Bug 1529247 - remove unused releaseMany method from thread client and actor; r=ochameau
authoryulia <ystartsev@mozilla.com>
Tue, 12 Mar 2019 14:06:35 +0000
changeset 521555 aa554fe5d8fe
parent 521554 eba70db52fa5
child 521556 4cb9ab350bcb
push id10867
push userdvarga@mozilla.com
push dateThu, 14 Mar 2019 15:20:45 +0000
treeherdermozilla-beta@abad13547875 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1529247
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1529247 - remove unused releaseMany method from thread client and actor; r=ochameau There is one spot I am unsure about, and that is the test: devtools/server/tests/unit/test_threadlifetime-06.js -- should this be kept or do we want to remove it as well? Differential Revision: https://phabricator.services.mozilla.com/D21674
devtools/docs/backend/protocol.md
devtools/server/actors/thread.js
devtools/server/tests/unit/test_threadlifetime-03.js
devtools/server/tests/unit/test_threadlifetime-05.js
devtools/server/tests/unit/test_threadlifetime-06.js
devtools/server/tests/unit/xpcshell.ini
devtools/shared/client/thread-client.js
--- a/devtools/docs/backend/protocol.md
+++ b/devtools/docs/backend/protocol.md
@@ -96,17 +96,17 @@ where *name* is a JSON string naming wha
 If an actor receives a packet whose type it does not recognize, it sends an error reply of the form:
 
 ```
 { "from":actor, "error":"unrecognizedPacketType", "message":message }
 ```
 
 where *message* provides details to help debugger developers understand what went wrong: what kind of actor actor is; the packet received; and so on.
 
-If an actor receives a packet which is missing needed parameters (say, a `"releaseMany"` packet with no `"actors"` parameter), it sends an error reply of the form:
+If an actor receives a packet which is missing needed parameters (say, an `"autocomplete"` packet with no `"text"` parameter), it sends an error reply of the form:
 
 ```
 { "from":actor, "error":"missingParameter", "message":message }
 ```
 
 where *message* provides details to help debugger developers fix the problem.
 
 If an actor receives a packet with a parameter whose value is inappropriate for the operation, it sends an error reply of the form:
@@ -544,24 +544,16 @@ The grip actor will reply, simply:
 ```
 
 This closes the grip actor. The `"release"` packet may only be sent to thread-lifetime grip actors; if a pause-lifetime grip actor receives a `"release"` packet, it sends an error reply of the form:
 
 ```
 { "from":<gripActor>, "error":"notReleasable", "message":<message> }
 ```
 
-where *message* includes whatever further information would be useful to the debugger developers.
-
-The client can release many thread-lifetime grips in a single operation by sending the thread actor a request of the form:
-
-```
-{ "to":<thread>, "type":"releaseMany", "actors":[ <gripActor>, ... ] }
-```
-
 where each *gripActor* is the name of a child of *thread* that should be freed. The thread actor will reply, simply:
 
 ```
 { "from":<thread> }
 ```
 
 Regardless of the lifetime of a grip, the client may only send messages to object grip actors while the thread to which they belong is paused; the client's interaction with mutable values cannot take place concurrently with the thread.
 
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -200,17 +200,17 @@ const ThreadActor = ActorClassWithSpec(t
     }
     this._sources = null;
     this._scripts = null;
   },
 
   /**
    * Clean up listeners, debuggees and clear actor pools associated with
    * the lifetime of this actor. This does not destroy the thread actor,
-   * it resets it. This is used in methods `onReleaseMany` `onDetatch` and
+   * it resets it. This is used in methods `onDetatch` and
    * `exit`. The actor is truely destroyed in the `exit method`.
    */
   destroy: function() {
     dumpn("in ThreadActor.prototype.destroy");
     if (this._state == "paused") {
       this.onResume();
     }
 
@@ -1107,44 +1107,16 @@ const ThreadActor = ActorClassWithSpec(t
       }
       frames.push(frameItem);
     }
 
     // Filter null values because createSourceActor can be falsey
     return { frames: frames.filter(x => !!x) };
   },
 
-  onReleaseMany: function(request) {
-    if (!request.actors) {
-      return { error: "missingParameter",
-               message: "no actors were specified" };
-    }
-
-    let res;
-    for (const actorID of request.actors) {
-      const actor = this.threadLifetimePool.get(actorID);
-      if (!actor) {
-        if (!res) {
-          res = { error: "notReleasable",
-                  message: "Only thread-lifetime actors can be released." };
-        }
-        continue;
-      }
-
-      // We can still have old-style actors (e.g. object/long-string) in the pool, so we
-      // need to check onRelease existence.
-      if (actor.onRelease) {
-        actor.onRelease();
-      } else if (actor.destroy) {
-        actor.destroy();
-      }
-    }
-    return res ? res : {};
-  },
-
   onSources: function(request) {
     for (const source of this.dbg.findSources()) {
       this._addSource(source);
     }
 
     // No need to flush the new source packets here, as we are sending the
     // list of sources out immediately and we don't need to invoke the
     // overhead of an RDP packet for every source right now. Let the default
@@ -1767,17 +1739,16 @@ const ThreadActor = ActorClassWithSpec(t
 Object.assign(ThreadActor.prototype.requestTypes, {
   "attach": ThreadActor.prototype.onAttach,
   "detach": ThreadActor.prototype.onDetach,
   "reconfigure": ThreadActor.prototype.onReconfigure,
   "resume": ThreadActor.prototype.onResume,
   "clientEvaluate": ThreadActor.prototype.onClientEvaluate,
   "frames": ThreadActor.prototype.onFrames,
   "interrupt": ThreadActor.prototype.onInterrupt,
-  "releaseMany": ThreadActor.prototype.onReleaseMany,
   "sources": ThreadActor.prototype.onSources,
   "threadGrips": ThreadActor.prototype.onThreadGrips,
   "skipBreakpoints": ThreadActor.prototype.onSkipBreakpoints,
   "dumpThread": ThreadActor.prototype.onDump,
 });
 
 exports.ThreadActor = ThreadActor;
 
deleted file mode 100644
--- a/devtools/server/tests/unit/test_threadlifetime-03.js
+++ /dev/null
@@ -1,86 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-/* eslint-disable no-shadow, max-nested-callbacks */
-
-"use strict";
-
-/**
- * Check that thread-lifetime grips last past a resume.
- */
-
-var gDebuggee;
-var gClient;
-var gThreadClient;
-
-function run_test() {
-  Services.prefs.setBoolPref("security.allow_eval_with_system_principal", true);
-  registerCleanupFunction(() => {
-    Services.prefs.clearUserPref("security.allow_eval_with_system_principal");
-  });
-  initTestDebuggerServer();
-  gDebuggee = addTestGlobal("test-grips");
-  gClient = new DebuggerClient(DebuggerServer.connectPipe());
-  gClient.connect().then(function() {
-    attachTestTabAndResume(gClient, "test-grips",
-                           function(response, targetFront, threadClient) {
-                             gThreadClient = threadClient;
-                             test_thread_lifetime();
-                           });
-  });
-  do_test_pending();
-}
-
-function test_thread_lifetime() {
-  // Get three thread-lifetime grips.
-  gThreadClient.addOneTimeListener("paused", function(event, packet) {
-    const grips = [];
-
-    const handler = function(response) {
-      if (response.error) {
-        Assert.equal(response.error, "");
-        finishClient(gClient);
-      }
-      grips.push(response.from);
-      if (grips.length == 3) {
-        test_release_many(grips);
-      }
-    };
-    for (let i = 0; i < 3; i++) {
-      gClient.request({ to: packet.frame.arguments[i].actor, type: "threadGrip" },
-                      handler);
-    }
-  });
-
-  gDebuggee.eval("(" + function() {
-    function stopMe(arg1, arg2, arg3) {
-      debugger;
-    }
-    stopMe({obj: 1}, {obj: 2}, {obj: 3});
-  } + ")()");
-}
-
-function test_release_many(grips) {
-  // Release the first two grips, leave the third alive.
-
-  const release = [grips[0], grips[1]];
-
-  gThreadClient.releaseMany(release, function(response) {
-    // First two actors should return a noSuchActor error, because
-    // they're gone now.
-    gClient.request({ to: grips[0], type: "bogusRequest" }, function(response) {
-      Assert.equal(response.error, "noSuchActor");
-      gClient.request({ to: grips[1], type: "bogusRequest" }, function(response) {
-        Assert.equal(response.error, "noSuchActor");
-
-        // Last actor should return unrecognizedPacketType, because it's still
-        // alive.
-        gClient.request({ to: grips[2], type: "bogusRequest" }, function(response) {
-          Assert.equal(response.error, "unrecognizedPacketType");
-          gThreadClient.resume(function() {
-            finishClient(gClient);
-          });
-        });
-      });
-    });
-  });
-}
deleted file mode 100644
--- a/devtools/server/tests/unit/test_threadlifetime-05.js
+++ /dev/null
@@ -1,85 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-/**
- * 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() {
-  Services.prefs.setBoolPref("security.allow_eval_with_system_principal", true);
-  registerCleanupFunction(() => {
-    Services.prefs.clearUserPref("security.allow_eval_with_system_principal");
-  });
-  initTestDebuggerServer();
-  gDebuggee = addTestGlobal("test-grips");
-  gClient = new DebuggerClient(DebuggerServer.connectPipe());
-  gClient.connect().then(function() {
-    attachTestTabAndResume(gClient, "test-grips",
-                           function(response, targetFront, threadClient) {
-                             gThreadClient = threadClient;
-                             test_thread_lifetime();
-                           });
-  });
-  do_test_pending();
-}
-
-function arg_grips(frameArgs, onResponse) {
-  const grips = [];
-  const handler = function(response) {
-    if (response.error) {
-      grips.push(response.error);
-    } else {
-      grips.push(response.from);
-    }
-    if (grips.length == frameArgs.length) {
-      onResponse(grips);
-    }
-  };
-  for (let i = 0; i < frameArgs.length; i++) {
-    gClient.request({ to: frameArgs[i].actor, type: "threadGrip" },
-                    handler);
-  }
-}
-
-function test_thread_lifetime() {
-  // Get two thread-lifetime grips.
-  gThreadClient.addOneTimeListener("paused", function(event, packet) {
-    const frameArgs = [ packet.frame.arguments[0], packet.frame.arguments[1] ];
-    gPauseGrip = packet.frame.arguments[2];
-    arg_grips(frameArgs, function(grips) {
-      release_grips(frameArgs, grips);
-    });
-  });
-
-  gDebuggee.eval("(" + function() {
-    function stopMe(arg1, arg2, arg3) {
-      debugger;
-    }
-    stopMe({obj: 1}, {obj: 2}, {obj: 3});
-  } + ")()");
-}
-
-function release_grips(frameArgs, threadGrips) {
-  // Release all actors with releaseMany...
-  const release = [threadGrips[0], threadGrips[1], gPauseGrip.actor];
-  gThreadClient.releaseMany(release, function(response) {
-    Assert.equal(response.error, "notReleasable");
-    // Now ask for thread grips again, they should not exist.
-    arg_grips(frameArgs, function(newGrips) {
-      for (let i = 0; i < newGrips.length; i++) {
-        Assert.equal(newGrips[i], "noSuchActor");
-      }
-      gThreadClient.resume(function() {
-        finishClient(gClient);
-      });
-    });
-  });
-}
deleted file mode 100644
--- a/devtools/server/tests/unit/test_threadlifetime-06.js
+++ /dev/null
@@ -1,77 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-/* eslint-disable no-shadow, max-nested-callbacks */
-
-"use strict";
-
-/**
- * Check that promoting multiple pause-lifetime actors to thread-lifetime actors
- * works as expected.
- */
-
-var gDebuggee;
-var gClient;
-var gThreadClient;
-
-function run_test() {
-  Services.prefs.setBoolPref("security.allow_eval_with_system_principal", true);
-  registerCleanupFunction(() => {
-    Services.prefs.clearUserPref("security.allow_eval_with_system_principal");
-  });
-  initTestDebuggerServer();
-  gDebuggee = addTestGlobal("test-grips");
-  gClient = new DebuggerClient(DebuggerServer.connectPipe());
-  gClient.connect().then(function() {
-    attachTestTabAndResume(gClient, "test-grips",
-                           function(response, targetFront, threadClient) {
-                             gThreadClient = threadClient;
-                             test_thread_lifetime();
-                           });
-  });
-  do_test_pending();
-}
-
-function test_thread_lifetime() {
-  gThreadClient.addOneTimeListener("paused", function(event, packet) {
-    const actors = [];
-    let last;
-    for (const grip of packet.frame.arguments) {
-      actors.push(grip.actor);
-      last = grip.actor;
-    }
-
-    // Create thread-lifetime actors for these objects.
-    gThreadClient.threadGrips(actors, function(response) {
-      // Successful promotion won't return an error.
-      Assert.equal(response.error, undefined);
-
-      gThreadClient.addOneTimeListener("paused", function(event, packet) {
-        // Verify that the promoted actors are returned again.
-        actors.forEach(function(actor, i) {
-          Assert.equal(actor, packet.frame.arguments[i].actor);
-        });
-        // Now that we've resumed, release the thread-lifetime grips.
-        gThreadClient.releaseMany(actors, function(response) {
-          // Successful release won't return an error.
-          Assert.equal(response.error, undefined);
-
-          gClient.request({ to: last, type: "bogusRequest" }, function(response) {
-            Assert.equal(response.error, "noSuchActor");
-            gThreadClient.resume(function(response) {
-              finishClient(gClient);
-            });
-          });
-        });
-      });
-      gThreadClient.resume();
-    });
-  });
-
-  gDebuggee.eval("(" + function() {
-    function stopMe(arg1, arg2, arg3) {
-      debugger;
-      debugger;
-    }
-    stopMe({obj: 1}, {obj: 2}, {obj: 3});
-  } + ")()");
-}
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -69,20 +69,17 @@ support-files =
 [test_getRuleText.js]
 [test_getTextAtLineColumn.js]
 [test_pauselifetime-01.js]
 [test_pauselifetime-02.js]
 [test_pauselifetime-03.js]
 [test_pauselifetime-04.js]
 [test_threadlifetime-01.js]
 [test_threadlifetime-02.js]
-[test_threadlifetime-03.js]
 [test_threadlifetime-04.js]
-[test_threadlifetime-05.js]
-[test_threadlifetime-06.js]
 [test_functiongrips-01.js]
 [test_front_destroy.js]
 [test_nativewrappers.js]
 [test_nodelistactor.js]
 [test_eval-01.js]
 [test_eval-02.js]
 [test_eval-03.js]
 [test_eval-04.js]
--- a/devtools/shared/client/thread-client.js
+++ b/devtools/shared/client/thread-client.js
@@ -341,29 +341,16 @@ ThreadClient.prototype = {
   }, {
     after: function(response) {
       this.client.unregisterClient(this);
       return response;
     },
   }),
 
   /**
-   * 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 actors
-   *        An array with actor IDs to release.
-   */
-  releaseMany: DebuggerClient.requester({
-    type: "releaseMany",
-    actors: arg(0),
-  }),
-
-  /**
    * Promote multiple pause-lifetime object actors to thread-lifetime ones.
    *
    * @param array actors
    *        An array with actor IDs to promote.
    */
   threadGrips: DebuggerClient.requester({
     type: "threadGrips",
     actors: arg(0),