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

Bug 1908422: Add closedGroups to SessionRestore for tab groups r=dao,sthompson,sessionstore-reviewers,tabbrowser-reviewers

Browse Source 
This patch adds a `closedGroups` array to the SessionRestore state, and
adds functionality that ensures closed tab groups end up in the closed
groups array and that closed tab counts respect closed groups.

This does not update `undoClosedTab` or any related methods. Attempting
to restore a closed tab group will result in an error.

Differential Revision: https://phabricator.services.mozilla.com/D226397
This commit is contained in:
Jeremy Swinarton 2024-11-08 17:29:38 +00:00
parent fd103eccc3
commit 192fcb4ace
9 changed files with 391 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -118,7 +118,8 @@ const TAB_EVENTS = [
"TabPinned",
"TabUnpinned",
"TabGroupCreate",
"TabGroupRemove",
"TabGroupRemoveRequested",
"TabGroupRemoved",
"TabGrouped",
"TabUngrouped",
"TabGroupCollapse",
@ -470,6 +471,26 @@ export var SessionStore = {
return SessionStoreInternal.forgetClosedTab(aSource, aIndex);
},
/**
* Forget a closed tab group associated with a given window
* Removes the record at the given index so it cannot be un-closed or appear
* in a list of recently-closed tabs
*
* @param {Window|Object} aSource
* Either a DOMWindow or an object with properties to resolve to the window
* the tab was previously open in.
* @param {String} aSource.sourceWindowId
A SessionStore window id used to look up the window where the tab group was closed
* @param {number} aSource.sourceClosedId
The closedId used to look up the closed window where the tab group was closed
* @param {Integer} [aIndex = 0]
* The index into the window's list of closed tabs
* @throws {InvalidArgumentError} if the window is not tracked by SessionStore, or index is out of bounds
*/
forgetClosedTabGroup: function ss_forgetClosedTabGroup(aSource, aIndex) {
return SessionStoreInternal.forgetClosedTabGroup(aSource, aIndex);
},
/**
* Forget a closed tab that corresponds to the closedId
* Removes the record with this closedId so it cannot be un-closed or appear
@ -776,10 +797,10 @@ export var SessionStore = {
/**
* Remove a tab group to the session's saved group list.
* @param {MozTabbrowserTabGroup} tabGroup - The group to remove
* @param {string} tabGroupId - The ID of the tab group to remove
*/
removeSavedTabGroup(tabGroup) {
return SessionStoreInternal.removeSavedTabGroup(tabGroup);
removeSavedTabGroup(tabGroupId) {
return SessionStoreInternal.removeSavedTabGroup(tabGroupId);
},
};
@ -1711,13 +1732,17 @@ var SessionStoreInternal = {
this.saveStateDelayed(win);
break;
case "TabGroupCreate":
case "TabGroupRemove":
case "TabGroupRemoved":
case "TabGrouped":
case "TabUngrouped":
case "TabGroupCollapse":
case "TabGroupExpand":
this.saveStateDelayed(win);
break;
case "TabGroupRemoveRequested":
this.onTabGroupRemoveRequested(win, target);
this._notifyOfClosedObjectsChange();
break;
case "oop-browser-crashed":
case "oop-browser-buildid-mismatch":
if (aEvent.isTopFrame) {
@ -1778,6 +1803,7 @@ var SessionStoreInternal = {
this._windows[aWindow.__SSi] = {
tabs: [],
groups: [],
closedGroups: [],
selected: 0,
_closedTabs: [],
_lastClosedTabGroupCount: -1,
@ -2616,6 +2642,10 @@ var SessionStoreInternal = {
this._windows[ix]._closedTabs = [];
this._closedObjectsChanged = true;
}
if (this._windows[ix].closedGroups.length) {
this._windows[ix].closedGroups = [];
this._closedObjectsChanged = true;
}
} else {
delete this._windows[ix];
}
@ -2833,6 +2863,38 @@ var SessionStoreInternal = {
this.maybeSaveClosedTab(aWindow, aTab, tabState);
},
onTabGroupRemoveRequested: function ssi_onTabGroupRemoveRequested(
win,
tabGroup
) {
// don't update our internal state if we don't have to
if (this._max_tabs_undo == 0) {
return;
}
let closedGroups = this._windows[win.__SSi].closedGroups;
let tabGroupState = lazy.TabGroupState.clone(tabGroup);
tabGroupState.closedAt = Date.now();
// Only save tab state for tabs that qualify
tabGroupState.tabs = tabGroupState.tabs.filter(t =>
this._shouldSaveTabState(t)
);
if (tabGroupState.tabs.length) {
// TODO(jswinarton) it's unclear if updating lastClosedTabGroupCount is
// necessary when restoring tab groups — it largely depends on how we
// decide to do the restore. If we add a _LAST_ACTION_CLOSED_TAB_GROUP
// item to the closed actions list, this may not be necessary.
// To address in bug1915174
this._windows[win.__SSi]._lastClosedTabGroupCount =
tabGroupState.tabs.length;
closedGroups.push(tabGroupState);
this._closedObjectsChanged = true;
}
},
/**
* Flush and copy tab state when moving a tab to a new window.
* @param aFromBrowser
@ -3230,11 +3292,21 @@ var SessionStoreInternal = {
this._closedWindows.map(winData => winData._closedTabs)
);
// Remove closed groups of closed windows
this._cleanupOldData(
this._closedWindows.map(winData => winData.closedGroups)
);
// Remove closed tabs of open windows
this._cleanupOldData(
Object.keys(this._windows).map(key => this._windows[key]._closedTabs)
);
// Remove closed groups of open windows
this._cleanupOldData(
Object.keys(this._windows).map(key => this._windows[key].closedGroups)
);
this._notifyOfClosedObjectsChange();
},
@ -3583,10 +3655,12 @@ var SessionStoreInternal = {
for (let privateness of privateValues) {
for (let window of this.getWindows({ private: privateness })) {
const windowState = this._windows[window.__SSi];
if (!windowState._closedTabs?.length) {
const closedTabs =
this._getStateForClosedTabsAndClosedGroupTabs(windowState);
if (!closedTabs.length) {
continue;
}
if (windowState._closedTabs.find(tab => tab.closedId === aClosedId)) {
if (closedTabs.find(tab => tab.closedId === aClosedId)) {
return window;
}
}
@ -3615,7 +3689,9 @@ var SessionStoreInternal = {
getClosedTabCountForWindow: function ssi_getClosedTabCountForWindow(aWindow) {
if ("__SSi" in aWindow) {
return this._windows[aWindow.__SSi]._closedTabs.length;
return this._getStateForClosedTabsAndClosedGroupTabs(
this._windows[aWindow.__SSi]
).length;
}
if (!DyingWindowCache.has(aWindow)) {
@ -3625,7 +3701,9 @@ var SessionStoreInternal = {
);
}
return DyingWindowCache.get(aWindow)._closedTabs.length;
return this._getStateForClosedTabsAndClosedGroupTabs(
DyingWindowCache.get(aWindow)
).length;
},
_prepareClosedTabOptions(aOptions = {}) {
@ -3671,7 +3749,10 @@ var SessionStoreInternal = {
getClosedTabCountFromClosedWindows:
function ssi_getClosedTabCountFromClosedWindows() {
const tabCount = this._closedWindows
.map(winData => winData._closedTabs.length)
.map(
winData =>
this._getStateForClosedTabsAndClosedGroupTabs(winData).length
)
.reduce((total, count) => total + count, 0);
return tabCount;
},
@ -3681,12 +3762,12 @@ var SessionStoreInternal = {
// objects containing FormDatas, which could be stored by
// form-associated custom elements.
let options = { wrapReflectors: true };
if ("__SSi" in aWindow) {
return Cu.cloneInto(
this._windows[aWindow.__SSi]._closedTabs,
{},
options
let closedTabs = this._getStateForClosedTabsAndClosedGroupTabs(
this._windows[aWindow.__SSi]
);
return Cu.cloneInto(closedTabs, {}, options);
}
if (!DyingWindowCache.has(aWindow)) {
@ -3697,7 +3778,8 @@ var SessionStoreInternal = {
}
let data = DyingWindowCache.get(aWindow);
return Cu.cloneInto(data._closedTabs, {}, options);
let closedTabs = this._getStateForClosedTabsAndClosedGroupTabs(data);
return Cu.cloneInto(closedTabs, {}, options);
},
getClosedTabData: function ssi_getClosedTabData(aOptions) {
@ -3731,6 +3813,15 @@ var SessionStoreInternal = {
return closedTabData;
},
_getStateForClosedTabsAndClosedGroupTabs:
function ssi_getStateForClosedTabsAndClosedGroupTabs(winData) {
const closedGroups = winData.closedGroups ?? [];
return closedGroups.reduce(
(acc, group) => acc.concat(group.tabs),
winData._closedTabs ?? []
);
},
undoCloseTab: function ssi_undoCloseTab(aSource, aIndex, aTargetWindow) {
const sourceWinData = this._resolveClosedDataSource(aSource);
const isPrivateSource = Boolean(sourceWinData.isPrivate);
@ -3893,6 +3984,26 @@ var SessionStoreInternal = {
this._notifyOfClosedObjectsChange();
},
forgetClosedTabGroup: function ssi_forgetClosedTabGroup(aSource, aIndex) {
const winData = this._resolveClosedDataSource(aSource);
// default to the most-recently closed tab
aIndex = aIndex || 0;
if (!(aIndex in winData.closedGroups)) {
throw Components.Exception(
"Invalid index: not in the closed groups",
Cr.NS_ERROR_INVALID_ARG
);
}
for (let i = 0; i < winData.closedGroups[aIndex].length; i++) {
this.removeClosedTabData(winData, winData.closedGroups[aIndex], i);
}
winData.closedGroups.splice(aIndex, 1);
// Notify of changes to closed objects.
this._notifyOfClosedObjectsChange();
},
forgetClosedWindowById(aClosedId) {
// We don't keep any record for closed private windows so privateness is not relevant here
let closedIndex = this._closedWindows.findIndex(
@ -5102,7 +5213,7 @@ var SessionStoreInternal = {
},
/**
* This function will restore window features and then retore window data.
* This function will restore window features and then restore window data.
*
* @param windows
* ordered array of windows to restore
@ -6253,7 +6364,7 @@ var SessionStoreInternal = {
/**
* Determine if the tab state we're passed is something we should save. This
* is used when closing a tab or closing a window with a single tab
* is used when closing a tab, tab group, or closing a window with a single tab
*
* @param aTabState
* The current tab state

2
browser/components/sessionstore/test/browser.toml
View File

@ -275,6 +275,8 @@ tags = "os_integration"
["browser_switch_remoteness.js"]
["browser_tab_groups_closed.js"]
["browser_tab_groups_empty.js"]
["browser_tab_groups_restore_multiple.js"]

45
browser/components/sessionstore/test/browser_cleaner.js
View File

@ -14,6 +14,8 @@ const URL_TAB1 =
"http://example.com/browser_cleaner.js?newtab1=" + Math.random();
const URL_TAB2 =
"http://example.com/browser_cleaner.js?newtab2=" + Math.random();
const URL_NEWGROUP =
"http://example.com/browser_cleaner.js?newgroup=" + Math.random();
const URL_NEWWIN =
"http://example.com/browser_cleaner.js?newwin=" + Math.random();
@ -37,6 +39,9 @@ var CLOSED_STATE;
add_setup(async function () {
forgetClosedWindows();
forgetClosedTabs(window);
try {
ss.forgetClosedTabGroup(window, 0);
} catch {}
});
add_task(async function test_open_and_close() {
@ -46,6 +51,10 @@ add_task(async function test_open_and_close() {
let newTab2 = BrowserTestUtils.addTab(gBrowser, URL_TAB2);
await promiseBrowserLoaded(newTab2.linkedBrowser);
let newGroupTab = BrowserTestUtils.addTab(gBrowser, URL_NEWGROUP);
await promiseBrowserLoaded(newGroupTab.linkedBrowser);
let newGroup = gBrowser.addTabGroup([newGroupTab]);
let newWin = await promiseNewWindowLoaded();
let tab = BrowserTestUtils.addTab(newWin.gBrowser, URL_NEWWIN);
@ -77,6 +86,16 @@ add_task(async function test_open_and_close() {
false,
"1. Second tab doesn't have closedAt"
);
is(
state.windows[0].tabs[2].closedAt || false,
false,
"1. Grouped tab doesn't have closedAt"
);
is(
state.windows[0].groups[0].closedAt || false,
false,
"1. Group doesn't have closedAt"
);
info("2. Making sure that after closing, we have closedAt");
@ -85,6 +104,13 @@ add_task(async function test_open_and_close() {
await promiseRemoveTabAndSessionState(newTab1);
await promiseRemoveTabAndSessionState(newTab2);
let removePromise = BrowserTestUtils.waitForEvent(
newGroup,
"TabGroupRemoved"
);
gBrowser.removeTabGroup(newGroup);
await removePromise;
state = CLOSED_STATE = JSON.parse(ss.getBrowserState());
is(
@ -104,6 +130,10 @@ add_task(async function test_open_and_close() {
isRecent(state.windows[0]._closedTabs[1].closedAt),
"2. Second tab was closed recently"
);
ok(
isRecent(state.windows[0].closedGroups[0].closedAt),
"2. Group was closed recently"
);
});
add_task(async function test_restore() {
@ -120,7 +150,6 @@ add_task(async function test_restore() {
await promiseTabRestored(newTab1);
let state = JSON.parse(ss.getBrowserState());
console.log("examining state:", state);
is(
state.windows[0].closedAt || false,
false,
@ -156,6 +185,7 @@ add_task(async function test_old_data() {
delete state._closedWindows[0].closedAt;
delete state.windows[0]._closedTabs[0].closedAt;
delete state.windows[0]._closedTabs[1].closedAt;
delete state.windows[0].closedGroups[0].closedAt;
await promiseBrowserState(state);
info("Sending idle-daily");
@ -180,6 +210,10 @@ add_task(async function test_old_data() {
isRecent(state.windows[0]._closedTabs[1].closedAt),
"4. Second tab was closed recently"
);
ok(
isRecent(state.windows[0].closedGroups[0].closedAt),
"4. Group was closed recently"
);
await promiseCleanup();
});
@ -187,6 +221,9 @@ add_task(async function test_cleanup() {
info(
"5. Altering closedAt to an old date, making sure that stuff gets collected, eventually"
);
// TODO this should also test that the closed tab group is cleaned up, but
// can't be done until restoring tab groups (and thus setBrowserState)
// supports tab groups. See bug1927527.
await promiseCleanup();
let state = getClosedState();
@ -210,5 +247,11 @@ add_task(async function test_cleanup() {
url,
"5. The second tab is still here"
);
await promiseCleanup();
// Cleanup closed tab group
try {
ss.forgetClosedTabGroup(window, 0);
} catch {}
});

189
browser/components/sessionstore/test/browser_tab_groups_closed.js Normal file
View File

@ -0,0 +1,189 @@
"use strict";
const ORIG_STATE = SessionStore.getBrowserState();
add_task(async function test_closingTabGroupAddsClosedGroup() {
let win = await promiseNewWindowLoaded();
let tabs = [
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
];
let group = win.gBrowser.addTabGroup(tabs);
await Promise.all(
tabs.map(async t => {
await BrowserTestUtils.browserLoaded(t.linkedBrowser);
await TabStateFlusher.flush(t.linkedBrowser);
})
);
let windowState = ss.getWindowState(win).windows[0];
Assert.equal(
windowState.closedGroups.length,
0,
"Window state starts with no closed groups"
);
win.gBrowser.removeTabGroup(group);
windowState = ss.getWindowState(win).windows[0];
Assert.equal(
windowState.closedGroups.length,
1,
"Window state gains a closed group"
);
Assert.equal(
windowState.closedGroups[0].tabs.length,
3,
"Closed group has 3 tabs"
);
await BrowserTestUtils.closeWindow(win);
await SessionStoreTestUtils.promiseBrowserState(ORIG_STATE);
});
add_task(async function test_closedTabGroupSkipsNotWorthSavingTabs() {
let win = await promiseNewWindowLoaded();
let tabs = [
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
BrowserTestUtils.addTab(win.gBrowser, "about:blank"),
];
let group = win.gBrowser.addTabGroup(tabs);
await Promise.all(
tabs.map(async t => {
await BrowserTestUtils.browserLoaded(t.linkedBrowser);
await TabStateFlusher.flush(t.linkedBrowser);
})
);
let windowState = ss.getWindowState(win).windows[0];
Assert.equal(
windowState.closedGroups.length,
0,
"Window state starts with no closed groups"
);
win.gBrowser.removeTabGroup(group);
windowState = ss.getWindowState(win).windows[0];
Assert.equal(
windowState.closedGroups.length,
1,
"Window state gains a closed group"
);
Assert.equal(
windowState.closedGroups[0].tabs.length,
1,
"Closed group has only 1 tab"
);
await BrowserTestUtils.closeWindow(win);
await SessionStoreTestUtils.promiseBrowserState(ORIG_STATE);
});
add_task(async function test_closedTabCountsRespectTabGroups() {
let win = await promiseNewWindowLoaded();
Assert.equal(
SessionStore.getClosedTabCount(),
0,
"Session store starts with 0 closed tabs"
);
Assert.equal(
SessionStore.getLastClosedTabCount(win),
0,
"Session store starts with 0 last closed tab count"
);
await SessionStoreTestUtils.openAndCloseTab(win, "https://example.com");
Assert.equal(
SessionStore.getClosedTabCount(),
1,
"Session store registers closed tab"
);
Assert.equal(
SessionStore.getLastClosedTabCount(win),
1,
"Session store reports last closed tab count as 1"
);
// We need to have at least one "worth saving" tab open in the window at time
// of close, or else the window will not be added to _closedWindows
BrowserTestUtils.addTab(win.gBrowser, "https://example.com");
let tabs = [
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
BrowserTestUtils.addTab(win.gBrowser, "https://example.com"),
];
let group = win.gBrowser.addTabGroup(tabs);
await Promise.all(
tabs.map(async t => {
await BrowserTestUtils.browserLoaded(t.linkedBrowser);
await TabStateFlusher.flush(t.linkedBrowser);
})
);
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemoved");
win.gBrowser.removeTabGroup(group);
await removePromise;
Assert.equal(
SessionStore.getClosedTabCount(),
4,
"Session store registers tabs from closed group"
);
Assert.equal(
SessionStore.getLastClosedTabCount(win),
3,
"Session store reports last closed tab count as 3"
);
Assert.equal(
SessionStore.getClosedTabCountForWindow(win),
4,
"Session store correctly reports closed tab count for window"
);
await BrowserTestUtils.closeWindow(win);
Assert.equal(
SessionStore.getClosedTabCountFromClosedWindows(),
4,
"Session store correctly reports closed tab count for closed windows"
);
await SessionStoreTestUtils.promiseBrowserState(ORIG_STATE);
});
add_task(async function test_purgingSessionHistoryClearsClosedTabGroups() {
let win = await promiseNewWindowLoaded();
let tab = BrowserTestUtils.addTab(win.gBrowser, "https://example.com");
await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
await TabStateFlusher.flush(tab.linkedBrowser);
let group = win.gBrowser.addTabGroup([tab]);
win.gBrowser.removeTabGroup(group);
let windowState = ss.getWindowState(win).windows[0];
Assert.equal(
windowState.closedGroups.length,
1,
"Window state gains a closed group"
);
Services.obs.notifyObservers(null, "browser:purge-session-history");
windowState = ss.getWindowState(win).windows[0];
Assert.equal(
windowState.closedGroups.length,
0,
"Session history purge removes closed groups"
);
await BrowserTestUtils.closeWindow(win);
await SessionStoreTestUtils.promiseBrowserState(ORIG_STATE);
});

9
browser/components/sessionstore/test/head.js
View File

@ -360,8 +360,13 @@ function forgetClosedWindows() {
// Forget all closed tabs for a window
function forgetClosedTabs(win) {
while (ss.getClosedTabCountForWindow(win) > 0) {
ss.forgetClosedTab(win, 0);
const closedTabCount = ss.getClosedTabCountForWindow(win);
for (let i = 0; i < closedTabCount; i++) {
try {
ss.forgetClosedTab(win, 0);
} catch (err) {
// This will fail if there are tab groups in here
}
}
}

15
browser/components/tabbrowser/content/tabbrowser.js
View File

@ -2983,6 +2983,17 @@
if (this.tabGroupMenu.panel.state != "closed") {
this.tabGroupMenu.panel.hidePopup(options.animate);
}
// This needs to be fired before tabs are removed because session store
// needs to respond to this while tabs are still part of the group
group.dispatchEvent(
new CustomEvent("TabGroupRemoveRequested", { bubbles: true })
);
// Skip session store on a per-tab basis since these tabs will get
// recorded as part of a group
options.skipSessionStore = true;
this.removeTabs(group.tabs, options);
},
@ -4280,7 +4291,9 @@
return;
}
SessionStore.resetLastClosedTabCount(window);
if (!skipSessionStore) {
SessionStore.resetLastClosedTabCount(window);
}
this._clearMultiSelectionLocked = true;
// Guarantee that _clearMultiSelectionLocked lock gets released.

2
browser/components/tabbrowser/content/tabgroup.js
View File

@ -74,7 +74,7 @@
}
if (!this.tabs.length) {
this.dispatchEvent(
new CustomEvent("TabGroupRemove", { bubbles: true })
new CustomEvent("TabGroupRemoved", { bubbles: true })
);
this.remove();
}

10
browser/components/tabbrowser/test/browser/tabs/browser_tab_groups.js
View File

@ -242,8 +242,8 @@ add_task(async function test_tabUngroup() {
Assert.equal(groupedTab._tPos, 2, "grouped tab starts in correct position");
Assert.equal(groupedTab.group, group, "tab belongs to group");
info("Calling ungroupTabs and waiting for TabGroupRemove event.");
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemove");
info("Calling ungroupTabs and waiting for TabGroupRemoved event.");
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemoved");
group.ungroupTabs();
await removePromise;
@ -286,8 +286,8 @@ add_task(async function test_tabGroupMoveToNewWindow() {
label: "test",
});
info("Calling adoptTabGroup and waiting for TabGroupRemove event.");
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemove");
info("Calling adoptTabGroup and waiting for TabGroupRemoved event.");
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemoved");
let fgWindow = await BrowserTestUtils.openNewBrowserWindow();
fgWindow.gBrowser.adoptTabGroup(group, 0);
@ -383,7 +383,7 @@ add_task(async function test_TabGroupEvents() {
"TabUngrouped fired with correct group"
);
let tabGroupRemoved = BrowserTestUtils.waitForEvent(group, "TabGroupRemove");
let tabGroupRemoved = BrowserTestUtils.waitForEvent(group, "TabGroupRemoved");
await removeTabGroup(group);
await tabGroupRemoved;

2
browser/components/tabbrowser/test/browser/tabs/head.js
View File

@ -597,7 +597,7 @@ async function removeTabGroup(group) {
ok(false, "group was already removed");
return;
}
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemove");
let removePromise = BrowserTestUtils.waitForEvent(group, "TabGroupRemoved");
group.ownerGlobal.gBrowser.removeTabGroup(group, { animate: false });
await removePromise;
}