Bug 1473511 - use protocol.js pool to manage thread actor; r=ochameau draft
authoryulia <ystartsev@mozilla.com>
Thu, 05 Jul 2018 12:22:27 +0200
changeset 815999 0f26a6315489e1d104ee92070a8d51e3c13a7883
parent 814369 90be04d99fc7941cb9b7186bf5f95e184a4e989a
push id115713
push userbmo:ystartsev@mozilla.com
push dateTue, 10 Jul 2018 12:49:28 +0000
reviewersochameau
bugs1473511
milestone63.0a1
Bug 1473511 - use protocol.js pool to manage thread actor; r=ochameau MozReview-Commit-ID: 6Ifmo0GCdwe
devtools/server/actors/targets/browsing-context.js
devtools/server/actors/thread.js
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -285,17 +285,16 @@ const browsingContextTargetPrototype = {
     if (this.exited) {
       return null;
     }
     const form = this.form();
     return this.conn._getOrCreateActor(form.consoleActor);
   },
 
   _targetScopedActorPool: null,
-  _contextPool: null,
 
   /**
    * A constant prefix that will be used to form the actor ID by the server.
    */
   typeName: "browsingContextTarget",
 
   /**
    * An object on which listen for DOMWindowCreated and pageshow events.
@@ -574,17 +573,17 @@ const browsingContextTargetPrototype = {
    * Does the actual work of attaching to a browsing context.
    */
   _attach() {
     if (this._attached) {
       return;
     }
 
     // Create a pool for context-lifetime actors.
-    this._pushContext();
+    this._createThreadActor();
 
     // on xpcshell, there is no document
     if (this.window) {
       this._progressListener = new DebuggerProgressListener(this);
 
       // Save references to the original document we attached to
       this._originalWindow = this.window;
 
@@ -851,38 +850,29 @@ const browsingContextTargetPrototype = {
     this.conn.send({
       from: this.actorID,
       type: "frameUpdate",
       destroyAll: true
     });
   },
 
   /**
-   * Creates a thread actor and a pool for context-lifetime actors. It then sets
-   * up the content window for debugging.
+   * Creates and manages the thread actor as part of the Browsing Context Target pool.
+   * This sets up the content window for being debugged
    */
-  _pushContext() {
-    assert(!this._contextPool, "Can't push multiple contexts");
-
-    this._contextPool = new ActorPool(this.conn);
-    this.conn.addActorPool(this._contextPool);
-
+  _createThreadActor() {
     this.threadActor = new ThreadActor(this, this.window);
-    this._contextPool.addActor(this.threadActor);
+    this.manage(this.threadActor);
   },
 
   /**
-   * Exits the current thread actor and removes the context-lifetime actor pool.
+   * Exits the current thread actor and removes it from the Browsing Context Target pool.
    * The content window is no longer being debugged after this call.
    */
-  _popContext() {
-    assert(!!this._contextPool, "No context to pop.");
-
-    this.conn.removeActorPool(this._contextPool);
-    this._contextPool = null;
+  _destroyThreadActor() {
     this.threadActor.exit();
     this.threadActor = null;
     this._sources = null;
   },
 
   /**
    * Does the actual work of detaching from a browsing context.
    *
@@ -906,17 +896,17 @@ const browsingContextTargetPrototype = {
 
       // Removes the observers being set in _watchDocShells
       if (this.listenForNewDocShells) {
         Services.obs.removeObserver(this, "webnavigation-create");
       }
       Services.obs.removeObserver(this, "webnavigation-destroy");
     }
 
-    this._popContext();
+    this._destroyThreadActor();
 
     // Shut down actors that belong to this target's pool.
     this._styleSheetActors.clear();
     if (this._targetScopedActorPool) {
       this.conn.removeActorPool(this._targetScopedActorPool);
       this._targetScopedActorPool = null;
     }
 
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -6,17 +6,17 @@
 
 "use strict";
 
 const Services = require("Services");
 const { Cr } = require("chrome");
 const { ActorPool, GeneratedLocation } = require("devtools/server/actors/common");
 const { createValueGrip } = require("devtools/server/actors/object/utils");
 const { longStringGrip } = require("devtools/server/actors/object/long-string");
-const { ActorClassWithSpec } = require("devtools/shared/protocol");
+const { ActorClassWithSpec, Actor } = require("devtools/shared/protocol");
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 const flags = require("devtools/shared/flags");
 const { assert, dumpn } = DevToolsUtils;
 const { DevToolsWorker } = require("devtools/shared/worker/worker");
 const { threadSpec } = require("devtools/shared/specs/script");
 
 loader.lazyRequireGetter(this, "findCssSelector", "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "BreakpointActor", "devtools/server/actors/breakpoint", true);
@@ -49,16 +49,17 @@ loader.lazyRequireGetter(this, "EventEmi
  *          - makeDebugger: A function that takes no arguments and instantiates
  *            a Debugger that manages its globals on its own.
  * @param aGlobal object [optional]
  *        An optional (for content debugging only) reference to the content
  *        window.
  */
 const ThreadActor = ActorClassWithSpec(threadSpec, {
   initialize: function(parent, global) {
+    Actor.prototype.initialize.call(this, parent.conn);
     this._state = "detached";
     this._frameActors = [];
     this._parent = parent;
     this._dbg = null;
     this._gripDepth = 0;
     this._threadLifetimePool = null;
     this._parentClosed = false;
     this._scripts = null;
@@ -202,16 +203,22 @@ const ThreadActor = ActorClassWithSpec(t
     this.conn.send({
       from: this.actorID,
       type: "newGlobal",
       // TODO: after bug 801084 lands see if we need to JSONify this.
       hostAnnotations: global.hostAnnotations
     });
   },
 
+  /**
+   * Clean up listeners, debuggees and clear actor pools associated with
+   * the lifetime of this actor. This does not destroy the thread actor,
+   * it resets it. This is used in methods `onReleaseMany` `onDetatch` and
+   * `exit`. The actor is truely destroyed in the `exit method`.
+   */
   destroy: function() {
     dumpn("in ThreadActor.prototype.destroy");
     if (this._state == "paused") {
       this.onResume();
     }
 
     // Blow away our source actor ID store because those IDs are only
     // valid for this connection. This is ok because we never keep
@@ -238,16 +245,24 @@ const ThreadActor = ActorClassWithSpec(t
   },
 
   /**
    * destroy the debugger and put the actor in the exited state.
    */
   exit: function() {
     this.destroy();
     this._state = "exited";
+    // This actor has a slightly different destroy behavior than other
+    // actors using Protocol.js. The thread actor may detach but still
+    // be in use, however detach calls the destroy method, even though it
+    // expects the actor to still be alive. Therefore, we are calling
+    // `Actor.prototype.destroy` in the `exit` method, after its state has
+    // been set to "exited", where we are certain that the actor is no
+    // longer in use.
+    Actor.prototype.destroy.call(this);
   },
 
   // Request handlers
   onAttach: function(request) {
     if (this.state === "exited") {
       return { type: "exited" };
     }