diff --git a/devtools/client/responsive.html/browser/swap.js b/devtools/client/responsive.html/browser/swap.js index 14747fd1642a..03f41972c0a9 100644 --- a/devtools/client/responsive.html/browser/swap.js +++ b/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, diff --git a/devtools/client/responsive.html/browser/tunnel.js b/devtools/client/responsive.html/browser/tunnel.js index 2c85726d691c..acd3099ee60b 100644 --- a/devtools/client/responsive.html/browser/tunnel.js +++ b/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); diff --git a/devtools/client/responsive.html/test/browser/browser.ini b/devtools/client/responsive.html/test/browser/browser.ini index 8d6a3b7d86b5..78c14ce77dd9 100644 --- a/devtools/client/responsive.html/test/browser/browser.ini +++ b/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] diff --git a/devtools/client/responsive.html/test/browser/browser_hide_container.js b/devtools/client/responsive.html/test/browser/browser_hide_container.js new file mode 100644 index 000000000000..cf1f79802462 --- /dev/null +++ b/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); +}); diff --git a/devtools/client/responsive.html/test/browser/browser_navigation.js b/devtools/client/responsive.html/test/browser/browser_navigation.js index 2c9f0027f1d2..b6f443d6065d 100644 --- a/devtools/client/responsive.html/test/browser/browser_navigation.js +++ b/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); }); diff --git a/devtools/client/responsive.html/test/browser/browser_page_state.js b/devtools/client/responsive.html/test/browser/browser_page_state.js index 306900535692..a390a66a94e0 100644 --- a/devtools/client/responsive.html/test/browser/browser_page_state.js +++ b/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); }); diff --git a/devtools/client/responsive.html/test/browser/head.js b/devtools/client/responsive.html/test/browser/head.js index 322c7cb62b35..f8d2e9f0836a 100644 --- a/devtools/client/responsive.html/test/browser/head.js +++ b/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 */ }); }