Bug 1276271 P1 Don't leak windows when response design tool is closed while active. r=jryans
authorBen Kelly <ben@wanderview.com>
Sat, 28 May 2016 05:59:09 -0700
changeset 338431 55dc8f7c505ce35a04ade9c2f00d0a3dd9dba790
parent 338430 650ba00927b41ceb006f20e86693c7ed262a09eb
child 338432 17d8abb5b1803418ea206cd60fe5386dc8dded98
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1276271
milestone49.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 1276271 P1 Don't leak windows when response design tool is closed while active. r=jryans
devtools/client/responsivedesign/responsivedesign.jsm
--- a/devtools/client/responsivedesign/responsivedesign.jsm
+++ b/devtools/client/responsivedesign/responsivedesign.jsm
@@ -182,16 +182,22 @@ function ResponsiveUI(aWindow, aTab)
   this.bound_stopResizing = this.stopResizing.bind(this);
   this.bound_onDrag = this.onDrag.bind(this);
   this.bound_changeUA = this.changeUA.bind(this);
   this.bound_onContentResize = this.onContentResize.bind(this);
 
   this.mm.addMessageListener("ResponsiveMode:OnContentResize",
                              this.bound_onContentResize);
 
+  // We must be ready to handle window or tab close now that we have saved
+  // ourselves in ActiveTabs.  Otherwise we risk leaking the window.
+  this.mainWindow.addEventListener("unload", this);
+  this.tab.addEventListener("TabClose", this);
+  this.tabContainer.addEventListener("TabSelect", this);
+
   ActiveTabs.set(this.tab, this);
 
   this.inited = this.init();
 }
 
 ResponsiveUI.prototype = {
   _transitionsEnabled: true,
   get transitionsEnabled() {
@@ -221,20 +227,16 @@ ResponsiveUI.prototype = {
       // Tests expect events on resize to yield on various size changes
       notifyOnResize: DevToolsUtils.testing,
     });
     yield started;
 
     // Load Presets
     this.loadPresets();
 
-    // Events
-    this.tab.addEventListener("TabClose", this);
-    this.tabContainer.addEventListener("TabSelect", this);
-
     // Setup the UI
     this.container.setAttribute("responsivemode", "true");
     this.stack.setAttribute("responsivemode", "true");
     this.buildUI();
     this.checkMenus();
 
     // Rotate the responsive mode if needed
     try {
@@ -338,35 +340,38 @@ ResponsiveUI.prototype = {
     debug(`CURRENT SIZE: ${this.stack.getAttribute("style")}`);
     let style = "max-width: none;" +
                 "min-width: 0;" +
                 "max-height: none;" +
                 "min-height: 0;";
     debug("RESET STACK SIZE");
     this.stack.setAttribute("style", style);
 
-    // Wait for resize message before stopping in the child when testing
-    if (DevToolsUtils.testing) {
+    // Wait for resize message before stopping in the child when testing,
+    // but only if we should expect to still get a message.
+    if (DevToolsUtils.testing && this.tab.linkedBrowser.messageManager) {
       yield this.waitForMessage("ResponsiveMode:OnContentResize");
     }
 
     if (this.isResizing)
       this.stopResizing();
 
     // Remove listeners.
     this.menulist.removeEventListener("select", this.bound_presetSelected, true);
     this.menulist.removeEventListener("change", this.bound_handleManualInput, true);
+    this.mainWindow.removeEventListener("unload", this);
     this.tab.removeEventListener("TabClose", this);
     this.tabContainer.removeEventListener("TabSelect", this);
     this.rotatebutton.removeEventListener("command", this.bound_rotate, true);
     this.screenshotbutton.removeEventListener("command", this.bound_screenshot, true);
     this.closebutton.removeEventListener("command", this.bound_close, true);
     this.addbutton.removeEventListener("command", this.bound_addPreset, true);
     this.removebutton.removeEventListener("command", this.bound_removePreset, true);
     this.touchbutton.removeEventListener("command", this.bound_touch, true);
+    this.userAgentInput.removeEventListener("blur", this.bound_changeUA, true);
 
     // Removed elements.
     this.container.removeChild(this.toolbar);
     if (this.bottomToolbar) {
       this.bottomToolbar.remove();
       delete this.bottomToolbar;
     }
     this.stack.removeChild(this.resizer);
@@ -386,19 +391,21 @@ ResponsiveUI.prototype = {
 
     yield new Promise((resolve, reject) => {
       this.client.close(resolve);
       this.client = this.tabClient = null;
     });
 
     this._telemetry.toolClosed("responsive");
 
-    let stopped = this.waitForMessage("ResponsiveMode:Stop:Done");
-    this.tab.linkedBrowser.messageManager.sendAsyncMessage("ResponsiveMode:Stop");
-    yield stopped;
+    if (this.tab.linkedBrowser.messageManager) {
+      let stopped = this.waitForMessage("ResponsiveMode:Stop:Done");
+      this.tab.linkedBrowser.messageManager.sendAsyncMessage("ResponsiveMode:Stop");
+      yield stopped;
+    }
 
     this.inited = null;
     ResponsiveUIManager.emit("off", { tab: this.tab });
   }),
 
   waitForMessage(message) {
     return new Promise(resolve => {
       let listener = () => {
@@ -421,16 +428,17 @@ ResponsiveUI.prototype = {
   },
 
   /**
    * Handle events
    */
   handleEvent: function (aEvent) {
     switch (aEvent.type) {
       case "TabClose":
+      case "unload":
         this.close();
         break;
       case "TabSelect":
         if (this.tab.selected) {
           this.checkMenus();
         } else if (!this.mainWindow.gBrowser.selectedTab.responsiveUI) {
           this.unCheckMenus();
         }
@@ -444,17 +452,20 @@ ResponsiveUI.prototype = {
   checkMenus: function RUI_checkMenus() {
     this.chromeDoc.getElementById("menu_responsiveUI").setAttribute("checked", "true");
   },
 
   /**
    * Uncheck the menu items.
    */
   unCheckMenus: function RUI_unCheckMenus() {
-    this.chromeDoc.getElementById("menu_responsiveUI").setAttribute("checked", "false");
+    let el = this.chromeDoc.getElementById("menu_responsiveUI");
+    if (el) {
+      el.setAttribute("checked", "false");
+    }
   },
 
   /**
    * Build the toolbar and the resizers.
    *
    * <vbox class="browserContainer"> From tabbrowser.xml
    *  <toolbar class="devtools-responsiveui-toolbar">
    *    <menulist class="devtools-responsiveui-menulist"/> // presets