Bug 795263 - Actor names should not change when promoted to thread-lifetime; r=rcampbell
authorPanos Astithas <past@mozilla.com>
Fri, 12 Oct 2012 11:26:49 +0300
changeset 110257 a8f4069538a6a93d0d82565bb808811e8a3835e2
parent 109899 ec10630b1a5406c28d1ac84bd314938374404d04
child 110258 b07f4a276e76b45589b9cd78192c977aea05359e
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersrcampbell
bugs795263
milestone19.0a1
Bug 795263 - Actor names should not change when promoted to thread-lifetime; r=rcampbell
toolkit/devtools/debugger/dbg-client.jsm
toolkit/devtools/debugger/server/dbg-browser-actors.js
toolkit/devtools/debugger/server/dbg-script-actors.js
toolkit/devtools/debugger/server/dbg-server.js
toolkit/devtools/debugger/tests/unit/test_threadlifetime-01.js
toolkit/devtools/debugger/tests/unit/test_threadlifetime-02.js
toolkit/devtools/debugger/tests/unit/test_threadlifetime-03.js
toolkit/devtools/debugger/tests/unit/test_threadlifetime-04.js
toolkit/devtools/debugger/tests/unit/test_threadlifetime-05.js
--- a/toolkit/devtools/debugger/dbg-client.jsm
+++ b/toolkit/devtools/debugger/dbg-client.jsm
@@ -376,23 +376,26 @@ DebuggerClient.prototype = {
     });
   },
 
   /**
    * Release an object actor.
    *
    * @param string aActor
    *        The actor ID to send the request to.
+   * @param aOnResponse function
+   *        If specified, will be called with the response packet when
+   *        debugging server responds.
    */
-  release: function DC_release(aActor) {
+  release: function DC_release(aActor, aOnResponse) {
     let packet = {
       to: aActor,
       type: "release",
     };
-    this.request(packet);
+    this.request(packet, aOnResponse);
   },
 
   /**
    * Send a request to the debugging server.
    *
    * @param aRequest object
    *        A JSON packet to send to the debugging server.
    * @param aOnResponse function
@@ -782,16 +785,33 @@ ThreadClient.prototype = {
         aOnResponse(aResponse);
         return;
       }
       doSetBreakpoint(this.resume.bind(this));
     }.bind(this));
   },
 
   /**
+   * Release multiple thread-lifetime object actors. If any pause-lifetime
+   * actors are included in the request, a |notReleasable| error will return,
+   * but all the thread-lifetime ones will have been released.
+   *
+   * @param array aActors
+   *        An array with actor IDs to release.
+   */
+  releaseMany: function TC_releaseMany(aActors, aOnResponse) {
+    let packet = {
+      to: this._actor,
+      type: "releaseMany",
+      actors: aActors
+    };
+    this._client.request(packet, aOnResponse);
+  },
+
+  /**
    * Request the loaded scripts for the current thread.
    *
    * @param aOnResponse integer
    *        Called with the thread's response.
    */
   getScripts: function TC_getScripts(aOnResponse) {
     let packet = { to: this._actor, type: "scripts" };
     this._client.request(packet, aOnResponse);
--- a/toolkit/devtools/debugger/server/dbg-browser-actors.js
+++ b/toolkit/devtools/debugger/server/dbg-browser-actors.js
@@ -275,18 +275,18 @@ BrowserTabActor.prototype = {
    */
   addToBreakpointPool: function BTA_addToBreakpointPool(aActor) {
     this.conn.addActor(aActor);
   },
 
   /**
    * Remove the specified breakpint from the default actor pool.
    *
-   * @param string aActor
-   *        The actor ID.
+   * @param BreakpointActor aActor
+   *        The actor object.
    */
   removeFromBreakpointPool: function BTA_removeFromBreakpointPool(aActor) {
     this.conn.removeActor(aActor);
   },
 
   // A constant prefix that will be used to form the actor ID by the server.
   actorPrefix: "tab",
 
--- a/toolkit/devtools/debugger/server/dbg-script-actors.js
+++ b/toolkit/devtools/debugger/server/dbg-script-actors.js
@@ -372,22 +372,29 @@ ThreadActor.prototype = {
   },
 
   onReleaseMany: function TA_onReleaseMany(aRequest) {
     if (!aRequest.actors) {
       return { error: "missingParameter",
                message: "no actors were specified" };
     }
 
+    let res;
     for each (let actorID in aRequest.actors) {
       let actor = this.threadLifetimePool.get(actorID);
-      this.threadLifetimePool.objectActors.delete(actor.obj);
-      this.threadLifetimePool.removeActor(actorID);
+      if (!actor) {
+        if (!res) {
+          res = { error: "notReleasable",
+                  message: "Only thread-lifetime actors can be released." };
+        }
+        continue;
+      }
+      actor.onRelease();
     }
-    return {};
+    return res ? res : {};
   },
 
   /**
    * Handle a protocol request to set a breakpoint.
    */
   onSetBreakpoint: function TA_onSetBreakpoint(aRequest) {
     if (this.state !== "paused") {
       return { error: "wrongState",
@@ -867,23 +874,30 @@ ThreadActor.prototype = {
     if (!this._pausePool) {
       throw "Object grip requested while not paused.";
     }
 
     return this.objectGrip(aValue, this._pausePool);
   },
 
   /**
-   * Create a grip for the given debuggee object with a thread lifetime.
+   * Extend the lifetime of the provided object actor to thread lifetime.
    *
-   * @param aValue Debugger.Object
-   *        The debuggee object value.
+   * @param aActor object
+   *        The object actor.
    */
-  threadObjectGrip: function TA_threadObjectGrip(aValue) {
-    return this.objectGrip(aValue, this.threadLifetimePool);
+  threadObjectGrip: function TA_threadObjectGrip(aActor) {
+    if (!this.threadLifetimePool.objectActors) {
+      this.threadLifetimePool.objectActors = new WeakMap();
+    }
+    // We want to reuse the existing actor ID, so we just remove it from the
+    // current pool's weak map and then let pool.addActor do the rest.
+    aActor.registeredPool.objectActors.delete(aActor.obj);
+    this.threadLifetimePool.addActor(aActor);
+    this.threadLifetimePool.objectActors.set(aActor.obj, aActor);
   },
 
   /**
    * Create a grip for the given string.
    *
    * @param aString String
    *        The string we are creating a grip for.
    * @param aPool ActorPool
@@ -1376,17 +1390,17 @@ update(ObjectActor.prototype, {
              "actor": this.actorID };
   },
 
   /**
    * Releases this actor from the pool.
    */
   release: function OA_release() {
     this.registeredPool.objectActors.delete(this.obj);
-    this.registeredPool.removeActor(this.actorID);
+    this.registeredPool.removeActor(this);
   },
 
   /**
    * Handle a protocol request to provide the names of the properties defined on
    * the object and not its prototype.
    *
    * @param aRequest object
    *        The protocol request object.
@@ -1538,29 +1552,30 @@ update(ObjectActor.prototype, {
   /**
    * Handle a protocol request to promote a pause-lifetime grip to a
    * thread-lifetime grip.
    *
    * @param aRequest object
    *        The protocol request object.
    */
   onThreadGrip: PauseScopedActor.withPaused(function OA_onThreadGrip(aRequest) {
-    return { threadGrip: this.threadActor.threadObjectGrip(this.obj) };
+    this.threadActor.threadObjectGrip(this);
+    return {};
   }),
 
   /**
    * Handle a protocol request to release a thread-lifetime grip.
    *
    * @param aRequest object
    *        The protocol request object.
    */
   onRelease: PauseScopedActor.withPaused(function OA_onRelease(aRequest) {
     if (this.registeredPool !== this.threadActor.threadLifetimePool) {
       return { error: "notReleasable",
-               message: "only thread-lifetime actors can be released." };
+               message: "Only thread-lifetime actors can be released." };
     }
 
     this.release();
     return {};
   }),
 });
 
 ObjectActor.prototype.requestTypes = {
@@ -1792,17 +1807,17 @@ BreakpointActor.prototype = {
    * @param aRequest object
    *        The protocol request object.
    */
   onDelete: function BA_onDelete(aRequest) {
     // 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.removeFromBreakpointPool(this.actorID);
+    this.threadActor._hooks.removeFromBreakpointPool(this);
     for (let script of this.scripts) {
       script.clearBreakpoint(this);
     }
     this.scripts = null;
 
     return { from: this.actorID };
   }
 };
--- a/toolkit/devtools/debugger/server/dbg-server.js
+++ b/toolkit/devtools/debugger/server/dbg-server.js
@@ -386,19 +386,19 @@ ActorPool.prototype = {
    */
   isEmpty: function AP_isEmpty() {
     return Object.keys(this._actors).length == 0;
   },
 
   /**
    * Remove an actor from the actor pool.
    */
-  removeActor: function AP_remove(aActorID) {
-    delete this._actors[aActorID];
-    delete this._cleanups[aActorID];
+  removeActor: function AP_remove(aActor) {
+    delete this._actors[aActor.actorID];
+    delete this._cleanups[aActor.actorID];
   },
 
   /**
    * Run all cleanups previously registered with addCleanup.
    */
   cleanup: function AP_cleanup() {
     for each (let actor in this._cleanups) {
       actor.disconnect();
--- a/toolkit/devtools/debugger/tests/unit/test_threadlifetime-01.js
+++ b/toolkit/devtools/debugger/tests/unit/test_threadlifetime-01.js
@@ -23,33 +23,27 @@ function run_test()
   do_test_pending();
 }
 
 function test_thread_lifetime()
 {
   gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
     let pauseGrip = aPacket.frame.arguments[0];
 
+    // Create a thread-lifetime actor for this object.
     gClient.request({ to: pauseGrip.actor, type: "threadGrip" }, function(aResponse) {
-      let threadGrip = aResponse.threadGrip;
-
-      do_check_neq(pauseGrip.actor, threadGrip.actor);
-
-      // Create a thread-lifetime actor for this object.
-
+      // Successful promotion won't return an error.
+      do_check_eq(aResponse.error, undefined);
       gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
-        // Now that we've resumed, should get noSuchActor for the
-        // pause grip, but unrecognizePacketType for the thread grip.
-        gClient.request({ to: pauseGrip.actor, type: "bogusRequest" }, function(aResponse) {
-          do_check_eq(aResponse.error, "noSuchActor");
-          gClient.request({ to: threadGrip.actor, type: "bogusRequest"}, function(aResponse) {
-            do_check_eq(aResponse.error, "unrecognizedPacketType");
-            gThreadClient.resume(function() {
-              finishClient(gClient);
-            });
+        // Now that we've resumed, should get unrecognizePacketType for the
+        // promoted grip.
+        gClient.request({ to: pauseGrip.actor, type: "bogusRequest"}, function(aResponse) {
+          do_check_eq(aResponse.error, "unrecognizedPacketType");
+          gThreadClient.resume(function() {
+            finishClient(gClient);
           });
         });
       });
       gThreadClient.resume();
     });
   });
 
   gDebuggee.eval("(" + function() {
--- a/toolkit/devtools/debugger/tests/unit/test_threadlifetime-02.js
+++ b/toolkit/devtools/debugger/tests/unit/test_threadlifetime-02.js
@@ -23,23 +23,24 @@ function run_test()
   do_test_pending();
 }
 
 function test_thread_lifetime()
 {
   gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
     let pauseGrip = aPacket.frame.arguments[0];
 
+    // Create a thread-lifetime actor for this object.
     gClient.request({ to: pauseGrip.actor, type: "threadGrip" }, function(aResponse) {
-      let threadGrip = aResponse.threadGrip;
-      // Create a thread-lifetime actor for this object.
+      // Successful promotion won't return an error.
+      do_check_eq(aResponse.error, undefined);
       gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
         // Now that we've resumed, release the thread-lifetime grip.
-        gClient.request({ to: threadGrip.actor, type: "release" }, function(aResponse) {
-          gClient.request({ to: threadGrip.actor, type: "bogusRequest" }, function(aResponse) {
+        gClient.release(pauseGrip.actor, function(aResponse) {
+          gClient.request({ to: pauseGrip.actor, type: "bogusRequest" }, function(aResponse) {
             do_check_eq(aResponse.error, "noSuchActor");
             gThreadClient.resume(function(aResponse) {
               finishClient(gClient);
             });
           });
         });
       });
       gThreadClient.resume();
--- a/toolkit/devtools/debugger/tests/unit/test_threadlifetime-03.js
+++ b/toolkit/devtools/debugger/tests/unit/test_threadlifetime-03.js
@@ -26,17 +26,21 @@ function run_test()
 function test_thread_lifetime()
 {
   // Get three thread-lifetime grips.
   gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
     aPacket.frame.arguments[0];
     let grips = [];
 
     let handler = function(aResponse) {
-      grips.push(aResponse.threadGrip);
+      if (aResponse.error) {
+        do_check_eq(aResponse.error, '');
+        finishClient(gClient);
+      }
+      grips.push(aResponse.from);
       if (grips.length == 3) {
         test_release_many(grips);
       }
     };
     for (let i = 0; i < 3; i++) {
       gClient.request({ to: aPacket.frame.arguments[i].actor, type: "threadGrip" },
                       handler);
     }
@@ -50,29 +54,29 @@ function test_thread_lifetime()
     ")"
   } + ")()");
 }
 
 function test_release_many(grips)
 {
   // Release the first two grips, leave the third alive.
 
-  let release = [grips[0].actor, grips[1].actor];
+  let release = [grips[0], grips[1]];
 
-  gClient.request({ to: gThreadClient.actor, type: "releaseMany", "actors": release }, function(aResponse) {
+  gThreadClient.releaseMany(release, function(aResponse) {
     // First two actors should return a noSuchActor error, because
     // they're gone now.
-    gClient.request({ to: grips[0].actor, type: "bogusRequest" }, function(aResponse) {
+    gClient.request({ to: grips[0], type: "bogusRequest" }, function(aResponse) {
       do_check_eq(aResponse.error, "noSuchActor");
-      gClient.request({ to: grips[1].actor, type: "bogusRequest" }, function(aResponse) {
+      gClient.request({ to: grips[1], type: "bogusRequest" }, function(aResponse) {
         do_check_eq(aResponse.error, "noSuchActor");
 
         // Last actor should return unrecognizedPacketType, because it's still
         // alive.
-        gClient.request({ to: grips[2].actor, type: "bogusRequest" }, function(aResponse) {
+        gClient.request({ to: grips[2], type: "bogusRequest" }, function(aResponse) {
           do_check_eq(aResponse.error, "unrecognizedPacketType");
           gThreadClient.resume(function() {
             finishClient(gClient);
           });
         });
       });
     });
   });
--- a/toolkit/devtools/debugger/tests/unit/test_threadlifetime-04.js
+++ b/toolkit/devtools/debugger/tests/unit/test_threadlifetime-04.js
@@ -25,22 +25,23 @@ function run_test()
 }
 
 function test_thread_lifetime()
 {
   gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
     let pauseGrip = aPacket.frame.arguments[0];
 
     gClient.request({ to: pauseGrip.actor, type: "threadGrip" }, function (aResponse) {
-      let threadGrip1 = aResponse.threadGrip;
+      // Successful promotion won't return an error.
+      do_check_eq(aResponse.error, undefined);
 
-      do_check_neq(pauseGrip.actor, threadGrip1.actor);
+      let threadGrip1 = aResponse.from;
 
       gClient.request({ to: pauseGrip.actor, type: "threadGrip" }, function (aResponse) {
-        do_check_eq(threadGrip1.actor, aResponse.threadGrip.actor);
+        do_check_eq(threadGrip1, aResponse.from);
         gThreadClient.resume(function() {
           finishClient(gClient);
         });
       });
     });
   });
 
   gDebuggee.eval("(" + function() {
--- a/toolkit/devtools/debugger/tests/unit/test_threadlifetime-05.js
+++ b/toolkit/devtools/debugger/tests/unit/test_threadlifetime-05.js
@@ -1,19 +1,20 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
- * Make sure that an actor will not be reused after a release or
- * releaseMany.
+ * Make sure that releasing a pause-lifetime actorin a releaseMany returns an
+ * error, but releases all the thread-lifetime actors.
  */
 
 var gDebuggee;
 var gClient;
 var gThreadClient;
+var gPauseGrip;
 
 function run_test()
 {
   initTestDebuggerServer();
   gDebuggee = addTestGlobal("test-grips");
   gClient = new DebuggerClient(DebuggerServer.connectPipe());
   gClient.connect(function() {
     attachTestGlobalClientAndResume(gClient, "test-grips", function(aResponse, aThreadClient) {
@@ -22,33 +23,38 @@ function run_test()
     });
   });
   do_test_pending();
 }
 
 function arg_grips(aFrameArgs, aOnResponse) {
   let grips = [];
   let handler = function (aResponse) {
-    grips.push(aResponse.threadGrip);
+    if (aResponse.error) {
+      grips.push(aResponse.error);
+    } else {
+      grips.push(aResponse.from);
+    }
     if (grips.length == aFrameArgs.length) {
       aOnResponse(grips);
     }
   };
   for (let i = 0; i < aFrameArgs.length; i++) {
     gClient.request({ to: aFrameArgs[i].actor, type: "threadGrip" },
                     handler);
   }
 }
 
 function test_thread_lifetime()
 {
-  // Get three thread-lifetime grips.
+  // Get two thread-lifetime grips.
   gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
 
-    let frameArgs = aPacket.frame.arguments;
+    let frameArgs = [ aPacket.frame.arguments[0], aPacket.frame.arguments[1] ];
+    gPauseGrip = aPacket.frame.arguments[2];
     arg_grips(frameArgs, function (aGrips) {
       release_grips(frameArgs, aGrips);
     });
   });
 
   gDebuggee.eval("(" + function() {
     function stopMe(arg1, arg2, arg3) {
       debugger;
@@ -56,25 +62,23 @@ function test_thread_lifetime()
     stopMe({obj: 1}, {obj: 2}, {obj: 3});
     ")"
   } + ")()");
 }
 
 
 function release_grips(aFrameArgs, aThreadGrips)
 {
-  // Release the first grip with release, and the second two with releaseMany...
-
-  gClient.request({ to: aThreadGrips[0].actor, type: "release" }, function (aResponse) {
-    let release = [aThreadGrips[1].actor, aThreadGrips[2].actor];
-    gClient.request({ to: gThreadClient.actor, type: "releaseMany", "actors": release }, function (aResponse) {
-      // Now ask for thread grips again, they should be different.
-      arg_grips(aFrameArgs, function (aNewGrips) {
-        for (let i = 0; i < aNewGrips.length; i++) {
-          do_check_neq(aThreadGrips[i].actor, aNewGrips[i].actor);
-        }
-        gThreadClient.resume(function () {
-          finishClient(gClient);
-        });
+  // Release all actors with releaseMany...
+  let release = [aThreadGrips[0], aThreadGrips[1], gPauseGrip.actor];
+  gThreadClient.releaseMany(release, function (aResponse) {
+    do_check_eq(aResponse.error, "notReleasable");
+    // Now ask for thread grips again, they should not exist.
+    arg_grips(aFrameArgs, function (aNewGrips) {
+      for (let i = 0; i < aNewGrips.length; i++) {
+        do_check_eq(aNewGrips[i], "noSuchActor");
+      }
+      gThreadClient.resume(function () {
+        finishClient(gClient);
       });
     });
   });
 }