From 3285eb9dd50ec2d6bb514f16c5296a41db32297e Mon Sep 17 00:00:00 2001 From: "mr. m" Date: Fri, 12 Dec 2025 11:43:28 +0100 Subject: [PATCH] feat: Make sure to always initialize an empty state with the sidebar object, b=no-bug, c=workspaces --- .../sessionstore/SessionStore-sys-mjs.patch | 8 +- .../sessionstore/ZenSessionManager.sys.mjs | 138 +++++++++++++----- src/zen/sessionstore/ZenWindowSync.sys.mjs | 6 +- src/zen/workspaces/ZenWorkspaces.mjs | 4 +- 4 files changed, 114 insertions(+), 42 deletions(-) diff --git a/src/browser/components/sessionstore/SessionStore-sys-mjs.patch b/src/browser/components/sessionstore/SessionStore-sys-mjs.patch index b1c2074d4..aa83c63a5 100644 --- a/src/browser/components/sessionstore/SessionStore-sys-mjs.patch +++ b/src/browser/components/sessionstore/SessionStore-sys-mjs.patch @@ -1,5 +1,5 @@ diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs -index 2c2f43bf743ef458b378e85e9ed44a971711e1d9..e1d95e97819718d70fab23850a8810dd216db7a8 100644 +index 2c2f43bf743ef458b378e85e9ed44a971711e1d9..667b76e3065aafcc60ab146f40013ce791e59747 100644 --- a/browser/components/sessionstore/SessionStore.sys.mjs +++ b/browser/components/sessionstore/SessionStore.sys.mjs @@ -127,6 +127,8 @@ const TAB_EVENTS = [ @@ -56,9 +56,9 @@ index 2c2f43bf743ef458b378e85e9ed44a971711e1d9..e1d95e97819718d70fab23850a8810dd + for (let window of this._browserWindows) { + windowPromises.push(lazy.TabStateFlusher.flushWindow(window)); + } -+ Promise.all(windowPromises).finally(() => { -+ lazy.ZenSessionStore.restoreNewWindow(aWindow, this); -+ }); ++ aWindow._zenPromiseNewWindowRestored = new Promise((resolve) => Promise.all(windowPromises).finally(() => { ++ lazy.ZenSessionStore.restoreNewWindow(aWindow, this, resolve); ++ })); + } if (this._restoreLastWindow && aWindow.toolbar.visible) { diff --git a/src/zen/sessionstore/ZenSessionManager.sys.mjs b/src/zen/sessionstore/ZenSessionManager.sys.mjs index 2f14f5350..0a1ae2511 100644 --- a/src/zen/sessionstore/ZenSessionManager.sys.mjs +++ b/src/zen/sessionstore/ZenSessionManager.sys.mjs @@ -35,17 +35,24 @@ class nsZenSidebarObject { #sidebar = {}; get data() { - return { ...this.#sidebar }; + return Cu.cloneInto(this.#sidebar, {}); } set data(data) { - console.log(data); this.#sidebar = data; } } export class nsZenSessionManager { - #file; + /** + * The JSON file instance used to read/write session data. + * @type {JSONFile} + */ + #file = null; + /** + * The sidebar object holding tabs, groups, folders and split view data. + * @type {nsZenSidebarObject} + */ #sidebarObject = new nsZenSidebarObject(); // Called from SessionComponents.manifest on app-startup @@ -69,16 +76,27 @@ export class nsZenSessionManager { } } + /** + * Reads the session file and populates the sidebar object. + * This should be only called once at startup. + * @see SessionFileInternal.read + */ async readFile() { - console.log(await this.#file.load()); try { - this.#sidebar = (await this.#file.load()) || {}; + await this.#file.load(); } catch (e) { console.error('ZenSessionManager: Failed to read session file', e); - this.#sidebar = {}; } + this.#sidebar = this.#file.data || {}; } + /** + * Called when the session file is read. Restores the sidebar data + * into all windows. + * + * @param initialState + * The initial session state read from the session file. + */ onFileRead(initialState) { // For the first time after migration, we restore the tabs // That where going to be restored by SessionStore. The sidebar @@ -88,17 +106,30 @@ export class nsZenSessionManager { Services.prefs.setBoolPref(MIGRATION_PREF, true); return; } + // If there's no initial state, nothing to restore. This would + // happen if the file is empty or corrupted. + if (!initialState) { + return; + } + // If there are no windows, we create an empty one. By default, + // firefox would create simply a new empty window, but we want + // to make sure that the sidebar object is properly initialized. + // This would happen on first run after having a single private window + // open when quitting the app, for example. + if (!initialState.windows?.length) { + initialState.windows = [{}]; + } // Restore all windows with the same sidebar object, this will // guarantee that all tabs, groups, folders and split view data // are properly synced across all windows. this.log(`Restoring Zen session data into ${initialState.windows?.length || 0} windows`); for (const winData of initialState.windows || []) { - this.restoreWindowData(winData); + this.#restoreWindowData(winData); } } get #sidebar() { - return { ...this.#sidebarObject.data }; + return this.#sidebarObject.data; } set #sidebar(data) { @@ -118,7 +149,8 @@ export class nsZenSessionManager { return; } this.#collectWindowData(state); - // This would save the data to disk asynchronously. + // This would save the data to disk asynchronously or when + // quitting the app. this.#file.data = this.#sidebar; this.#file.saveSoon(); this.log(`Saving Zen session data with ${this.#sidebar.tabs?.length || 0} tabs`); @@ -178,9 +210,16 @@ export class nsZenSessionManager { sidebarData.groups = state.windows[0].groups; } - restoreWindowData(aWindowData) { + /** + * Restores the sidebar data into a given window data object. + * We do this in order to make sure all new window objects + * have the same sidebar data. + * + * @param aWindowData + * The window data object to restore into. + */ + #restoreWindowData(aWindowData) { const sidebar = this.#sidebar; - console.log(sidebar); if (!sidebar) { return; } @@ -190,35 +229,64 @@ export class nsZenSessionManager { aWindowData.groups = sidebar.groups; } - restoreNewWindow(aWindow, SessionStoreInternal) { + /** + * Restores a new window with Zen session data. This should be called + * not at startup, but when a new window is opened by the user. + * + * @param aWindow + * The window to restore. + * @param SessionStoreInternal + * The SessionStore module instance. + * @param resolvePromise + * The promise resolver to call when done. We use a promise + * here because out workspace manager always waits for SessionStore + * to restore all the windows before initializing, but when opening + * a new window, that promise is always resolved, meaning it may run + * into a race condition if we try to restore the window synchronously + * here. + */ + restoreNewWindow(aWindow, SessionStoreInternal, resolvePromise) { if (aWindow.gZenWorkspaces?.privateWindowOrDisabled) { return; } this.log('Restoring new window with Zen session data'); - aWindow._zenPromiseNewWindowRestored = new Promise((resolve) => { - lazy.setTimeout(() => { - const state = lazy.SessionStore.getCurrentState(true); - const windows = (state.windows || []).find( - (win) => !win.isPrivate && !win.isPopup && !win.isTaskbarTab && !win.isZenUnsynced - ); - let windowToClone = windows[0]; - let newWindow = Cu.cloneInto(windowToClone, {}); - if (windows.length < 2) { - // We only want to restore the sidebar object if we found - // only one normal window to clone from (which is the one - // we are opening). - this.log('Restoring sidebar data into new window'); - this.restoreWindowData(newWindow); - } - newWindow.tabs = this.#filterUnusedTabs(newWindow.tabs || []); - delete newWindow.selected; - const newState = { windows: [newWindow] }; - this.log(`Cloning window with ${newWindow.tabs.length} tabs`); - SessionStoreInternal.restoreWindows(aWindow, newState, { - firstWindow: true, - }); - resolve(); + lazy.setTimeout(() => { + const state = lazy.SessionStore.getCurrentState(true); + const windows = (state.windows || []).filter( + (win) => !win.isPrivate && !win.isPopup && !win.isTaskbarTab && !win.isZenUnsynced + ); + let windowToClone = windows[0] || {}; + let newWindow = Cu.cloneInto(windowToClone, {}); + if (windows.length < 2) { + // We only want to restore the sidebar object if we found + // only one normal window to clone from (which is the one + // we are opening). + this.log('Restoring sidebar data into new window'); + this.#restoreWindowData(newWindow); + } + newWindow.tabs = this.#filterUnusedTabs(newWindow.tabs || []); + + // These are window-specific from the previous window state that + // we don't want to restore into the new window. Otherwise, new + // windows would appear overlapping the previous one, or with + // the same size and position, which should be decided by the + // window manager. + delete newWindow.selected; + delete newWindow.screenX; + delete newWindow.screenY; + delete newWindow.width; + delete newWindow.height; + delete newWindow.sizemode; + delete newWindow.sizemodeBeforeMinimized; + delete newWindow.zIndex; + + const newState = { windows: [newWindow] }; + this.log(`Cloning window with ${newWindow.tabs.length} tabs`); + SessionStoreInternal.restoreWindows(aWindow, newState, { + firstWindow: true, }); + + resolvePromise(); }); } } diff --git a/src/zen/sessionstore/ZenWindowSync.sys.mjs b/src/zen/sessionstore/ZenWindowSync.sys.mjs index 89248877d..08eddfd32 100644 --- a/src/zen/sessionstore/ZenWindowSync.sys.mjs +++ b/src/zen/sessionstore/ZenWindowSync.sys.mjs @@ -493,6 +493,7 @@ class nsZenWindowSync { // firefox doesn't expect an unloaded + selected tab, so we need to get // around this limitation somehow. if (!onClose && aOurTab.linkedBrowser?.currentURI.spec !== 'about:blank') { + this.log(`Loading about:blank in our tab ${aOurTab.id} before swap`); aOurTab.linkedBrowser.loadURI(Services.io.newURI('about:blank'), { triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), loadFlags: Ci.nsIWebNavigation.LOAD_FLAGS_REPLACE_HISTORY, @@ -504,6 +505,7 @@ class nsZenWindowSync { this.#withRestoreTabProgressListener( aOtherTab, () => { + this.log(`Swapping docshells between windows for tab ${aOurTab.id}`); aOurTab.ownerGlobal.gBrowser.swapBrowsersAndCloseOther(aOurTab, aOtherTab, false); }, onClose @@ -637,6 +639,7 @@ class nsZenWindowSync { const targetTab = this.#getItemFromWindow(mostRecentWindow, tab.id); if (targetTab) { targetTab._zenContentsVisible = true; + this.log(`Moving active tab ${tab.id} to most recent window on close`); this.#swapBrowserDocSheellsInner(targetTab, tab, targetTab.selected, /* onClose =*/ true); // We can animate later, whats important is to always stay on the same // process and avoid async operations here to avoid the closed window @@ -666,7 +669,7 @@ class nsZenWindowSync { this.#swapBrowserDocShellsAsync(otherTabToShow, aPreviousTab); } } - if (selectedTab._zenContentsVisible) { + if (selectedTab._zenContentsVisible || selectedTab.hasAttribute('zen-empty-tab')) { return; } const otherSelectedTab = this.#getActiveTabFromOtherWindows(aWindow, selectedTab.id); @@ -737,6 +740,7 @@ class nsZenWindowSync { } }; if (!win) { + this.log('No synced window found, creating a new one'); win = aWindow.gBrowser.replaceTabWithWindow(selectedTab, {}, /* zenForceSync = */ true); win.gZenWorkspaces.promiseInitialized.then(() => { moveAllTabsToWindow(); diff --git a/src/zen/workspaces/ZenWorkspaces.mjs b/src/zen/workspaces/ZenWorkspaces.mjs index e24ddcac6..99ec1e86e 100644 --- a/src/zen/workspaces/ZenWorkspaces.mjs +++ b/src/zen/workspaces/ZenWorkspaces.mjs @@ -412,6 +412,7 @@ class nsZenWorkspaces extends nsZenMultiWindowFeature { perifery.setAttribute('hidden', 'true'); await new Promise((resolve) => { setTimeout(async () => { + await window._zenPromiseNewWindowRestored; const tabs = gBrowser.tabContainer.allTabs; const workspaces = await this.getWorkspaces(); for (const workspace of workspaces.workspaces) { @@ -934,8 +935,6 @@ class nsZenWorkspaces extends nsZenMultiWindowFeature { let activeWorkspace = await this.getActiveWorkspace(); this.activeWorkspace = activeWorkspace?.uuid; await gZenSessionStore.promiseInitialized; - await window._zenPromiseNewWindowRestored; - delete window._zenPromiseNewWindowRestored; try { if (activeWorkspace) { window.gZenThemePicker = new nsZenThemePicker(); @@ -952,6 +951,7 @@ class nsZenWorkspaces extends nsZenMultiWindowFeature { this.onWindowResize(); this._resolveInitialized(); this.#clearAnyZombieTabs(); // Dont call with await + delete window._zenPromiseNewWindowRestored; const tabUpdateListener = this.updateTabsContainers.bind(this); window.addEventListener('TabOpen', tabUpdateListener);