Bug 1302148 - Fix various code leaking devtools toolbox. r=bgrins
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 14 Sep 2016 06:33:49 -0700
changeset 314484 d772aeb63c38a896a00453229e309601dae88c35
parent 314483 cbe6f8892338d106afe6e49ea5425f380935858d
child 314485 d0b6ed087223c1be7d9ce55bfc4dbd663551dd32
push id81903
push usercbook@mozilla.com
push dateTue, 20 Sep 2016 10:04:26 +0000
treeherdermozilla-inbound@150109898e5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1302148
milestone51.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 1302148 - Fix various code leaking devtools toolbox. r=bgrins MozReview-Commit-ID: 9oqcXeuM9i
devtools/client/framework/devtools-browser.js
devtools/client/framework/toolbox.js
devtools/client/shared/developer-toolbar.js
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -629,17 +629,17 @@ var gDevToolsBrowser = exports.gDevTools
     }
     gDevToolsBrowser._trackedBrowserWindows.delete(win);
     win.removeEventListener("unload", this);
 
     BrowserMenus.removeMenus(win.document);
 
     // Destroy toolboxes for closed window
     for (let [target, toolbox] of gDevTools._toolboxes) {
-      if (toolbox.win == win) {
+      if (toolbox.win.top == win) {
         toolbox.destroy();
       }
     }
 
     // Destroy the Developer toolbar if it has been accessed
     let desc = Object.getOwnPropertyDescriptor(win, "DeveloperToolbar");
     if (desc && !desc.get) {
       win.DeveloperToolbar.destroy();
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -818,19 +818,17 @@ Toolbox.prototype = {
         continue;
       }
 
       let button = this.doc.createElementNS(HTML_NS, "button");
       button.id = "toolbox-dock-" + position;
       button.className = "toolbox-dock-button devtools-button";
       button.setAttribute("title", L10N.getStr("toolboxDockButtons." +
                                                   position + ".tooltip"));
-      button.addEventListener("click", () => {
-        this.switchHost(position);
-      });
+      button.addEventListener("click", this.switchHost.bind(this, position));
 
       dockBox.appendChild(button);
     }
   },
 
   _getMinimizeButtonShortcutTooltip: function () {
     let str = L10N.getStr("toolbox.minimize.key");
     let key = KeyShortcuts.parseElectronKey(this.win, str);
@@ -1131,19 +1129,17 @@ Toolbox.prototype = {
     radio.setAttribute("ordinal", toolDefinition.ordinal);
     radio.setAttribute("tooltiptext", toolDefinition.tooltip);
     if (toolDefinition.invertIconForLightTheme) {
       radio.setAttribute("icon-invertable", "light-theme");
     } else if (toolDefinition.invertIconForDarkTheme) {
       radio.setAttribute("icon-invertable", "dark-theme");
     }
 
-    radio.addEventListener("command", () => {
-      this.selectTool(id);
-    });
+    radio.addEventListener("command", this.selectTool.bind(this, id));
 
     // spacer lets us center the image and label, while allowing cropping
     let spacer = this.doc.createElement("spacer");
     spacer.setAttribute("flex", "1");
     radio.appendChild(spacer);
 
     if (toolDefinition.icon) {
       let image = this.doc.createElement("image");
@@ -2071,23 +2067,33 @@ Toolbox.prototype = {
       this._sourceMapService.destroy();
       this._sourceMapService = null;
     }
 
     if (this.webconsolePanel) {
       this._saveSplitConsoleHeight();
       this.webconsolePanel.removeEventListener("resize",
         this._saveSplitConsoleHeight);
+      this.webconsolePanel = null;
     }
-    this.closeButton.removeEventListener("click", this.destroy, true);
-    this.textboxContextMenuPopup.removeEventListener("popupshowing",
-      this._updateTextboxMenuItems, true);
-    this.tabbar.removeEventListener("focus", this._onTabbarFocus, true);
-    this.tabbar.removeEventListener("click", this._onTabbarFocus, true);
-    this.tabbar.removeEventListener("keypress", this._onTabbarArrowKeypress);
+    if (this.closeButton) {
+      this.closeButton.removeEventListener("click", this.destroy, true);
+      this.closeButton = null;
+    }
+    if (this.textboxContextMenuPopup) {
+      this.textboxContextMenuPopup.removeEventListener("popupshowing",
+        this._updateTextboxMenuItems, true);
+      this.textboxContextMenuPopup = null;
+    }
+    if (this.tabbar) {
+      this.tabbar.removeEventListener("focus", this._onTabbarFocus, true);
+      this.tabbar.removeEventListener("click", this._onTabbarFocus, true);
+      this.tabbar.removeEventListener("keypress", this._onTabbarArrowKeypress);
+      this.tabbar = null;
+    }
 
     let outstanding = [];
     for (let [id, panel] of this._toolPanels) {
       try {
         gDevTools.emit(id + "-destroy", this, panel);
         this.emit(id + "-destroy", panel);
 
         outstanding.push(panel.destroy());
--- a/devtools/client/shared/developer-toolbar.js
+++ b/devtools/client/shared/developer-toolbar.js
@@ -100,38 +100,38 @@ var CommandUtils = {
 
         if (command.tooltipText != null) {
           button.setAttribute("title", command.tooltipText);
         }
         else if (command.description != null) {
           button.setAttribute("title", command.description);
         }
 
-        button.addEventListener("click", () => {
-          requisition.updateExec(typed);
-        }, false);
+        button.addEventListener("click",
+          requisition.updateExec.bind(requisition, typed));
 
         button.addEventListener("keypress", (event) => {
           if (ViewHelpers.isSpaceOrReturn(event)) {
             event.preventDefault();
             requisition.updateExec(typed);
           }
         }, false);
 
         // Allow the command button to be toggleable
+        let onChange = null;
         if (command.state) {
           button.setAttribute("autocheck", false);
 
           /**
            * The onChange event should be called with an event object that
            * contains a target property which specifies which target the event
            * applies to. For legacy reasons the event object can also contain
            * a tab property.
            */
-          let onChange = (eventName, ev) => {
+          onChange = (eventName, ev) => {
             if (ev.target == target || ev.tab == target.tab) {
 
               let updateChecked = (checked) => {
                 if (checked) {
                   button.setAttribute("checked", true);
                 }
                 else if (button.hasAttribute("checked")) {
                   button.removeAttribute("checked");
@@ -150,22 +150,24 @@ var CommandUtils = {
               else {
                 updateChecked(reply);
               }
             }
           };
 
           command.state.onChange(target, onChange);
           onChange("", { target: target });
-          document.defaultView.addEventListener("unload", () => {
-            if (command.state.offChange) {
-              command.state.offChange(target, onChange);
-            }
-          }, false);
-        }
+        };
+        document.defaultView.addEventListener("unload", function (event) {
+          if (onChange && command.state.offChange) {
+            command.state.offChange(target, onChange);
+          }
+          button.remove();
+          button = null;
+        }, { once: true });
 
         requisition.clear();
 
         return button;
       });
     });
   },