Bug 1473513 - remove other instances of ActorPool from browsingContext and root; r=ochameau
☠☠ backed out by 98b5ff9533ee ☠ ☠
authoryulia <ystartsev@mozilla.com>
Tue, 24 Jul 2018 14:48:39 +0200
changeset 487947 12eb1139a80286f7ee6adf378c421cee981a454f
parent 487946 ce86ea60a31cc1944347306e28aaff1fbff38f59
child 487948 6e157bea362acd8a51fdf4863ebb38dbe6e74df6
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1473513
milestone63.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 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");
@@ -559,19 +558,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
 };