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

--HG--
rename : browser/components/sessionstore/src/PrivacyLevelFilter.jsm => browser/components/sessionstore/src/PrivacyFilter.jsm

browser/components/sessionstore/src/PrivacyFilter.jsm

@@ -4,7 +4,7 @@
 
 "use strict";
 
-this.EXPORTED_SYMBOLS = ["PrivacyLevelFilter"];
+this.EXPORTED_SYMBOLS = ["PrivacyFilter"];
 
 const Cu = Components.utils;
 
@@ -30,7 +30,7 @@ function checkPrivacyLevel(url, 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
@@ -87,5 +87,64 @@ this.PrivacyLevelFilter = Object.freeze({
     }
 
     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.
   }
 });

browser/components/sessionstore/src/SessionSaver.jsm

@@ -15,10 +15,12 @@ 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, "SessionStore",
-  "resource:///modules/sessionstore/SessionStore.jsm");
 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, "SessionFile",
   "resource:///modules/sessionstore/SessionFile.jsm");
 
@@ -192,39 +194,7 @@ let SessionSaverInternal = {
 
     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.

browser/components/sessionstore/src/SessionStore.jsm

@@ -125,6 +125,8 @@ 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",
@@ -1109,13 +1111,20 @@ let SessionStoreInternal = {
 
       // 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;
+      if (!winData.isPrivate) {
+        // Remove any open private tabs the window may contain.
+        PrivacyFilter.filterPrivateTabs(winData);
 
-        this._closedWindows.unshift(winData);
-        this._capClosedWindows();
+        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
@@ -1406,7 +1415,8 @@ let SessionStoreInternal = {
     let tabState = TabState.collectSync(aTab);
 
     // Don't save private tabs
-    if (tabState.isPrivate || false) {
+    let isPrivateWindow = PrivateBrowsingUtils.isWindowPrivate(aWindow);
+    if (!isPrivateWindow && tabState.isPrivate) {
       return;
     }
 

browser/components/sessionstore/src/TabState.jsm

@@ -16,8 +16,8 @@ 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",
@@ -366,9 +366,9 @@ let TabStateInternal = {
       // 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);
         }
       }
 

browser/components/sessionstore/src/moz.build

@@ -20,8 +20,8 @@ EXTRA_JS_MODULES = [
     'GlobalState.jsm',
     'Messenger.jsm',
     'PageStyle.jsm',
+    'PrivacyFilter.jsm',
     'PrivacyLevel.jsm',
-    'PrivacyLevelFilter.jsm',
     'RecentlyClosedTabsAndWindowsMenuUtils.jsm',
     'ScrollPosition.jsm',
     'SessionCookies.jsm',

browser/components/sessionstore/test/browser_privatetabs.js

@@ -68,6 +68,11 @@ 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);
@@ -82,8 +87,52 @@ add_task(function () {
   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) {