Bug 1315391 - Rename all `disconnect` methods to `destroy` in actors. r=ochameau

Ever since protocol.js was added as a way to create DevTools actors, we've had
lots of confusion about the correct way to implement actor destruction.  If your
actor's _parent_ was the legacy kind, you had to use `disconnect`.  If it was
protocol.js, you had to use `destroy`.

There is no reason for this madness, which makes reasoning about destruction
quite hard.  Here we rename `disconnect` to `destroy` so there is only one name
for every destruction path.

MozReview-Commit-ID: C1Yw9NfUUR2

--HG--
extra : rebase_source : 4d018622b7547d404510e0b563c6324c0127aafc

devtools/server/actors/addon.js

@@ -70,7 +70,6 @@ BrowserAddonActor.prototype = {
     return this._sources;
   },
 
-
   form: function BAA_form() {
     assert(this.actorID, "addon should have an actorID.");
     if (!this._consoleActor) {
@@ -95,7 +94,7 @@ BrowserAddonActor.prototype = {
     };
   },
 
-  disconnect: function BAA_disconnect() {
+  destroy() {
     this.conn.removeActorPool(this._contextPool);
     this._contextPool = null;
     this._consoleActor = null;
@@ -140,7 +139,7 @@ BrowserAddonActor.prototype = {
       this.conn.send({ from: this.actorID, type: "tabDetached" });
     }
 
-    this.disconnect();
+    this.destroy();
   },
 
   onAttach: function BAA_onAttach() {
@@ -309,9 +308,8 @@ update(AddonConsoleActor.prototype, {
   /**
    * Destroy the current AddonConsoleActor instance.
    */
-  disconnect: function ACA_disconnect()
-  {
-    WebConsoleActor.prototype.disconnect.call(this);
+  destroy() {
+    WebConsoleActor.prototype.destroy.call(this);
     this.addon = null;
   },
 

devtools/server/actors/animation.js

@@ -497,14 +497,6 @@ var AnimationsActor = exports.AnimationsActor = protocol.ActorClassWithSpec(anim
     this.tabActor = this.observer = this.actors = this.walker = null;
   },
 
-  /**
-   * Since AnimationsActor doesn't have a protocol.js parent actor that takes
-   * care of its lifetime, implementing disconnect is required to cleanup.
-   */
-  disconnect: function () {
-    this.destroy();
-  },
-
   /**
    * Clients can optionally call this with a reference to their WalkerActor.
    * If they do, then AnimationPlayerActor's forms are going to also include

devtools/server/actors/breakpoint.js

@@ -54,7 +54,7 @@ let BreakpointActor = ActorClassWithSpec(breakpointSpec, {
     this.isPending = true;
   },
 
-  disconnect: function () {
+  destroy: function () {
     this.removeScripts();
   },
 

devtools/server/actors/child-process.js

@@ -121,7 +121,7 @@ ChildProcessActor.prototype = {
     this._workerList.onListChanged = null;
   },
 
-  disconnect: function () {
+  destroy: function () {
     this.conn.removeActorPool(this._contextPool);
     this._contextPool = null;
 

devtools/server/actors/common.js

@@ -254,13 +254,17 @@ ActorPool.prototype = {
   },
 
   /**
-   * Remove an actor from the pool. If the actor has a disconnect method, call
-   * it.
+   * Remove an actor from the pool. If the actor has a destroy method, call it.
    */
-  removeActor: function AP_remove(aActor) {
-    delete this._actors[aActor.actorID];
-    if (aActor.disconnect) {
-      aActor.disconnect();
+  removeActor(actor) {
+    delete this._actors[actor.actorID];
+    if (actor.destroy) {
+      actor.destroy();
+      return;
+    }
+    // Obsolete destruction method name (might still be used by custom actors)
+    if (actor.disconnect) {
+      actor.disconnect();
     }
   },
 

devtools/server/actors/emulation.js

@@ -31,10 +31,6 @@ let EmulationActor = protocol.ActorClassWithSpec(emulationSpec, {
     this.simulatorCore = new SimulatorCore(tabActor.chromeEventHandler);
   },
 
-  disconnect() {
-    this.destroy();
-  },
-
   destroy() {
     this.clearDPPXOverride();
     this.clearNetworkThrottling();

devtools/server/actors/frame.js

@@ -44,7 +44,7 @@ let FrameActor = ActorClassWithSpec(frameSpec, {
    * Finalization handler that is called when the actor is being evicted from
    * the pool.
    */
-  disconnect: function () {
+  destroy: function () {
     this.conn.removeActorPool(this._frameLifetimePool);
     this._frameLifetimePool = null;
   },

devtools/server/actors/gcli.js

@@ -25,10 +25,6 @@ const GcliActor = ActorClassWithSpec(gcliSpec, {
     this._requisitionPromise = undefined; // see _getRequisition()
   },
 
-  disconnect: function () {
-    return this.destroy();
-  },
-
   destroy: function () {
     Actor.prototype.destroy.call(this);
 

devtools/server/actors/inspector.js

@@ -2641,12 +2641,6 @@ exports.InspectorActor = protocol.ActorClassWithSpec(inspectorSpec, {
     this.tabActor = null;
   },
 
-  // Forces destruction of the actor and all its children
-  // like highlighter, walker and style actors.
-  disconnect: function () {
-    this.destroy();
-  },
-
   get window() {
     return this.tabActor.window;
   },

devtools/server/actors/monitor.js

@@ -48,7 +48,7 @@ MonitorActor.prototype = {
     return {};
   },
 
-  disconnect: function () {
+  destroy: function () {
     this.stop();
   },
 

devtools/server/actors/object.js

@@ -2081,9 +2081,9 @@ function LongStringActor(string) {
 LongStringActor.prototype = {
   actorPrefix: "longString",
 
-  disconnect: function () {
+  destroy: function () {
     // Because longStringActors is not a weak map, we won't automatically leave
-    // it so we need to manually leave on disconnect so that we don't leak
+    // it so we need to manually leave on destroy so that we don't leak
     // memory.
     this._releaseActor();
   },

devtools/server/actors/performance-entries.js

@@ -42,10 +42,6 @@ var PerformanceEntriesActor = ActorClassWithSpec(performanceSpec, {
     }
   },
 
-  disconnect: function () {
-    this.destroy();
-  },
-
   destroy: function () {
     this.stop();
     Actor.prototype.destroy.call(this);

devtools/server/actors/performance.js

@@ -53,14 +53,6 @@ var PerformanceActor = ActorClassWithSpec(performanceSpec, {
     events.on(this.bridge, "*", this._onRecorderEvent);
   },
 
-  /**
-   * `disconnect` method required to call destroy, since this
-   * actor is not managed by a parent actor.
-   */
-  disconnect: function () {
-    this.destroy();
-  },
-
   destroy: function () {
     events.off(this.bridge, "*", this._onRecorderEvent);
     this.bridge.destroy();

devtools/server/actors/profiler.js

@@ -25,14 +25,6 @@ var ProfilerActor = exports.ProfilerActor = ActorClassWithSpec(profilerSpec, {
     events.on(this.bridge, "*", this._onProfilerEvent);
   },
 
-  /**
-   * `disconnect` method required to call destroy, since this
-   * actor is not managed by a parent actor.
-   */
-  disconnect: function () {
-    this.destroy();
-  },
-
   destroy: function () {
     events.off(this.bridge, "*", this._onProfilerEvent);
     this.bridge.destroy();

devtools/server/actors/reflow.js

@@ -46,15 +46,6 @@ var ReflowActor = exports.ReflowActor = protocol.ActorClassWithSpec(reflowSpec,
     this._isStarted = false;
   },
 
-  /**
-   * The reflow actor is the first (and last) in its hierarchy to use
-   * protocol.js so it doesn't have a parent protocol actor that takes care of
-   * its lifetime. So it needs a disconnect method to cleanup.
-   */
-  disconnect: function () {
-    this.destroy();
-  },
-
   destroy: function () {
     this.stop();
     releaseLayoutChangesObserver(this.tabActor);

devtools/server/actors/root.js

@@ -47,7 +47,7 @@ loader.lazyGetter(this, "ppmm", () => {
  *       reply whose value is the name of an actor constructed by
  *       |A[P]|.
  *
- *     - onShutdown: a function to call when the root actor is disconnected.
+ *     - onShutdown: a function to call when the root actor is destroyed.
  *
  * Instance properties:
  *
@@ -80,7 +80,7 @@ loader.lazyGetter(this, "ppmm", () => {
  * The root actor registers an 'onListChanged' handler on the appropriate
  * list when it may need to send the client 'tabListChanged' notifications,
  * and is careful to remove the handler whenever it does not need to send
- * such notifications (including when it is disconnected). This means that
+ * such notifications (including when it is destroyed). This means that
  * live list implementations can use the state of the handler property (set
  * or null) to install and remove observers and event listeners.
  *
@@ -206,9 +206,9 @@ RootActor.prototype = {
   },
 
   /**
-   * Disconnects the actor from the browser window.
+   * Destroys the actor from the browser window.
    */
-  disconnect: function () {
+  destroy: function () {
     /* Tell the live lists we aren't watching any more. */
     if (this._parameters.tabList) {
       this._parameters.tabList.onListChanged = null;

devtools/server/actors/script.js

@@ -561,8 +561,8 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     });
   },
 
-  disconnect: function () {
-    dumpn("in ThreadActor.prototype.disconnect");
+  destroy: function () {
+    dumpn("in ThreadActor.prototype.destroy");
     if (this._state == "paused") {
       this.onResume();
     }
@@ -592,10 +592,10 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
   },
 
   /**
-   * Disconnect the debugger and put the actor in the exited state.
+   * destroy the debugger and put the actor in the exited state.
    */
   exit: function () {
-    this.disconnect();
+    this.destroy();
     this._state = "exited";
   },
 
@@ -654,7 +654,7 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
   },
 
   onDetach: function (aRequest) {
-    this.disconnect();
+    this.destroy();
     this._state = "detached";
     this._debuggerSourcesSeen = null;
 

devtools/server/actors/source.js

@@ -226,7 +226,7 @@ let SourceActor = ActorClassWithSpec(sourceSpec, {
     };
   },
 
-  disconnect: function () {
+  destroy: function () {
     if (this.registeredPool && this.registeredPool.sourceActors) {
       delete this.registeredPool.sourceActors[this.actorID];
     }

devtools/server/actors/stylesheets.js

@@ -258,14 +258,6 @@ var StyleSheetActor = protocol.ActorClassWithSpec(styleSheetSpec, {
     }
   },
 
-  /**
-   * Since StyleSheetActor doesn't have a protocol.js parent actor that take
-   * care of its lifetime, implementing disconnect is required to cleanup.
-   */
-  disconnect: function () {
-    this.destroy();
-  },
-
   initialize: function (aStyleSheet, aParentActor, aWindow) {
     protocol.Actor.prototype.initialize.call(this, null);
 

devtools/server/actors/timeline.js

@@ -39,15 +39,6 @@ var TimelineActor = exports.TimelineActor = protocol.ActorClassWithSpec(timeline
     events.on(this.bridge, "*", this._onTimelineEvent);
   },
 
-  /**
-   * The timeline actor is the first (and last) in its hierarchy to use
-   * protocol.js so it doesn't have a parent protocol actor that takes care of
-   * its lifetime. So it needs a disconnect method to cleanup.
-   */
-  disconnect: function () {
-    this.destroy();
-  },
-
   /**
    * Destroys this actor, stopping recording first.
    */

devtools/server/actors/webbrowser.js

@@ -484,8 +484,7 @@ BrowserTabList.prototype._handleActorClose = function (actor, browser) {
 /**
  * Make sure we are listening or not listening for activity elsewhere in
  * the browser, as appropriate. Other than setting up newly created XUL
- * windows, all listener / observer connection and disconnection should
- * happen here.
+ * windows, all listener / observer management should happen here.
  */
 BrowserTabList.prototype._checkListening = function () {
   /*
@@ -887,7 +886,7 @@ function TabActor(connection) {
   this._onWorkerActorListChanged = this._onWorkerActorListChanged.bind(this);
 }
 
-// XXX (bug 710213): TabActor attach/detach/exit/disconnect is a
+// XXX (bug 710213): TabActor attach/detach/exit/destroy is a
 // *complete* mess, needs to be rethought asap.
 
 TabActor.prototype = {
@@ -1137,7 +1136,7 @@ TabActor.prototype = {
   /**
    * Called when the actor is removed from the connection.
    */
-  disconnect() {
+  destroy() {
     this.exit();
   },
 

devtools/server/actors/webconsole.js

@@ -336,8 +336,7 @@ WebConsoleActor.prototype =
   /**
    * Destroy the current WebConsoleActor instance.
    */
-  disconnect: function WCA_disconnect()
-  {
+  destroy() {
     if (this.consoleServiceListener) {
       this.consoleServiceListener.destroy();
       this.consoleServiceListener = null;

devtools/server/actors/worker.js

@@ -123,10 +123,6 @@ let WorkerActor = protocol.ActorClassWithSpec(workerSpec, {
     }
   },
 
-  disconnect() {
-    this.destroy();
-  },
-
   connect(options) {
     if (!this._attached) {
       return { error: "wrongState" };
@@ -446,10 +442,6 @@ protocol.ActorClassWithSpec(serviceWorkerRegistrationSpec, {
     this._activeWorker = null;
   },
 
-  disconnect() {
-    this.destroy();
-  },
-
   /**
    * Standard observer interface to listen to push messages and changes.
    */

devtools/server/main.js

@@ -1494,14 +1494,14 @@ DebuggerServerConnection.prototype = {
    * @param ActorPool actorPool
    *        The ActorPool instance you want to remove.
    * @param boolean noCleanup [optional]
-   *        True if you don't want to disconnect each actor from the pool, false
+   *        True if you don't want to destroy each actor from the pool, false
    *        otherwise.
    */
   removeActorPool(actorPool, noCleanup) {
     // When a connection is closed, it removes each of its actor pools. When an
-    // actor pool is removed, it calls the disconnect method on each of its
+    // actor pool is removed, it calls the destroy method on each of its
     // actors. Some actors, such as ThreadActor, manage their own actor pools.
-    // When the disconnect method is called on these actors, they manually
+    // When the destroy method is called on these actors, they manually
     // remove their actor pools. Consequently, this method is reentrant.
     //
     // In addition, some actors, such as ThreadActor, perform asynchronous work
@@ -1510,7 +1510,7 @@ DebuggerServerConnection.prototype = {
     // be completed, we can end up in this function recursively after the
     // connection already set this._extraPools to null.
     //
-    // This is a bug: if the disconnect method can perform asynchronous work,
+    // This is a bug: if the destroy method can perform asynchronous work,
     // then we should wait for that work to be completed before setting this.
     // _extraPools to null. As a temporary solution, it should be acceptable
     // to just return early (if this._extraPools has been set to null, all

devtools/server/tests/mochitest/test_connectToChild.html

@@ -42,7 +42,7 @@ function runTests() {
   let mm = iframe.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader.messageManager;
 
   // Register a test actor in the child process so that we can know if and when
-  // this fake actor is disconnected.
+  // this fake actor is destroyed.
   mm.loadFrameScript("data:text/javascript,new " + function FrameScriptScope() {
     const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
     const { DebuggerServer } = require("devtools/server/main");
@@ -55,8 +55,8 @@ function runTests() {
     TestActor.prototype = {
       actorPrefix: "test",
 
-      disconnect: function () {
-        sendAsyncMessage("test-actor-disconnected", null);
+      destroy: function () {
+        sendAsyncMessage("test-actor-destroyed", null);
       },
       hello: function () {
         return {msg:"world"};
@@ -85,7 +85,7 @@ function runTests() {
       ok(actor.testActor, "Got the test actor");
 
       // Ensure sending at least one request to our actor,
-      // otherwise it won't be instanciated, nor be disconnected...
+      // otherwise it won't be instanciated, nor be destroyed...
       client.request({
         to: actor.testActor,
         type: "hello",
@@ -95,10 +95,10 @@ function runTests() {
         client.close();
 
         // Ensure that our test actor got cleaned up;
-        // its disconnect method should be called
-        mm.addMessageListener("test-actor-disconnected", function listener() {
-          mm.removeMessageListener("test-actor-disconnected", listener);
-          ok(true, "Actor is cleaned  up");
+        // its destroy method should be called
+        mm.addMessageListener("test-actor-destroyed", function listener() {
+          mm.removeMessageListener("test-actor-destroyed", listener);
+          ok(true, "Actor is cleaned up");
 
           secondClient(actor.testActor);
         });

devtools/server/tests/unit/test_longstringactor.js

@@ -7,7 +7,7 @@
 const { LongStringActor } = require("devtools/server/actors/object");
 
 function run_test() {
-  test_LSA_disconnect();
+  test_LSA_destroy();
   test_LSA_grip();
   test_LSA_onSubstring();
 }
@@ -27,12 +27,12 @@ function makeMockLongStringActor()
   return actor;
 }
 
-function test_LSA_disconnect()
+function test_LSA_destroy()
 {
   let actor = makeMockLongStringActor();
   do_check_eq(actor.registeredPool.longStringActors[TEST_STRING], actor);
 
-  actor.disconnect();
+  actor.destroy();
   do_check_eq(actor.registeredPool.longStringActors[TEST_STRING], void 0);
 }