Bug 1352814 - Force session history off for RDM container. r=ochameau

In bug 1313933, we removed the session store black magic that RDM used to do in
order to hide the container tab.

Unfortunately, that fix appears to have been imperfect.  Session store has a
fallback path that can still record the current URL, causing the container URL
to be recorded anyway, even though we asked nicely to please not do that.

In this change, we try a fresh approach of wedging the session history listener
for the container tab so it can't record anything.  This avoids the racy
approach that was used before bug 1313933 while still appearing to block the
container URL from being recorded.

MozReview-Commit-ID: JZTYzMAvaEM

--HG--
extra : rebase_source : dff8a35b25994b49f2e31888d49ffcb6b55402bb

devtools/client/responsive.html/browser/swap.js

@@ -69,6 +69,19 @@ function swapToInnerBrowser({ tab, containerURL, getInnerBrowser }) {
       });
       gBrowser.hideTab(containerTab);
       let containerBrowser = containerTab.linkedBrowser;
+      // Even though we load the `containerURL` with `LOAD_FLAGS_BYPASS_HISTORY` below,
+      // `SessionHistory.jsm` has a fallback path for tabs with no history which
+      // fabricates a history entry by reading the current URL, and this can cause the
+      // container URL to be recorded in the session store.  To avoid this, we send a
+      // bogus `epoch` value to our container tab, which causes all future history
+      // messages to be ignored.  (Actual navigations are still correctly recorded because
+      // this only affects the container frame, not the content.)  A better fix would be
+      // to just not load the `content-sessionStore.js` frame script at all in the
+      // container tab, but it's loaded for all tab browsers, so this seems a bit harder
+      // to achieve in a nice way.
+      containerBrowser.messageManager.sendAsyncMessage("SessionStore:flush", {
+        epoch: -1,
+      });
       // Prevent the `containerURL` from ending up in the tab's history.
       containerBrowser.loadURIWithFlags(containerURL, {
         flags: Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_HISTORY,

devtools/client/responsive.html/browser/tunnel.js

@@ -12,6 +12,8 @@ const { getStack } = require("devtools/shared/platform/stack");
 
 // A symbol used to hold onto the frame loader from the outer browser while tunneling.
 const FRAME_LOADER = Symbol("devtools/responsive/frame-loader");
+// Export for use in tests.
+exports.OUTER_FRAME_LOADER_SYMBOL = FRAME_LOADER;
 
 function debug(msg) {
   // console.log(msg);

devtools/client/responsive.html/test/browser/browser.ini

@@ -25,6 +25,7 @@ support-files =
 [browser_dpr_change.js]
 [browser_exit_button.js]
 [browser_frame_script_active.js]
+[browser_hide_container.js]
 [browser_menu_item_01.js]
 [browser_menu_item_02.js]
 [browser_mouse_resize.js]

devtools/client/responsive.html/test/browser/browser_hide_container.js

@@ -0,0 +1,64 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Ensure that the RDM container tab URL is not recorded in session history.
+
+const TEST_URL = "http://example.com/";
+const CONTAINER_URL = "chrome://devtools/content/responsive.html/index.xhtml";
+
+const { TabStateFlusher } =
+  Cu.import("resource:///modules/sessionstore/TabStateFlusher.jsm", {});
+const SessionStore =
+  Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
+const { OUTER_FRAME_LOADER_SYMBOL } =
+  require("devtools/client/responsive.html/browser/tunnel");
+
+function flushContainerTabState(tab) {
+  let browser = tab.linkedBrowser;
+  let outerBrowser = {
+    permanentKey: browser.permanentKey,
+    messageManager: browser[OUTER_FRAME_LOADER_SYMBOL].messageManager,
+  };
+  // Try to flush the tab, but if that hangs for a while, resolve anyway.
+  // During this test, we actually expect this to hang because the container tab
+  // doesn't have the right epoch value to reply to the flush correctly.
+  return new Promise(resolve => {
+    TabStateFlusher.flush(outerBrowser).then(resolve);
+    waitForTime(10000).then(resolve);
+  });
+}
+
+add_task(function* () {
+  // Load test URL
+  let tab = yield addTab(TEST_URL);
+  let browser = tab.linkedBrowser;
+
+  // Check session history state
+  let history = yield getSessionHistory(browser);
+  is(history.index - 1, 0, "At page 0 in history");
+  is(history.entries.length, 1, "1 page in history");
+  is(history.entries[0].url, TEST_URL, "Page 0 URL matches");
+
+  // Open RDM
+  yield openRDM(tab);
+
+  // Checking session history directly in content does show the container URL
+  // that we're trying to hide...
+  history = yield getSessionHistory(browser);
+  is(history.index - 1, 0, "At page 0 in history");
+  is(history.entries.length, 1, "1 page in history");
+  is(history.entries[0].url, CONTAINER_URL, "Page 0 URL matches");
+
+  // However, checking the recorded tab state for the outer browser shows the
+  // container URL has been ignored correctly.
+  yield flushContainerTabState(tab);
+  let tabState = JSON.parse(SessionStore.getTabState(tab));
+  is(tabState.index - 1, 0, "At page 0 in history");
+  is(tabState.entries.length, 1, "1 page in history");
+  is(tabState.entries[0].url, TEST_URL, "Page 0 URL matches");
+
+  yield closeRDM(tab);
+  yield removeTab(tab);
+});

devtools/client/responsive.html/test/browser/browser_navigation.js

@@ -22,22 +22,22 @@ add_task(function* () {
 
   // Check session history state
   let history = yield getSessionHistory(browser);
-  is(history.index, 2, "At page 2 in history");
+  is(history.index - 1, 2, "At page 2 in history");
   is(history.entries.length, 3, "3 pages in history");
-  is(history.entries[0].uri, DUMMY_1_URL, "Page 0 URL matches");
-  is(history.entries[1].uri, TEST_URL, "Page 1 URL matches");
-  is(history.entries[2].uri, DUMMY_2_URL, "Page 2 URL matches");
+  is(history.entries[0].url, DUMMY_1_URL, "Page 0 URL matches");
+  is(history.entries[1].url, TEST_URL, "Page 1 URL matches");
+  is(history.entries[2].url, DUMMY_2_URL, "Page 2 URL matches");
 
   // Go back one so we're at the test page
   yield back(browser);
 
   // Check session history state
   history = yield getSessionHistory(browser);
-  is(history.index, 1, "At page 1 in history");
+  is(history.index - 1, 1, "At page 1 in history");
   is(history.entries.length, 3, "3 pages in history");
-  is(history.entries[0].uri, DUMMY_1_URL, "Page 0 URL matches");
-  is(history.entries[1].uri, TEST_URL, "Page 1 URL matches");
-  is(history.entries[2].uri, DUMMY_2_URL, "Page 2 URL matches");
+  is(history.entries[0].url, DUMMY_1_URL, "Page 0 URL matches");
+  is(history.entries[1].url, TEST_URL, "Page 1 URL matches");
+  is(history.entries[2].url, DUMMY_2_URL, "Page 2 URL matches");
 
   yield openRDM(tab);
 
@@ -89,10 +89,10 @@ add_task(function* () {
 
   // Check session history state
   history = yield getSessionHistory(browser);
-  is(history.index, 1, "At page 1 in history");
+  is(history.index - 1, 1, "At page 1 in history");
   is(history.entries.length, 2, "2 pages in history");
-  is(history.entries[0].uri, DUMMY_1_URL, "Page 0 URL matches");
-  is(history.entries[1].uri, DUMMY_3_URL, "Page 1 URL matches");
+  is(history.entries[0].url, DUMMY_1_URL, "Page 0 URL matches");
+  is(history.entries[1].url, DUMMY_3_URL, "Page 1 URL matches");
 
   yield removeTab(tab);
 });

devtools/client/responsive.html/test/browser/browser_page_state.js

@@ -22,22 +22,22 @@ add_task(function* () {
 
   // Check session history state
   let history = yield getSessionHistory(browser);
-  is(history.index, 2, "At page 2 in history");
+  is(history.index - 1, 2, "At page 2 in history");
   is(history.entries.length, 3, "3 pages in history");
-  is(history.entries[0].uri, DUMMY_1_URL, "Page 0 URL matches");
-  is(history.entries[1].uri, TEST_URL, "Page 1 URL matches");
-  is(history.entries[2].uri, DUMMY_2_URL, "Page 2 URL matches");
+  is(history.entries[0].url, DUMMY_1_URL, "Page 0 URL matches");
+  is(history.entries[1].url, TEST_URL, "Page 1 URL matches");
+  is(history.entries[2].url, DUMMY_2_URL, "Page 2 URL matches");
 
   // Go back one so we're at the test page
   yield back(browser);
 
   // Check session history state
   history = yield getSessionHistory(browser);
-  is(history.index, 1, "At page 1 in history");
+  is(history.index - 1, 1, "At page 1 in history");
   is(history.entries.length, 3, "3 pages in history");
-  is(history.entries[0].uri, DUMMY_1_URL, "Page 0 URL matches");
-  is(history.entries[1].uri, TEST_URL, "Page 1 URL matches");
-  is(history.entries[2].uri, DUMMY_2_URL, "Page 2 URL matches");
+  is(history.entries[0].url, DUMMY_1_URL, "Page 0 URL matches");
+  is(history.entries[1].url, TEST_URL, "Page 1 URL matches");
+  is(history.entries[2].url, DUMMY_2_URL, "Page 2 URL matches");
 
   // Click on content to set an altered state that would be lost on reload
   yield BrowserTestUtils.synthesizeMouseAtCenter("body", {}, browser);
@@ -66,11 +66,11 @@ add_task(function* () {
 
   // Check session history state
   history = yield getSessionHistory(browser);
-  is(history.index, 1, "At page 1 in history");
+  is(history.index - 1, 1, "At page 1 in history");
   is(history.entries.length, 3, "3 pages in history");
-  is(history.entries[0].uri, DUMMY_1_URL, "Page 0 URL matches");
-  is(history.entries[1].uri, TEST_URL, "Page 1 URL matches");
-  is(history.entries[2].uri, DUMMY_2_URL, "Page 2 URL matches");
+  is(history.entries[0].url, DUMMY_1_URL, "Page 0 URL matches");
+  is(history.entries[1].url, TEST_URL, "Page 1 URL matches");
+  is(history.entries[2].url, DUMMY_2_URL, "Page 2 URL matches");
 
   yield removeTab(tab);
 });

devtools/client/responsive.html/test/browser/head.js

@@ -274,23 +274,10 @@ const selectNetworkThrottling = (ui, value) => Promise.all([
 function getSessionHistory(browser) {
   return ContentTask.spawn(browser, {}, function* () {
     /* eslint-disable no-undef */
-    let { interfaces: Ci } = Components;
-    let webNav = docShell.QueryInterface(Ci.nsIWebNavigation);
-    let sessionHistory = webNav.sessionHistory;
-    let result = {
-      index: sessionHistory.index,
-      entries: []
-    };
-
-    for (let i = 0; i < sessionHistory.count; i++) {
-      let entry = sessionHistory.getEntryAtIndex(i, false);
-      result.entries.push({
-        uri: entry.URI.spec,
-        title: entry.title
-      });
-    }
-
-    return result;
+    let { utils: Cu } = Components;
+    const { SessionHistory } =
+      Cu.import("resource://gre/modules/sessionstore/SessionHistory.jsm", {});
+    return SessionHistory.collect(docShell);
     /* eslint-enable no-undef */
   });
 }