From 80cfd9ffb670045b4ae03aa80106bad755e8433b Mon Sep 17 00:00:00 2001 From: Panos Astithas Date: Wed, 1 May 2013 18:29:33 +0300 Subject: [PATCH] Bug 832231 - After a reload, breakpoints require multiple resumes to allow execution to continue; r=vporof --- browser/devtools/debugger/test/Makefile.in | 3 + .../test/browser_dbg_location-changes-bp.js | 163 ++++++++++++++++++ .../test/test-location-changes-bp.html | 17 ++ .../debugger/test/test-location-changes-bp.js | 7 + .../debugger/server/dbg-browser-actors.js | 4 +- .../debugger/server/dbg-script-actors.js | 40 ++++- 6 files changed, 224 insertions(+), 10 deletions(-) create mode 100644 browser/devtools/debugger/test/browser_dbg_location-changes-bp.js create mode 100644 browser/devtools/debugger/test/test-location-changes-bp.html create mode 100644 browser/devtools/debugger/test/test-location-changes-bp.js diff --git a/browser/devtools/debugger/test/Makefile.in b/browser/devtools/debugger/test/Makefile.in index d8f9b1ee8f0b..9f36eba1a313 100644 --- a/browser/devtools/debugger/test/Makefile.in +++ b/browser/devtools/debugger/test/Makefile.in @@ -61,6 +61,7 @@ MOCHITEST_BROWSER_TESTS = \ browser_dbg_location-changes.js \ browser_dbg_location-changes-new.js \ browser_dbg_location-changes-blank.js \ + browser_dbg_location-changes-bp.js \ browser_dbg_sources-cache.js \ browser_dbg_scripts-switching.js \ browser_dbg_scripts-sorting.js \ @@ -133,6 +134,8 @@ MOCHITEST_BROWSER_PAGES = \ binary_search.coffee \ binary_search.js \ binary_search.map \ + test-location-changes-bp.js \ + test-location-changes-bp.html \ $(NULL) MOCHITEST_BROWSER_FILES_PARTS = MOCHITEST_BROWSER_TESTS MOCHITEST_BROWSER_PAGES diff --git a/browser/devtools/debugger/test/browser_dbg_location-changes-bp.js b/browser/devtools/debugger/test/browser_dbg_location-changes-bp.js new file mode 100644 index 000000000000..b5be16bf929b --- /dev/null +++ b/browser/devtools/debugger/test/browser_dbg_location-changes-bp.js @@ -0,0 +1,163 @@ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Make sure that reloading a page with a breakpoint set does not cause it to + * fire more than once. + */ + +const TAB_URL = EXAMPLE_URL + "test-location-changes-bp.html"; +const SCRIPT_URL = EXAMPLE_URL + "test-location-changes-bp.js"; + +var gPane = null; +var gTab = null; +var gDebuggee = null; +var gDebugger = null; +var sourcesShown = false; +var tabNavigated = false; + +function test() +{ + debug_tab_pane(TAB_URL, function(aTab, aDebuggee, aPane) { + gTab = aTab; + gDebuggee = aDebuggee; + gPane = aPane; + gDebugger = gPane.panelWin; + + testAddBreakpoint(); + }); +} + +function testAddBreakpoint() +{ + let controller = gDebugger.DebuggerController; + controller.activeThread.addOneTimeListener("framesadded", function() { + Services.tm.currentThread.dispatch({ run: function() { + + var frames = gDebugger.DebuggerView.StackFrames._container._list; + + is(controller.activeThread.state, "paused", + "The debugger statement was reached."); + + is(frames.querySelectorAll(".dbg-stackframe").length, 1, + "Should have one frame."); + + gPane.addBreakpoint({ url: SCRIPT_URL, line: 5 }, testResume); + }}, 0); + }); + + gDebuggee.runDebuggerStatement(); +} + +function testResume() +{ + is(gDebugger.DebuggerController.activeThread.state, "paused", + "The breakpoint wasn't hit yet."); + + let thread = gDebugger.DebuggerController.activeThread; + thread.addOneTimeListener("resumed", function() { + thread.addOneTimeListener("paused", function() { + executeSoon(testBreakpointHit); + }); + + EventUtils.sendMouseEvent({ type: "click" }, + content.document.querySelector("button")); + }); + + thread.resume(); +} + +function testBreakpointHit() +{ + is(gDebugger.DebuggerController.activeThread.state, "paused", + "The breakpoint was hit."); + + let thread = gDebugger.DebuggerController.activeThread; + thread.addOneTimeListener("paused", function test(aEvent, aPacket) { + thread.addOneTimeListener("resumed", function() { + executeSoon(testReloadPage); + }); + + is(aPacket.why.type, "debuggerStatement", "Execution has advanced to the next line."); + isnot(aPacket.why.type, "breakpoint", "No ghost breakpoint was hit."); + thread.resume(); + }); + + thread.resume(); +} + +function testReloadPage() +{ + let controller = gDebugger.DebuggerController; + controller._target.once("navigate", function onTabNavigated(aEvent, aPacket) { + tabNavigated = true; + ok(true, "tabNavigated event was fired."); + info("Still attached to the tab."); + clickAgain(); + }); + + gDebugger.addEventListener("Debugger:SourceShown", function onSourcesShown() { + sourcesShown = true; + gDebugger.removeEventListener("Debugger:SourceShown", onSourcesShown); + clickAgain(); + }); + + content.location.reload(); +} + +function clickAgain() +{ + if (!sourcesShown || !tabNavigated) { + return; + } + + let controller = gDebugger.DebuggerController; + controller.activeThread.addOneTimeListener("framesadded", function() { + is(gDebugger.DebuggerController.activeThread.state, "paused", + "The breakpoint was hit."); + + let thread = gDebugger.DebuggerController.activeThread; + thread.addOneTimeListener("paused", function test(aEvent, aPacket) { + thread.addOneTimeListener("resumed", function() { + executeSoon(closeDebuggerAndFinish); + }); + + is(aPacket.why.type, "debuggerStatement", "Execution has advanced to the next line."); + isnot(aPacket.why.type, "breakpoint", "No ghost breakpoint was hit."); + thread.resume(); + }); + + thread.resume(); + }); + + EventUtils.sendMouseEvent({ type: "click" }, + content.document.querySelector("button")); +} + +function testBreakpointHitAfterReload() +{ + is(gDebugger.DebuggerController.activeThread.state, "paused", + "The breakpoint was hit."); + + let thread = gDebugger.DebuggerController.activeThread; + thread.addOneTimeListener("paused", function test(aEvent, aPacket) { + thread.addOneTimeListener("resumed", function() { + executeSoon(closeDebuggerAndFinish); + }); + + is(aPacket.why.type, "debuggerStatement", "Execution has advanced to the next line."); + isnot(aPacket.why.type, "breakpoint", "No ghost breakpoint was hit."); + thread.resume(); + }); + + thread.resume(); +} + +registerCleanupFunction(function() { + removeTab(gTab); + gPane = null; + gTab = null; + gDebuggee = null; + gDebugger = null; +}); diff --git a/browser/devtools/debugger/test/test-location-changes-bp.html b/browser/devtools/debugger/test/test-location-changes-bp.html new file mode 100644 index 000000000000..0e18318947e2 --- /dev/null +++ b/browser/devtools/debugger/test/test-location-changes-bp.html @@ -0,0 +1,17 @@ + + + + + + + + + + + + + diff --git a/browser/devtools/debugger/test/test-location-changes-bp.js b/browser/devtools/debugger/test/test-location-changes-bp.js new file mode 100644 index 000000000000..d164b8bdf64d --- /dev/null +++ b/browser/devtools/debugger/test/test-location-changes-bp.js @@ -0,0 +1,7 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +function myFunction() { + var a = 1; + debugger; +} diff --git a/toolkit/devtools/debugger/server/dbg-browser-actors.js b/toolkit/devtools/debugger/server/dbg-browser-actors.js index 6e541c7005dd..5645991a754f 100644 --- a/toolkit/devtools/debugger/server/dbg-browser-actors.js +++ b/toolkit/devtools/debugger/server/dbg-browser-actors.js @@ -668,9 +668,6 @@ DebuggerProgressListener.prototype = { } if (isStart && aRequest instanceof Ci.nsIChannel) { - // If the request is about to happen in a new window, we are not concerned - // about the request. - // Proceed normally only if the debuggee is not paused. if (this._tabActor.threadActor.state == "paused") { aRequest.suspend(); @@ -679,6 +676,7 @@ DebuggerProgressListener.prototype = { this._tabActor._pendingNavigation = aRequest; } + this._tabActor.threadActor.disableAllBreakpoints(); 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 1eea8b4c13f1..c1a846833eda 100644 --- a/toolkit/devtools/debugger/server/dbg-script-actors.js +++ b/toolkit/devtools/debugger/server/dbg-script-actors.js @@ -740,6 +740,22 @@ ThreadActor.prototype = { }); }, + /** + * Disassociate all breakpoint actors from their scripts and clear the + * breakpoint handlers. This method can be used when the thread actor intends + * to keep the breakpoint store, but needs to clear any actual breakpoints, + * e.g. due to a page navigation. This way the breakpoint actors' script + * caches won't hold on to the Debugger.Script objects leaking memory. + */ + disableAllBreakpoints: function () { + for (let url in this._breakpointStore) { + for (let line in this._breakpointStore[url]) { + let bp = this._breakpointStore[url][line]; + bp.actor.removeScripts(); + } + } + }, + /** * Handle a protocol request to pause the debuggee. */ @@ -1268,8 +1284,11 @@ ThreadActor.prototype = { // affect the loop. for (let line = existing.length - 1; line >= 0; line--) { let bp = existing[line]; - // Limit search to the line numbers contained in the new script. - if (bp && line >= aScript.startLine && line <= endLine) { + // Only consider breakpoints that are not already associated with + // scripts, and limit search to the line numbers contained in the new + // script. + if (bp && !bp.actor.scripts.length && + line >= aScript.startLine && line <= endLine) { this._setBreakpoint(bp); } } @@ -2050,6 +2069,16 @@ BreakpointActor.prototype = { this.scripts.push(aScript); }, + /** + * Remove the breakpoints from associated scripts and clear the script cache. + */ + removeScripts: function () { + for (let script of this.scripts) { + script.clearBreakpoint(this); + } + this.scripts = []; + }, + /** * A function that the engine calls when a breakpoint has been hit. * @@ -2079,12 +2108,9 @@ BreakpointActor.prototype = { // Remove from the breakpoint store. let scriptBreakpoints = this.threadActor._breakpointStore[this.location.url]; delete scriptBreakpoints[this.location.line]; - // Remove the actual breakpoint. this.threadActor._hooks.removeFromParentPool(this); - for (let script of this.scripts) { - script.clearBreakpoint(this); - } - this.scripts = null; + // Remove the actual breakpoint from the associated scripts. + this.removeScripts(); return { from: this.actorID }; }