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 314417 d772aeb63c38a896a00453229e309601dae88c35
parent 314416 cbe6f8892338d106afe6e49ea5425f380935858d
child 314418 d0b6ed087223c1be7d9ce55bfc4dbd663551dd32
push id20574
push usercbook@mozilla.com
push dateTue, 20 Sep 2016 10:05:16 +0000
treeherderfx-team@14705f779a46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1302148
milestone51.0a1
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;
       });
     });
   },