Bug 628270 - Undo close (hidden) tab causes panorama and session restore(?) to break unrecoverably [r=ian, r=zpao, a=blocking2.0:betaN+]
authorTim Taubert <tim.taubert@gmx.de>
Tue, 01 Feb 2011 13:53:04 -0800
changeset 61753 1c4fcf164f1836bf2ce127d26496c4cfd23ff250
parent 61752 3fab3dca98175177b3976f5d7e884d9285225814
child 61754 6de40288cc33484324ad06e2da274abd0597fed8
push id18468
push userposhannessy@mozilla.com
push dateTue, 01 Feb 2011 21:57:50 +0000
treeherdermozilla-central@1c4fcf164f18 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersian, zpao, blocking2.0
bugs628270
milestone2.0b11pre
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 628270 - Undo close (hidden) tab causes panorama and session restore(?) to break unrecoverably [r=ian, r=zpao, a=blocking2.0:betaN+]
browser/base/content/test/tabview/Makefile.in
browser/base/content/test/tabview/browser_tabview_bug628270.js
browser/components/sessionstore/src/nsSessionStore.js
browser/components/sessionstore/test/browser/Makefile.in
browser/components/sessionstore/test/browser/browser_628270.js
--- a/browser/base/content/test/tabview/Makefile.in
+++ b/browser/base/content/test/tabview/Makefile.in
@@ -93,16 +93,17 @@ include $(topsrcdir)/config/rules.mk
                  browser_tabview_bug624727.js \
                  browser_tabview_bug624953.js \
                  browser_tabview_bug625269.js \
                  browser_tabview_bug625424.js \
                  browser_tabview_bug626368.js \
                  browser_tabview_bug627288.js \
                  browser_tabview_bug627736.js \
                  browser_tabview_bug628165.js \
+                 browser_tabview_bug628270.js \
                  browser_tabview_dragdrop.js \
                  browser_tabview_exit_button.js \
                  browser_tabview_expander.js \
                  browser_tabview_group.js \
                  browser_tabview_launch.js \
                  browser_tabview_multiwindow_search.js \
                  browser_tabview_orphaned_tabs.js \
                  browser_tabview_privatebrowsing.js \
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabview/browser_tabview_bug628270.js
@@ -0,0 +1,113 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function test() {
+  let cw;
+  let prefix = 'start';
+
+  let createGroupItem = function () {
+    let bounds = new cw.Rect(20, 20, 200, 200);
+    let groupItem = new cw.GroupItem([], {bounds: bounds, immediately: true});
+
+    cw.GroupItems.setActiveGroupItem(groupItem);
+    gBrowser.loadOneTab('http://mochi.test:8888/', {inBackground: true});
+    gBrowser.loadOneTab('http://mochi.test:8888/', {inBackground: true});
+
+    let groupItemId = groupItem.id;
+    registerCleanupFunction(function() {
+      let groupItem = cw.GroupItems.groupItem(groupItemId);
+      if (groupItem)
+        groupItem.close();
+    });
+  }
+
+  let getGroupItem = function (index) {
+    return cw.GroupItems.groupItems[index];
+  }
+
+  let restoreTab = function (callback) {
+    let tab = undoCloseTab(0);
+
+    if (tab._tabViewTabItem._reconnected) {
+      callback();
+      return;
+    }
+
+    tab._tabViewTabItem.addSubscriber(tab, 'reconnected', function () {
+      tab._tabViewTabItem.removeSubscriber(tab, 'reconnected');
+      afterAllTabsLoaded(callback);
+    });
+  }
+
+  let activateFirstGroupItem = function () {
+    let activeTabItem = getGroupItem(0).getChild(0);
+    cw.GroupItems.updateActiveGroupItemAndTabBar(activeTabItem);
+  }
+
+  let assertTabViewIsHidden = function () {
+    ok(!TabView.isVisible(), prefix + ': tabview is hidden');
+  }
+
+  let assertNumberOfGroups = function (num) {
+    is(cw.GroupItems.groupItems.length, num, prefix + ': there are ' + num + ' groups');
+  }
+
+  let assertNumberOfTabs = function (num) {
+    is(gBrowser.visibleTabs.length, num, prefix + ': there are ' + num + ' tabs');
+  }
+
+  let assertNumberOfPinnedTabs = function (num) {
+    is(gBrowser._numPinnedTabs, num, prefix + ': there are ' + num + ' pinned tabs');
+  }
+
+  let assertNumberOfTabsInGroup = function (groupItem, num) {
+    is(groupItem.getChildren().length, num, prefix + ': there are ' + num + ' tabs in the group');
+  }
+
+  let assertValidPrerequisites = function () {
+    assertNumberOfTabs(1);
+    assertNumberOfGroups(1);
+    assertNumberOfPinnedTabs(0);
+  }
+
+  let finishTest = function () {
+    prefix = 'finish';
+    assertValidPrerequisites();
+    assertTabViewIsHidden();
+    finish();
+  }
+
+  let testRestoreTabFromInactiveGroup = function () {
+    prefix = 'restore';
+    activateFirstGroupItem();
+
+    let groupItem = getGroupItem(1);
+    let tabItem = groupItem.getChild(0);
+
+    gBrowser.removeTab(tabItem.tab);
+    assertNumberOfTabsInGroup(groupItem, 1);
+
+    restoreTab(function () {
+      assertNumberOfTabsInGroup(groupItem, 2);
+
+      activateFirstGroupItem();
+      gBrowser.removeTab(gBrowser.tabs[1]);
+      gBrowser.removeTab(gBrowser.tabs[1]);
+      finishTest();
+    });
+  }
+
+  waitForExplicitFinish();
+  assertTabViewIsHidden();
+  registerCleanupFunction(function () TabView.hide());
+
+  showTabView(function () {
+    hideTabView(function () {
+      cw = TabView.getContentWindow();
+      assertValidPrerequisites();
+
+      createGroupItem();
+      afterAllTabsLoaded(testRestoreTabFromInactiveGroup);
+    });
+  });
+}
--- a/browser/components/sessionstore/src/nsSessionStore.js
+++ b/browser/components/sessionstore/src/nsSessionStore.js
@@ -2467,17 +2467,31 @@ SessionStoreService.prototype = {
             self.restoreHistoryPrecursor(aWindow, aTabs, aTabData, aSelectTab, aIx, aCount + 1);
           }
           aWindow.setTimeout(restoreHistoryFunc, 100, this);
           return;
         }
       }
     }
 
-    if (aTabs.length > 0) {
+    if (!this._isWindowLoaded(aWindow)) {
+      // from now on, the data will come from the actual window
+      delete this._statesToRestore[aWindow.__SS_restoreID];
+      delete aWindow.__SS_restoreID;
+      delete this._windows[aWindow.__SSi]._restoring;
+    }
+
+    if (aTabs.length == 0) {
+      // this is normally done in restoreHistory() but as we're returning early
+      // here we need to take care of it.
+      this._sendWindowStateEvent(aWindow, "Ready");
+      return;
+    }
+
+    if (aTabs.length > 1) {
       // Load hidden tabs last, by pushing them to the end of the list
       let unhiddenTabs = aTabs.length;
       for (let t = 0; t < unhiddenTabs; ) {
         if (aTabData[t].hidden) {
           aTabs = aTabs.concat(aTabs.splice(t, 1));
           aTabData = aTabData.concat(aTabData.splice(t, 1));
           if (aSelectTab > t)
             --aSelectTab;
@@ -2500,23 +2514,23 @@ SessionStoreService.prototype = {
         } else {
           // aSelectTab is rightmost or no more room to scroll right
           firstVisibleTab = unhiddenTabs - maxVisibleTabs;
         }
         aTabs = aTabs.splice(firstVisibleTab, maxVisibleTabs).concat(aTabs);
         aTabData = aTabData.splice(firstVisibleTab, maxVisibleTabs).concat(aTabData);
         aSelectTab -= firstVisibleTab;
       }
-
-      // make sure to restore the selected tab first (if any)
-      if (aSelectTab-- && aTabs[aSelectTab]) {
-        aTabs.unshift(aTabs.splice(aSelectTab, 1)[0]);
-        aTabData.unshift(aTabData.splice(aSelectTab, 1)[0]);
-        tabbrowser.selectedTab = aTabs[0];
-      }
+    }
+
+    // make sure to restore the selected tab first (if any)
+    if (aSelectTab-- && aTabs[aSelectTab]) {
+      aTabs.unshift(aTabs.splice(aSelectTab, 1)[0]);
+      aTabData.unshift(aTabData.splice(aSelectTab, 1)[0]);
+      tabbrowser.selectedTab = aTabs[0];
     }
 
     // Prepare the tabs so that they can be properly restored. We'll pin/unpin
     // and show/hide tabs as necessary. We'll also set the labels, user typed
     // value, and attach a copy of the tab's data in case we close it before
     // it's been restored.
     for (t = 0; t < aTabs.length; t++) {
       let tab = aTabs[t];
@@ -2573,23 +2587,16 @@ SessionStoreService.prototype = {
           tab.crop = "end";
         } else if (activePageData.url != "about:blank") {
           tab.label = activePageData.url;
           tab.crop = "center";
         }
       }
     }
 
-    if (!this._isWindowLoaded(aWindow)) {
-      // from now on, the data will come from the actual window
-      delete this._statesToRestore[aWindow.__SS_restoreID];
-      delete aWindow.__SS_restoreID;
-      delete this._windows[aWindow.__SSi]._restoring;
-    }
-    
     // helper hashes for ensuring unique frame IDs and unique document
     // identifiers.
     var idMap = { used: {} };
     var docIdentMap = {};
     this.restoreHistory(aWindow, aTabs, aTabData, idMap, docIdentMap);
   },
 
   /**
--- a/browser/components/sessionstore/test/browser/Makefile.in
+++ b/browser/components/sessionstore/test/browser/Makefile.in
@@ -136,16 +136,17 @@ include $(topsrcdir)/config/rules.mk
 	browser_600545.js \
 	browser_601955.js \
 	browser_607016.js \
 	browser_615394-SSWindowState_events.js \
 	browser_618151.js \
 	browser_623779.js \
 	browser_624727.js \
 	browser_625257.js \
+	browser_628270.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_628270.js
@@ -0,0 +1,54 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+let ss = Cc["@mozilla.org/browser/sessionstore;1"].
+         getService(Ci.nsISessionStore);
+
+function test() {
+  let assertNumberOfTabs = function (num, msg) {
+    is(gBrowser.tabs.length, num, msg);
+  }
+
+  let assertNumberOfVisibleTabs = function (num, msg) {
+    is(gBrowser.visibleTabs.length, num, msg);
+  }
+
+  let assertNumberOfPinnedTabs = function (num, msg) {
+    is(gBrowser._numPinnedTabs, num, msg);
+  }
+
+  waitForExplicitFinish();
+
+  // check prerequisites
+  assertNumberOfTabs(1, "we start off with one tab");
+
+  // setup
+  let tab = gBrowser.addTab("about:robots");
+
+  whenTabIsLoaded(tab, function () {
+    // hide the newly created tab
+    assertNumberOfVisibleTabs(2, "there are two visible tabs");
+    gBrowser.showOnlyTheseTabs([gBrowser.tabs[0]]);
+    assertNumberOfVisibleTabs(1, "there is one visible tab");
+    ok(tab.hidden, "newly created tab is now hidden");
+
+    // close and restore hidden tab
+    gBrowser.removeTab(tab);
+    tab = ss.undoCloseTab(window, 0);
+
+    // check that everything was restored correctly, clean up and finish
+    whenTabIsLoaded(tab, function () {
+      is(tab.linkedBrowser.currentURI.spec, "about:robots", "restored tab has correct url");
+
+      gBrowser.removeTab(tab);
+      finish();
+    });
+  });
+}
+
+function whenTabIsLoaded(tab, callback) {
+  tab.linkedBrowser.addEventListener("load", function onLoad() {
+    tab.linkedBrowser.removeEventListener("load", onLoad, true);
+    callback();
+  }, true);
+}