Bug 1145049 - Clean inspector on detach/disconnect. r=pbrosset
☠☠ backed out by fed4fafd1877 ☠ ☠
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 15 Apr 2015 00:35:32 +0200
changeset 239316 fb97412bef73d9e9e7a4b71ce510ae7b0f699f0a
parent 239315 9d319ca0f937765ebbbb5a095b0c5125ecf77fbb
child 239317 69819d44541fce0bb91c6d20093e6670c771e59d
push id28589
push userryanvm@gmail.com
push dateWed, 15 Apr 2015 19:13:10 +0000
treeherdermozilla-central@24ccca4707eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbrosset
bugs1145049
milestone40.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 1145049 - Clean inspector on detach/disconnect. r=pbrosset
toolkit/devtools/server/actors/highlighter.js
toolkit/devtools/server/actors/inspector.js
toolkit/devtools/server/actors/styles.js
--- a/toolkit/devtools/server/actors/highlighter.js
+++ b/toolkit/devtools/server/actors/highlighter.js
@@ -177,27 +177,34 @@ let HighlighterActor = exports.Highlight
     // Only rebuild the highlighter if the window type changed.
     if (isXUL(this._tabActor) !== this._isPreviousWindowXUL) {
       this._destroyHighlighter();
       this._createHighlighter();
     }
   },
 
   destroy: function() {
+    if (!this._inspector) {
+      return;
+    }
     protocol.Actor.prototype.destroy.call(this);
 
     this._destroyHighlighter();
     events.off(this._tabActor, "navigate", this._onNavigate);
     this._autohide = null;
     this._inspector = null;
     this._walker = null;
     this._tabActor = null;
     this._layoutHelpers = null;
   },
 
+  disconnect: function () {
+    this.destroy();
+  },
+
   /**
    * Display the box model highlighting on a given NodeActor.
    * There is only one instance of the box model highlighter, so calling this
    * method several times won't display several highlighters, it will just move
    * the highlighter instance to these nodes.
    *
    * @param NodeActor The node to be highlighted
    * @param Options See the request part for existing options. Note that not
--- a/toolkit/devtools/server/actors/inspector.js
+++ b/toolkit/devtools/server/actors/inspector.js
@@ -215,16 +215,22 @@ var NodeActor = exports.NodeActor = prot
    */
   get conn() this.walker.conn,
 
   isDocumentElement: function() {
     return this.rawNode.ownerDocument &&
         this.rawNode.ownerDocument.documentElement === this.rawNode;
   },
 
+  destroy: function () {
+    protocol.Actor.prototype.destroy.call(this);
+    this.rawNode = null;
+    this.walker = null;
+  },
+
   // Returns the JSON representation of this object over the wire.
   form: function(detail) {
     if (detail === "actorid") {
       return this.actorID;
     }
 
     let parentNode = this.walker.parentNode(this);
 
@@ -1200,23 +1206,37 @@ var WalkerActor = protocol.ActorClass({
   getDocumentWalker: function(node, whatToShow) {
     // Allow native anon content (like <video> controls) if preffed on
     let nodeFilter = this.showAllAnonymousContent ? allAnonymousContentTreeWalkerFilter : standardTreeWalkerFilter;
     return new DocumentWalker(node, this.rootWin, whatToShow, nodeFilter);
   },
 
   destroy: function() {
     try {
+      if (this._destroyed) {
+        return;
+      }
       this._destroyed = true;
 
       this.clearPseudoClassLocks();
       this._activePseudoClassLocks = null;
 
       this._hoveredNode = null;
       this.rootDoc = null;
+      this.rootWin = null;
+      this.rootNode = null;
+      this.tabActor = null;
+      this.layoutHelpers = null;
+      this._orphaned = null;
+      this._retainedOrphans = null;
+      this._refMap.forEach(actor => {
+        this.unmanage(actor);
+        actor.destroy();
+      });
+      this._refMap = null;
 
       this.reflowObserver.off("reflows", this._onReflows);
       this.reflowObserver = null;
       releaseLayoutChangesObserver(this.tabActor);
 
       events.emit(this, "destroyed");
     } catch(e) {
       console.error(e);
@@ -3164,16 +3184,40 @@ var AttributeModificationList = Class({
  */
 var InspectorActor = exports.InspectorActor = protocol.ActorClass({
   typeName: "inspector",
   initialize: function(conn, tabActor) {
     protocol.Actor.prototype.initialize.call(this, conn);
     this.tabActor = tabActor;
   },
 
+  destroy: function () {
+    protocol.Actor.prototype.destroy.call(this);
+    this._highlighterPromise = null;
+    this._pageStylePromise = null;
+    this._walkerPromise = null;
+    if (this.walker) {
+      this.walker.destroy();
+    }
+    this.walker = null;
+    if (this.pageStyle) {
+      this.pageStyle.destroy();
+    }
+    this.pageStyle = null;
+    if (this.highlighter) {
+      this.highlighter.destroy();
+    }
+    this.highlighter = null;
+    this.tabActor = null;
+  },
+
+  disconnect: function () {
+    this.destroy();
+  },
+
   get window() this.tabActor.window,
 
   getWalker: method(function(options={}) {
     if (this._walkerPromise) {
       return this._walkerPromise;
     }
 
     let deferred = promise.defer();
@@ -3208,17 +3252,17 @@ var InspectorActor = exports.InspectorAc
   }),
 
   getPageStyle: method(function() {
     if (this._pageStylePromise) {
       return this._pageStylePromise;
     }
 
     this._pageStylePromise = this.getWalker().then(walker => {
-      return PageStyleActor(this);
+      return this.pageStyle = PageStyleActor(this);
     });
     return this._pageStylePromise;
   }, {
     request: {},
     response: {
       pageStyle: RetVal("pagestyle")
     }
   }),
@@ -3237,17 +3281,17 @@ var InspectorActor = exports.InspectorAc
    * @return {HighlighterActor}
    */
   getHighlighter: method(function (autohide) {
     if (this._highlighterPromise) {
       return this._highlighterPromise;
     }
 
     this._highlighterPromise = this.getWalker().then(walker => {
-      return HighlighterActor(this, autohide);
+      return this.highlighter = HighlighterActor(this, autohide);
     });
     return this._highlighterPromise;
   }, {
     request: {
       autohide: Arg(0, "boolean")
     },
     response: {
       highligter: RetVal("highlighter")
--- a/toolkit/devtools/server/actors/styles.js
+++ b/toolkit/devtools/server/actors/styles.js
@@ -119,20 +119,42 @@ var PageStyleActor = protocol.ActorClass
                    "creating a PageStyleActor.");
     }
     this.walker = inspector.walker;
     this.cssLogic = new CssLogic;
 
     // Stores the association of DOM objects -> actors
     this.refMap = new Map;
 
+    // Keep the list of StyleRuleActor created for font
+    // in order to clean them up when the PageStyleActor is collected
+    this.fontStyleActors = new Set;
+
     this.onFrameUnload = this.onFrameUnload.bind(this);
     events.on(this.inspector.tabActor, "will-navigate", this.onFrameUnload);
   },
 
+  destroy: function () {
+    if (!this.walker)
+      return;
+    protocol.Actor.prototype.destroy.call(this);
+    events.off(this.inspector.tabActor, "will-navigate", this.onFrameUnload);
+    this.inspector = null;
+    this.walker = null;
+    this.refMap.forEach(actor => {
+      this.unmanage(actor);
+      actor.destroy();
+    });
+    this.fontStyleActors.forEach(actor => actor.destroy());
+    this.fontStyleActors = null;
+    this.refMap = null;
+    this.cssLogic = null;
+    this._styleElement = null;
+  },
+
   get conn() this.inspector.conn,
 
   form: function(detail) {
     if (detail === "actorid") {
       return this.actorID;
     }
 
     return {
@@ -302,17 +324,19 @@ var PageStyleActor = protocol.ActorClass
         URI: font.URI,
         format: font.format,
         localName: font.localName,
         metadata: font.metadata
       }
 
       // If this font comes from a @font-face rule
       if (font.rule) {
-        fontFace.rule = StyleRuleActor(this, font.rule);
+        let styleActor = StyleRuleActor(this, font.rule);
+        this.fontStyleActors.add(styleActor);
+        fontFace.rule = styleActor;
         fontFace.ruleText = font.rule.cssText;
       }
 
       // Get the weight and style of this font for the preview and sort order
       let weight = NORMAL_FONT_WEIGHT, style = "";
       if (font.rule) {
         weight = font.rule.style.getPropertyValue("font-weight")
                  || NORMAL_FONT_WEIGHT;
@@ -954,16 +978,26 @@ var StyleRuleActor = protocol.ActorClass
         style: item.style,
         toString: function() "[element rule " + this.style + "]"
       }
     }
   },
 
   get conn() this.pageStyle.conn,
 
+  destroy: function () {
+    if (!this.rawStyle)
+      return;
+    protocol.Actor.prototype.destroy.call(this);
+    this.rawStyle = null;
+    this.pageStyle = null;
+    this.rawNode = null;
+    this.rawRule = null;
+  },
+
   // Objects returned by this actor are owned by the PageStyleActor
   // to which this rule belongs.
   get marshallPool() this.pageStyle,
 
   getDocument: function(sheet) {
     let document;
 
     if (sheet.ownerNode instanceof Ci.nsIDOMHTMLDocument) {