Bug 1306294 - "Restarting browser while Simplify Page mode on, restores 2 new empty tabs". r=mdeboer
☠☠ backed out by 4772a0f4bed2 ☠ ☠
authorMatheus Longaray <mlongaray>
Tue, 22 Nov 2016 06:30:00 +0800
changeset 323927 4b75d4672954a617791487fd13fc878b1b5461e0
parent 323926 17ce37597d5c9699464e10b56838864fb549c6bb
child 323928 8c3caa5cf8a6001113c6007090964a37ab0019bc
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersmdeboer
bugs1306294
milestone53.0a1
Bug 1306294 - "Restarting browser while Simplify Page mode on, restores 2 new empty tabs". r=mdeboer
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/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
@@ -367,16 +367,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");
--- 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");