Bug 1309631 - Add guards to ensure RDM destroys on tab close. r=ochameau draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Mon, 24 Oct 2016 17:39:01 -0500
changeset 429296 c23b151236505e4351600179b7c6193280ef9c38
parent 428736 c845bfd0accb7e0c29b41713255963b08006e701
child 534934 b82c29638b5b85851dac0b5131aec0c1d9c2454b
push id33525
push userbmo:jryans@gmail.com
push dateTue, 25 Oct 2016 15:18:59 +0000
reviewersochameau
bugs1309631
milestone52.0a1
Bug 1309631 - Add guards to ensure RDM destroys on tab close. r=ochameau In the tab close case, RDM destroy has to complete with yielding. It's possible to open RDM in a tab and then close the tab before RDM init has finished, which means any properties set during RDM init can't be assumed to exist. In this change, we guard all usages of such properties to ensure destroy can still finish. MozReview-Commit-ID: Eb5is9YJtY5
devtools/client/responsive.html/browser/swap.js
devtools/client/responsive.html/manager.js
--- a/devtools/client/responsive.html/browser/swap.js
+++ b/devtools/client/responsive.html/browser/swap.js
@@ -108,18 +108,20 @@ function swapToInnerBrowser({ tab, conta
       // Force the browser UI to match the new state of the tab and browser.
       thawNavigationState(tab);
       gBrowser.setTabTitle(tab);
       gBrowser.updateCurrentBrowser(true);
     }),
 
     stop() {
       // 1. Stop the tunnel between outer and inner browsers.
-      tunnel.stop();
-      tunnel = null;
+      if (tunnel) {
+        tunnel.stop();
+        tunnel = null;
+      }
 
       // 2. Create a temporary, hidden tab to hold the content.
       let contentTab = gBrowser.addTab("about:blank", {
         skipAnimation: true,
       });
       gBrowser.hideTab(contentTab);
       let contentBrowser = contentTab.linkedBrowser;
 
--- a/devtools/client/responsive.html/manager.js
+++ b/devtools/client/responsive.html/manager.js
@@ -373,39 +373,43 @@ ResponsiveUI.prototype = {
     // Ensure init has finished before starting destroy
     if (!isTabContentDestroying) {
       yield this.inited;
     }
 
     this.tab.removeEventListener("TabClose", this);
     this.tab.removeEventListener("BeforeTabRemotenessChange", this);
     this.browserWindow.removeEventListener("unload", this);
-    this.toolWindow.removeEventListener("message", this);
+    if (this.toolWindow) {
+      this.toolWindow.removeEventListener("message", this);
+    }
 
-    if (!isTabContentDestroying) {
+    if (!isTabContentDestroying && this.toolWindow) {
       // Notify the inner browser to stop the frame script
       yield message.request(this.toolWindow, "stop-frame-script");
     }
 
     // Destroy local state
     let swap = this.swap;
     this.browserWindow = null;
     this.tab = null;
     this.inited = null;
     this.toolWindow = null;
     this.swap = null;
 
     // Close the debugger client used to speak with emulation actor.
     // The actor handles clearing any overrides itself, so it's not necessary to clear
     // anything on shutdown client side.
-    let clientClosed = this.client.close();
-    if (!isTabContentDestroying) {
-      yield clientClosed;
+    if (this.client) {
+      let clientClosed = this.client.close();
+      if (!isTabContentDestroying) {
+        yield clientClosed;
+      }
+      this.client = this.emulationFront = null;
     }
-    this.client = this.emulationFront = null;
 
     if (!isWindowClosing) {
       // Undo the swap and return the content back to a normal tab
       swap.stop();
     }
 
     this.destroyed = true;