Bug 1306294 - Filters out not worth-saving tabs from session state. r=mikedeboer
authorMatheus Longaray <mlongaray@hp.com>
Tue, 06 Dec 2016 03:30:00 -0500
changeset 370941 cbe1b40baf579ffbc7c254a4f38cdb732a0ffd28
parent 370940 6cec2e289658c0fd22675bab8b3658460e73a373
child 370942 05f393387c3759c97d2c727d5fa7178985817665
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1306294
milestone53.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 1306294 - Filters out not worth-saving tabs from session state. r=mikedeboer
browser/base/content/test/urlbar/browser_urlbarAboutHomeLoading.js
browser/components/sessionstore/SessionSaver.jsm
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/test/browser_625016.js
browser/components/sessionstore/test/browser_819510_perwindowpb.js
browser/components/sessionstore/test/browser_newtab_userTypedValue.js
browser/components/sessionstore/test/browser_scrollPositions.js
browser/components/sessionstore/test/browser_scrollPositionsReaderMode.js
--- a/browser/base/content/test/urlbar/browser_urlbarAboutHomeLoading.js
+++ b/browser/base/content/test/urlbar/browser_urlbarAboutHomeLoading.js
@@ -1,10 +1,11 @@
 "use strict";
 
+const {SessionSaver} = Cu.import("resource:///modules/sessionstore/SessionSaver.jsm", {});
 const {TabStateFlusher} = Cu.import("resource:///modules/sessionstore/TabStateFlusher.jsm", {});
 
 /**
  * Test what happens if loading a URL that should clear the
  * location bar after a parent process URL.
  */
 add_task(function* clearURLBarAfterParentProcessURL() {
   let tab = yield new Promise(resolve => {
@@ -79,16 +80,18 @@ add_task(function* dontTemporarilyShowAb
   yield windowOpenedPromise;
   let promiseTabSwitch = BrowserTestUtils.switchTab(win.gBrowser, () => {});
   win.BrowserOpenTab();
   yield promiseTabSwitch;
   yield TabStateFlusher.flush(win.gBrowser.selectedBrowser);
   yield BrowserTestUtils.closeWindow(win);
   ok(SessionStore.getClosedWindowCount(), "Should have a closed window");
 
+  yield SessionSaver.run();
+
   windowOpenedPromise = BrowserTestUtils.waitForNewWindow();
   win = SessionStore.undoCloseWindow(0);
   yield windowOpenedPromise;
   let wpl = {
     onLocationChange() {
       is(win.gURLBar.value, "", "URL bar value should stay empty.");
     },
   };
--- a/browser/components/sessionstore/SessionSaver.jsm
+++ b/browser/components/sessionstore/SessionSaver.jsm
@@ -181,16 +181,19 @@ var SessionSaverInternal = {
       this.updateLastSaveTime();
       return Promise.resolve();
     }
 
     stopWatchStart("COLLECT_DATA_MS", "COLLECT_DATA_LONGEST_OP_MS");
     let state = SessionStore.getCurrentState(forceUpdateAllWindows);
     PrivacyFilter.filterPrivateWindowsAndTabs(state);
 
+    // Make sure we only write worth saving tabs to disk.
+    SessionStore.keepOnlyWorthSavingTabs(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;
     }
 
     if (AppConstants.platform != "macosx") {
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -370,16 +370,44 @@ this.SessionStore = {
       return false;
     }
     let number = Number.parseFloat(version[1]);
     if (Number.isNaN(number)) {
       return false;
     }
     return number <= FORMAT_VERSION;
   },
+
+  /**
+   * Filters out not worth-saving tabs from a given browser state object.
+   *
+   * @param aState (object)
+   *        The browser state for which we remove worth-saving tabs.
+   *        The given object will be modified.
+   */
+  keepOnlyWorthSavingTabs: function (aState) {
+    for (let i = aState.windows.length - 1; i >= 0; i--) {
+      let win = aState.windows[i];
+      for (let j = win.tabs.length - 1; j >= 0; j--) {
+        let tab = win.tabs[j];
+        if (!SessionStoreInternal._shouldSaveTabState(tab)) {
+          win.tabs.splice(j, 1);
+          if (win.selected > j) {
+            win.selected--;
+          }
+        }
+      }
+      if (!win.tabs.length) {
+        aState.windows.splice(i, 1);
+        if (aState.selectedWindow > i) {
+          aState.selectedWindow--;
+        }
+      }
+    }
+  },
 };
 
 // Freeze the SessionStore object. We don't want anyone to modify it.
 Object.freeze(SessionStore);
 
 var SessionStoreInternal = {
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsIDOMEventListener,
--- a/browser/components/sessionstore/test/browser_625016.js
+++ b/browser/components/sessionstore/test/browser_625016.js
@@ -30,18 +30,18 @@ add_task(function* new_window() {
 
     // Double check that we have no closed windows
     is(ss.getClosedWindowCount(), 0, "no closed windows on first save");
 
     yield BrowserTestUtils.closeWindow(newWin);
     newWin = null;
 
     let state = JSON.parse((yield promiseRecoveryFileContents()));
-    is(state.windows.length, 2,
-      "observe1: 2 windows in data written to disk");
+    is(state.windows.length, 1,
+      "observe1: 1 window in data written to disk");
     is(state._closedWindows.length, 0,
       "observe1: no closed windows in data written to disk");
 
     // The API still treats the closed window as closed, so ensure that window is there
     is(ss.getClosedWindowCount(), 1,
       "observe1: 1 closed window according to API");
   } finally {
     if (newWin) {
@@ -52,31 +52,36 @@ add_task(function* new_window() {
 });
 
 // We'll open a tab, which should trigger another state save which would wipe
 // the _shouldRestore attribute from the closed window
 add_task(function* new_tab() {
   let newTab;
   try {
     newTab = gBrowser.addTab("about:mozilla");
+    yield promiseBrowserLoaded(newTab.linkedBrowser);
+    yield TabStateFlusher.flush(newTab.linkedBrowser);
 
     let state = JSON.parse((yield promiseRecoveryFileContents()));
     is(state.windows.length, 1,
       "observe2: 1 window in data being written to disk");
     is(state._closedWindows.length, 1,
       "observe2: 1 closed window in data being written to disk");
 
     // The API still treats the closed window as closed, so ensure that window is there
     is(ss.getClosedWindowCount(), 1,
       "observe2: 1 closed window according to API");
   } finally {
-    gBrowser.removeTab(newTab);
+    if (newTab) {
+      gBrowser.removeTab(newTab);
+    }
   }
 });
 
 
 add_task(function* done() {
   // The API still represents the closed window as closed, so we can clear it
   // with the API, but just to make sure...
 //  is(ss.getClosedWindowCount(), 1, "1 closed window according to API");
+  yield promiseAllButPrimaryWindowClosed();
   forgetClosedWindows();
   Services.prefs.clearUserPref("browser.sessionstore.interval");
 });
--- a/browser/components/sessionstore/test/browser_819510_perwindowpb.js
+++ b/browser/components/sessionstore/test/browser_819510_perwindowpb.js
@@ -3,87 +3,87 @@
 
 // Test opening default mochitest-normal-private-normal-private windows
 // (saving the state with last window being private)
 
 requestLongerTimeout(2);
 
 add_task(function* test_1() {
   let win = yield promiseNewWindowLoaded();
-  win.gBrowser.addTab("http://www.example.com/1");
+  yield promiseTabLoad(win, "http://www.example.com/#1");
 
   win = yield promiseNewWindowLoaded({private: true});
-  win.gBrowser.addTab("http://www.example.com/2");
+  yield promiseTabLoad(win, "http://www.example.com/#2");
 
   win = yield promiseNewWindowLoaded();
-  win.gBrowser.addTab("http://www.example.com/3");
+  yield promiseTabLoad(win, "http://www.example.com/#3");
 
   win = yield promiseNewWindowLoaded({private: true});
-  win.gBrowser.addTab("http://www.example.com/4");
+  yield promiseTabLoad(win, "http://www.example.com/#4");
 
   let curState = JSON.parse(ss.getBrowserState());
   is(curState.windows.length, 5, "Browser has opened 5 windows");
   is(curState.windows[2].isPrivate, true, "Window is private");
   is(curState.windows[4].isPrivate, true, "Last window is private");
   is(curState.selectedWindow, 5, "Last window opened is the one selected");
 
   let state = JSON.parse(yield promiseRecoveryFileContents());
 
-  is(state.windows.length, 3,
-     "sessionstore state: 3 windows in data being written to disk");
-  is(state.selectedWindow, 3,
+  is(state.windows.length, 2,
+     "sessionstore state: 2 windows in data being written to disk");
+  is(state.selectedWindow, 2,
      "Selected window is updated to match one of the saved windows");
   ok(state.windows.every(win => !win.isPrivate),
     "Saved windows are not private");
   is(state._closedWindows.length, 0,
      "sessionstore state: no closed windows in data being written to disk");
 
   // Cleanup.
   yield promiseAllButPrimaryWindowClosed();
   forgetClosedWindows();
 });
 
 // Test opening default mochitest window + 2 private windows
 add_task(function* test_2() {
   let win = yield promiseNewWindowLoaded({private: true});
-  win.gBrowser.addTab("http://www.example.com/1");
+  yield promiseTabLoad(win, "http://www.example.com/#1");
 
   win = yield promiseNewWindowLoaded({private: true});
-  win.gBrowser.addTab("http://www.example.com/2");
+  yield promiseTabLoad(win, "http://www.example.com/#2");
 
   let curState = JSON.parse(ss.getBrowserState());
   is(curState.windows.length, 3, "Browser has opened 3 windows");
   is(curState.windows[1].isPrivate, true, "Window 1 is private");
   is(curState.windows[2].isPrivate, true, "Window 2 is private");
   is(curState.selectedWindow, 3, "Last window opened is the one selected");
 
   let state = JSON.parse(yield promiseRecoveryFileContents());
 
-  is(state.windows.length, 1,
-     "sessionstore state: 1 windows in data being written to disk");
-  is(state.selectedWindow, 1,
-     "Selected window is updated to match one of the saved windows");
+  is(state.windows.length, 0,
+     "sessionstore state: no window in data being written to disk");
+  is(state.selectedWindow, 0,
+     "Selected window updated to 0 given there are no saved windows");
   is(state._closedWindows.length, 0,
      "sessionstore state: no closed windows in data being written to disk");
 
   // Cleanup.
   yield promiseAllButPrimaryWindowClosed();
   forgetClosedWindows();
 });
 
 // Test opening default-normal-private-normal windows and closing a normal window
 add_task(function* test_3() {
   let normalWindow = yield promiseNewWindowLoaded();
-  yield promiseTabLoad(normalWindow, "http://www.example.com/");
+  yield promiseTabLoad(normalWindow, "http://www.example.com/#1");
 
   let win = yield promiseNewWindowLoaded({private: true});
-  yield promiseTabLoad(win, "http://www.example.com/");
+  yield promiseTabLoad(win, "http://www.example.com/#2");
 
   win = yield promiseNewWindowLoaded();
-  yield promiseTabLoad(win, "http://www.example.com/");
+  yield promiseTabLoad(win, "http://www.example.com/#3");
 
   let curState = JSON.parse(ss.getBrowserState());
   is(curState.windows.length, 4, "Browser has opened 4 windows");
   is(curState.windows[2].isPrivate, true, "Window 2 is private");
   is(curState.selectedWindow, 4, "Last window opened is the one selected");
 
   yield BrowserTestUtils.closeWindow(normalWindow);
 
@@ -91,30 +91,29 @@ add_task(function* test_3() {
   // the list of restoring windows gets cleared. Otherwise the
   // window we just closed would be marked as not closed.
   let tab = win.gBrowser.tabs[0];
   win.gBrowser.pinTab(tab);
   win.gBrowser.unpinTab(tab);
 
   let state = JSON.parse(yield promiseRecoveryFileContents());
 
-  is(state.windows.length, 2,
-     "sessionstore state: 2 windows in data being written to disk");
-  is(state.selectedWindow, 2,
+  is(state.windows.length, 1,
+     "sessionstore state: 1 window in data being written to disk");
+  is(state.selectedWindow, 1,
      "Selected window is updated to match one of the saved windows");
   ok(state.windows.every(win => !win.isPrivate),
     "Saved windows are not private");
   is(state._closedWindows.length, 1,
      "sessionstore state: 1 closed window in data being written to disk");
   ok(state._closedWindows.every(win => !win.isPrivate),
     "Closed windows are not private");
 
   // Cleanup.
   yield promiseAllButPrimaryWindowClosed();
   forgetClosedWindows();
 });
 
 function* promiseTabLoad(win, url) {
-  let browser = win.gBrowser.selectedBrowser;
-  browser.loadURI(url);
-  yield promiseBrowserLoaded(browser);
-  yield TabStateFlusher.flush(browser);
+  let tab = win.gBrowser.addTab(url);
+  yield promiseBrowserLoaded(tab.linkedBrowser);
+  yield TabStateFlusher.flush(tab.linkedBrowser);
 }
--- a/browser/components/sessionstore/test/browser_newtab_userTypedValue.js
+++ b/browser/components/sessionstore/test/browser_newtab_userTypedValue.js
@@ -20,16 +20,18 @@ add_task(function* () {
   let state = JSON.parse(SessionStore.getTabState(tab));
   ok(!state.userTypedValue, "userTypedValue should be undefined on the tab's state");
   tab = null;
 
   yield BrowserTestUtils.closeWindow(win);
 
   ok(SessionStore.getClosedWindowCount(), "Should have a closed window");
 
+  yield forceSaveState();
+
   win = SessionStore.undoCloseWindow(0);
   yield TestUtils.topicObserved("sessionstore-single-window-restored",
                                 subject => subject == win);
   // Don't wait for load here because it's about:newtab and we may have swapped in
   // a preloaded browser.
   yield TabStateFlusher.flush(win.gBrowser.selectedBrowser);
 
   is(win.gURLBar.value, "", "URL bar should be empty");
@@ -45,16 +47,18 @@ add_task(function* () {
       continue; // We tested about:newtab using BrowserOpenTab() above.
     }
     info("Testing " + url + " - " + new Date());
     yield BrowserTestUtils.openNewForegroundTab(win.gBrowser, url);
     yield BrowserTestUtils.closeWindow(win);
 
     ok(SessionStore.getClosedWindowCount(), "Should have a closed window");
 
+    yield forceSaveState();
+
     win = SessionStore.undoCloseWindow(0);
     yield TestUtils.topicObserved("sessionstore-single-window-restored",
                                   subject => subject == win);
     yield BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
     yield TabStateFlusher.flush(win.gBrowser.selectedBrowser);
 
     is(win.gURLBar.value, "", "URL bar should be empty");
     tab = win.gBrowser.selectedTab;
--- a/browser/components/sessionstore/test/browser_scrollPositions.js
+++ b/browser/components/sessionstore/test/browser_scrollPositions.js
@@ -119,16 +119,18 @@ add_task(function test_scroll_background
 
   // Scroll down a little.
   yield sendMessage(browser, "ss-test:setScrollPosition", {x: SCROLL_X, y: SCROLL_Y});
   yield checkScroll(tab, {scroll: SCROLL_STR}, "scroll is fine");
 
   // Close the window
   yield BrowserTestUtils.closeWindow(newWin);
 
+  yield forceSaveState();
+
   // Now restore the window
   newWin = ss.undoCloseWindow(0);
 
   // Make sure to wait for the window to be restored.
   yield BrowserTestUtils.waitForEvent(newWin, "SSWindowStateReady");
 
   is(newWin.gBrowser.tabs.length, 2, "There should be two tabs");
 
--- a/browser/components/sessionstore/test/browser_scrollPositionsReaderMode.js
+++ b/browser/components/sessionstore/test/browser_scrollPositionsReaderMode.js
@@ -30,16 +30,18 @@ add_task(function test_scroll_background
 
   // Scroll down a little.
   yield sendMessage(browser, "ss-test:setScrollPosition", {x: 0, y: SCROLL_READER_MODE_Y});
   yield checkScroll(tab, {scroll: SCROLL_READER_MODE_STR}, "scroll is fine");
 
   // Close the window
   yield BrowserTestUtils.closeWindow(newWin);
 
+  yield forceSaveState();
+
   // Now restore the window
   newWin = ss.undoCloseWindow(0);
 
   // Make sure to wait for the window to be restored.
   yield BrowserTestUtils.waitForEvent(newWin, "SSWindowStateReady");
 
   is(newWin.gBrowser.tabs.length, 2, "There should be two tabs");