Bug 1442694 - Fix failures due to removing selected tab r=Gijs

This adds test which reproduce the failure as well as the fix. Essentially,
if we hit the edited case in SessionStore with `tab` equal to `tabbrowser.tabs[t]`,
we remove the tab and then try to pin it, which obviously blows up.

Note: the additional method in SessionStore.jsm was largely to get around
complexity requirements inside restoreWindow. Cleaner solutions welcome.

Differential Revision: https://phabricator.services.mozilla.com/D21383
This commit is contained in:
Doug Thayer
2019-03-01 18:29:09 +00:00
parent c372c1eb9f
commit df0855848d
2 changed files with 68 additions and 118 deletions

View File

@@ -3600,6 +3600,36 @@ var SessionStoreInternal = {
return Promise.all([...WINDOW_SHOWING_PROMISES.values()].map(deferred => deferred.promise));
},
/**
* Handles the pinning / unpinning of a selected tab restored with
* restoreWindow.
*
* @param aWindow
* Window reference to the window used for restoration
* @param aWinData
* The window data we're restoring
* @param aRestoreIndex
* The index of the tab data we're currently restoring
* @returns the selected tab
*/
_updateRestoredSelectedTabPinnedState(aWindow, aWinData, aRestoreIndex) {
let tabbrowser = aWindow.gBrowser;
let tabData = aWinData.tabs[aRestoreIndex];
let tab = tabbrowser.selectedTab;
let needsUnpin = tab.pinned && !tabData.pinned;
let needsPin = !tab.pinned && tabData.pinned;
if (needsUnpin) {
tabbrowser.unpinTab(tab);
} else if (needsPin && tab == tabbrowser.tabs[aRestoreIndex]) {
tabbrowser.pinTab(tab);
} else if (needsPin) {
tabbrowser.removeTab(tabbrowser.tabs[aRestoreIndex]);
tabbrowser.pinTab(tab);
tabbrowser.moveTabTo(tab, aRestoreIndex);
}
return tab;
},
/**
* restore features to a single window
* @param aWindow
@@ -3685,14 +3715,7 @@ var SessionStoreInternal = {
// selecting a new tab.
if (select &&
tabbrowser.selectedTab.userContextId == userContextId) {
tab = tabbrowser.selectedTab;
if (tab.pinned && !tabData.pinned) {
tabbrowser.unpinTab(tab);
} else if (!tab.pinned && tabData.pinned) {
tabbrowser.removeTab(tabbrowser.tabs[t]);
tabbrowser.pinTab(tab);
tabbrowser.moveTabTo(tab, t);
}
tab = this._updateRestoredSelectedTabPinnedState(aWindow, winData, t);
tabbrowser.moveTabToEnd();
if (aWindow.gMultiProcessBrowser && !tab.linkedBrowser.isRemoteBrowser) {

View File

@@ -23,116 +23,43 @@ registerCleanupFunction(function() {
Services.prefs.clearUserPref(PREF_NUM_PINNED_TABS);
});
add_task(async function testPrefSynced() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, 3);
let testCases = [
{numPinnedPref: 5, selected: 3, overrideTabs: false},
{numPinnedPref: 5, selected: 3, overrideTabs: true},
{numPinnedPref: 5, selected: 1, overrideTabs: false},
{numPinnedPref: 5, selected: 1, overrideTabs: true},
{numPinnedPref: 3, selected: 3, overrideTabs: false},
{numPinnedPref: 3, selected: 3, overrideTabs: true},
{numPinnedPref: 3, selected: 1, overrideTabs: false},
{numPinnedPref: 3, selected: 1, overrideTabs: true},
{numPinnedPref: 1, selected: 3, overrideTabs: false},
{numPinnedPref: 1, selected: 3, overrideTabs: true},
{numPinnedPref: 1, selected: 1, overrideTabs: false},
{numPinnedPref: 1, selected: 1, overrideTabs: true},
{numPinnedPref: 0, selected: 3, overrideTabs: false},
{numPinnedPref: 0, selected: 3, overrideTabs: true},
{numPinnedPref: 0, selected: 1, overrideTabs: false},
{numPinnedPref: 0, selected: 1, overrideTabs: true},
];
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true },
], selected: 3 }] };
for (let {numPinnedPref, selected, overrideTabs} of testCases) {
add_task(async function testPrefSynced() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, numPinnedPref);
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, false, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 4);
await BrowserTestUtils.closeWindow(win);
});
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true, userContextId: 0 },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true, userContextId: 0 },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true, userContextId: 0 },
], selected }] };
add_task(async function testPrefSyncedSelected() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, 3);
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true },
], selected: 1 }] };
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, false, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 4);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testPrefTooHigh() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, 5);
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true },
], selected: 3 }] };
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 5);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, false, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 4);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testPrefTooLow() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, 1);
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true },
], selected: 3 }] };
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 1);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, false, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 4);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testPrefTooHighSelected() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, 5);
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true },
], selected: 1 }] };
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 5);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, false, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 4);
await BrowserTestUtils.closeWindow(win);
});
add_task(async function testPrefTooLowSelected() {
Services.prefs.setIntPref(PREF_NUM_PINNED_TABS, 1);
let state = { windows: [{ tabs: [
{ entries: [{ url: "http://example.org/#1", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#2", triggeringPrincipal_base64 }], pinned: true },
{ entries: [{ url: "http://example.org/#3", triggeringPrincipal_base64 }], pinned: true },
], selected: 1 }] };
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 1);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, false, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 4);
await BrowserTestUtils.closeWindow(win);
});
muffleMainWindowType();
let win = await BrowserTestUtils.openNewBrowserWindow();
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, numPinnedPref);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length, 1);
await setWindowState(win, state, overrideTabs, true);
Assert.equal([...win.gBrowser.tabs].filter(t => t.pinned).length, 3);
Assert.equal([...win.gBrowser.tabs].filter(t => !!t.linkedPanel).length,
overrideTabs ? 3 : 4);
await BrowserTestUtils.closeWindow(win);
});
}