From 3caff3c473a9bf7b41ee0a65d67d6dd629d16083 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Thu, 23 Aug 2012 14:37:29 -0700 Subject: [PATCH] Back out b2900e347f5c (bug 783393) for Windows xpcshell hangs in test_listscripts-01.js on a CLOSED TREE --- .../devtools/debugger/debugger-controller.js | 106 +++++++----------- .../debugger/test/browser_dbg_bfcache.js | 30 ++--- .../browser_dbg_location-changes-blank.js | 31 ++--- .../test/browser_dbg_location-changes-new.js | 31 ++--- .../test/browser_dbg_location-changes.js | 6 +- toolkit/devtools/debugger/dbg-client.jsm | 47 ++------ .../debugger/server/dbg-browser-actors.js | 14 +-- .../debugger/server/dbg-script-actors.js | 50 +++------ .../devtools/debugger/server/dbg-server.js | 5 +- 9 files changed, 113 insertions(+), 207 deletions(-) diff --git a/browser/devtools/debugger/debugger-controller.js b/browser/devtools/debugger/debugger-controller.js index dedd40f7c1b0..3fc445558b7e 100644 --- a/browser/devtools/debugger/debugger-controller.js +++ b/browser/devtools/debugger/debugger-controller.js @@ -185,14 +185,21 @@ let DebuggerController = { }, /** - * This function is called on each location change in this tab. + * Starts debugging the current tab. This function is called on each location + * change in this tab. */ _onTabNavigated: function DC__onTabNavigated(aNotification, aPacket) { - DebuggerController.ThreadState._handleTabNavigation(function() { - DebuggerController.StackFrames._handleTabNavigation(function() { - DebuggerController.SourceScripts._handleTabNavigation(); - }); - }); + let client = this.client; + + client.activeThread.detach(function() { + client.activeTab.detach(function() { + client.listTabs(function(aResponse) { + let tab = aResponse.tabs[aResponse.selected]; + this._startDebuggingTab(client, tab); + this.dispatchEvent("Debugger:Connecting"); + }.bind(this)); + }.bind(this)); + }.bind(this)); }, /** @@ -320,7 +327,7 @@ ThreadState.prototype = { this.activeThread.addListener("resumed", this._update); this.activeThread.addListener("detached", this._update); - this._handleTabNavigation(); + this._update(); aCallback && aCallback(); }, @@ -337,15 +344,6 @@ ThreadState.prototype = { this.activeThread.removeListener("detached", this._update); }, - /** - * Handles any initialization on a tab navigation event issued by the client. - */ - _handleTabNavigation: function TS__handleTabNavigation(aCallback) { - DebuggerView.StackFrames.updateState(this.activeThread.state); - - aCallback && aCallback(); - }, - /** * Update the UI after a thread state change. */ @@ -399,12 +397,12 @@ StackFrames.prototype = { */ connect: function SF_connect(aCallback) { window.addEventListener("Debugger:FetchedVariables", this._onFetchedVars, false); + this.activeThread.addListener("paused", this._onPaused); this.activeThread.addListener("resumed", this._onResume); this.activeThread.addListener("framesadded", this._onFrames); this.activeThread.addListener("framescleared", this._onFramesCleared); - this._handleTabNavigation(); this.updatePauseOnExceptions(this.pauseOnExceptions); aCallback && aCallback(); @@ -414,25 +412,17 @@ StackFrames.prototype = { * Disconnect from the client. */ disconnect: function SF_disconnect() { + window.removeEventListener("Debugger:FetchedVariables", this._onFetchedVars, false); + if (!this.activeThread) { return; } - window.removeEventListener("Debugger:FetchedVariables", this._onFetchedVars, false); this.activeThread.removeListener("paused", this._onPaused); this.activeThread.removeListener("resumed", this._onResume); this.activeThread.removeListener("framesadded", this._onFrames); this.activeThread.removeListener("framescleared", this._onFramesCleared); }, - /** - * Handles any initialization on a tab navigation event issued by the client. - */ - _handleTabNavigation: function SF__handleTabNavigation(aCallback) { - // Nothing to do here yet. - - aCallback && aCallback(); - }, - /** * Handler for the thread client's paused notification. * @@ -446,7 +436,6 @@ StackFrames.prototype = { if (aPacket.why.type == "exception") { this.exception = aPacket.why.exception; } - this.activeThread.fillFrames(this.pageSize); DebuggerView.editor.focus(); }, @@ -845,6 +834,7 @@ StackFrames.prototype = { function SourceScripts() { this._onNewScript = this._onNewScript.bind(this); this._onScriptsAdded = this._onScriptsAdded.bind(this); + this._onScriptsCleared = this._onScriptsCleared.bind(this); this._onShowScript = this._onShowScript.bind(this); this._onLoadSource = this._onLoadSource.bind(this); this._onLoadSourceFinished = this._onLoadSourceFinished.bind(this); @@ -879,9 +869,17 @@ SourceScripts.prototype = { */ connect: function SS_connect(aCallback) { window.addEventListener("Debugger:LoadSource", this._onLoadSource, false); - this.debuggerClient.addListener("newScript", this._onNewScript); - this._handleTabNavigation(); + this.debuggerClient.addListener("newScript", this._onNewScript); + this.activeThread.addListener("scriptsadded", this._onScriptsAdded); + this.activeThread.addListener("scriptscleared", this._onScriptsCleared); + + this._clearLabelsCache(); + this._onScriptsCleared(); + + // Retrieve the list of scripts known to the server from before the client + // was ready to handle new script notifications. + this.activeThread.fillScripts(); aCallback && aCallback(); }, @@ -889,26 +887,15 @@ SourceScripts.prototype = { /** * Disconnect from the client. */ - disconnect: function SS_disconnect() { + disconnect: function TS_disconnect() { + window.removeEventListener("Debugger:LoadSource", this._onLoadSource, false); + if (!this.activeThread) { return; } - window.removeEventListener("Debugger:LoadSource", this._onLoadSource, false); this.debuggerClient.removeListener("newScript", this._onNewScript); - }, - - /** - * Handles any initialization on a tab navigation event issued by the client. - */ - _handleTabNavigation: function SS__handleTabNavigation(aCallback) { - this._clearLabelsCache(); - this._onScriptsCleared(); - - // Retrieve the list of scripts known to the server from before the client - // was ready to handle new script notifications. - this.activeThread.getScripts(this._onScriptsAdded); - - aCallback && aCallback(); + this.activeThread.removeListener("scriptsadded", this._onScriptsAdded); + this.activeThread.removeListener("scriptscleared", this._onScriptsCleared); }, /** @@ -922,16 +909,10 @@ SourceScripts.prototype = { this._addScript({ url: aPacket.url, startLine: aPacket.startLine }, true); - let preferredScriptUrl = DebuggerView.Scripts.preferredScriptUrl; - - // Select this script if it's the preferred one. + // Select the script if it's the preferred one. if (aPacket.url === DebuggerView.Scripts.preferredScriptUrl) { DebuggerView.Scripts.selectScript(aPacket.url); } - // ..or the first entry if there's not one selected yet. - else if (!DebuggerView.Scripts.selected) { - DebuggerView.Scripts.selectIndex(0); - } // If there are any stored breakpoints for this script, display them again, // both in the editor and the pane. @@ -940,36 +921,29 @@ SourceScripts.prototype = { DebuggerController.Breakpoints.displayBreakpoint(breakpoint); } } - - DebuggerController.dispatchEvent("Debugger:AfterNewScript"); }, /** - * Callback for the getScripts() method. + * Handler for the thread client's scriptsadded notification. */ - _onScriptsAdded: function SS__onScriptsAdded(aResponse) { - for each (let script in aResponse.scripts) { + _onScriptsAdded: function SS__onScriptsAdded() { + for each (let script in this.activeThread.cachedScripts) { this._addScript(script, false); } DebuggerView.Scripts.commitScripts(); DebuggerController.Breakpoints.updatePaneBreakpoints(); + // Select the preferred script if one exists, the first entry otherwise. let preferredScriptUrl = DebuggerView.Scripts.preferredScriptUrl; - - // Select the preferred script if it exists and was part of the response. if (preferredScriptUrl && DebuggerView.Scripts.contains(preferredScriptUrl)) { DebuggerView.Scripts.selectScript(preferredScriptUrl); - } - // ..or the first entry if there's not one selected yet. - else if (!DebuggerView.Scripts.selected) { + } else { DebuggerView.Scripts.selectIndex(0); } - - DebuggerController.dispatchEvent("Debugger:AfterScriptsAdded"); }, /** - * Called during navigation to clear the currently-loaded scripts. + * Handler for the thread client's scriptscleared notification. */ _onScriptsCleared: function SS__onScriptsCleared() { DebuggerView.Scripts.empty(); diff --git a/browser/devtools/debugger/test/browser_dbg_bfcache.js b/browser/devtools/debugger/test/browser_dbg_bfcache.js index e936f28334e4..9ff46888b655 100644 --- a/browser/devtools/debugger/test/browser_dbg_bfcache.js +++ b/browser/devtools/debugger/test/browser_dbg_bfcache.js @@ -40,13 +40,9 @@ function testInitialLoad() { function testLocationChange() { gDebugger.DebuggerController.activeThread.resume(function() { - gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) { - ok(true, "tabNavigated event was fired."); - info("Still attached to the tab."); - - gDebugger.addEventListener("Debugger:AfterScriptsAdded", function _onEvent(aEvent) { - gDebugger.removeEventListener(aEvent.type, _onEvent); - + gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) { + ok(true, "Successfully reattached to the tab again."); + gDebugger.DebuggerController.client.addOneTimeListener("resumed", function(aEvent, aPacket) { executeSoon(function() { validateSecondPage(); testBack(); @@ -59,13 +55,9 @@ function testLocationChange() function testBack() { - gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) { - ok(true, "tabNavigated event was fired after going back."); - info("Still attached to the tab."); - - gDebugger.addEventListener("Debugger:AfterScriptsAdded", function _onEvent(aEvent) { - gDebugger.removeEventListener(aEvent.type, _onEvent); - + gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) { + ok(true, "Successfully reattached to the tab after going back."); + gDebugger.DebuggerController.client.addOneTimeListener("resumed", function(aEvent, aPacket) { executeSoon(function() { validateFirstPage(); testForward(); @@ -79,13 +71,9 @@ function testBack() function testForward() { - gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) { - ok(true, "tabNavigated event was fired after going forward."); - info("Still attached to the tab."); - - gDebugger.addEventListener("Debugger:AfterScriptsAdded", function _onEvent(aEvent) { - gDebugger.removeEventListener(aEvent.type, _onEvent); - + gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) { + ok(true, "Successfully reattached to the tab after going forward."); + gDebugger.DebuggerController.client.addOneTimeListener("resumed", function(aEvent, aPacket) { executeSoon(function() { validateSecondPage(); closeDebuggerAndFinish(); diff --git a/browser/devtools/debugger/test/browser_dbg_location-changes-blank.js b/browser/devtools/debugger/test/browser_dbg_location-changes-blank.js index 9df0fc2c5fe2..0366656a966d 100644 --- a/browser/devtools/debugger/test/browser_dbg_location-changes-blank.js +++ b/browser/devtools/debugger/test/browser_dbg_location-changes-blank.js @@ -57,24 +57,25 @@ function testLocationChange() gDebugger.DebuggerController.activeThread.resume(function() { gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) { ok(true, "tabNavigated event was fired."); - info("Still attached to the tab."); + gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) { + ok(true, "Successfully reattached to the tab again."); - gDebugger.addEventListener("Debugger:AfterScriptsAdded", function _onEvent(aEvent) { - gDebugger.removeEventListener(aEvent.type, _onEvent); + // Wait for the initial resume... + gDebugger.gClient.addOneTimeListener("resumed", function() { + is(gDebugger.DebuggerView.Scripts.selected, null, + "There should be no selected script."); + is(gDebugger.editor.getText().length, 0, + "The source editor not have any text displayed."); - is(gDebugger.DebuggerView.Scripts.selected, null, - "There should be no selected script."); - is(gDebugger.editor.getText().length, 0, - "The source editor not have any text displayed."); + let menulist = gDebugger.DebuggerView.Scripts._scripts; + let noScripts = gDebugger.L10N.getStr("noScriptsText"); + is(menulist.getAttribute("label"), noScripts, + "The menulist should display a notice that there are no scripts availalble."); + is(menulist.getAttribute("tooltiptext"), "", + "The menulist shouldn't have any tooltip text attributed when there are no scripts available."); - let menulist = gDebugger.DebuggerView.Scripts._scripts; - let noScripts = gDebugger.L10N.getStr("noScriptsText"); - is(menulist.getAttribute("label"), noScripts, - "The menulist should display a notice that there are no scripts availalble."); - is(menulist.getAttribute("tooltiptext"), "", - "The menulist shouldn't have any tooltip text attributed when there are no scripts available."); - - closeDebuggerAndFinish(); + closeDebuggerAndFinish(); + }); }); }); content.location = "about:blank"; diff --git a/browser/devtools/debugger/test/browser_dbg_location-changes-new.js b/browser/devtools/debugger/test/browser_dbg_location-changes-new.js index 4e9b2800cf4f..2be2dd892c07 100644 --- a/browser/devtools/debugger/test/browser_dbg_location-changes-new.js +++ b/browser/devtools/debugger/test/browser_dbg_location-changes-new.js @@ -57,24 +57,25 @@ function testLocationChange() gDebugger.DebuggerController.activeThread.resume(function() { gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) { ok(true, "tabNavigated event was fired."); - info("Still attached to the tab."); + gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) { + ok(true, "Successfully reattached to the tab again."); - gDebugger.addEventListener("Debugger:AfterScriptsAdded", function _onEvent(aEvent) { - gDebugger.removeEventListener(aEvent.type, _onEvent); + // Wait for the initial resume... + gDebugger.gClient.addOneTimeListener("resumed", function() { + isnot(gDebugger.DebuggerView.Scripts.selected, null, + "There should be a selected script."); + isnot(gDebugger.editor.getText().length, 0, + "The source editor should have some text displayed."); - isnot(gDebugger.DebuggerView.Scripts.selected, null, - "There should be a selected script."); - isnot(gDebugger.editor.getText().length, 0, - "The source editor should have some text displayed."); + let menulist = gDebugger.DebuggerView.Scripts._scripts; + let noScripts = gDebugger.L10N.getStr("noScriptsText"); + isnot(menulist.getAttribute("label"), noScripts, + "The menulist should not display a notice that there are no scripts availalble."); + isnot(menulist.getAttribute("tooltiptext"), "", + "The menulist should have a tooltip text attributed."); - let menulist = gDebugger.DebuggerView.Scripts._scripts; - let noScripts = gDebugger.L10N.getStr("noScriptsText"); - isnot(menulist.getAttribute("label"), noScripts, - "The menulist should not display a notice that there are no scripts availalble."); - isnot(menulist.getAttribute("tooltiptext"), "", - "The menulist should have a tooltip text attributed."); - - closeDebuggerAndFinish(); + closeDebuggerAndFinish(); + }); }); }); content.location = EXAMPLE_URL + "browser_dbg_iframes.html"; diff --git a/browser/devtools/debugger/test/browser_dbg_location-changes.js b/browser/devtools/debugger/test/browser_dbg_location-changes.js index ce5de5141ced..b67a8d0ff078 100644 --- a/browser/devtools/debugger/test/browser_dbg_location-changes.js +++ b/browser/devtools/debugger/test/browser_dbg_location-changes.js @@ -52,9 +52,11 @@ function testLocationChange() gDebugger.DebuggerController.activeThread.resume(function() { gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) { ok(true, "tabNavigated event was fired."); - info("Still attached to the tab."); + gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) { + ok(true, "Successfully reattached to the tab again."); - closeDebuggerAndFinish(); + closeDebuggerAndFinish(); + }); }); content.location = TAB1_URL; }); diff --git a/toolkit/devtools/debugger/dbg-client.jsm b/toolkit/devtools/debugger/dbg-client.jsm index 26114f924049..dd8648707036 100644 --- a/toolkit/devtools/debugger/dbg-client.jsm +++ b/toolkit/devtools/debugger/dbg-client.jsm @@ -177,17 +177,6 @@ const UnsolicitedNotifications = { "tabNavigated": "tabNavigated" }; -/** - * Set of pause types that are sent by the server and not as an immediate - * response to a client request. - */ -const UnsolicitedPauses = { - "resumeLimit": "resumeLimit", - "debuggerStatement": "debuggerStatement", - "breakpoint": "breakpoint", - "watchpoint": "watchpoint" -}; - /** * Set of debug protocol request types that specify the protocol request being * sent to the server. @@ -411,28 +400,18 @@ DebuggerClient.prototype = { } let onResponse; - // Don't count unsolicited notifications or pauses as responses. + // Don't count unsolicited notifications as responses. if (aPacket.from in this._activeRequests && - !(aPacket.type in UnsolicitedNotifications) && - !(aPacket.type == ThreadStateTypes.paused && - aPacket.why.type in UnsolicitedPauses)) { + !(aPacket.type in UnsolicitedNotifications)) { onResponse = this._activeRequests[aPacket.from].onResponse; delete this._activeRequests[aPacket.from]; } - // Packets that indicate thread state changes get special treatment. + // paused/resumed/detached get special treatment... if (aPacket.type in ThreadStateTypes && aPacket.from in this._threadClients) { this._threadClients[aPacket.from]._onThreadState(aPacket); } - // On navigation the server resumes, so the client must resume as well. - // We achive that by generating a fake resumption packet that triggers - // the client's thread state change listeners. - if (aPacket.type == UnsolicitedNotifications.tabNavigated && - aPacket.from in this._tabClients) { - let resumption = { from: this.activeThread._actor, type: "resumed" }; - this.activeThread._onThreadState(resumption); - } this.notify(aPacket.type, aPacket); if (onResponse) { @@ -746,20 +725,12 @@ ThreadClient.prototype = { this._client.request(packet, aOnResponse); }, - _doInterrupted: function TC_doInterrupted(aAction, aError) { - if (this.paused) { - aAction(); - return; - } - this.interrupt(function(aResponse) { - if (aResponse) { - aError(aResponse); - return; - } - aAction(); - this.resume(function() {}); - }.bind(this)); - }, + /** + * A cache of source scripts. Clients can observe the scriptsadded and + * scriptscleared event to keep up to date on changes to this cache, + * and can fill it using the fillScripts method. + */ + get cachedScripts() { return this._scriptCache; }, /** * Ensure that source scripts have been loaded in the diff --git a/toolkit/devtools/debugger/server/dbg-browser-actors.js b/toolkit/devtools/debugger/server/dbg-browser-actors.js index 50346f58ae80..c386b162f6b6 100644 --- a/toolkit/devtools/debugger/server/dbg-browser-actors.js +++ b/toolkit/devtools/debugger/server/dbg-browser-actors.js @@ -446,19 +446,10 @@ BrowserTabActor.prototype = { return; } if (this._attached) { - this.threadActor.clearDebuggees(); - this.threadActor.dbg.enabled = true; - if (this._progressListener) { - delete this._progressListener._needsTabNavigated; - } this.conn.send({ from: this.actorID, type: "tabNavigated", url: this.browser.contentDocument.URL }); } } - - if (this._attached) { - this._addDebuggees(evt.target.defaultView.wrappedJSObject); - } } }; @@ -508,13 +499,10 @@ DebuggerProgressListener.prototype = { } aRequest.suspend(); - this._tabActor.threadActor.onResume(); - this._tabActor.threadActor.dbg.enabled = false; this._tabActor._pendingNavigation = aRequest; + this._tabActor._detach(); this._needsTabNavigated = true; } else if (isStop && isWindow && isNetwork && this._needsTabNavigated) { - delete this._needsTabNavigated; - this._tabActor.threadActor.dbg.enabled = true; this._tabActor.conn.send({ from: this._tabActor.actorID, type: "tabNavigated", diff --git a/toolkit/devtools/debugger/server/dbg-script-actors.js b/toolkit/devtools/debugger/server/dbg-script-actors.js index 3e051467662b..4deac682c0e3 100644 --- a/toolkit/devtools/debugger/server/dbg-script-actors.js +++ b/toolkit/devtools/debugger/server/dbg-script-actors.js @@ -51,24 +51,6 @@ ThreadActor.prototype = { return this._threadLifetimePool; }, - clearDebuggees: function TA_clearDebuggees() { - if (this._dbg) { - let debuggees = this._dbg.getDebuggees(); - for (let debuggee of debuggees) { - this._dbg.removeDebuggee(debuggee); - } - } - this.conn.removeActorPool(this._threadLifetimePool || undefined); - this._threadLifetimePool = null; - // Unless we carefully take apart the scripts table this way, we end up - // leaking documents. It would be nice to track this down carefully, once - // we have the appropriate tools. - for (let url in this._scripts) { - delete this._scripts[url]; - } - this._scripts = {}; - }, - /** * Add a debuggee global to the Debugger object. */ @@ -80,17 +62,14 @@ ThreadActor.prototype = { if (!this._dbg) { this._dbg = new Debugger(); - this._dbg.uncaughtExceptionHook = this.uncaughtExceptionHook.bind(this); - this._dbg.onDebuggerStatement = this.onDebuggerStatement.bind(this); - this._dbg.onNewScript = this.onNewScript.bind(this); - // Keep the debugger disabled until a client attaches. - this.dbg.enabled = this._state != "detached"; } this.dbg.addDebuggee(aGlobal); - for (let s of this.dbg.findScripts()) { - this._addScript(s); - } + this.dbg.uncaughtExceptionHook = this.uncaughtExceptionHook.bind(this); + this.dbg.onDebuggerStatement = this.onDebuggerStatement.bind(this); + this.dbg.onNewScript = this.onNewScript.bind(this); + // Keep the debugger disabled until a client attaches. + this.dbg.enabled = false; }, /** @@ -111,14 +90,19 @@ ThreadActor.prototype = { } this._state = "exited"; - - this.clearDebuggees(); - - if (!this._dbg) { - return; + if (this.dbg) { + this.dbg.enabled = false; + this._dbg = null; } - this._dbg.enabled = false; - this._dbg = null; + this.conn.removeActorPool(this._threadLifetimePool || undefined); + this._threadLifetimePool = null; + // Unless we carefully take apart the scripts table this way, we end up + // leaking documents. It would be nice to track this down carefully, once + // we have the appropriate tools. + for (let url in this._scripts) { + delete this._scripts[url]; + } + this._scripts = {}; }, /** diff --git a/toolkit/devtools/debugger/server/dbg-server.js b/toolkit/devtools/debugger/server/dbg-server.js index 2cfab0ef7ce8..8ba6607745b8 100644 --- a/toolkit/devtools/debugger/server/dbg-server.js +++ b/toolkit/devtools/debugger/server/dbg-server.js @@ -366,10 +366,7 @@ DebuggerServerConnection.prototype = { * Remove a previously-added pool of actors to the connection. */ removeActorPool: function DSC_removeActorPool(aActorPool) { - let index = this._extraPools.lastIndexOf(aActorPool); - if (index > -1) { - this._extraPools.splice(index, 1); - } + let index = this._extraPools.splice(this._extraPools.lastIndexOf(aActorPool), 1); }, /**