Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-12-11 16:32:59 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1822854 - ensure session store doesn't assume _tPos is set and has good stub info for recently restored tabs so we don't break manual session restore, r=dao

Browse Source 
Either of these changes (ie dropping the setTabState call for batch restored
tabs, or ensuring the restoreTabs code correctly fills its array with dummy
entries) is sufficient here. I chose to do both because I think in both cases
the brokenness is not limited to this scenario or the issues at hand.

Specifically, the setTabState call was added in bug 1521346 to deal with
moved lazy tabs, but is now being invoked for session restore because of
the batchInsertingTabs optimization work. It doesn't actually need to be,
as far as I can tell, and the lacking _tPos in this case (because we don't
insert the tab into the tabstrip a few lines above) is what breaks things
inside _ensureNoNullsInTabDataList. Note that this _already_ was breaking
things in restoreTab(), which would assign into tabs[undefined] on the
window state object, so just dropping the call seemed better than wallpapering
the absence of _tPos.

The restoreTabs code, pre-patch, calls _ensureNoNullsInTabDataList but that
will never do anything, because right before calling it we change the array
length, so maxPos was always smaller than the size of the list. This meant
we still had empty slots in the array, which was also causing confusion down
the line.

I added the explicit exception for the broken _tPos in restoreTab so that we
notice any future issues with this more quickly. Doing so without any of the
other fixes broke the pre-existing browser_586068-apptabs.js test, so
hopefully that will catch any future changes that break the code's assumptions.

Differential Revision: https://phabricator.services.mozilla.com/D173070
This commit is contained in:
Gijs Kruitbosch 2023-03-21 16:29:06 +00:00
parent afd2db475b
commit 78ae8aa771
2 changed files with 35 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
browser/base/content/tabbrowser.js
View File

@ -2745,21 +2745,26 @@
);
b.registeredOpenURI = lazyBrowserURI;
}
SessionStore.setTabState(t, {
entries: [
{
url: lazyBrowserURI?.spec || "about:blank",
title: lazyTabTitle,
triggeringPrincipal_base64: E10SUtils.serializePrincipal(
triggeringPrincipal
),
},
],
// Make sure to store the userContextId associated to the lazy tab
// otherwise it would be created as a default tab when recreated on a
// session restore (See Bug 1819794).
userContextId,
});
// If we're batch inserting, we can't set the tab state meaningfully
// because the tab won't be in the DOM yet. The consumer (normally
// session restore) will have to do this work itself.
if (!batchInsertingTabs) {
SessionStore.setTabState(t, {
entries: [
{
url: lazyBrowserURI?.spec || "about:blank",
title: lazyTabTitle,
triggeringPrincipal_base64: E10SUtils.serializePrincipal(
triggeringPrincipal
),
},
],
// Make sure to store the userContextId associated to the lazy tab
// otherwise it would be created as a default tab when recreated on a
// session restore (See Bug 1819794).
userContextId,
});
}
} else {
this._insertBrowser(t, true);
// If we were called by frontend and don't have openWindowInfo,

20
browser/components/sessionstore/SessionStore.sys.mjs
View File

@ -4654,12 +4654,17 @@ var SessionStoreInternal = {
tabsDataArray.splice(numTabsInWindow - numTabsToRestore);
}
// Let the tab data array have the right number of slots.
tabsDataArray.length = numTabsInWindow;
// Remove items from aTabData if there is no corresponding tab:
if (numTabsInWindow < tabsDataArray.length) {
tabsDataArray.length = numTabsInWindow;
}
// Fill out any empty items in the list:
let maxPos = Math.max(...aTabs.map(tab => tab._tPos));
this._ensureNoNullsInTabDataList(tabbrowser.tabs, tabsDataArray, maxPos);
// Ensure the tab data array has items for each of the tabs
this._ensureNoNullsInTabDataList(
tabbrowser.tabs,
tabsDataArray,
numTabsInWindow - 1
);
if (aSelectTab > 0 && aSelectTab <= aTabs.length) {
// Update the window state in case we shut down without being notified.
@ -4735,6 +4740,11 @@ var SessionStoreInternal = {
// we collect their data for the first time when saving state.
DirtyWindows.add(window);
if (!tab.hasOwnProperty("_tPos")) {
throw new Error(
"Shouldn't be trying to restore a tab that has no position"
);
}
// Update the tab state in case we shut down without being notified.
this._windows[window.__SSi].tabs[tab._tPos] = tabData;