Bug 1315391 - Clean up actor destruction after changing to `destroy`. r=ochameau draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 11 Nov 2016 18:24:41 -0600
changeset 439475 7b5656671c394f2d15a924375c3988e45d4aefca
parent 438608 5756d6f47a66814810c3976c12a47b9045aeae9f
child 537173 8d0085bbd36fbdecf669b500929db12dfa897019
push id36010
push userbmo:jryans@gmail.com
push dateWed, 16 Nov 2016 02:36:02 +0000
reviewersochameau
bugs1315391
milestone52.0a1
Bug 1315391 - Clean up actor destruction after changing to `destroy`. r=ochameau The `destroy` method in some actors would throw errors or was incomplete, leading to various issues closing the toolbox after the previous patch. This cleans up all such cases noticed through manual testing of the toolbox. MozReview-Commit-ID: 6EZYFwjSri
devtools/server/actors/css-properties.js
devtools/server/actors/promises.js
devtools/server/actors/storage.js
devtools/server/primitive.js
--- a/devtools/server/actors/css-properties.js
+++ b/devtools/server/actors/css-properties.js
@@ -14,19 +14,18 @@ const protocol = require("devtools/share
 const { ActorClassWithSpec, Actor } = protocol;
 const { cssPropertiesSpec } = require("devtools/shared/specs/css-properties");
 const { CSS_PROPERTIES, CSS_TYPES } = require("devtools/shared/css/properties-db");
 const { cssColors } = require("devtools/shared/css/color-db");
 
 exports.CssPropertiesActor = ActorClassWithSpec(cssPropertiesSpec, {
   typeName: "cssProperties",
 
-  initialize(conn, parent) {
+  initialize(conn) {
     Actor.prototype.initialize.call(this, conn);
-    this.parent = parent;
   },
 
   destroy() {
     Actor.prototype.destroy.call(this);
   },
 
   getCSSDatabase() {
     const properties = generateCssProperties();
--- a/devtools/server/actors/promises.js
+++ b/devtools/server/actors/promises.js
@@ -13,46 +13,46 @@ loader.lazyRequireGetter(this, "events",
 
 /**
  * The Promises Actor provides support for getting the list of live promises and
  * observing changes to their settlement state.
  */
 var PromisesActor = protocol.ActorClassWithSpec(promisesSpec, {
   /**
    * @param conn DebuggerServerConnection.
-   * @param parent TabActor|RootActor
+   * @param parentActor TabActor|RootActor
    */
-  initialize: function (conn, parent) {
+  initialize: function (conn, parentActor) {
     protocol.Actor.prototype.initialize.call(this, conn);
 
     this.conn = conn;
-    this.parent = parent;
+    this.parentActor = parentActor;
     this.state = "detached";
     this._dbg = null;
     this._gripDepth = 0;
     this._navigationLifetimePool = null;
     this._newPromises = null;
     this._promisesSettled = null;
 
     this.objectGrip = this.objectGrip.bind(this);
     this._makePromiseEventHandler = this._makePromiseEventHandler.bind(this);
     this._onWindowReady = this._onWindowReady.bind(this);
   },
 
   destroy: function () {
-    protocol.Actor.prototype.destroy.call(this, this.conn);
-
     if (this.state === "attached") {
       this.detach();
     }
+
+    protocol.Actor.prototype.destroy.call(this, this.conn);
   },
 
   get dbg() {
     if (!this._dbg) {
-      this._dbg = this.parent.makeDebugger();
+      this._dbg = this.parentActor.makeDebugger();
     }
     return this._dbg;
   },
 
   /**
    * Attach to the PromisesActor.
    */
   attach: expectState("detached", function () {
@@ -60,24 +60,24 @@ var PromisesActor = protocol.ActorClassW
 
     this._navigationLifetimePool = this._createActorPool();
     this.conn.addActorPool(this._navigationLifetimePool);
 
     this._newPromises = [];
     this._promisesSettled = [];
 
     this.dbg.findScripts().forEach(s => {
-      this.parent.sources.createSourceActors(s.source);
+      this.parentActor.sources.createSourceActors(s.source);
     });
 
     this.dbg.onNewScript = s => {
-      this.parent.sources.createSourceActors(s.source);
+      this.parentActor.sources.createSourceActors(s.source);
     };
 
-    events.on(this.parent, "window-ready", this._onWindowReady);
+    events.on(this.parentActor, "window-ready", this._onWindowReady);
 
     this.state = "attached";
   }, "attaching to the PromisesActor"),
 
   /**
    * Detach from the PromisesActor upon Debugger closing.
    */
   detach: expectState("attached", function () {
@@ -87,17 +87,17 @@ var PromisesActor = protocol.ActorClassW
     this._newPromises = null;
     this._promisesSettled = null;
 
     if (this._navigationLifetimePool) {
       this.conn.removeActorPool(this._navigationLifetimePool);
       this._navigationLifetimePool = null;
     }
 
-    events.off(this.parent, "window-ready", this._onWindowReady);
+    events.off(this.parentActor, "window-ready", this._onWindowReady);
 
     this.state = "detached";
   }),
 
   _createActorPool: function () {
     let pool = new ActorPool(this.conn);
     pool.objectActors = new WeakMap();
     return pool;
@@ -117,17 +117,17 @@ var PromisesActor = protocol.ActorClassW
     }
 
     let actor = new ObjectActor(promise, {
       getGripDepth: () => this._gripDepth,
       incrementGripDepth: () => this._gripDepth++,
       decrementGripDepth: () => this._gripDepth--,
       createValueGrip: v =>
         createValueGrip(v, this._navigationLifetimePool, this.objectGrip),
-      sources: () => this.parent.sources,
+      sources: () => this.parentActor.sources,
       createEnvironmentActor: () => DevToolsUtils.reportException(
         "PromisesActor", Error("createEnvironmentActor not yet implemented")),
       getGlobalDebugObject: () => DevToolsUtils.reportException(
         "PromisesActor", Error("getGlobalDebugObject not yet implemented")),
     });
 
     this._navigationLifetimePool.addActor(actor);
     this._navigationLifetimePool.objectActors.set(promise, actor);
--- a/devtools/server/actors/storage.js
+++ b/devtools/server/actors/storage.js
@@ -156,16 +156,19 @@ StorageActors.defaults = function (typeN
     destroy() {
       if (observationTopic) {
         Services.obs.removeObserver(this, observationTopic, false);
       }
       events.off(this.storageActor, "window-ready", this.onWindowReady);
       events.off(this.storageActor, "window-destroyed", this.onWindowDestroyed);
 
       this.hostVsStores.clear();
+
+      protocol.Actor.prototype.destroy.call(this);
+
       this.storageActor = null;
     },
 
     getNamesForHost(host) {
       return [...this.hostVsStores.get(host).keys()];
     },
 
     getValuesForHost(host, name) {
@@ -410,17 +413,21 @@ StorageActors.createActor({
     // single process mode.
     if (!DebuggerServer.isInChildProcess) {
       this.removeCookieObservers();
     }
 
     events.off(this.storageActor, "window-ready", this.onWindowReady);
     events.off(this.storageActor, "window-destroyed", this.onWindowDestroyed);
 
-    this._pendingResponse = this.storageActor = null;
+    this._pendingResponse = null;
+
+    protocol.Actor.prototype.destroy.call(this);
+
+    this.storageActor = null;
   },
 
   /**
    * Given a cookie object, figure out all the matching hosts from the page that
    * the cookie belong to.
    */
   getMatchingHosts(cookies) {
     if (!cookies.length) {
@@ -1439,16 +1446,20 @@ StorageActors.createActor({
   },
 
   destroy() {
     this.hostVsStores.clear();
     this.objectsSize = null;
 
     events.off(this.storageActor, "window-ready", this.onWindowReady);
     events.off(this.storageActor, "window-destroyed", this.onWindowDestroyed);
+
+    protocol.Actor.prototype.destroy.call(this);
+
+    this.storageActor = null;
   },
 
   /**
    * Remove an indexedDB database from given host with a given name.
    */
   removeDatabase: Task.async(function* (host, name) {
     let win = this.storageActor.getWindowFromHost(host);
     if (!win) {
@@ -2228,19 +2239,18 @@ let StorageActor = protocol.ActorClassWi
     return this.parentActor.window.document;
   },
 
   get windows() {
     return this.childWindowPool;
   },
 
   initialize(conn, tabActor) {
-    protocol.Actor.prototype.initialize.call(this, null);
+    protocol.Actor.prototype.initialize.call(this, conn);
 
-    this.conn = conn;
     this.parentActor = tabActor;
 
     this.childActorPool = new Map();
     this.childWindowPool = new Set();
 
     // Fetch all the inner iframe windows in this tab.
     this.fetchChildWindows(this.parentActor.docShell);
 
@@ -2266,30 +2276,34 @@ let StorageActor = protocol.ActorClassWi
   destroy() {
     clearTimeout(this.batchTimer);
     this.batchTimer = null;
     // Remove observers
     Services.obs.removeObserver(this, "content-document-global-created", false);
     Services.obs.removeObserver(this, "inner-window-destroyed", false);
     this.destroyed = true;
     if (this.parentActor.browser) {
-      this.parentActor.browser.removeEventListener(
-        "pageshow", this.onPageChange, true);
-      this.parentActor.browser.removeEventListener(
-        "pagehide", this.onPageChange, true);
+      this.parentActor.browser.removeEventListener("pageshow", this.onPageChange, true);
+      this.parentActor.browser.removeEventListener("pagehide", this.onPageChange, true);
     }
     // Destroy the registered store types
     for (let actor of this.childActorPool.values()) {
       actor.destroy();
     }
     this.childActorPool.clear();
     this.childWindowPool.clear();
-    this.childWindowPool = this.childActorPool = this.__poolMap = this.conn =
-      this.parentActor = this.boundUpdate = this.registeredPool =
-      this._pendingResponse = null;
+
+    this.childActorPool = null;
+    this.childWindowPool = null;
+    this.parentActor = null;
+    this.boundUpdate = null;
+    this.registeredPool = null;
+    this._pendingResponse = null;
+
+    protocol.Actor.prototype.destroy.call(this);
   },
 
   /**
    * Given a docshell, recursively find out all the child windows from it.
    *
    * @param {nsIDocShell} item
    *        The docshell from which all inner windows need to be extracted.
    */
--- a/devtools/server/primitive.js
+++ b/devtools/server/primitive.js
@@ -23,19 +23,17 @@ const WebGLPrimitivesType = {
 const WebGLDrawArrays = "drawArrays";
 const WebGLDrawElements = "drawElements";
 
 var WebGLPrimitiveCounter = exports.WebGLPrimitiveCounter = Class({
   initialize: function (tabActor) {
     this.tabActor = tabActor;
   },
 
-  destroy: function () {
-    this.stopRecording();
-  },
+  destroy: function () {},
 
   /**
    * Starts monitoring primitive draws, storing the primitives count per tick.
    */
   resetCounts: function () {
     this._tris = 0;
     this._vertices = 0;
     this._points = 0;