Bug 1407830 - Verify that RDM swaps take place. r=ochameau draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 13 Oct 2017 20:19:49 -0500
changeset 680417 4e0a8a128c13c260ac1c421584b81c5a664dedb6
parent 680416 146858c8c768e66dd056cc9d28f75d567115e584
child 680418 3ea69d5f332845c9637016faefbe7dd866fa543d
push id84507
push userbmo:jryans@gmail.com
push dateSat, 14 Oct 2017 04:45:44 +0000
reviewersochameau
bugs1407830
milestone58.0a1
Bug 1407830 - Verify that RDM swaps take place. r=ochameau It is possible for the frame loader swap within `gBrowser._swapBrowserDocShells` to fail when various frame state is either not ready yet or doesn't match between the two browsers you're trying to swap. However, such errors are currently caught and silenced in the browser, because they are apparently expected in certain cases. So, here we do our own check to verify that the swap actually did in fact take place, making it much easier to track such errors when they happen. MozReview-Commit-ID: LwuCXJQRRVW
devtools/client/responsive.html/browser/swap.js
--- a/devtools/client/responsive.html/browser/swap.js
+++ b/devtools/client/responsive.html/browser/swap.js
@@ -69,16 +69,35 @@ function swapToInnerBrowser({ tab, conta
   // about it.  This hides the temporary tab from things like WebExtensions.
   let swapBrowsersAndCloseOtherSilently = (ourTab, otherTab) => {
     browserWindow.addEventListener("TabClose", event => {
       event.stopImmediatePropagation();
     }, { capture: true, once: true });
     gBrowser.swapBrowsersAndCloseOther(ourTab, otherTab);
   };
 
+  // It is possible for the frame loader swap within `gBrowser._swapBrowserDocShells` to
+  // fail when various frame state is either not ready yet or doesn't match between the
+  // two browsers you're trying to swap.  However, such errors are currently caught and
+  // silenced in the browser, because they are apparently expected in certain cases.
+  // So, here we do our own check to verify that the swap actually did in fact take place,
+  // making it much easier to track such errors when they happen.
+  let swapBrowserDocShells = (ourTab, otherBrowser) => {
+    // The verification step here assumes both browsers are remote.
+    if (!ourTab.linkedBrowser.isRemoteBrowser || !otherBrowser.isRemoteBrowser) {
+      throw new Error("Both browsers should be remote before swapping.");
+    }
+    let contentTabId = ourTab.linkedBrowser.frameLoader.tabParent.tabId;
+    gBrowser._swapBrowserDocShells(ourTab, otherBrowser);
+    if (otherBrowser.frameLoader.tabParent.tabId != contentTabId) {
+      // Bug 1408602: Try to unwind to save tab content from being lost.
+      throw new Error("Swapping tab content between browsers failed.");
+    }
+  };
+
   return {
 
     start: Task.async(function* () {
       tab.isResponsiveDesignMode = true;
 
       // Hide the browser content temporarily while things move around to avoid displaying
       // strange intermediate states.
       tab.linkedBrowser.style.visibility = "hidden";
@@ -144,17 +163,17 @@ function swapToInnerBrowser({ tab, conta
                         "original tab.");
       }
 
       // 4. Swap tab content from the regular browser tab to the browser within
       //    the viewport in the tool UI, preserving all state via
       //    `gBrowser._swapBrowserDocShells`.
       dispatchDevToolsBrowserSwap(tab.linkedBrowser, innerBrowser);
       debug("Swap content to inner browser");
-      gBrowser._swapBrowserDocShells(tab, innerBrowser);
+      swapBrowserDocShells(tab, innerBrowser);
 
       // 5. Force the original browser tab to be non-remote since the tool UI
       //    must be loaded in the parent process, and we're about to swap the
       //    tool UI into this tab.
       debug("Flip original tab to remote false");
       gBrowser.updateBrowserRemoteness(tab.linkedBrowser, false);
 
       // 6. Swap the tool UI (with viewport showing the content) into the
@@ -209,17 +228,17 @@ function swapToInnerBrowser({ tab, conta
       // 3. Mark the content tab browser's docshell as active so the frame
       //    is created eagerly and will be ready to swap.
       contentBrowser.docShellIsActive = true;
 
       // 4. Swap tab content from the browser within the viewport in the tool UI
       //    to the regular browser tab, preserving all state via
       //    `gBrowser._swapBrowserDocShells`.
       dispatchDevToolsBrowserSwap(innerBrowser, contentBrowser);
-      gBrowser._swapBrowserDocShells(contentTab, innerBrowser);
+      swapBrowserDocShells(contentTab, innerBrowser);
       innerBrowser = null;
 
       // Copy tab listener state flags to content tab.  See similar comment in `start`
       // above for more details.
       let stateFlags = gBrowser._tabListeners.get(tab).mStateFlags;
       gBrowser._tabListeners.get(contentTab).mStateFlags = stateFlags;
 
       // 5. Force the original browser tab to be remote since web content is