Bug 1296736 - Exit RDM when loading non-remote URLs. r=ochameau draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 14 Oct 2016 16:46:54 -0500
changeset 425510 b2504ee1c620a648d26ad64824c3f491e6e16c18
parent 425499 bf3585a4e045f386eaeedf375904e1a82f3db9a8
child 533934 691a68dfb35b35dee9b01d1cfcf7856090541edd
push id32440
push userbmo:jryans@gmail.com
push dateFri, 14 Oct 2016 21:47:49 +0000
reviewersochameau
bugs1296736
milestone52.0a1
Bug 1296736 - Exit RDM when loading non-remote URLs. r=ochameau MozReview-Commit-ID: 8GUZkq2fsZb
devtools/client/responsive.html/browser/tunnel.js
devtools/client/responsive.html/manager.js
devtools/client/responsive.html/test/browser/browser.ini
devtools/client/responsive.html/test/browser/browser_shutdown_close_sync.js
devtools/client/responsive.html/test/browser/browser_tab_close.js
devtools/client/responsive.html/test/browser/browser_tab_remoteness_change.js
devtools/client/responsive.html/test/browser/head.js
--- a/devtools/client/responsive.html/browser/tunnel.js
+++ b/devtools/client/responsive.html/browser/tunnel.js
@@ -94,23 +94,17 @@ function tunnelToInnerBrowser(outer, inn
       // so this has the effect of overriding the message manager for browser UI code.
       mmTunnel = new MessageManagerTunnel(outer, inner);
 
       // We are tunneling to an inner browser with a specific remoteness, so it is simpler
       // for the logic of the browser UI to assume this tab has taken on that remoteness,
       // even though it's not true.  Since the actions the browser UI performs are sent
       // down to the inner browser by this tunnel, the tab's remoteness effectively is the
       // remoteness of the inner browser.
-      Object.defineProperty(outer, "isRemoteBrowser", {
-        get() {
-          return true;
-        },
-        configurable: true,
-        enumerable: true,
-      });
+      outer.setAttribute("remote", "true");
 
       // Clear out any cached state that references the current non-remote XBL binding,
       // such as form fill controllers.  Otherwise they will remain in place and leak the
       // outer docshell.
       outer.destroy();
       // The XBL binding for remote browsers uses the message manager for many actions in
       // the UI and that works well here, since it gives us one main thing we need to
       // route to the inner browser (the messages), instead of having to tweak many
@@ -217,21 +211,23 @@ function tunnelToInnerBrowser(outer, inn
 
       // Reset the XBL binding back to the default.
       outer.destroy();
       outer.style.MozBinding = "";
 
       // Reset overridden XBL properties and methods.  Deleting the override
       // means it will fallback to the original XBL binding definitions which
       // are on the prototype.
-      delete outer.isRemoteBrowser;
       delete outer.hasContentOpener;
       delete outer.docShellIsActive;
       delete outer.preserveLayers;
 
+      // Reset @remote since this is now back to a regular, non-remote browser
+      outer.setAttribute("remote", "false");
+
       // Delete the PopupNotifications getter added for permission doorhangers
       delete inner.ownerGlobal.PopupNotifications;
 
       mmTunnel.destroy();
       mmTunnel = null;
 
       // Invalidate outer's permanentKey so that SessionStore stops associating
       // things that happen to the outer browser with the content inside in the
--- a/devtools/client/responsive.html/manager.js
+++ b/devtools/client/responsive.html/manager.js
@@ -319,16 +319,18 @@ ResponsiveUI.prototype = {
         yield message.request(toolWindow, "init");
         toolWindow.addInitialViewport("about:blank");
         yield message.wait(toolWindow, "browser-mounted");
         return ui.getViewportBrowser();
       })
     });
     yield this.swap.start();
 
+    this.tab.addEventListener("BeforeTabRemotenessChange", this);
+
     // Notify the inner browser to start the frame script
     yield message.request(this.toolWindow, "start-frame-script");
 
     // Get the protocol ready to speak with emulation actor
     yield this.connectToServer();
   }),
 
   /**
@@ -345,28 +347,31 @@ ResponsiveUI.prototype = {
       return false;
     }
     this.destroying = true;
 
     // If our tab is about to be closed, there's not enough time to exit
     // gracefully, but that shouldn't be a problem since the tab will go away.
     // So, skip any yielding when we're about to close the tab.
     let isWindowClosing = options && options.reason === "unload";
-    let isTabClosing = (options && options.reason === "TabClose") || isWindowClosing;
+    let isTabContentDestroying =
+      isWindowClosing || (options && (options.reason === "TabClose" ||
+                                      options.reason === "BeforeTabRemotenessChange"));
 
     // Ensure init has finished before starting destroy
-    if (!isTabClosing) {
+    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 (!isTabClosing) {
+    if (!isTabContentDestroying) {
       // Stop the touch event simulator if it was running
       yield this.emulationFront.clearTouchEventsOverride();
 
       // Notify the inner browser to stop the frame script
       yield message.request(this.toolWindow, "stop-frame-script");
     }
 
     // Destroy local state
@@ -374,17 +379,17 @@ ResponsiveUI.prototype = {
     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
     let clientClosed = this.client.close();
-    if (!isTabClosing) {
+    if (!isTabContentDestroying) {
       yield clientClosed;
     }
     this.client = this.emulationFront = null;
 
     if (!isWindowClosing) {
       // Undo the swap and return the content back to a normal tab
       swap.stop();
     }
@@ -407,18 +412,19 @@ ResponsiveUI.prototype = {
 
   handleEvent(event) {
     let { browserWindow, tab } = this;
 
     switch (event.type) {
       case "message":
         this.handleMessage(event);
         break;
+      case "BeforeTabRemotenessChange":
+      case "TabClose":
       case "unload":
-      case "TabClose":
         ResponsiveUIManager.closeIfNeeded(browserWindow, tab, {
           reason: event.type,
         });
         break;
     }
   },
 
   handleMessage(event) {
--- a/devtools/client/responsive.html/test/browser/browser.ini
+++ b/devtools/client/responsive.html/test/browser/browser.ini
@@ -26,15 +26,16 @@ support-files =
 [browser_menu_item_01.js]
 [browser_menu_item_02.js]
 [browser_mouse_resize.js]
 [browser_navigation.js]
 [browser_page_state.js]
 [browser_permission_doorhanger.js]
 [browser_resize_cmd.js]
 [browser_screenshot_button.js]
-[browser_shutdown_close_sync.js]
+[browser_tab_close.js]
+[browser_tab_remoteness_change.js]
 [browser_toolbox_computed_view.js]
 [browser_toolbox_rule_view.js]
 [browser_toolbox_swap_browsers.js]
 [browser_touch_simulation.js]
 [browser_viewport_basics.js]
 [browser_window_close.js]
deleted file mode 100644
--- a/devtools/client/responsive.html/test/browser/browser_shutdown_close_sync.js
+++ /dev/null
@@ -1,50 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-// Verify RDM closes synchronously when tabs are closed.
-
-const TEST_URL = "http://example.com/";
-
-function waitForClientClose(ui) {
-  return new Promise(resolve => {
-    info("RDM's debugger client is now closed");
-    ui.client.addOneTimeListener("closed", resolve);
-  });
-}
-
-add_task(function* () {
-  let tab = yield addTab(TEST_URL);
-
-  let { ui } = yield openRDM(tab);
-  let clientClosed = waitForClientClose(ui);
-
-  closeRDM(tab, {
-    reason: "TabClose",
-  });
-
-  // This flag is set at the end of `ResponsiveUI.destroy`.  If it is true
-  // without yielding on `closeRDM` above, then we must have closed
-  // synchronously.
-  is(ui.destroyed, true, "RDM closed synchronously");
-
-  yield clientClosed;
-  yield removeTab(tab);
-});
-
-add_task(function* () {
-  let tab = yield addTab(TEST_URL);
-
-  let { ui } = yield openRDM(tab);
-  let clientClosed = waitForClientClose(ui);
-
-  yield removeTab(tab);
-
-  // This flag is set at the end of `ResponsiveUI.destroy`.  If it is true without
-  // yielding on `closeRDM` itself and only removing the tab, then we must have closed
-  // synchronously in response to tab closing.
-  is(ui.destroyed, true, "RDM closed synchronously");
-
-  yield clientClosed;
-});
new file mode 100644
--- /dev/null
+++ b/devtools/client/responsive.html/test/browser/browser_tab_close.js
@@ -0,0 +1,43 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Verify RDM closes synchronously when tabs are closed.
+
+const TEST_URL = "http://example.com/";
+
+add_task(function* () {
+  let tab = yield addTab(TEST_URL);
+
+  let { ui } = yield openRDM(tab);
+  let clientClosed = waitForClientClose(ui);
+
+  closeRDM(tab, {
+    reason: "TabClose",
+  });
+
+  // This flag is set at the end of `ResponsiveUI.destroy`.  If it is true
+  // without yielding on `closeRDM` above, then we must have closed
+  // synchronously.
+  is(ui.destroyed, true, "RDM closed synchronously");
+
+  yield clientClosed;
+  yield removeTab(tab);
+});
+
+add_task(function* () {
+  let tab = yield addTab(TEST_URL);
+
+  let { ui } = yield openRDM(tab);
+  let clientClosed = waitForClientClose(ui);
+
+  yield removeTab(tab);
+
+  // This flag is set at the end of `ResponsiveUI.destroy`.  If it is true without
+  // yielding on `closeRDM` itself and only removing the tab, then we must have closed
+  // synchronously in response to tab closing.
+  is(ui.destroyed, true, "RDM closed synchronously");
+
+  yield clientClosed;
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/responsive.html/test/browser/browser_tab_remoteness_change.js
@@ -0,0 +1,44 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Verify RDM closes synchronously when tabs change remoteness.
+
+const TEST_URL = "http://example.com/";
+
+add_task(function* () {
+  let tab = yield addTab(TEST_URL);
+
+  let { ui } = yield openRDM(tab);
+  let clientClosed = waitForClientClose(ui);
+
+  closeRDM(tab, {
+    reason: "BeforeTabRemotenessChange",
+  });
+
+  // This flag is set at the end of `ResponsiveUI.destroy`.  If it is true
+  // without yielding on `closeRDM` above, then we must have closed
+  // synchronously.
+  is(ui.destroyed, true, "RDM closed synchronously");
+
+  yield clientClosed;
+  yield removeTab(tab);
+});
+
+add_task(function* () {
+  let tab = yield addTab(TEST_URL);
+
+  let { ui } = yield openRDM(tab);
+  let clientClosed = waitForClientClose(ui);
+
+  // Load URL that requires the main process, forcing a remoteness flip
+  yield load(tab.linkedBrowser, "about:robots");
+
+  // This flag is set at the end of `ResponsiveUI.destroy`.  If it is true without
+  // yielding on `closeRDM` itself and only removing the tab, then we must have closed
+  // synchronously in response to tab closing.
+  is(ui.destroyed, true, "RDM closed synchronously");
+
+  yield clientClosed;
+});
--- a/devtools/client/responsive.html/test/browser/head.js
+++ b/devtools/client/responsive.html/test/browser/head.js
@@ -323,8 +323,15 @@ function addDeviceForTest(device) {
   info(`Adding Test Device "${device.name}" to the list.`);
   addDevice(device);
 
   registerCleanupFunction(() => {
     // Note that assertions in cleanup functions are not displayed unless they failed.
     ok(removeDevice(device), `Removed Test Device "${device.name}" from the list.`);
   });
 }
+
+function waitForClientClose(ui) {
+  return new Promise(resolve => {
+    info("RDM's debugger client is now closed");
+    ui.client.addOneTimeListener("closed", resolve);
+  });
+}