Bug 783393 - Breakpoints not getting caught on reload, r=rcampbell f=rcampbell,dcamp,vporof

browser/devtools/debugger/debugger-controller.js

@@ -185,21 +185,14 @@ let DebuggerController = {
   },
 
   /**
-   * Starts debugging the current tab. This function is called on each location
-   * change in this tab.
+   * This function is called on each location change in this tab.
    */
   _onTabNavigated: function DC__onTabNavigated(aNotification, aPacket) {
-    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));
+    DebuggerController.ThreadState._handleTabNavigation(function() {
+      DebuggerController.StackFrames._handleTabNavigation(function() {
+        DebuggerController.SourceScripts._handleTabNavigation();
+      });
+    });
   },
 
   /**
@@ -327,7 +320,7 @@ ThreadState.prototype = {
     this.activeThread.addListener("resumed", this._update);
     this.activeThread.addListener("detached", this._update);
 
-    this._update();
+    this._handleTabNavigation();
 
     aCallback && aCallback();
   },
@@ -344,6 +337,15 @@ 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.
    */
@@ -397,12 +399,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();
@@ -412,17 +414,25 @@ 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.
    *
@@ -436,6 +446,7 @@ StackFrames.prototype = {
     if (aPacket.why.type == "exception") {
       this.exception = aPacket.why.exception;
     }
+
     this.activeThread.fillFrames(this.pageSize);
     DebuggerView.editor.focus();
   },
@@ -834,7 +845,6 @@ 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);
@@ -869,17 +879,9 @@ SourceScripts.prototype = {
    */
   connect: function SS_connect(aCallback) {
     window.addEventListener("Debugger:LoadSource", this._onLoadSource, false);
-
     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();
+    this._handleTabNavigation();
 
     aCallback && aCallback();
   },
@@ -887,15 +889,26 @@ SourceScripts.prototype = {
   /**
    * Disconnect from the client.
    */
-  disconnect: function TS_disconnect() {
-    window.removeEventListener("Debugger:LoadSource", this._onLoadSource, false);
-
+  disconnect: function SS_disconnect() {
     if (!this.activeThread) {
       return;
     }
+    window.removeEventListener("Debugger:LoadSource", this._onLoadSource, false);
     this.debuggerClient.removeListener("newScript", this._onNewScript);
-    this.activeThread.removeListener("scriptsadded", this._onScriptsAdded);
-    this.activeThread.removeListener("scriptscleared", this._onScriptsCleared);
+  },
+
+  /**
+   * 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();
   },
 
   /**
@@ -909,10 +922,16 @@ SourceScripts.prototype = {
 
     this._addScript({ url: aPacket.url, startLine: aPacket.startLine }, true);
 
-    // Select the script if it's the preferred one.
+    let preferredScriptUrl = DebuggerView.Scripts.preferredScriptUrl;
+
+    // Select this 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.
@@ -921,29 +940,36 @@ SourceScripts.prototype = {
         DebuggerController.Breakpoints.displayBreakpoint(breakpoint);
       }
     }
+
+    DebuggerController.dispatchEvent("Debugger:AfterNewScript");
   },
 
   /**
-   * Handler for the thread client's scriptsadded notification.
+   * Callback for the getScripts() method.
    */
-  _onScriptsAdded: function SS__onScriptsAdded() {
-    for each (let script in this.activeThread.cachedScripts) {
+  _onScriptsAdded: function SS__onScriptsAdded(aResponse) {
+    for each (let script in aResponse.scripts) {
       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);
-    } else {
+    }
+    // ..or the first entry if there's not one selected yet.
+    else if (!DebuggerView.Scripts.selected) {
       DebuggerView.Scripts.selectIndex(0);
     }
+
+    DebuggerController.dispatchEvent("Debugger:AfterScriptsAdded");
   },
 
   /**
-   * Handler for the thread client's scriptscleared notification.
+   * Called during navigation to clear the currently-loaded scripts.
    */
   _onScriptsCleared: function SS__onScriptsCleared() {
     DebuggerView.Scripts.empty();

browser/devtools/debugger/test/browser_dbg_bfcache.js

@@ -40,9 +40,13 @@ function testInitialLoad() {
 function testLocationChange()
 {
   gDebugger.DebuggerController.activeThread.resume(function() {
-    gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) {
-      ok(true, "Successfully reattached to the tab again.");
-      gDebugger.DebuggerController.client.addOneTimeListener("resumed", function(aEvent, aPacket) {
+    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);
+
         executeSoon(function() {
           validateSecondPage();
           testBack();
@@ -55,9 +59,13 @@ function testLocationChange()
 
 function testBack()
 {
-  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) {
+  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);
+
       executeSoon(function() {
         validateFirstPage();
         testForward();
@@ -71,9 +79,13 @@ function testBack()
 
 function testForward()
 {
-  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) {
+  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);
+
       executeSoon(function() {
         validateSecondPage();
         closeDebuggerAndFinish();

browser/devtools/debugger/test/browser_dbg_location-changes-blank.js

@@ -57,11 +57,11 @@ function testLocationChange()
   gDebugger.DebuggerController.activeThread.resume(function() {
     gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) {
       ok(true, "tabNavigated event was fired.");
-      gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) {
-        ok(true, "Successfully reattached to the tab again.");
+      info("Still attached to the tab.");
+
+      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,
@@ -77,7 +77,6 @@ function testLocationChange()
         closeDebuggerAndFinish();
       });
     });
-    });
     content.location = "about:blank";
   });
 }

browser/devtools/debugger/test/browser_dbg_location-changes-new.js

@@ -57,11 +57,11 @@ function testLocationChange()
   gDebugger.DebuggerController.activeThread.resume(function() {
     gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) {
       ok(true, "tabNavigated event was fired.");
-      gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) {
-        ok(true, "Successfully reattached to the tab again.");
+      info("Still attached to the tab.");
+
+      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,
@@ -77,7 +77,6 @@ function testLocationChange()
         closeDebuggerAndFinish();
       });
     });
-    });
     content.location = EXAMPLE_URL + "browser_dbg_iframes.html";
   });
 }

browser/devtools/debugger/test/browser_dbg_location-changes.js

@@ -52,12 +52,10 @@ function testLocationChange()
   gDebugger.DebuggerController.activeThread.resume(function() {
     gDebugger.DebuggerController.client.addOneTimeListener("tabNavigated", function(aEvent, aPacket) {
       ok(true, "tabNavigated event was fired.");
-      gDebugger.DebuggerController.client.addOneTimeListener("tabAttached", function(aEvent, aPacket) {
-        ok(true, "Successfully reattached to the tab again.");
+      info("Still attached to the tab.");
 
       closeDebuggerAndFinish();
     });
-    });
     content.location = TAB1_URL;
   });
 }

toolkit/devtools/debugger/dbg-client.jsm

@@ -177,6 +177,17 @@ 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.
@@ -400,18 +411,28 @@ DebuggerClient.prototype = {
       }
 
       let onResponse;
-      // Don't count unsolicited notifications as responses.
+      // Don't count unsolicited notifications or pauses as responses.
       if (aPacket.from in this._activeRequests &&
-          !(aPacket.type in UnsolicitedNotifications)) {
+          !(aPacket.type in UnsolicitedNotifications) &&
+          !(aPacket.type == ThreadStateTypes.paused &&
+            aPacket.why.type in UnsolicitedPauses)) {
         onResponse = this._activeRequests[aPacket.from].onResponse;
         delete this._activeRequests[aPacket.from];
       }
 
-      // paused/resumed/detached get special treatment...
+      // Packets that indicate thread state changes 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) {
@@ -725,12 +746,20 @@ ThreadClient.prototype = {
     this._client.request(packet, aOnResponse);
   },
 
-  /**
-   * 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; },
+  _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));
+  },
 
   /**
    * Ensure that source scripts have been loaded in the

toolkit/devtools/debugger/server/dbg-browser-actors.js

@@ -446,10 +446,19 @@ 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);
+    }
   }
 };
 
@@ -499,10 +508,13 @@ 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",

toolkit/devtools/debugger/server/dbg-script-actors.js

@@ -51,6 +51,24 @@ 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.
    */
@@ -62,14 +80,17 @@ 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);
-    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;
+    for (let s of this.dbg.findScripts()) {
+      this._addScript(s);
+    }
   },
 
   /**
@@ -90,19 +111,14 @@ ThreadActor.prototype = {
     }
 
     this._state = "exited";
-    if (this.dbg) {
-      this.dbg.enabled = false;
+
+    this.clearDebuggees();
+
+    if (!this._dbg) {
+      return;
+    }
+    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 = {};
   },
 
   /**

toolkit/devtools/debugger/server/dbg-server.js

@@ -366,7 +366,10 @@ DebuggerServerConnection.prototype = {
    * Remove a previously-added pool of actors to the connection.
    */
   removeActorPool: function DSC_removeActorPool(aActorPool) {
-    let index = this._extraPools.splice(this._extraPools.lastIndexOf(aActorPool), 1);
+    let index = this._extraPools.lastIndexOf(aActorPool);
+    if (index > -1) {
+      this._extraPools.splice(index, 1);
+    }
   },
 
   /**