Bug 961380 - Should be able to undo closing tabs in private windows r=yoric, ui-r=phlsa
authorTim Taubert <ttaubert@mozilla.com>
Sat, 18 Jan 2014 09:51:46 +0100
changeset 164459 cdca987f43ce1dd49f95c84e51d127b09d48dea9
parent 164458 4694a2bdb14d971b52835d0c167191b5ad8e86df
child 164460 85b7689257760bf80e31ba3030291899f05e06eb
push id26047
push userryanvm@gmail.com
push dateTue, 21 Jan 2014 20:18:36 +0000
treeherdermozilla-central@dcb0a6baeadb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric, phlsa
bugs961380
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 961380 - Should be able to undo closing tabs in private windows r=yoric, ui-r=phlsa From 0e17ede5f79d9894b8fc7d391615767766dd9aa6 Mon Sep 17 00:00:00 2001
browser/components/sessionstore/src/PrivacyFilter.jsm
browser/components/sessionstore/src/PrivacyLevelFilter.jsm
browser/components/sessionstore/src/SessionSaver.jsm
browser/components/sessionstore/src/SessionStore.jsm
browser/components/sessionstore/src/TabState.jsm
browser/components/sessionstore/src/moz.build
browser/components/sessionstore/test/browser_privatetabs.js
rename from browser/components/sessionstore/src/PrivacyLevelFilter.jsm
rename to browser/components/sessionstore/src/PrivacyFilter.jsm
--- a/browser/components/sessionstore/src/PrivacyLevelFilter.jsm
+++ b/browser/components/sessionstore/src/PrivacyFilter.jsm
@@ -1,15 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this file,
 * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-this.EXPORTED_SYMBOLS = ["PrivacyLevelFilter"];
+this.EXPORTED_SYMBOLS = ["PrivacyFilter"];
 
 const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 
 XPCOMUtils.defineLazyModuleGetter(this, "PrivacyLevel",
   "resource:///modules/sessionstore/PrivacyLevel.jsm");
 
@@ -25,17 +25,17 @@ function checkPrivacyLevel(url, isPinned
   let isHttps = url.startsWith("https:");
   return PrivacyLevel.canSave({isHttps: isHttps, isPinned: isPinned});
 }
 
 /**
  * A module that provides methods to filter various kinds of data collected
  * from a tab by the current privacy level as set by the user.
  */
-this.PrivacyLevelFilter = Object.freeze({
+this.PrivacyFilter = Object.freeze({
   /**
    * Filters the given (serialized) session storage |data| according to the
    * current privacy level and returns a new object containing only data that
    * we're allowed to store.
    *
    * @param data The session storage data as collected from a tab.
    * @param isPinned Whether the tab we collected from is pinned.
    * @return object
@@ -82,10 +82,69 @@ this.PrivacyLevelFilter = Object.freeze(
       // Only copy keys other than "children" if we have a valid URL in
       // data.url and we thus passed the privacy level check.
       } else if (data.url) {
         retval[key] = data[key];
       }
     }
 
     return Object.keys(retval).length ? retval : null;
+  },
+
+  /**
+   * Removes any private windows and tabs from a given browser state object.
+   *
+   * @param browserState (object)
+   *        The browser state for which we remove any private windows and tabs.
+   *        The given object will be modified.
+   */
+  filterPrivateWindowsAndTabs: function (browserState) {
+    // Remove private opened windows.
+    for (let i = browserState.windows.length - 1; i >= 0; i--) {
+      let win = browserState.windows[i];
+
+      if (win.isPrivate) {
+        browserState.windows.splice(i, 1);
+
+        if (browserState.selectedWindow >= i) {
+          browserState.selectedWindow--;
+        }
+      } else {
+        // Remove private tabs from all open non-private windows.
+        this.filterPrivateTabs(win);
+      }
+    }
+
+    // Remove private closed windows.
+    browserState._closedWindows =
+      browserState._closedWindows.filter(win => !win.isPrivate);
+
+    // Remove private tabs from all remaining closed windows.
+    browserState._closedWindows.forEach(win => this.filterPrivateTabs(win));
+  },
+
+  /**
+   * Removes open private tabs from a given window state object.
+   *
+   * @param winState (object)
+   *        The window state for which we remove any private tabs.
+   *        The given object will be modified.
+   */
+  filterPrivateTabs: function (winState) {
+    // Remove open private tabs.
+    for (let i = winState.tabs.length - 1; i >= 0 ; i--) {
+      let tab = winState.tabs[i];
+
+      if (tab.isPrivate) {
+        winState.tabs.splice(i, 1);
+
+        if (winState.selected >= i) {
+          winState.selected--;
+        }
+      }
+    }
+
+    // Note that closed private tabs are only stored for private windows.
+    // There is no need to call this function for private windows as the
+    // whole window state should just be discarded so we explicitly don't
+    // try to remove closed private tabs as an optimization.
   }
 });
--- a/browser/components/sessionstore/src/SessionSaver.jsm
+++ b/browser/components/sessionstore/src/SessionSaver.jsm
@@ -10,20 +10,22 @@ const Cu = Components.utils;
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 
 Cu.import("resource://gre/modules/Timer.jsm", this);
 Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/TelemetryStopwatch.jsm", this);
 
+XPCOMUtils.defineLazyModuleGetter(this, "console",
+  "resource://gre/modules/devtools/Console.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PrivacyFilter",
+  "resource:///modules/sessionstore/PrivacyFilter.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionStore",
   "resource:///modules/sessionstore/SessionStore.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "console",
-  "resource://gre/modules/devtools/Console.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionFile",
   "resource:///modules/sessionstore/SessionFile.jsm");
 
 // Minimal interval between two save operations (in milliseconds).
 XPCOMUtils.defineLazyGetter(this, "gInterval", function () {
   const PREF = "browser.sessionstore.interval";
 
   // Observer that updates the cached value when the preference changes.
@@ -187,49 +189,17 @@ let SessionSaverInternal = {
    *        update the corresponding caches.
    */
   _saveState: function (forceUpdateAllWindows = false) {
     // Cancel any pending timeouts.
     this.cancel();
 
     stopWatchStart("COLLECT_DATA_MS", "COLLECT_DATA_LONGEST_OP_MS");
     let state = SessionStore.getCurrentState(forceUpdateAllWindows);
-
-    // Forget about private windows and tabs.
-    for (let i = state.windows.length - 1; i >= 0; i--) {
-      let win = state.windows[i];
-      if (win.isPrivate || false) { // The whole window is private, remove it
-         state.windows.splice(i, 1);
-         if (state.selectedWindow >= i) {
-           state.selectedWindow--;
-         }
-        continue;
-      }
-      // The window is not private, but its tabs still might
-      for (let j = win.tabs.length - 1; j >= 0 ; --j) {
-        let tab = win.tabs[j];
-        if (tab.isPrivate || false) {
-          win.tabs.splice(j, 1);
-          if (win.selected >= j) {
-            win.selected--;
-          }
-        }
-      }
-    }
-
-    // Remove private windows from the list of closed windows.
-    for (let i = state._closedWindows.length - 1; i >= 0; i--) {
-      if (state._closedWindows[i].isPrivate) {
-        state._closedWindows.splice(i, 1);
-      }
-    }
-
-    // Note that closed private tabs are never stored (see
-    // SessionStoreInternal.onTabClose), so we do not need to remove
-    // them.
+    PrivacyFilter.filterPrivateWindowsAndTabs(state);
 
     // Make sure that we keep the previous session if we started with a single
     // private window and no non-private windows have been opened, yet.
     if (state.deferredInitialState) {
       state.windows = state.deferredInitialState.windows || [];
       delete state.deferredInitialState;
     }
 
--- a/browser/components/sessionstore/src/SessionStore.jsm
+++ b/browser/components/sessionstore/src/SessionStore.jsm
@@ -120,16 +120,18 @@ XPCOMUtils.defineLazyServiceGetter(this,
   "@mozilla.org/base/telemetry;1", "nsITelemetry");
 
 XPCOMUtils.defineLazyModuleGetter(this, "console",
   "resource://gre/modules/devtools/Console.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "GlobalState",
   "resource:///modules/sessionstore/GlobalState.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Messenger",
   "resource:///modules/sessionstore/Messenger.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PrivacyFilter",
+  "resource:///modules/sessionstore/PrivacyFilter.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
   "resource:///modules/RecentWindow.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ScratchpadManager",
   "resource:///modules/devtools/scratchpad-manager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionSaver",
   "resource:///modules/sessionstore/SessionSaver.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionCookies",
   "resource:///modules/sessionstore/SessionCookies.jsm");
@@ -1104,23 +1106,30 @@ let SessionStoreInternal = {
 
       // Store the window's close date to figure out when each individual tab
       // was closed. This timestamp should allow re-arranging data based on how
       // recently something was closed.
       winData.closedAt = Date.now();
 
       // Save the window if it has multiple tabs or a single saveable tab and
       // it's not private.
-      if (!winData.isPrivate && (winData.tabs.length > 1 ||
-          (winData.tabs.length == 1 && this._shouldSaveTabState(winData.tabs[0])))) {
-        // we don't want to save the busy state
-        delete winData.busy;
-
-        this._closedWindows.unshift(winData);
-        this._capClosedWindows();
+      if (!winData.isPrivate) {
+        // Remove any open private tabs the window may contain.
+        PrivacyFilter.filterPrivateTabs(winData);
+
+        let hasSingleTabToSave =
+          winData.tabs.length == 1 && this._shouldSaveTabState(winData.tabs[0]);
+
+        if (hasSingleTabToSave || winData.tabs.length > 1) {
+          // we don't want to save the busy state
+          delete winData.busy;
+
+          this._closedWindows.unshift(winData);
+          this._capClosedWindows();
+        }
       }
 
       // clear this window from the list
       delete this._windows[aWindow.__SSi];
 
       // save the state without this window to disk
       this.saveStateDelayed();
     }
@@ -1401,17 +1410,18 @@ let SessionStoreInternal = {
 
     // Flush all data queued in the content script before the tab is gone.
     TabState.flush(aTab.linkedBrowser);
 
     // Get the latest data for this tab (generally, from the cache)
     let tabState = TabState.collectSync(aTab);
 
     // Don't save private tabs
-    if (tabState.isPrivate || false) {
+    let isPrivateWindow = PrivateBrowsingUtils.isWindowPrivate(aWindow);
+    if (!isPrivateWindow && tabState.isPrivate) {
       return;
     }
 
     // store closed-tab data for undo
     if (this._shouldSaveTabState(tabState)) {
       let tabTitle = aTab.label;
       let tabbrowser = aWindow.gBrowser;
       tabTitle = this._replaceLoadingTitle(tabTitle, tabbrowser, aTab);
--- a/browser/components/sessionstore/src/TabState.jsm
+++ b/browser/components/sessionstore/src/TabState.jsm
@@ -11,18 +11,18 @@ const Cu = Components.utils;
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/Promise.jsm", this);
 Cu.import("resource://gre/modules/Task.jsm", this);
 
 XPCOMUtils.defineLazyModuleGetter(this, "console",
   "resource://gre/modules/devtools/Console.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Messenger",
   "resource:///modules/sessionstore/Messenger.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PrivacyLevelFilter",
-  "resource:///modules/sessionstore/PrivacyLevelFilter.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PrivacyFilter",
+  "resource:///modules/sessionstore/PrivacyFilter.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TabStateCache",
   "resource:///modules/sessionstore/TabStateCache.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TabAttributes",
   "resource:///modules/sessionstore/TabAttributes.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Utils",
   "resource:///modules/sessionstore/Utils.jsm");
 
 /**
@@ -361,19 +361,19 @@ let TabStateInternal = {
     let includePrivateData = options && options.includePrivateData;
 
     for (let key of Object.keys(data)) {
       let value = data[key];
 
       // Filter sensitive data according to the current privacy level.
       if (!includePrivateData) {
         if (key === "storage") {
-          value = PrivacyLevelFilter.filterSessionStorageData(value, tab.pinned);
+          value = PrivacyFilter.filterSessionStorageData(value, tab.pinned);
         } else if (key === "formdata") {
-          value = PrivacyLevelFilter.filterFormData(value, tab.pinned);
+          value = PrivacyFilter.filterFormData(value, tab.pinned);
         }
       }
 
       if (value) {
         tabData[key] = value;
       }
     }
   },
--- a/browser/components/sessionstore/src/moz.build
+++ b/browser/components/sessionstore/src/moz.build
@@ -15,18 +15,18 @@ JS_MODULES_PATH = 'modules/sessionstore'
 EXTRA_JS_MODULES = [
     'ContentRestore.jsm',
     'DocShellCapabilities.jsm',
     'FormData.jsm',
     'FrameTree.jsm',
     'GlobalState.jsm',
     'Messenger.jsm',
     'PageStyle.jsm',
+    'PrivacyFilter.jsm',
     'PrivacyLevel.jsm',
-    'PrivacyLevelFilter.jsm',
     'RecentlyClosedTabsAndWindowsMenuUtils.jsm',
     'ScrollPosition.jsm',
     'SessionCookies.jsm',
     'SessionFile.jsm',
     'SessionHistory.jsm',
     'SessionMigration.jsm',
     'SessionStorage.jsm',
     'SessionWorker.js',
--- a/browser/components/sessionstore/test/browser_privatetabs.js
+++ b/browser/components/sessionstore/test/browser_privatetabs.js
@@ -63,30 +63,79 @@ add_task(function() {
     }
   }
 });
 
 add_task(function () {
   const FRAME_SCRIPT = "data:," +
     "docShell.QueryInterface%28Ci.nsILoadContext%29.usePrivateBrowsing%3Dtrue";
 
+  // Clear the list of closed windows.
+  while (ss.getClosedWindowCount()) {
+    ss.forgetClosedWindow(0);
+  }
+
   // Create a new window to attach our frame script to.
   let win = yield promiseNewWindowLoaded();
   win.messageManager.loadFrameScript(FRAME_SCRIPT, true);
 
   // Create a new tab in the new window that will load the frame script.
   let tab = win.gBrowser.addTab("about:mozilla");
   let browser = tab.linkedBrowser;
   yield promiseBrowserLoaded(browser);
   SyncHandlers.get(browser).flush();
 
   // Check that we consider the tab as private.
   let state = JSON.parse(ss.getTabState(tab));
   ok(state.isPrivate, "tab considered private");
 
-  // Cleanup.
+  // Ensure we don't allow restoring closed private tabs in non-private windows.
+  win.gBrowser.removeTab(tab);
+  is(ss.getClosedTabCount(win), 0, "no tabs to restore");
+
+  // Create a new tab in the new window that will load the frame script.
+  let tab = win.gBrowser.addTab("about:mozilla");
+  let browser = tab.linkedBrowser;
+  yield promiseBrowserLoaded(browser);
+  SyncHandlers.get(browser).flush();
+
+  // Check that we consider the tab as private.
+  let state = JSON.parse(ss.getTabState(tab));
+  ok(state.isPrivate, "tab considered private");
+
+  // Check that all private tabs are removed when the non-private
+  // window is closed and we don't save windows without any tabs.
   yield promiseWindowClosed(win);
+  is(ss.getClosedWindowCount(), 0, "no windows to restore");
+});
+
+add_task(function () {
+  // Clear the list of closed windows.
+  while (ss.getClosedWindowCount()) {
+    ss.forgetClosedWindow(0);
+  }
+
+  // Create a new window to attach our frame script to.
+  let win = yield promiseNewWindowLoaded({private: true});
+
+  // Create a new tab in the new window that will load the frame script.
+  let tab = win.gBrowser.addTab("about:mozilla");
+  let browser = tab.linkedBrowser;
+  yield promiseBrowserLoaded(browser);
+  SyncHandlers.get(browser).flush();
+
+  // Check that we consider the tab as private.
+  let state = JSON.parse(ss.getTabState(tab));
+  ok(state.isPrivate, "tab considered private");
+
+  // Ensure that closed tabs in a private windows can be restored.
+  win.gBrowser.removeTab(tab);
+  is(ss.getClosedTabCount(win), 1, "there is a single tab to restore");
+
+  // Ensure that closed private windows can never be restored.
+  yield promiseWindowClosed(win);
+  is(ss.getClosedWindowCount(), 0, "no windows to restore");
 });
 
 function setUsePrivateBrowsing(browser, val) {
   return sendMessage(browser, "ss-test:setUsePrivateBrowsing", val);
 }