From 64b491034c0373755a2f34db5db1810b8d90187a Mon Sep 17 00:00:00 2001 From: mgiambalvo Date: Fri, 2 Sep 2016 17:26:19 -0700 Subject: [PATCH] fix(debugger): Fix issues when calling pause() multiple times (#3501) (#3504) --- .gitignore | 1 + lib/browser.ts | 49 +++++++++++++++++++++--------- lib/debugger/clients/wddebugger.js | 2 +- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index 2ae1543f3..ef32b6fce 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ chromedriver.log libpeerconnection.log xmloutput* npm-debug.log +.idea/ *.swp globals.js diff --git a/lib/browser.ts b/lib/browser.ts index efd58f4a8..eae3e427a 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -241,9 +241,13 @@ export class ProtractorBrowser extends Webdriver { */ debuggerServerPort_: number; + /** + * Set to true when we validate that the debug port is open. Since the debug + * port is held open forever once the debugger is attached, it's important + * we only do validation once. + */ debuggerValidated_: boolean; - /** * If true, Protractor will interpret any angular apps it comes across as * hybrid angular1/angular2 apps. @@ -983,10 +987,12 @@ export class ProtractorBrowser extends Webdriver { } }); - return doneDeferred.then(null, (err: string) => { - console.error(err); - process.exit(1); - }); + return doneDeferred.then( + () => { this.debuggerValidated_ = true; }, + (err: string) => { + console.error(err); + process.exit(1); + }); } private dbgCodeExecutor_: any; @@ -1043,10 +1049,10 @@ export class ProtractorBrowser extends Webdriver { let browserUnderDebug = this; let debuggerReadyPromise = webdriver.promise.defer(); - flow.execute(function() { + flow.execute(() => { process['debugPort'] = opt_debugPort || process['debugPort']; browserUnderDebug.validatePortAvailability_(process['debugPort']) - .then(function(firstTime: boolean) { + .then((firstTime: boolean) => { onStartFn(firstTime); let args = [process.pid, process['debugPort']]; @@ -1056,11 +1062,19 @@ export class ProtractorBrowser extends Webdriver { let nodedebug = require('child_process').fork(debuggerClientPath, args); process.on('exit', function() { nodedebug.kill('SIGTERM'); }); - nodedebug.on('message', function(m: string) { - if (m === 'ready') { - debuggerReadyPromise.fulfill(); - } - }); + nodedebug + .on('message', + (m: string) => { + if (m === 'ready') { + debuggerReadyPromise.fulfill(); + } + }) + .on('exit', () => { + logger.info('Debugger exiting'); + // Clear this so that we know it's ok to attach a debugger + // again. + this.dbgCodeExecutor_ = null; + }); }); }); @@ -1167,6 +1181,8 @@ export class ProtractorBrowser extends Webdriver { return this.execPromiseResult_; } }; + + return pausePromise; } /** @@ -1221,7 +1237,12 @@ export class ProtractorBrowser extends Webdriver { * @param {number=} opt_debugPort Optional port to use for the debugging * process */ - pause(opt_debugPort?: number) { + pause(opt_debugPort?: number): webdriver.promise.Promise { + if (this.dbgCodeExecutor_) { + logger.info( + 'Encountered browser.pause(), but debugger already attached.'); + return webdriver.promise.fulfilled(true); + } let debuggerClientPath = __dirname + '/debugger/clients/wddebugger.js'; let onStartFn = (firstTime: boolean) => { logger.info(); @@ -1240,7 +1261,7 @@ export class ProtractorBrowser extends Webdriver { logger.info(); } }; - this.initDebugger_(debuggerClientPath, onStartFn, opt_debugPort); + return this.initDebugger_(debuggerClientPath, onStartFn, opt_debugPort); } /** diff --git a/lib/debugger/clients/wddebugger.js b/lib/debugger/clients/wddebugger.js index 7be4427fa..767e05af3 100644 --- a/lib/debugger/clients/wddebugger.js +++ b/lib/debugger/clients/wddebugger.js @@ -100,7 +100,7 @@ WdDebugger.prototype.initRepl_ = function() { self.replServer.on('exit', function() { console.log('Resuming code execution'); self.client.req({command: 'disconnect'}, function() { - // Intentionally blank. + process.exit(); }); }); });