Bug 1473513 - use Protocol.js pools for browsing context target actor; r=ochameau draft
authoryulia <ystartsev@mozilla.com>
Fri, 06 Jul 2018 14:32:14 +0200
changeset 814923 215fa102ee5a80283e8d8fb53a88c53435d4eeb6
parent 814369 90be04d99fc7941cb9b7186bf5f95e184a4e989a
push id115380
push userbmo:ystartsev@mozilla.com
push dateFri, 06 Jul 2018 12:35:50 +0000
reviewersochameau
bugs1473513
milestone63.0a1
Bug 1473513 - use Protocol.js pools for browsing context target actor; r=ochameau MozReview-Commit-ID: GQqLGjzP3F0
devtools/server/actors/targets/browsing-context.js
devtools/server/main.js
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -19,32 +19,32 @@
  * 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 {
-  ActorPool, createExtraActors, appendExtraActors
+  ActorPool, 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 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);
@@ -93,16 +93,118 @@ exports.getChildDocShells = getChildDocS
  * Browser-specific actors.
  */
 
 function getInnerId(window) {
   return window.QueryInterface(Ci.nsIInterfaceRequestor)
                .getInterface(Ci.nsIDOMWindowUtils).currentInnerWindowID;
 }
 
+/*
+ * Methods shared between RootActor and BrowsingContextTargetActor.
+ */
+
+/**
+ * Populate |this._extraActors| as specified by |factories|, reusing whatever
+ * actors are already there. Add all actors in the final extra actors table to
+ * |pool|.
+ *
+ * The target actor uses this to instantiate actors that other
+ * parts of the browser have specified with DebuggerServer.addTargetScopedActor
+ *
+ * @param factories
+ *     An object whose own property names are the names of properties to add to
+ *     some reply packet (say, a target actor grip or the "listTabs" response
+ *     form), and whose own property values are actor constructor functions, as
+ *     documented for addTargetScopedActor
+ *
+ * @param this
+ *     The BrowsingContextTargetActor with which the new actors
+ *     will be associated. It should support whatever API the |factories|
+ *     constructor functions might be interested in, as it is passed to them.
+ *     For the sake of CommonCreateExtraActors itself, it should have at least
+ *     the following properties:
+ *
+ *     - _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.
+ *
+ *     - conn
+ *        The DebuggerServerConnection in which the new actors will participate.
+ *
+ *     - actorID
+ *        The actor's name, for use as the new actors' parentID.
+ * @param pool
+ *     An object which implements the protocol.js Pool interface, and has the
+ *     following properties
+ *
+ *     - manage
+ *       a function which adds a given actor to an actor pool
+ */
+function createExtraActors(factories, pool) {
+  // Walk over global actors added by extensions.
+  for (const name in factories) {
+    let actor = this._extraActors[name];
+    if (!actor) {
+      // Register another factory, but this time specific to this connection.
+      // It creates a fake actor that looks like an regular actor in the pool,
+      // but without actually instantiating the actor.
+      // It will only be instantiated on the first request made to the actor.
+      actor = new LazyActor(factories[name], name, this, pool);
+      this._extraActors[name] = actor;
+    }
+
+    // If the actor already exists in the pool, it may have been instantiated,
+    // so make sure not to overwrite it by a non-instantiated version.
+    if (!pool.has(actor.actorID)) {
+      pool.manage(actor);
+    }
+  }
+}
+
+function LazyActor(actorFactory, name, parent, pool) {
+  this._factory = actorFactory;
+  this._parentActor = parent;
+  this._name = name;
+  this._pool = pool;
+
+  // needed for taking a place in a pool
+  this.typeName = actorFactory.prefix;
+  this.actorID = actorFactory.actorID;
+  this.isLazy = true;
+}
+
+LazyActor.prototype.destroy = function() {
+  this.actorID = null;
+  this._pool = null;
+};
+
+LazyActor.prototype.createActor = function() {
+  // Fetch the actor constructor
+  const C = this._factory._getConstructor();
+  // Instantiate a new actor instance
+  const conn = this._parentActor.conn;
+  const instance = new C(conn, this._parentActor);
+
+  // this should be taken care of once all actors are moved to protocol.js
+  instance.conn = conn;
+
+  instance.parentID = this._parentActor.actorID;
+  // We want the newly-constructed actor to completely replace the factory
+  // actor. Reusing the existing actor ID will make sure Pool.manage
+  // replaces the old actor with the new actor.
+  instance.actorID = this.actorID;
+
+  this._pool.manage(instance);
+  this.destroy();
+
+  return instance;
+};
+
 const browsingContextTargetPrototype = {
 
   /**
    * BrowsingContextTargetActor is an abstract class used by target actors that
    * hold documents, such as frames, chrome windows, etc.  The term "browsing
    * context" is defined in the HTML spec as "an environment in which `Document`
    * objects are presented to the user".  In Gecko, this means a browsing context
    * is a `docShell`.
@@ -478,18 +580,17 @@ const browsingContextTargetPrototype = {
       response.title = this.title;
       response.url = this.url;
       response.outerWindowID = this.outerWindowID;
     }
 
     // Always use the same ActorPool, so existing actor instances
     // (created in createExtraActors) are not lost.
     if (!this._targetScopedActorPool) {
-      this._targetScopedActorPool = new ActorPool(this.conn);
-      this.conn.addActorPool(this._targetScopedActorPool);
+      this._targetScopedActorPool = new Pool(this.conn);
     }
 
     // Walk over target-scoped actor factories and make sure they are all
     // instantiated and added into the ActorPool.
     this._createExtraActors(DebuggerServer.targetScopedActorFactories,
       this._targetScopedActorPool);
 
     this._appendExtraActors(response);
@@ -651,24 +752,23 @@ 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);
+      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())
       };
     });
@@ -911,28 +1011,28 @@ const browsingContextTargetPrototype = {
       Services.obs.removeObserver(this, "webnavigation-destroy");
     }
 
     this._popContext();
 
     // Shut down actors that belong to this target's pool.
     this._styleSheetActors.clear();
     if (this._targetScopedActorPool) {
-      this.conn.removeActorPool(this._targetScopedActorPool);
+      this.targetScopedActorPool.destroy();
       this._targetScopedActorPool = null;
     }
 
     // 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.conn.send({ from: this.actorID,
                      type: "tabDetached" });
 
@@ -1453,27 +1553,28 @@ const browsingContextTargetPrototype = {
   createStyleSheetActor(styleSheet) {
     assert(!this.exited, "Target must not be exited to create a sheet actor.");
     if (this._styleSheetActors.has(styleSheet)) {
       return this._styleSheetActors.get(styleSheet);
     }
     const actor = new StyleSheetActor(styleSheet, this);
     this._styleSheetActors.set(styleSheet, actor);
 
-    this._targetScopedActorPool.addActor(actor);
+    this._targetScopedActorPool.manage(actor);
     this.emit("stylesheet-added", actor);
 
     return actor;
   },
 
   removeActorByName(name) {
     if (name in this._extraActors) {
-      const actor = this._extraActors[name];
-      if (this._targetScopedActorPool.has(actor)) {
-        this._targetScopedActorPool.removeActor(actor);
+      const actorFactory = this._extraActors[name];
+      const actor = this._targetScopedActorPool.get(actorFactory.actorID);
+      if (actor) {
+        actor.destroy();
       }
       delete this._extraActors[name];
     }
   },
 };
 
 exports.browsingContextTargetPrototype = browsingContextTargetPrototype;
 exports.BrowsingContextTargetActor =
--- a/devtools/server/main.js
+++ b/devtools/server/main.js
@@ -1602,17 +1602,17 @@ DebuggerServerConnection.prototype = {
     if (!actor) {
       this.transport.send({ from: actorID ? actorID : "root",
                             error: "noSuchActor",
                             message: "No such actor for ID: " + actorID });
       return null;
     }
 
     // Dynamically-loaded actors have to be created lazily.
-    if (actor instanceof ObservedActorFactory) {
+    if (actor instanceof ObservedActorFactory || actor.isLazy) {
       try {
         actor = actor.createActor();
       } catch (error) {
         const prefix = "Error occurred while creating actor '" + actor.name;
         this.transport.send(this._unknownError(actorID, prefix, error));
       }
     } else if (typeof (actor) !== "object") {
       // ActorPools should now contain only actor instances (i.e. objects)