From 2c846a49c3860d9444036dbf05dfe4436b46e7f8 Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Thu, 4 Aug 2011 03:49:38 +0200 Subject: [PATCH] Bug 651643 - Private browsing service executes transition even when no mode switch required; r=ehsan,zpao --- browser/base/content/tabview/ui.js | 3 +- .../browser_tabview_privatebrowsing.js | 2 + .../src/nsPrivateBrowsingService.js | 54 ++++++++------- .../test/browser/browser_588426.js | 34 ++++++--- .../test/browser/browser_590563.js | 69 +++++++++++-------- 5 files changed, 98 insertions(+), 64 deletions(-) diff --git a/browser/base/content/tabview/ui.js b/browser/base/content/tabview/ui.js index ed2740ac598..d9e18083bd0 100644 --- a/browser/base/content/tabview/ui.js +++ b/browser/base/content/tabview/ui.js @@ -433,7 +433,8 @@ let UI = { Utils.assert(item, "item must be given"); if (item.isATabItem) { - GroupItems.setActiveGroupItem(item.parent); + if (item.parent) + GroupItems.setActiveGroupItem(item.parent); this._setActiveTab(item); } else { GroupItems.setActiveGroupItem(item); diff --git a/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js b/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js index a6f6ae27861..aae8979f149 100644 --- a/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js +++ b/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js @@ -103,6 +103,8 @@ function onTabViewHidden() { // end game ok(!TabView.isVisible(), "we finish with Tab View not visible"); registerCleanupFunction(verifyCleanState); // verify after all cleanups + + gBrowser.selectedTab = gBrowser.tabs[0]; finish(); }); }); diff --git a/browser/components/privatebrowsing/src/nsPrivateBrowsingService.js b/browser/components/privatebrowsing/src/nsPrivateBrowsingService.js index 42fdf81d190..484482f24b1 100644 --- a/browser/components/privatebrowsing/src/nsPrivateBrowsingService.js +++ b/browser/components/privatebrowsing/src/nsPrivateBrowsingService.js @@ -503,41 +503,43 @@ PrivateBrowsingService.prototype = { if (this._currentStatus != STATE_IDLE) throw Cr.NS_ERROR_FAILURE; + if (val == this._inPrivateBrowsing) + return; + try { + if (val) { + if (!this._canEnterPrivateBrowsingMode()) + return; + } + else { + if (!this._canLeavePrivateBrowsingMode()) + return; + } + + this._ensureCanCloseWindows(); + + // start the transition now that we know that we can this._currentStatus = STATE_TRANSITION_STARTED; - if (val != this._inPrivateBrowsing) { - if (val) { - if (!this._canEnterPrivateBrowsingMode()) - return; - } - else { - if (!this._canLeavePrivateBrowsingMode()) - return; - } + this._autoStarted = this._prefs.getBoolPref("browser.privatebrowsing.autostart"); + this._inPrivateBrowsing = val != false; - this._ensureCanCloseWindows(); + let data = val ? "enter" : "exit"; - this._autoStarted = this._prefs.getBoolPref("browser.privatebrowsing.autostart"); - this._inPrivateBrowsing = val != false; + let quitting = Cc["@mozilla.org/supports-PRBool;1"]. + createInstance(Ci.nsISupportsPRBool); + quitting.data = this._quitting; - let data = val ? "enter" : "exit"; + // notify observers of the pending private browsing mode change + this._obs.notifyObservers(quitting, "private-browsing-change-granted", data); - let quitting = Cc["@mozilla.org/supports-PRBool;1"]. - createInstance(Ci.nsISupportsPRBool); - quitting.data = this._quitting; + // destroy the current session and start initial cleanup + this._onBeforePrivateBrowsingModeChange(); - // notify observers of the pending private browsing mode change - this._obs.notifyObservers(quitting, "private-browsing-change-granted", data); + this._obs.notifyObservers(quitting, "private-browsing", data); - // destroy the current session and start initial cleanup - this._onBeforePrivateBrowsingModeChange(); - - this._obs.notifyObservers(quitting, "private-browsing", data); - - // load the appropriate session - this._onAfterPrivateBrowsingModeChange(); - } + // load the appropriate session + this._onAfterPrivateBrowsingModeChange(); } catch (ex) { // We aborted the transition to/from private browsing, we must restore the // beforeunload handling on all the windows for which we switched it off. diff --git a/browser/components/sessionstore/test/browser/browser_588426.js b/browser/components/sessionstore/test/browser/browser_588426.js index 1e414e5f282..fc9146e7265 100644 --- a/browser/components/sessionstore/test/browser/browser_588426.js +++ b/browser/components/sessionstore/test/browser/browser_588426.js @@ -7,19 +7,35 @@ function test() { {entries: [{url: "about:robots"}], hidden: true} ] }] }; - let finalState = { windows: [{ tabs: [ - {entries: [{url: "about:blank"}]} - ] }] }; - waitForExplicitFinish(); - waitForBrowserState(state, function () { - is(gBrowser.tabs.length, 2, "two tabs were restored"); - is(gBrowser.visibleTabs.length, 1, "one tab is visible"); + newWindowWithState(state, function (win) { + registerCleanupFunction(function () win.close()); - let tab = gBrowser.visibleTabs[0]; + is(win.gBrowser.tabs.length, 2, "two tabs were restored"); + is(win.gBrowser.visibleTabs.length, 1, "one tab is visible"); + + let tab = win.gBrowser.visibleTabs[0]; is(tab.linkedBrowser.currentURI.spec, "about:mozilla", "visible tab is about:mozilla"); - waitForBrowserState(finalState, finish); + finish(); }); } + +function newWindowWithState(state, callback) { + let opts = "chrome,all,dialog=no,height=800,width=800"; + let win = window.openDialog(getBrowserURL(), "_blank", opts); + + win.addEventListener("load", function onLoad() { + win.removeEventListener("load", onLoad, false); + + executeSoon(function () { + win.addEventListener("SSWindowStateReady", function onReady() { + win.removeEventListener("SSWindowStateReady", onReady, false); + executeSoon(function () callback(win)); + }, false); + + ss.setWindowState(win, JSON.stringify(state), true); + }); + }, false); +} diff --git a/browser/components/sessionstore/test/browser/browser_590563.js b/browser/components/sessionstore/test/browser/browser_590563.js index c30c901da93..1fefc4c63d6 100644 --- a/browser/components/sessionstore/test/browser/browser_590563.js +++ b/browser/components/sessionstore/test/browser/browser_590563.js @@ -1,11 +1,7 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -let stateBackup = ss.getBrowserState(); - function test() { - waitForExplicitFinish(); - let oldState = { windows: [{ tabs: [ @@ -20,24 +16,26 @@ function test() { }; let state = { windows: [{ tabs: [{ entries: [pageData] }] }] }; - // The form data will be restored before SSTabRestored, so we want to listen - // for that on the currently selected tab (it will be reused) - gBrowser.selectedTab.addEventListener("SSTabRestored", onSSTabRestored, true); + waitForExplicitFinish(); - ss.setBrowserState(JSON.stringify(state)); + newWindowWithState(state, function (win) { + registerCleanupFunction(function () win.close()); + + is(gBrowser.tabs.length, 1, "The total number of tabs should be 1"); + is(gBrowser.visibleTabs.length, 1, "The total number of visible tabs should be 1"); + + executeSoon(function () { + waitForFocus(function () { + middleClickTest(win); + finish(); + }, win); + }); + }); } -function onSSTabRestored(aEvent) { - gBrowser.selectedTab.removeEventListener("SSTabRestored", onSSTabRestored, true); - - is(gBrowser.tabs.length, 1, "The total number of tabs should be 1"); - is(gBrowser.visibleTabs.length, 1, "The total number of visible tabs should be 1"); - - executeSoon(middleClickTest); -} - -function middleClickTest() { - let tree = gBrowser.selectedBrowser.contentDocument.getElementById("tabList"); +function middleClickTest(win) { + let browser = win.gBrowser.selectedBrowser; + let tree = browser.contentDocument.getElementById("tabList"); is(tree.view.rowCount, 3, "There should be three items"); let x = {}, y = {}, width = {}, height = {}; @@ -45,21 +43,36 @@ function middleClickTest() { // click on the first tab item tree.treeBoxObject.getCoordsForCellItem(1, tree.columns[1], "text", x, y, width, height); EventUtils.synthesizeMouse(tree.body, x.value, y.value, { button: 1 }, - gBrowser.selectedBrowser.contentWindow); + browser.contentWindow); // click on the second tab item tree.treeBoxObject.getCoordsForCellItem(2, tree.columns[1], "text", x, y, width, height); EventUtils.synthesizeMouse(tree.body, x.value, y.value, { button: 1 }, - gBrowser.selectedBrowser.contentWindow); + browser.contentWindow); - is(gBrowser.tabs.length, 3, + is(win.gBrowser.tabs.length, 3, "The total number of tabs should be 3 after restoring 2 tabs by middle click."); - is(gBrowser.visibleTabs.length, 3, + is(win.gBrowser.visibleTabs.length, 3, "The total number of visible tabs should be 3 after restoring 2 tabs by middle click"); - - cleanup(); } -function cleanup() { - ss.setBrowserState(stateBackup); - executeSoon(finish); +function newWindowWithState(state, callback) { + let opts = "chrome,all,dialog=no,height=800,width=800"; + let win = window.openDialog(getBrowserURL(), "_blank", opts); + + win.addEventListener("load", function onLoad() { + win.removeEventListener("load", onLoad, false); + + let tab = win.gBrowser.selectedTab; + + // The form data will be restored before SSTabRestored, so we want to listen + // for that on the currently selected tab (it will be reused) + tab.addEventListener("SSTabRestored", function onRestored() { + tab.removeEventListener("SSTabRestored", onRestored, true); + callback(win); + }, true); + + executeSoon(function () { + ss.setWindowState(win, JSON.stringify(state), true); + }); + }, false); }