Forbid out of order resumption when two tabs with the same URL are paused (bug 947830); r=fitzgen

Also consider stepping actions as a form of resumption and require the same resumption order when multiple tabs are paused.

browser/devtools/debugger/debugger-toolbar.js

@@ -149,7 +149,8 @@ ToolbarView.prototype = {
   _onStepOverPressed: function() {
     if (DebuggerController.activeThread.paused) {
       DebuggerController.StackFrames.currentFrameDepth = -1;
-      DebuggerController.activeThread.stepOver();
+      let warn = DebuggerController._ensureResumptionOrder;
+      DebuggerController.activeThread.stepOver(warn);
     }
   },
 
@@ -159,7 +160,8 @@ ToolbarView.prototype = {
   _onStepInPressed: function() {
     if (DebuggerController.activeThread.paused) {
       DebuggerController.StackFrames.currentFrameDepth = -1;
-      DebuggerController.activeThread.stepIn();
+      let warn = DebuggerController._ensureResumptionOrder;
+      DebuggerController.activeThread.stepIn(warn);
     }
   },
 
@@ -169,7 +171,8 @@ ToolbarView.prototype = {
   _onStepOutPressed: function() {
     if (DebuggerController.activeThread.paused) {
       DebuggerController.StackFrames.currentFrameDepth = -1;
-      DebuggerController.activeThread.stepOut();
+      let warn = DebuggerController._ensureResumptionOrder;
+      DebuggerController.activeThread.stepOut(warn);
     }
   },
 

toolkit/devtools/server/actors/script.js

@@ -261,19 +261,22 @@ BreakpointStore.prototype = {
  * @param nsIJSInspector inspector
  *        The underlying JS inspector we use to enter and exit nested event
  *        loops.
+ * @param ThreadActor thread
+ *        The thread actor instance that owns this EventLoopStack.
+ * @param DebuggerServerConnection connection
+ *        The remote protocol connection associated with this event loop stack.
  * @param Object hooks
  *        An object with the following properties:
  *          - url: The URL string of the debuggee we are spinning an event loop
  *                 for.
  *          - preNest: function called before entering a nested event loop
  *          - postNest: function called after exiting a nested event loop
- * @param ThreadActor thread
- *        The thread actor instance that owns this EventLoopStack.
  */
-function EventLoopStack({ inspector, thread, hooks }) {
+function EventLoopStack({ inspector, thread, connection, hooks }) {
   this._inspector = inspector;
   this._hooks = hooks;
   this._thread = thread;
+  this._connection = connection;
 }
 
 EventLoopStack.prototype = {
@@ -301,6 +304,14 @@ EventLoopStack.prototype = {
     return url;
   },
 
+  /**
+   * The DebuggerServerConnection of the debugger who pushed the event loop on
+   * top of the stack
+   */
+  get lastConnection() {
+    return this._inspector.lastNestRequestor._connection;
+  },
+
   /**
    * Push a new nested event loop onto the stack.
    *
@@ -310,6 +321,7 @@ EventLoopStack.prototype = {
     return new EventLoop({
       inspector: this._inspector,
       thread: this._thread,
+      connection: this._connection,
       hooks: this._hooks
     });
   }
@@ -323,14 +335,17 @@ EventLoopStack.prototype = {
  *        The JS Inspector that runs nested event loops.
  * @param ThreadActor thread
  *        The thread actor that is creating this nested event loop.
+ * @param DebuggerServerConnection connection
+ *        The remote protocol connection associated with this event loop.
  * @param Object hooks
  *        The same hooks object passed into EventLoopStack during its
  *        initialization.
  */
-function EventLoop({ inspector, thread, hooks }) {
+function EventLoop({ inspector, thread, connection, hooks }) {
   this._inspector = inspector;
   this._thread = thread;
   this._hooks = hooks;
+  this._connection = connection;
 
   this.enter = this.enter.bind(this);
   this.resolve = this.resolve.bind(this);
@@ -414,11 +429,6 @@ function ThreadActor(aHooks, aGlobal)
   this._frameActors = [];
   this._hooks = aHooks;
   this.global = aGlobal;
-  this._nestedEventLoops = new EventLoopStack({
-    inspector: DebuggerServer.xpcInspector,
-    hooks: aHooks,
-    thread: this
-  });
   // A map of actorID -> actor for breakpoints created and managed by the server.
   this._hiddenBreakpoints = new Map();
 
@@ -675,6 +685,15 @@ ThreadActor.prototype = {
 
     update(this._options, aRequest.options || {});
 
+    // Initialize an event loop stack. This can't be done in the constructor,
+    // because this.conn is not yet initialized by the actor pool at that time.
+    this._nestedEventLoops = new EventLoopStack({
+      inspector: DebuggerServer.xpcInspector,
+      hooks: this._hooks,
+      connection: this.conn,
+      thread: this
+    });
+
     if (!this.dbg) {
       this._initDebugger();
     }
@@ -992,7 +1011,8 @@ ThreadActor.prototype = {
     // different tabs or multiple debugger clients connected to the same tab)
     // only allow resumption in a LIFO order.
     if (this._nestedEventLoops.size && this._nestedEventLoops.lastPausedUrl
-        && this._nestedEventLoops.lastPausedUrl !== this._hooks.url) {
+        && (this._nestedEventLoops.lastPausedUrl !== this._hooks.url
+        || this._nestedEventLoops.lastConnection !== this.conn)) {
       return {
         error: "wrongOrder",
         message: "trying to resume in the wrong order.",

toolkit/devtools/server/tests/unit/test_nesting-01.js

@@ -6,15 +6,26 @@
 // ThreadActor.prototype.synchronize.
 
 const { defer } = devtools.require("sdk/core/promise");
+var gClient;
+var gThreadActor;
 
 function run_test() {
   initTestDebuggerServer();
+  let gDebuggee = addTestGlobal("test-nesting");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function () {
+    attachTestTabAndResume(gClient, "test-nesting", function (aResponse, aTabClient, aThreadClient) {
+      // Reach over the protocol connection and get a reference to the thread actor.
+      gThreadActor = aThreadClient._transport._serverConnection.getActor(aThreadClient._actor);
+
+      test_nesting();
+    });
+  });
   do_test_pending();
-  test_nesting();
 }
 
 function test_nesting() {
-  const thread = new DebuggerServer.ThreadActor(DebuggerServer);
+  const thread = gThreadActor;
   const { resolve, reject, promise } = defer();
 
   let currentStep = 0;
@@ -34,5 +45,5 @@ function test_nesting() {
   // There shouldn't be any nested event loops anymore
   do_check_eq(thread._nestedEventLoops.size, 0);
 
-  do_test_finished();
+  finishClient(gClient);
 }

toolkit/devtools/server/tests/unit/test_nesting-02.js

@@ -6,15 +6,26 @@
 // loops when requested.
 
 const { defer } = devtools.require("sdk/core/promise");
+var gClient;
+var gThreadActor;
 
 function run_test() {
   initTestDebuggerServer();
+  let gDebuggee = addTestGlobal("test-nesting");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function () {
+    attachTestTabAndResume(gClient, "test-nesting", function (aResponse, aTabClient, aThreadClient) {
+      // Reach over the protocol connection and get a reference to the thread actor.
+      gThreadActor = aThreadClient._transport._serverConnection.getActor(aThreadClient._actor);
+
+      test_nesting();
+    });
+  });
   do_test_pending();
-  test_nesting();
 }
 
 function test_nesting() {
-  const thread = new DebuggerServer.ThreadActor(DebuggerServer);
+  const thread = gThreadActor;
   const { resolve, reject, promise } = defer();
 
   // The following things should happen (in order):
@@ -64,5 +75,5 @@ function test_nesting() {
   // There shouldn't be any nested event loops anymore
   do_check_eq(thread._nestedEventLoops.size, 0);
 
-  do_test_finished();
+  finishClient(gClient);
 }

toolkit/devtools/server/tests/unit/test_nesting-03.js

@@ -0,0 +1,50 @@
+/* -*- Mode: javascript; js-indent-level: 2; -*- */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that we can detect nested event loops in tabs with the same URL.
+
+const { defer } = devtools.require("sdk/core/promise");
+var gClient1, gClient2;
+
+function run_test() {
+  initTestDebuggerServer();
+  addTestGlobal("test-nesting1");
+  addTestGlobal("test-nesting1");
+  // Conect the first client to the first debuggee.
+  gClient1 = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient1.connect(function () {
+    attachTestThread(gClient1, "test-nesting1", function (aResponse, aTabClient, aThreadClient) {
+      start_second_connection();
+    });
+  });
+  do_test_pending();
+}
+
+function start_second_connection() {
+  gClient2 = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient2.connect(function () {
+    attachTestThread(gClient2, "test-nesting1", function (aResponse, aTabClient, aThreadClient) {
+      test_nesting();
+    });
+  });
+}
+
+function test_nesting() {
+  const { resolve, reject, promise } = defer();
+
+  gClient1.activeThread.resume(aResponse => {
+    do_check_eq(aResponse.error, "wrongOrder");
+    gClient2.activeThread.resume(aResponse => {
+      do_check_true(!aResponse.error);
+      do_check_eq(aResponse.from, gClient2.activeThread.actor);
+
+      gClient1.activeThread.resume(aResponse => {
+        do_check_true(!aResponse.error);
+        do_check_eq(aResponse.from, gClient1.activeThread.actor);
+
+        gClient1.close(() => finishClient(gClient2));
+      });
+    });
+  });
+}

toolkit/devtools/server/tests/unit/testactors.js

@@ -71,6 +71,10 @@ TestTabActor.prototype = {
     return { wrappedJSObject: this._global };
   },
 
+  get url() {
+    return this._global.__name;
+  },
+
   form: function() {
     let response = { actor: this.actorID, title: this._global.__name };
 

toolkit/devtools/server/tests/unit/xpcshell.ini

@@ -18,6 +18,7 @@ support-files =
 
 [test_nesting-01.js]
 [test_nesting-02.js]
+[test_nesting-03.js]
 [test_forwardingprefix.js]
 [test_getyoungestframe.js]
 [test_nsjsinspector.js]