From a7e4b965c23ee5740c5b34eb00acf784f60b0bce Mon Sep 17 00:00:00 2001 From: Sarah Clements Date: Mon, 4 Dec 2023 11:28:16 +0000 Subject: [PATCH] Bug 1863692 - Only transfer window attributes for pinned tabs with deferred sessions r=sessionstore-reviewers,dao * add new marionette test Differential Revision: https://phabricator.services.mozilla.com/D195006 --- .../sessionstore/SessionStore.sys.mjs | 20 +-- .../sessionstore/test/marionette/manifest.ini | 1 + .../test/marionette/test_restore_manually.py | 133 ++++++++++++++++++ 3 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 browser/components/sessionstore/test/marionette/test_restore_manually.py diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs index 5f05ae404a95f..2e5150ae990d7 100644 --- a/browser/components/sessionstore/SessionStore.sys.mjs +++ b/browser/components/sessionstore/SessionStore.sys.mjs @@ -6560,20 +6560,16 @@ var SessionStoreInternal = { hasPinnedTabs ||= !!newWindowState.tabs.length; - // At this point the window in the state object has been modified (or not) - // We want to build the rest of this new window object if we have pinnedTabs. - if ( - newWindowState.tabs.length || - (PERSIST_SESSIONS && newWindowState._closedTabs.length) - ) { - // First get the other attributes off the window + // Only transfer over window attributes for pinned tabs, which has + // already been extracted into newWindowState.tabs. + if (newWindowState.tabs.length) { WINDOW_ATTRIBUTES.forEach(function (attr) { if (attr in window) { newWindowState[attr] = window[attr]; delete window[attr]; } }); - // We're just copying position data into the pinned window. + // We're just copying position data into the window for pinned tabs. // Not copying over: // - extData // - isPopup @@ -6583,8 +6579,14 @@ var SessionStoreInternal = { // remaining data window.__lastSessionWindowID = newWindowState.__lastSessionWindowID = "" + Date.now() + Math.random(); + } - // Actually add this window to our defaultState + // If this newWindowState contains pinned tabs (stored in tabs) or + // closed tabs, add it to the defaultState so they're available immediately. + if ( + newWindowState.tabs.length || + (PERSIST_SESSIONS && newWindowState._closedTabs.length) + ) { defaultState.windows.push(newWindowState); // Remove the window from the state if it doesn't have any tabs if (!window.tabs.length) { diff --git a/browser/components/sessionstore/test/marionette/manifest.ini b/browser/components/sessionstore/test/marionette/manifest.ini index 8210415f328fe..60ef5037af9c7 100644 --- a/browser/components/sessionstore/test/marionette/manifest.ini +++ b/browser/components/sessionstore/test/marionette/manifest.ini @@ -3,6 +3,7 @@ tags = local [test_persist_closed_tabs_restore_manually.py] [test_restore_loading_tab.py] +[test_restore_manually.py] [test_restore_manually_with_pinned_tabs.py] [test_restore_windows_after_restart_and_quit.py] [test_restore_windows_after_windows_shutdown.py] diff --git a/browser/components/sessionstore/test/marionette/test_restore_manually.py b/browser/components/sessionstore/test/marionette/test_restore_manually.py new file mode 100644 index 0000000000000..cb80aa8a94999 --- /dev/null +++ b/browser/components/sessionstore/test/marionette/test_restore_manually.py @@ -0,0 +1,133 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import os +import sys + +# add this directory to the path +sys.path.append(os.path.dirname(__file__)) + +from session_store_test_case import SessionStoreTestCase + + +def inline(title): + return "data:text/html;charset=utf-8,{}".format( + title + ) + + +class TestSessionRestoreManually(SessionStoreTestCase): + """ + Test that window attributes for each window are restored + correctly (with manual session restore) in a new session. + """ + + def setUp(self): + super(TestSessionRestoreManually, self).setUp( + startup_page=1, + include_private=False, + restore_on_demand=True, + test_windows=set( + [ + # Window 1 + ( + inline("lorem ipsom"), + inline("dolor"), + ), + # Window 2 + ( + inline("sit"), + ) + ] + ), + ) + + def test_restore(self): + self.marionette.execute_script( + """ + Services.prefs.setBoolPref("browser.sessionstore.persist_closed_tabs_between_sessions", true); + """ + ) + + self.wait_for_windows( + self.all_windows, "Not all requested windows have been opened" + ) + + self.assertEqual( + len(self.marionette.chrome_window_handles), + 2, + msg="Should have 3 windows open.", + ) + self.assertEqual( + self.marionette.execute_script( + """ + const lazy = {}; + ChromeUtils.defineESModuleGetters(lazy, { + SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs", + }); + function getAllBrowserWindows() { + return Array.from(Services.wm.getEnumerator("navigator:browser")); + } + let windows = getAllBrowserWindows(); + windows[1].resizeTo(500, 500) + return windows[1].document.documentElement.getAttribute("height") + """ + ), + "500", + "Window has been set to correct height" + ) + + self.marionette.quit() + self.marionette.start_session() + self.marionette.set_context("chrome") + + + # restore the previous session + self.marionette.execute_script( + """ + const lazy = {}; + ChromeUtils.defineESModuleGetters(lazy, { + SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs", + }); + function observeClosedObjectsChange() { + return new Promise(resolve => { + function observe(subject, topic, data) { + if (topic == "sessionstore-closed-objects-changed") { + Services.obs.removeObserver(this, "sessionstore-closed-objects-changed"); + resolve('observed closed objects changed'); + }; + } + Services.obs.addObserver(observe, "sessionstore-closed-objects-changed"); + }); + }; + + async function checkForWindowHeight() { + let closedWindowsObserver = observeClosedObjectsChange(); + lazy.SessionStore.restoreLastSession(); + await closedWindowsObserver; + } + checkForWindowHeight(); + """ + ) + + self.assertEqual( + len(self.marionette.chrome_window_handles), + 2, + msg="Windows from last session have been restored.", + ) + + self.assertEqual( + self.marionette.execute_script( + """ + const lazy = {}; + ChromeUtils.defineESModuleGetters(lazy, { + SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs", + }); + let state = SessionStore.getCurrentState() + return state.windows[1]["height"] + """ + ), + 500, + "Second window has correct height" + )