Bug 1495728 - Remove unused ActorPool class. r=ochameau.

This class isn't used anymore, and it's safe to remove it.
We take this as an opportunity to remove Pool#cleanup and
Pool#isEmpty, which each only had one callsite.
Some comments are updated to not mention ActorPool.

Differential Revision: https://phabricator.services.mozilla.com/D80602

devtools/client/devtools-client.js

@@ -614,19 +614,19 @@ DevToolsClient.prototype = {
     //
     // In the normal case where we shutdown cleanly, the toolbox tells each tool
     // to close, and they each call |destroy| on any fronts they were using.
-    // When |destroy| or |cleanup| is called on a protocol.js front, it also
+    // When |destroy| is called on a protocol.js front, it also
     // removes itself from the |_pools| array.  Once the toolbox has shutdown,
     // the connection is closed, and we reach here.  All fronts (should have
     // been) |destroy|ed, so |_pools| should empty.
     //
     // If the connection instead aborts unexpectedly, we may end up here with
-    // all fronts used during the life of the connection.  So, we call |cleanup|
+    // all fronts used during the life of the connection.  So, we call |destroy|
     // on them clear their state, reject pending requests, and remove themselves
     // from |_pools|.  This saves the toolbox from hanging indefinitely, in case
     // it waits for some server response before shutdown that will now never
     // arrive.
     for (const pool of this._pools) {
-      pool.cleanup();
+      pool.destroy();
     }
   },
 

devtools/server/actors/common.js

@@ -6,126 +6,6 @@
 
 const { method } = require("devtools/shared/protocol");
 
-/**
- * Construct an ActorPool.
- *
- * ActorPools are actorID -> actor mapping and storage.  These are
- * used to accumulate and quickly dispose of groups of actors that
- * share a lifetime.
- */
-function ActorPool(connection, label) {
-  this.conn = connection;
-  this.label = label;
-  this._actors = {};
-}
-
-ActorPool.prototype = {
-  /**
-   * Destroy the pool. This will remove all actors from the pool.
-   */
-  destroy: function APDestroy() {
-    for (const id in this._actors) {
-      this.removeActor(this._actors[id]);
-    }
-  },
-
-  /**
-   * Add an actor to the pool. If the actor doesn't have an ID, allocate one
-   * from the connection.
-   *
-   * @param Object actor
-   *        The actor to be added to the pool.
-   */
-  addActor: function APAddActor(actor) {
-    actor.conn = this.conn;
-    if (!actor.actorID) {
-      if (!actor.typeName) {
-        throw new Error("Actor should a specify a `typeName` attribute");
-      }
-      actor.actorID = this.conn.allocID(actor.typeName);
-    }
-
-    // If the actor is already in a pool, remove it without destroying it.
-    if (actor.registeredPool) {
-      delete actor.registeredPool._actors[actor.actorID];
-    }
-    actor.registeredPool = this;
-
-    this._actors[actor.actorID] = actor;
-  },
-
-  /**
-   * Remove an actor from the pool. If the actor has a destroy method, call it.
-   */
-  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();
-    }
-  },
-
-  get: function APGet(actorID) {
-    return this._actors[actorID] || undefined;
-  },
-
-  has: function APHas(actorID) {
-    return actorID in this._actors;
-  },
-
-  /**
-   * Returns true if the pool is empty.
-   */
-  isEmpty: function APIsEmpty() {
-    return Object.keys(this._actors).length == 0;
-  },
-
-  /**
-   * Match the api expected by the protocol library.
-   */
-  unmanage: function(actor) {
-    return this.removeActor(actor);
-  },
-
-  forEach: function(callback) {
-    for (const name in this._actors) {
-      callback(this._actors[name]);
-    }
-  },
-
-  // Generator that yields each non-self child of the pool.
-  *poolChildren() {
-    if (!this._actors) {
-      return;
-    }
-    for (const actor of Object.values(this._actors)) {
-      // Self-owned actors are ok, but don't need visiting twice.
-      if (actor === this) {
-        continue;
-      }
-      yield actor;
-    }
-  },
-
-  dumpPool() {
-    for (const actor in this._actors) {
-      console.log(`>> ${actor}`);
-    }
-  },
-
-  // For consistency with Pool.js
-  // Will be checked by devtools-server-connection.
-  isTopPool() {
-    return true;
-  },
-};
-
-exports.ActorPool = ActorPool;
-
 /**
  * A SourceLocation represents a location in a source.
  *

devtools/server/actors/targets/browsing-context.js

@@ -542,14 +542,14 @@ const browsingContextTargetPrototype = {
   },
 
   _createExtraActors() {
-    // Always use the same ActorPool, so existing actor instances
+    // Always use the same Pool, so existing actor instances
     // (created in createExtraActors) are not lost.
     if (!this._targetScopedActorPool) {
       this._targetScopedActorPool = new LazyPool(this.conn);
     }
 
     // Walk over target-scoped actor factories and make sure they are all
-    // instantiated and added into the ActorPool.
+    // instantiated and added into the Pool.
     return createExtraActors(
       ActorRegistry.targetScopedActorFactories,
       this._targetScopedActorPool,

devtools/server/actors/utils/actor-registry.js

@@ -27,8 +27,7 @@ const ActorRegistry = {
    *        An object with 3 mandatory attributes:
    *        - prefix (string):
    *          The prefix of an actor is used to compute:
-   *          - the `actorID` of each new actor instance (ex: prefix1).
-   *            (See ActorPool.addActor)
+   *          - the `actorID` of each new actor instance (ex: prefix1). (See Pool.manage)
    *          - the actor name in the listTabs request. Sending a listTabs
    *            request to the root actor returns actor IDs. IDs are in
    *            dictionaries, with actor names as keys and actor IDs as values.

devtools/server/devtools-server-connection.js

@@ -121,8 +121,8 @@ DevToolsServerConnection.prototype = {
   /**
    * Remove a previously-added pool of actors to the connection.
    *
-   * @param ActorPool actorPool
-   *        The ActorPool instance you want to remove.
+   * @param Pool actorPool
+   *        The Pool instance you want to remove.
    */
   removeActorPool(actorPool) {
     // When a connection is closed, it removes each of its actor pools. When an
@@ -205,12 +205,9 @@ DevToolsServerConnection.prototype = {
       }
 
       if (typeof actor !== "object") {
-        // ActorPools should now contain only actor instances (i.e. objects)
+        // Pools should now contain only actor instances (i.e. objects)
         throw new Error(
-          "Unexpected actor constructor/function in ActorPool " +
-            "for actorID=" +
-            actorID +
-            "."
+          `Unexpected actor constructor/function in Pool for actorID "${actorID}".`
         );
       }
 
@@ -494,32 +491,25 @@ DevToolsServerConnection.prototype = {
   },
 
   dumpPool(pool, output = [], dumpedPools) {
-    let label;
-    let actorIds = [];
-    let children = [];
+    const actorIds = [];
+    const children = [];
 
     if (dumpedPools.has(pool)) {
       return;
     }
     dumpedPools.add(pool);
-    // TRUE if the pool is an ActorPool
-    if (pool._actors) {
-      actorIds = Object.keys(pool._actors);
-      children = Object.values(pool._actors);
-      label = pool.label || "";
-    }
 
     // TRUE if the pool is a Pool
-    else if (pool.__poolMap) {
-      for (const actor of pool.poolChildren()) {
-        children.push(actor);
-        actorIds.push(actor.actorID);
-      }
-      label = pool.label || pool.actorID;
-    } else {
+    if (!pool.__poolMap) {
       return;
     }
 
+    for (const actor of pool.poolChildren()) {
+      children.push(actor);
+      actorIds.push(actor.actorID);
+    }
+    const label = pool.label || pool.actorID;
+
     output.push([label, actorIds]);
     dump(`- ${label}: ${JSON.stringify(actorIds)}\n`);
     children.forEach(childPool =>

devtools/server/tests/xpcshell/test_connection_closes_all_pools.js

@@ -76,7 +76,7 @@ add_task(async function() {
   //      |
   //      \- childActor
   //
-  // Since parentActor is also an ActorPool from the point of view of the
+  // Since parentActor is also a Pool from the point of view of the
   // DevToolsServerConnection, it will attempt to destroy it when looping on
   // this._extraPools. But since `parentActor` is also a direct child of `pool`,
   // it has already been destroyed by the Pool destroy() mechanism.

devtools/server/tests/xpcshell/testactors.js

@@ -206,7 +206,7 @@ const TestTargetActor = protocol.ActorClassWithSpec(browsingContextTargetSpec, {
       actorPool,
       this
     );
-    if (!actorPool.isEmpty()) {
+    if (actorPool?._poolMap.size > 0) {
       this._descriptorActorPool = actorPool;
       this.conn.addActorPool(this._descriptorActorPool);
     }

devtools/shared/protocol/Pool.js

@@ -132,11 +132,6 @@ class Pool extends EventEmitter {
     return null;
   }
 
-  // True if this pool has no children.
-  isEmpty() {
-    return !this.__poolMap || this._poolMap.size == 0;
-  }
-
   // Generator that yields each non-self child of the pool.
   *poolChildren() {
     if (!this.__poolMap) {
@@ -204,14 +199,6 @@ class Pool extends EventEmitter {
     this.__poolMap.clear();
     this.__poolMap = null;
   }
-
-  /**
-   * For getting along with the devtools server pools, should be removable
-   * eventually.
-   */
-  cleanup() {
-    this.destroy();
-  }
 }
 
 exports.Pool = Pool;

devtools/shared/protocol/lazy-pool.js

@@ -69,7 +69,7 @@ exports.LazyPool = LazyPool;
  *     - _extraActors
  *        An object whose own property names are factory table (and packet)
  *        property names, and whose values are no-argument actor constructors,
- *        of the sort that one can add to an ActorPool.
+ *        of the sort that one can add to a Pool.
  *
  *     - conn
  *        The DevToolsServerConnection in which the new actors will participate.
@@ -130,7 +130,7 @@ exports.createExtraActors = createExtraActors;
  *     - _extraActors
  *        An object whose own property names are factory table (and packet)
  *        property names, and whose values are no-argument actor constructors,
- *        of the sort that one can add to an ActorPool.
+ *        of the sort that one can add to a Pool.
  *
  *     - conn
  *        The DevToolsServerConnection in which the new actors will participate.