Bug 618151 - Overwriting state can lead to unrestored tabs [r=dietrich, a=blocking2.0:betaN+]
authorPaul O’Shannessy <paul@oshannessy.com>
Mon, 27 Dec 2010 16:22:47 -0500
changeset 59697 98b8d44e0096e67229a8aa283480a083565778d3
parent 59696 8531d0ed4546193a9d174f35c337463448088ee2
child 59698 92104acff2b39debc947477377d3982afb7e84a5
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersdietrich, blocking2
bugs618151
milestone2.0b9pre
Bug 618151 - Overwriting state can lead to unrestored tabs [r=dietrich, a=blocking2.0:betaN+]
browser/components/sessionstore/src/nsSessionStore.js
browser/components/sessionstore/test/browser/Makefile.in
browser/components/sessionstore/test/browser/browser_618151.js
--- a/browser/components/sessionstore/src/nsSessionStore.js
+++ b/browser/components/sessionstore/src/nsSessionStore.js
@@ -1002,17 +1002,21 @@ SessionStoreService.prototype = {
    * @param aBrowser
    *        Browser reference
    * @param aEvent
    *        Event obj
    */
   onTabLoad: function sss_onTabLoad(aWindow, aBrowser, aEvent) { 
     // react on "load" and solitary "pageshow" events (the first "pageshow"
     // following "load" is too late for deleting the data caches)
-    if (aEvent.type != "load" && !aEvent.persisted) {
+    // It's possible to get a load event after calling stop on a browser (when
+    // overwriting tabs). We want to return early if the tab hasn't been restored yet.
+    if ((aEvent.type != "load" && !aEvent.persisted) ||
+        (aBrowser.__SS_restoreState &&
+         aBrowser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE)) {
       return;
     }
     
     delete aBrowser.__SS_data;
     this.saveStateDelayed(aWindow);
     
     // attempt to update the current URL we send in a crash report
     this._updateCrashReportURL(aWindow);
--- a/browser/components/sessionstore/test/browser/Makefile.in
+++ b/browser/components/sessionstore/test/browser/Makefile.in
@@ -126,16 +126,17 @@ include $(topsrcdir)/config/rules.mk
 	browser_597315_index.html \
 	browser_597315_a.html \
 	browser_597315_b.html \
 	browser_597315_c.html \
 	browser_597315_c1.html \
 	browser_597315_c2.html \
 	browser_600545.js \
 	browser_607016.js \
+	browser_618151.js \
 	$(NULL)
 
 ifneq ($(OS_ARCH),Darwin)
 _BROWSER_TEST_FILES += \
 	browser_597071.js \
 	$(NULL)
 endif
 
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/test/browser/browser_618151.js
@@ -0,0 +1,113 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is sessionstore test code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ * Paul O’Shannessy <paul@oshannessy.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+Cu.import("resource://gre/modules/Services.jsm");
+const ss = Cc["@mozilla.org/browser/sessionstore;1"].
+           getService(Ci.nsISessionStore);
+
+const stateBackup = ss.getBrowserState();
+const testState = {
+  windows: [{
+    tabs: [
+      { entries: [{ url: "about:blank" }] },
+      { entries: [{ url: "about:robots" }] }
+    ]
+  }]
+};
+
+
+function test() {
+  /** Test for Bug 618151 - Overwriting state can lead to unrestored tabs **/
+  waitForExplicitFinish();
+  runNextTest();
+}
+
+// Just a subset of tests from bug 615394 that causes a timeout.
+let tests = [test_setup, test_hang];
+function runNextTest() {
+  // set an empty state & run the next test, or finish
+  if (tests.length) {
+    // Enumerate windows and close everything but our primary window. We can't
+    // use waitForFocus() because apparently it's buggy. See bug 599253.
+    var windowsEnum = Services.wm.getEnumerator("navigator:browser");
+    while (windowsEnum.hasMoreElements()) {
+      var currentWindow = windowsEnum.getNext();
+      if (currentWindow != window) {
+        currentWindow.close();
+      }
+    }
+
+    let currentTest = tests.shift();
+    info("running " + currentTest.name);
+    waitForBrowserState(testState, currentTest);
+  }
+  else {
+    ss.setBrowserState(stateBackup);
+    executeSoon(finish);
+  }
+}
+
+// helper, works only for single window
+function waitForBrowserState(aState, aSetStateCallback) {
+  let tabsRestored = 0;
+  gBrowser.tabContainer.addEventListener("SSTabRestored", function() {
+    if (++tabsRestored == aState.windows[0].tabs.length) {
+      gBrowser.tabContainer.removeEventListener("SSTabRestored", arguments.callee, true);
+      executeSoon(aSetStateCallback);
+    }
+  }, true);
+  ss.setBrowserState(JSON.stringify(aState));
+}
+
+
+function test_setup() {
+  function onSSTabRestored(aEvent) {
+    gBrowser.tabContainer.removeEventListener("SSTabRestored", onSSTabRestored, false);
+    runNextTest();
+  }
+
+  gBrowser.tabContainer.addEventListener("SSTabRestored", onSSTabRestored, false);
+  ss.setTabState(gBrowser.tabs[1], JSON.stringify({
+    entries: [{ url: "http://example.org" }],
+    extData: { foo: "bar" } }));
+}
+
+function test_hang() {
+  ok(true, "test didn't time out");
+  runNextTest();
+}
+