Bug 1473513 - use Protocol.js pools for workerTargetActorPool in Target Actors; r=ochameau draft
authoryulia <ystartsev@mozilla.com>
Tue, 10 Jul 2018 17:07:54 +0200
changeset 823480 11176b362149099d85f22cf918e04e7726b40921
parent 823465 87bcafe428a4ad6017e59b915581ae00aa863407
child 823481 efa1b7728c9a9a5e542637b28153e2d0db7170bc
push id117694
push userbmo:ystartsev@mozilla.com
push dateFri, 27 Jul 2018 13:09:11 +0000
reviewersochameau
bugs1473513
milestone63.0a1
Bug 1473513 - use Protocol.js pools for workerTargetActorPool in Target Actors; r=ochameau MozReview-Commit-ID: 5uIWwOR7CHp
devtools/server/actors/root.js
devtools/server/actors/targets/browsing-context.js
devtools/server/actors/targets/content-process.js
devtools/shared/protocol.js
--- a/devtools/server/actors/root.js
+++ b/devtools/server/actors/root.js
@@ -4,16 +4,17 @@
  * 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, createExtraActors } = require("devtools/server/actors/common");
+const { Pool } = require("devtools/shared/protocol");
 const { DebuggerServer } = require("devtools/server/main");
 
 loader.lazyRequireGetter(this, "ChromeWindowTargetActor",
   "devtools/server/actors/targets/chrome-window", true);
 
 /* Root actor for the remote debugging protocol. */
 
 /**
@@ -425,24 +426,28 @@ RootActor.prototype = {
       return { from: this.actorID, error: "noWorkers",
                message: "This root actor has no workers." };
     }
 
     // Reattach the onListChanged listener now that a client requested the list.
     workerList.onListChanged = this._onWorkerListChanged;
 
     return workerList.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._workerTargetActorPool);
+      // Do not destroy the pool before transfering ownership to the newly created
+      // pool, so that we do not accidently destroy actors that are still in use.
+      if (this._workerTargetActorPool) {
+        this._workerTargetActorPool.destroy();
+      }
+
       this._workerTargetActorPool = pool;
-      this.conn.addActorPool(this._workerTargetActorPool);
 
       return {
         "from": this.actorID,
         "workers": actors.map(actor => actor.form())
       };
     });
   },
 
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -36,17 +36,17 @@ const ReplayDebugger = require("devtools
 const InspectorUtils = require("InspectorUtils");
 
 const EXTENSION_CONTENT_JSM = "resource://gre/modules/ExtensionContent.jsm";
 
 const { LocalizationHelper } = require("devtools/shared/l10n");
 const STRINGS_URI = "devtools/shared/locales/browsing-context.properties";
 const L10N = new LocalizationHelper(STRINGS_URI);
 
-const { ActorClassWithSpec, Actor } = require("devtools/shared/protocol");
+const { ActorClassWithSpec, Actor, Pool } = require("devtools/shared/protocol");
 const { browsingContextTargetSpec } = require("devtools/shared/specs/targets/browsing-context");
 
 loader.lazyRequireGetter(this, "ThreadActor", "devtools/server/actors/thread", true);
 loader.lazyRequireGetter(this, "unwrapDebuggerObjectGlobal", "devtools/server/actors/thread", true);
 loader.lazyRequireGetter(this, "WorkerTargetActorList", "devtools/server/actors/worker/worker-list", true);
 loader.lazyImporter(this, "ExtensionContent", EXTENSION_CONTENT_JSM);
 
 loader.lazyRequireGetter(this, "StyleSheetActor", "devtools/server/actors/stylesheets", true);
@@ -655,25 +655,28 @@ const browsingContextTargetPrototype = {
     if (this._workerTargetActorList === null) {
       this._workerTargetActorList = new WorkerTargetActorList(this.conn, {
         type: Ci.nsIWorkerDebugger.TYPE_DEDICATED,
         window: this.window
       });
     }
 
     return this._workerTargetActorList.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._workerTargetActorPool);
+      // Do not destroy the pool before transfering ownership to the newly created
+      // pool, so that we do not accidently destroy actors that are still in use.
+      if (this._workerTargetActorPool) {
+        this._workerTargetActorPool.destroy();
+      }
+
       this._workerTargetActorPool = pool;
-      this.conn.addActorPool(this._workerTargetActorPool);
-
       this._workerTargetActorList.onListChanged = this._onWorkerTargetActorListChanged;
 
       return {
         "from": this.actorID,
         "workers": actors.map((actor) => actor.form())
       };
     });
   },
@@ -903,17 +906,17 @@ const browsingContextTargetPrototype = {
 
     // Make sure that no more workerListChanged notifications are sent.
     if (this._workerTargetActorList !== null) {
       this._workerTargetActorList.onListChanged = null;
       this._workerTargetActorList = null;
     }
 
     if (this._workerTargetActorPool !== null) {
-      this.conn.removeActorPool(this._workerTargetActorPool);
+      this._workerTargetActorPool.destroy();
       this._workerTargetActorPool = null;
     }
 
     this._attached = false;
 
     this.emit("tabDetached");
 
     return true;
--- a/devtools/server/actors/targets/content-process.js
+++ b/devtools/server/actors/targets/content-process.js
@@ -13,16 +13,17 @@
 
 const { Cc, Ci, Cu } = require("chrome");
 const Services = require("Services");
 
 const { ChromeDebuggerActor } = require("devtools/server/actors/thread");
 const { WebConsoleActor } = require("devtools/server/actors/webconsole");
 const makeDebugger = require("devtools/server/actors/utils/make-debugger");
 const { ActorPool } = require("devtools/server/actors/common");
+const { Pool } = require("devtools/shared/protocol");
 const { assert } = require("devtools/shared/DevToolsUtils");
 const { TabSources } = require("devtools/server/actors/utils/TabSources");
 
 loader.lazyRequireGetter(this, "WorkerTargetActorList", "devtools/server/actors/worker/worker-list", true);
 loader.lazyRequireGetter(this, "MemoryActor", "devtools/server/actors/memory", true);
 
 function ContentProcessTargetActor(connection) {
   this.conn = connection;
@@ -124,25 +125,28 @@ ContentProcessTargetActor.prototype = {
     };
   },
 
   onListWorkers: function() {
     if (!this._workerList) {
       this._workerList = new WorkerTargetActorList(this.conn, {});
     }
     return this._workerList.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._workerTargetActorPool);
+      // Do not destroy the pool before transfering ownership to the newly created
+      // pool, so that we do not accidently destroy actors that are still in use.
+      if (this._workerTargetActorPool) {
+        this._workerTargetActorPool.destroy();
+      }
+
       this._workerTargetActorPool = pool;
-      this.conn.addActorPool(this._workerTargetActorPool);
-
       this._workerList.onListChanged = this._onWorkerListChanged;
 
       return {
         "from": this.actorID,
         "workers": actors.map(actor => actor.form())
       };
     });
   },
--- a/devtools/shared/protocol.js
+++ b/devtools/shared/protocol.js
@@ -852,16 +852,22 @@ Pool.prototype = extend(EventEmitter.pro
   /**
    * Add an actor as a child of this pool.
    */
   manage: function(actor) {
     if (!actor.actorID) {
       actor.actorID = this.conn.allocID(actor.actorPrefix || actor.typeName);
     }
 
+    // If the actor is already in a pool, remove it without destroying it.
+    const parent = actor.parent();
+    if (parent) {
+      parent.unmanage(actor);
+    }
+
     this._poolMap.set(actor.actorID, actor);
     return actor;
   },
 
   /**
    * Remove an actor as a child of this pool.
    */
   unmanage: function(actor) {