From 706087d0e678ef5a6b49134e519da86278b96986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Tue, 9 Nov 2010 10:00:30 -0800 Subject: [PATCH] Bug 589246 - Closed window state getting corrupted when closing and reopening last browser window without exiting browser [r=dietrich, a=blocking2.0:betaN] --- .../sessionstore/src/nsSessionStore.js | 63 +++- .../sessionstore/test/browser/Makefile.in | 1 + .../test/browser/browser_589246.js | 276 ++++++++++++++++++ 3 files changed, 326 insertions(+), 14 deletions(-) create mode 100644 browser/components/sessionstore/test/browser/browser_589246.js diff --git a/browser/components/sessionstore/src/nsSessionStore.js b/browser/components/sessionstore/src/nsSessionStore.js index b5a27c5bb40f..8e67ff9a5a62 100644 --- a/browser/components/sessionstore/src/nsSessionStore.js +++ b/browser/components/sessionstore/src/nsSessionStore.js @@ -745,24 +745,59 @@ SessionStoreService.prototype = { !this._inPrivateBrowsing) { // default to the most-recently closed window // don't use popup windows - let state = null; - let newClosedWindows = this._closedWindows.filter(function(aWinState) { - if (!state && !aWinState.isPopup) { - state = aWinState; - return false; + let closedWindowState = null; + let closedWindowIndex; + for (let i = 0; i < this._closedWindows.length; i++) { + // Take the first non-popup, point our object at it, and break out. + if (!this._closedWindows[i].isPopup) { + closedWindowState = this._closedWindows[i]; + closedWindowIndex = i; + break; } - return true; - }); - if (state) { - delete state.hidden; + } + + if (closedWindowState) { + let newWindowState; #ifndef XP_MACOSX - if (!this._doResumeSession()) + if (!this._doResumeSession()) { #endif - state.tabs = state.tabs.filter(function (tab) tab.pinned); - if (state.tabs.length > 0) { - this._closedWindows = newClosedWindows; + // We want to split the window up into pinned tabs and unpinned tabs. + // Pinned tabs should be restored. If there are any remaining tabs, + // they should be added back to _closedWindows. + // We'll cheat a little bit and reuse _prepDataForDeferredRestore + // even though it wasn't built exactly for this. + let [appTabsState, normalTabsState] = + this._prepDataForDeferredRestore(JSON.stringify({ windows: [closedWindowState] })); + + // These are our pinned tabs, which we should restore + if (appTabsState.windows.length) { + newWindowState = appTabsState.windows[0]; + delete newWindowState.__lastSessionWindowID; + } + + // In case there were no unpinned tabs, remove the window from _closedWindows + if (!normalTabsState.windows.length) { + this._closedWindows.splice(closedWindowIndex, 1); + } + // Or update _closedWindows with the modified state + else { + delete normalTabsState.windows[0].__lastSessionWindowID; + this._closedWindows[closedWindowIndex] = normalTabsState.windows[0]; + } +#ifndef XP_MACOSX + } + else { + // If we're just restoring the window, make sure it gets removed from + // _closedWindows. + this._closedWindows.splice(closedWindowIndex, 1); + newWindowState = closedWindowState; + delete newWindowState.hidden; + } +#endif + if (newWindowState) { + // Ensure that the window state isn't hidden this._restoreCount = 1; - state = { windows: [state] }; + let state = { windows: [newWindowState] }; this.restoreWindow(aWindow, state, this._isCmdLineEmpty(aWindow, state)); } } diff --git a/browser/components/sessionstore/test/browser/Makefile.in b/browser/components/sessionstore/test/browser/Makefile.in index 83f41faf2e45..6f00f1abf943 100644 --- a/browser/components/sessionstore/test/browser/Makefile.in +++ b/browser/components/sessionstore/test/browser/Makefile.in @@ -120,6 +120,7 @@ _BROWSER_TEST_FILES = \ browser_581593.js \ browser_586147.js \ browser_586068-cascaded_restore.js \ + browser_589246.js \ browser_590268.js \ browser_600545.js \ $(NULL) diff --git a/browser/components/sessionstore/test/browser/browser_589246.js b/browser/components/sessionstore/test/browser/browser_589246.js new file mode 100644 index 000000000000..07233bbce36f --- /dev/null +++ b/browser/components/sessionstore/test/browser/browser_589246.js @@ -0,0 +1,276 @@ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is sessionstore test code. + * + * The Initial Developer of the Original Code is + * Mozilla Foundation. + * Portions created by the Initial Developer are Copyright (C) 2010 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Paul O’Shannessy + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +// Mirrors WINDOW_ATTRIBUTES IN nsSessionStore.js +const WINDOW_ATTRIBUTES = ["width", "height", "screenX", "screenY", "sizemode"]; + +Cu.import("resource://gre/modules/Services.jsm"); +let ss = Cc["@mozilla.org/browser/sessionstore;1"]. + getService(Ci.nsISessionStore); + +let stateBackup = ss.getBrowserState(); + +let originalWarnOnClose = gPrefService.getBoolPref("browser.tabs.warnOnClose"); +let originalStartupPage = gPrefService.getIntPref("browser.startup.page"); +let originalWindowType = document.documentElement.getAttribute("windowtype"); + +let gotLastWindowClosedTopic = false; +let shouldPinTab = false; +let shouldOpenTabs = false; +let shouldCloseTab = false; +let testNum = 0; +let afterTestCallback; + +// Set state so we know the closed windows content +let testState = { + windows: [ + { tabs: [{ entries: [{ url: "http://example.org" }] }] } + ], + _closedWindows: [] +}; + +// We'll push a set of conditions and callbacks into this array +// Ideally we would also test win/linux under a complete set of conditions, but +// the tests for osx mirror the other set of conditions possible on win/linux. +let tests = []; + +// the third & fourth test share a condition check, keep it DRY +function checkOSX34Generator(num) { + return function(aPreviousState, aCurState) { + // In here, we should have restored the pinned tab, so only the unpinned tab + // should be in aCurState. So let's shape our expectations. + let expectedState = JSON.parse(aPreviousState); + expectedState[0].tabs.shift(); + // size attributes are stripped out in _prepDataForDeferredRestore in nsSessionStore. + // This isn't the best approach, but neither is comparing JSON strings + WINDOW_ATTRIBUTES.forEach(function (attr) delete expectedState[0][attr]); + + is(aCurState, JSON.stringify(expectedState), + "test #" + num + ": closedWindowState is as expected"); + }; +} +function checkNoWindowsGenerator(num) { + return function(aPreviousState, aCurState) { + is(aCurState, "[]", "test #" + num + ": there should be no closedWindowsLeft"); + }; +} + +// The first test has 0 pinned tabs and 1 unpinned tab +tests.push({ + pinned: false, + extra: false, + close: false, + checkWinLin: checkNoWindowsGenerator(1), + checkOSX: function(aPreviousState, aCurState) { + is(aCurState, aPreviousState, "test #1: closed window state is unchanged"); + } +}); + +// The second test has 1 pinned tab and 0 unpinned tabs. +tests.push({ + pinned: true, + extra: false, + close: false, + checkWinLin: checkNoWindowsGenerator(2), + checkOSX: checkNoWindowsGenerator(2) +}); + +// The third test has 1 pinned tab and 2 unpinned tabs. +tests.push({ + pinned: true, + extra: true, + close: false, + checkWinLin: checkNoWindowsGenerator(3), + checkOSX: checkOSX34Generator(3) +}); + +// The fourth test has 1 pinned tab, 2 unpinned tabs, and closes one unpinned tab. +tests.push({ + pinned: true, + extra: true, + close: "one", + checkWinLin: checkNoWindowsGenerator(4), + checkOSX: checkOSX34Generator(4) +}); + +// The fifth test has 1 pinned tab, 2 unpinned tabs, and closes both unpinned tabs. +tests.push({ + pinned: true, + extra: true, + close: "both", + checkWinLin: checkNoWindowsGenerator(5), + checkOSX: checkNoWindowsGenerator(5) +}); + + +function test() { + /** Test for Bug 589246 - Closed window state getting corrupted when closing + and reopening last browser window without exiting browser **/ + waitForExplicitFinish(); + // windows opening & closing, so extending the timeout + requestLongerTimeout(2); + + // We don't want the quit dialog pref + gPrefService.setBoolPref("browser.tabs.warnOnClose", false); + // Ensure that we would restore the session (important for Windows) + gPrefService.setIntPref("browser.startup.page", 3); + + runNextTestOrFinish(); +} + +function runNextTestOrFinish() { + if (tests.length) { + setupForTest(tests.shift()) + } + else { + // some state is cleaned up at the end of each test, but not all + ["browser.tabs.warnOnClose", "browser.startup.page"].forEach(function(p) { + if (gPrefService.prefHasUserValue(p)) + gPrefService.clearUserPref(p); + }); + + ss.setBrowserState(stateBackup); + executeSoon(finish); + } +} + +function setupForTest(aConditions) { + // reset some checks + gotLastWindowClosedTopic = false; + shouldPinTab = aConditions.pinned; + shouldOpenTabs = aConditions.extra; + shouldCloseTab = aConditions.close; + testNum++; + + // set our test callback + afterTestCallback = /Mac/.test(navigator.platform) ? aConditions.checkOSX + : aConditions.checkWinLin; + + // Add observers + Services.obs.addObserver(onLastWindowClosed, "browser-lastwindow-close-granted", false); + + // Set the state + Services.obs.addObserver(onStateRestored, "sessionstore-browser-state-restored", false); + ss.setBrowserState(JSON.stringify(testState)); +} + +function onStateRestored(aSubject, aTopic, aData) { + info("test #" + testNum + ": onStateRestored"); + Services.obs.removeObserver(onStateRestored, "sessionstore-browser-state-restored", false); + + // change this window's windowtype so that closing a new window will trigger + // browser-lastwindow-close-granted. + document.documentElement.setAttribute("windowtype", "navigator:testrunner"); + + let newWin = openDialog(location, "_blank", "chrome,all,dialog=no", "http://example.com"); + newWin.addEventListener("load", function(aEvent) { + newWin.removeEventListener("load", arguments.callee, false); + + newWin.gBrowser.selectedBrowser.addEventListener("load", function() { + newWin.gBrowser.selectedBrowser.removeEventListener("load", arguments.callee, true); + + // pin this tab + if (shouldPinTab) + newWin.gBrowser.pinTab(newWin.gBrowser.selectedTab); + + newWin.addEventListener("unload", onWindowUnloaded, false); + // Open a new tab as well. On Windows/Linux this will be restored when the + // new window is opened below (in onWindowUnloaded). On OS X we'll just + // restore the pinned tabs, leaving the unpinned tab in the closedWindowsData. + if (shouldOpenTabs) { + let newTab = newWin.gBrowser.addTab("about:config"); + let newTab2 = newWin.gBrowser.addTab("about:buildconfig"); + + newTab.linkedBrowser.addEventListener("load", function() { + newTab.linkedBrowser.removeEventListener("load", arguments.callee, true); + + if (shouldCloseTab == "one") { + newWin.gBrowser.removeTab(newTab2); + } + else if (shouldCloseTab == "both") { + newWin.gBrowser.removeTab(newTab); + newWin.gBrowser.removeTab(newTab2); + } + newWin.BrowserTryToCloseWindow(); + }, true); + } + else { + newWin.BrowserTryToCloseWindow(); + } + }, true); + }, false); +} + +// This will be called before the window is actually closed +function onLastWindowClosed(aSubject, aTopic, aData) { + info("test #" + testNum + ": onLastWindowClosed"); + Services.obs.removeObserver(onLastWindowClosed, "browser-lastwindow-close-granted", false); + gotLastWindowClosedTopic = true; +} + +// This is the unload event listener on the new window (from onStateRestored). +// Unload is fired after the window is closed, so sessionstore has already +// updated _closedWindows (which is important). We'll open a new window here +// which should actually trigger the bug. +function onWindowUnloaded() { + info("test #" + testNum + ": onWindowClosed"); + ok(gotLastWindowClosedTopic, "test #" + testNum + ": browser-lastwindow-close-granted was notified prior"); + + let previousClosedWindowData = ss.getClosedWindowData(); + + // Now we want to open a new window + let newWin = openDialog(location, "_blank", "chrome,all,dialog=no", "about:robots"); + newWin.addEventListener("load", function(aEvent) { + newWin.removeEventListener("load", arguments.callee, false); + + newWin.gBrowser.selectedBrowser.addEventListener("load", function () { + // Good enough for checking the state + afterTestCallback(previousClosedWindowData, ss.getClosedWindowData()); + afterTestCleanup(newWin); + }, true); + + }, false); +} + +function afterTestCleanup(aNewWin) { + executeSoon(function() { + aNewWin.close(); + document.documentElement.setAttribute("windowtype", originalWindowType); + runNextTestOrFinish(); + }); +} +