Bug 1473513 - remove other instances of ActorPool from browsingContext and root; r=ochameau draft
authoryulia <ystartsev@mozilla.com>
Tue, 24 Jul 2018 14:48:39 +0200
changeset 823483 fd1412038cfe2518892c6176531465f5109b7a7f
parent 823482 f98b9503a19c122480c3b9c222ec7daa3aa03c12
child 823484 7451bc6d162a22932757ccc56a43172571093ee5
push id117694
push userbmo:ystartsev@mozilla.com
push dateFri, 27 Jul 2018 13:09:11 +0000
reviewersochameau
bugs1473513
milestone63.0a1
Bug 1473513 - remove other instances of ActorPool from browsingContext and root; r=ochameau MozReview-Commit-ID: GxkLzvxJgdY
devtools/server/actors/common.js
devtools/server/actors/root.js
devtools/server/actors/targets/browsing-context.js
devtools/server/tests/unit/testactors.js
devtools/shared/security/tests/unit/testactors.js
devtools/shared/transport/tests/unit/testactors.js
--- a/devtools/server/actors/common.js
+++ b/devtools/server/actors/common.js
@@ -4,36 +4,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { method } = require("devtools/shared/protocol");
 
 /**
- * Append the extra actors in |this._extraActors|, constructed by a prior call
- * to CommonCreateExtraActors, to |object|.
- *
- * @param object
- *     The object to which the extra actors should be added, under the
- *     property names given in the |factories| table passed to
- *     CommonCreateExtraActors.
- *
- * @param this
- *     The RootActor or BrowsingContextTargetActor whose |_extraActors| table we
- *     should use; see above.
- */
-exports.appendExtraActors = function appendExtraActors(object) {
-  for (const name in this._extraActors) {
-    const actor = this._extraActors[name];
-    object[name] = actor.actorID;
-  }
-};
-
-/**
  * 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) {
   this.conn = connection;
--- a/devtools/server/actors/root.js
+++ b/devtools/server/actors/root.js
@@ -3,17 +3,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { Cu } = require("chrome");
 const Services = require("Services");
-const { ActorPool, appendExtraActors } = require("devtools/server/actors/common");
 const { Pool } = require("devtools/shared/protocol");
 const { LazyPool, createExtraActors } = require("devtools/shared/protocol/lazy-pool");
 const { DebuggerServer } = require("devtools/server/main");
 
 loader.lazyRequireGetter(this, "ChromeWindowTargetActor",
   "devtools/server/actors/targets/chrome-window", true);
 
 /* Root actor for the remote debugging protocol. */
@@ -229,30 +228,27 @@ RootActor.prototype = {
   },
 
   /**
    * Gets the "root" form, which lists all the global actors that affect the entire
    * browser.  This can replace usages of `listTabs` that only wanted the global actors
    * and didn't actually care about tabs.
    */
   onGetRoot: function() {
-    const reply = {
-      from: this.actorID,
-    };
-
     // Create global actors
     if (!this._globalActorPool) {
       this._globalActorPool = new LazyPool(this.conn);
     }
-    createExtraActors(this._parameters.globalActorFactories, this._globalActorPool, this);
+    const actors = createExtraActors(
+      this._parameters.globalActorFactories,
+      this._globalActorPool,
+      this
+    );
 
-    // List the global actors
-    this._appendExtraActors(reply);
-
-    return reply;
+    return actors;
   },
 
   /* The 'listTabs' request and the 'tabListChanged' notification. */
 
   /**
    * Handles the listTabs request. The actors will survive until at least
    * the next listTabs request.
    *
@@ -269,48 +265,47 @@ RootActor.prototype = {
                message: "This root actor has no browser tabs." };
     }
 
     // Now that a client has requested the list of tabs, we reattach the onListChanged
     // listener in order to be notified if the list of tabs changes again in the future.
     tabList.onListChanged = this._onTabListChanged;
 
     // Walk the tab list, accumulating the array of target actors for the reply, and
-    // moving all the actors to a new ActorPool. We'll replace the old tab target actor
+    // moving all the actors to a new Pool. We'll replace the old tab target actor
     // pool with the one we build here, thus retiring any actors that didn't get listed
     // again, and preparing any new actors to receive packets.
-    const newActorPool = new ActorPool(this.conn);
+    const newActorPool = new Pool(this.conn);
     const targetActorList = [];
     let selected;
 
     const options = request.options || {};
     const targetActors = await tabList.getList(options);
     for (const targetActor of targetActors) {
       if (targetActor.exited) {
         // Target actor may have exited while we were gathering the list.
         continue;
       }
       if (targetActor.selected) {
         selected = targetActorList.length;
       }
       targetActor.parentID = this.actorID;
-      newActorPool.addActor(targetActor);
+      newActorPool.manage(targetActor);
       targetActorList.push(targetActor);
     }
 
     // Start with the root reply, which includes the global actors for the whole browser.
     const reply = this.onGetRoot();
 
     // Drop the old actorID -> actor map. Actors that still mattered were added to the
     // new map; others will go away.
     if (this._tabTargetActorPool) {
-      this.conn.removeActorPool(this._tabTargetActorPool);
+      this._tabTargetActorPool.destroy();
     }
     this._tabTargetActorPool = newActorPool;
-    this.conn.addActorPool(this._tabTargetActorPool);
 
     // We'll extend the reply here to also mention all the tabs.
     Object.assign(reply, {
       selected: selected || 0,
       tabs: targetActorList.map(actor => actor.form()),
     });
 
     return reply;
@@ -318,18 +313,17 @@ RootActor.prototype = {
 
   onGetTab: async function(options) {
     const tabList = this._parameters.tabList;
     if (!tabList) {
       return { error: "noTabs",
                message: "This root actor has no browser tabs." };
     }
     if (!this._tabTargetActorPool) {
-      this._tabTargetActorPool = new ActorPool(this.conn);
-      this.conn.addActorPool(this._tabTargetActorPool);
+      this._tabTargetActorPool = new Pool(this.conn);
     }
 
     let targetActor;
     try {
       targetActor = await tabList.getTab(options);
     } catch (error) {
       if (error.error) {
         // Pipe expected errors as-is to the client
@@ -337,17 +331,17 @@ RootActor.prototype = {
       }
       return {
         error: "noTab",
         message: "Unexpected error while calling getTab(): " + error
       };
     }
 
     targetActor.parentID = this.actorID;
-    this._tabTargetActorPool.addActor(targetActor);
+    this._tabTargetActorPool.manage(targetActor);
 
     return { tab: targetActor.form() };
   },
 
   onGetWindow: function({ outerWindowID }) {
     if (!DebuggerServer.allowChromeProcess) {
       return {
         from: this.actorID,
@@ -360,23 +354,22 @@ RootActor.prototype = {
       return {
         from: this.actorID,
         error: "notFound",
         message: `No window found with outerWindowID ${outerWindowID}`,
       };
     }
 
     if (!this._chromeWindowActorPool) {
-      this._chromeWindowActorPool = new ActorPool(this.conn);
-      this.conn.addActorPool(this._chromeWindowActorPool);
+      this._chromeWindowActorPool = new Pool(this.conn);
     }
 
     const actor = new ChromeWindowTargetActor(this.conn, window);
     actor.parentID = this.actorID;
-    this._chromeWindowActorPool.addActor(actor);
+    this._chromeWindowActorPool.manage(actor);
 
     return {
       from: this.actorID,
       window: actor.form(),
     };
   },
 
   onTabListChanged: function() {
@@ -391,26 +384,25 @@ RootActor.prototype = {
       return { from: this.actorID, error: "noAddons",
                message: "This root actor has no browser addons." };
     }
 
     // Reattach the onListChanged listener now that a client requested the list.
     addonList.onListChanged = this._onAddonListChanged;
 
     return addonList.getList().then((addonTargetActors) => {
-      const addonTargetActorPool = new ActorPool(this.conn);
+      const addonTargetActorPool = new Pool(this.conn);
       for (const addonTargetActor of addonTargetActors) {
-        addonTargetActorPool.addActor(addonTargetActor);
+        addonTargetActorPool.manage(addonTargetActor);
       }
 
       if (this._addonTargetActorPool) {
-        this.conn.removeActorPool(this._addonTargetActorPool);
+        this._addonTargetActorPool.destroy();
       }
       this._addonTargetActorPool = addonTargetActorPool;
-      this.conn.addActorPool(this._addonTargetActorPool);
 
       return {
         "from": this.actorID,
         "addons": addonTargetActors.map(addonTargetActor => addonTargetActor.form())
       };
     });
   },
 
@@ -461,24 +453,25 @@ RootActor.prototype = {
       return { from: this.actorID, error: "noServiceWorkerRegistrations",
                message: "This root actor has no service worker registrations." };
     }
 
     // Reattach the onListChanged listener now that a client requested the list.
     registrationList.onListChanged = this._onServiceWorkerRegistrationListChanged;
 
     return registrationList.getList().then(actors => {
-      const pool = new ActorPool(this.conn);
+      const pool = new Pool(this.conn);
       for (const actor of actors) {
-        pool.addActor(actor);
+        pool.manage(actor);
       }
 
-      this.conn.removeActorPool(this._serviceWorkerRegistrationActorPool);
+      if (this._serviceWorkerRegistrationActorPool) {
+        this._serviceWorkerRegistrationActorPool.destroy();
+      }
       this._serviceWorkerRegistrationActorPool = pool;
-      this.conn.addActorPool(this._serviceWorkerRegistrationActorPool);
 
       return {
         "from": this.actorID,
         "registrations": actors.map(actor => actor.form())
       };
     });
   },
 
@@ -558,35 +551,32 @@ RootActor.prototype = {
      */
     return Cu.cloneInto(request, {});
   },
 
   onProtocolDescription: function() {
     return require("devtools/shared/protocol").dumpProtocolSpec();
   },
 
-  /* Support for DebuggerServer.addGlobalActor. */
-  _appendExtraActors: appendExtraActors,
-
   /**
    * Remove the extra actor (added by DebuggerServer.addGlobalActor or
    * DebuggerServer.addTargetScopedActor) name |name|.
    */
   removeActorByName: function(name) {
     if (name in this._extraActors) {
       const actor = this._extraActors[name];
       if (this._globalActorPool.has(actor.actorID)) {
         actor.destroy();
       }
       if (this._tabTargetActorPool) {
         // Iterate over BrowsingContextTargetActor instances to also remove target-scoped
         // actors created during listTabs for each document.
-        this._tabTargetActorPool.forEach(tab => {
+        for (const tab in this._tabTargetActorPool.poolChildren()) {
           tab.removeActorByName(name);
-        });
+        }
       }
       delete this._extraActors[name];
     }
   }
 };
 
 RootActor.prototype.requestTypes = {
   getRoot: RootActor.prototype.onGetRoot,
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -18,17 +18,16 @@
  * For performance matters, this file should only be loaded in the targeted context's
  * process. For example, it shouldn't be evaluated in the parent process until we try to
  * debug a document living in the parent process.
  */
 
 var { Ci, Cu, Cr, Cc } = require("chrome");
 var Services = require("Services");
 const ChromeUtils = require("ChromeUtils");
-var { appendExtraActors } = require("devtools/server/actors/common");
 var { DebuggerServer } = require("devtools/server/main");
 var DevToolsUtils = require("devtools/shared/DevToolsUtils");
 var { assert } = DevToolsUtils;
 var { TabSources } = require("devtools/server/actors/utils/TabSources");
 var makeDebugger = require("devtools/server/actors/utils/make-debugger");
 const Debugger = require("Debugger");
 const ReplayDebugger = require("devtools/server/actors/replay/debugger");
 const InspectorUtils = require("InspectorUtils");
@@ -566,19 +565,16 @@ const browsingContextTargetPrototype = {
         && metadata["inner-window-id"]
         && metadata["inner-window-id"] == id) {
       return true;
     }
 
     return false;
   },
 
-  /* Support for DebuggerServer.addTargetScopedActor. */
-  _appendExtraActors: appendExtraActors,
-
   /**
    * Does the actual work of attaching to a browsing context.
    */
   _attach() {
     if (this._attached) {
       return;
     }
 
--- a/devtools/server/tests/unit/testactors.js
+++ b/devtools/server/tests/unit/testactors.js
@@ -1,14 +1,13 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-const { appendExtraActors } = require("devtools/server/actors/common");
 const { LazyPool, createExtraActors } = require("devtools/shared/protocol/lazy-pool");
 const { RootActor } = require("devtools/server/actors/root");
 const { ThreadActor } = require("devtools/server/actors/thread");
 const { DebuggerServer } = require("devtools/server/main");
 const { TabSources } = require("devtools/server/actors/utils/TabSources");
 const makeDebugger = require("devtools/server/actors/utils/make-debugger");
 
 var gTestGlobals = [];
@@ -147,18 +146,15 @@ TestTargetActor.prototype = {
 
   removeActorByName: function(name) {
     const actor = this._extraActors[name];
     if (this._targetActorPool) {
       this._targetActorPool.removeActor(actor);
     }
     delete this._extraActors[name];
   },
-
-  /* Support for DebuggerServer.addTargetScopedActor. */
-  _appendExtraActors: appendExtraActors
 };
 
 TestTargetActor.prototype.requestTypes = {
   "attach": TestTargetActor.prototype.onAttach,
   "detach": TestTargetActor.prototype.onDetach,
   "reload": TestTargetActor.prototype.onReload
 };
--- a/devtools/shared/security/tests/unit/testactors.js
+++ b/devtools/shared/security/tests/unit/testactors.js
@@ -1,14 +1,13 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-const { appendExtraActors } = require("devtools/server/actors/common");
 const { LazyPool, createExtraActors } = require("devtools/shared/protocol/lazy-pool");
 const { RootActor } = require("devtools/server/actors/root");
 const { ThreadActor } = require("devtools/server/actors/thread");
 const { DebuggerServer } = require("devtools/server/main");
 const promise = require("promise");
 
 var gTestGlobals = [];
 DebuggerServer.addTestGlobal = function(global) {
@@ -104,18 +103,15 @@ TestTargetActor.prototype = {
     return { type: "tabAttached", threadActor: this._threadActor.actorID };
   },
 
   onDetach: function(request) {
     if (!this._attached) {
       return { "error": "wrongState" };
     }
     return { type: "detached" };
-  },
-
-  /* Support for DebuggerServer.addTargetScopedActor. */
-  _appendExtraActors: appendExtraActors
+  }
 };
 
 TestTargetActor.prototype.requestTypes = {
   "attach": TestTargetActor.prototype.onAttach,
   "detach": TestTargetActor.prototype.onDetach
 };
--- a/devtools/shared/transport/tests/unit/testactors.js
+++ b/devtools/shared/transport/tests/unit/testactors.js
@@ -1,13 +1,12 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
-const { appendExtraActors } = require("devtools/server/actors/common");
 const { LazyPool, createExtraActors } = require("devtools/shared/protocol/lazy-pool");
 const { RootActor } = require("devtools/server/actors/root");
 const { ThreadActor } = require("devtools/server/actors/thread");
 const { DebuggerServer } = require("devtools/server/main");
 const promise = require("promise");
 
 var gTestGlobals = [];
 DebuggerServer.addTestGlobal = function(global) {
@@ -103,18 +102,15 @@ TestTargetActor.prototype = {
     return { type: "tabAttached", threadActor: this._threadActor.actorID };
   },
 
   onDetach: function(request) {
     if (!this._attached) {
       return { "error": "wrongState" };
     }
     return { type: "detached" };
-  },
-
-  /* Support for DebuggerServer.addTargetScopedActor. */
-  _appendExtraActors: appendExtraActors
+  }
 };
 
 TestTargetActor.prototype.requestTypes = {
   "attach": TestTargetActor.prototype.onAttach,
   "detach": TestTargetActor.prototype.onDetach
 };