Bug 674452 - Always restore pinned tabs (regardless of restore_on_demand) [r=dietrich]

browser/components/sessionstore/src/nsSessionStore.js

@@ -230,7 +230,7 @@ SessionStoreService.prototype = {
   _restoreLastWindow: false,
 
   // tabs to restore in order
-  _tabsToRestore: { visible: [], hidden: [] },
+  _tabsToRestore: { priority: [], visible: [], hidden: [] },
   _tabsRestoringCount: 0,
 
   // overrides MAX_CONCURRENT_TAB_RESTORES and _restoreHiddenTabs when true
@@ -437,6 +437,7 @@ SessionStoreService.prototype = {
     this.saveState(true);
 
     // clear out _tabsToRestore in case it's still holding refs
+    this._tabsToRestore.priority = null;
     this._tabsToRestore.visible = null;
     this._tabsToRestore.hidden = null;
 
@@ -2936,7 +2937,9 @@ SessionStoreService.prototype = {
     }
     else {
       // Put the tab into the right bucket
-      if (tabData.hidden)
+      if (tabData.pinned)
+        this._tabsToRestore.priority.push(tab);
+      else if (tabData.hidden)
         this._tabsToRestore.hidden.push(tab);
       else
         this._tabsToRestore.visible.push(tab);
@@ -3065,13 +3068,16 @@ SessionStoreService.prototype = {
       return;
 
     // If it's not possible to restore anything, then just bail out.
-    if (this._restoreOnDemand ||
+    if ((!this._tabsToRestore.priority.length && this._restoreOnDemand) ||
         this._tabsRestoringCount >= MAX_CONCURRENT_TAB_RESTORES)
       return;
 
-    // Look in visible, then hidden
+    // Look in priority, then visible, then hidden
     let nextTabArray;
-    if (this._tabsToRestore.visible.length) {
+    if (this._tabsToRestore.priority.length) {
+      nextTabArray = this._tabsToRestore.priority
+    }
+    else if (this._tabsToRestore.visible.length) {
       nextTabArray = this._tabsToRestore.visible;
     }
     else if (this._restoreHiddenTabs && this._tabsToRestore.hidden.length) {
@@ -4129,7 +4135,7 @@ SessionStoreService.prototype = {
    * Reset state to prepare for a new session state to be restored.
    */
   _resetRestoringState: function sss__initRestoringState() {
-    this._tabsToRestore = { visible: [], hidden: [] };
+    this._tabsToRestore = { priority: [], visible: [], hidden: [] };
     this._tabsRestoringCount = 0;
   },
 
@@ -4174,13 +4180,19 @@ SessionStoreService.prototype = {
   },
 
   /**
-   * Remove the tab from this._tabsToRestore[visible/hidden]
+   * Remove the tab from this._tabsToRestore[priority/visible/hidden]
    *
    * @param aTab
    */
   _removeTabFromTabsToRestore: function sss__removeTabFromTabsToRestore(aTab) {
-    let arr = this._tabsToRestore[aTab.hidden ? "hidden" : "visible"];
+    // We'll always check priority first since we don't have an indicator if
+    // a tab will be there or not.
+    let arr = this._tabsToRestore.priority;
     let index = arr.indexOf(aTab);
+    if (index == -1) {
+      arr = this._tabsToRestore[aTab.hidden ? "hidden" : "visible"];
+      index = arr.indexOf(aTab);
+    }
     if (index > -1)
       arr.splice(index, 1);
   },

browser/components/sessionstore/test/browser/browser_586068-cascaded_restore.js

@@ -56,7 +56,7 @@ let tests = [test_cascade, test_select, test_multiWindowState,
              test_setWindowStateNoOverwrite, test_setWindowStateOverwrite,
              test_setBrowserStateInterrupted, test_reload,
              /* test_reloadReload, */ test_reloadCascadeSetup,
-             /* test_reloadCascade */];
+             /* test_reloadCascade, */ test_apptabs_only];
 function runNextTest() {
   // Reset the pref
   try {
@@ -720,6 +720,63 @@ function _test_reloadAfter(aTestName, aState, aCallback) {
 }
 
 
+// This test ensures that app tabs are restored regardless of restore_on_demand
+function test_apptabs_only() {
+  // Set the pref to true so we know exactly how many tabs should be restoring at
+  // any given time. This guarantees that a finishing load won't start another.
+  Services.prefs.setBoolPref("browser.sessionstore.restore_on_demand", true);
+
+  // We have our own progress listener for this test, which we'll attach before our state is set
+  let progressListener = {
+    onStateChange: function (aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) {
+      if (aBrowser.__SS_restoreState == TAB_STATE_RESTORING &&
+          aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
+          aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK &&
+          aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW)
+        test_apptabs_only_progressCallback(aBrowser);
+    }
+  }
+
+  let state = { windows: [{ tabs: [
+    { entries: [{ url: "http://example.org/#1" }], extData: { "uniq": r() }, pinned: true },
+    { entries: [{ url: "http://example.org/#2" }], extData: { "uniq": r() }, pinned: true },
+    { entries: [{ url: "http://example.org/#3" }], extData: { "uniq": r() }, pinned: true },
+    { entries: [{ url: "http://example.org/#4" }], extData: { "uniq": r() } },
+    { entries: [{ url: "http://example.org/#5" }], extData: { "uniq": r() } },
+    { entries: [{ url: "http://example.org/#6" }], extData: { "uniq": r() } },
+    { entries: [{ url: "http://example.org/#7" }], extData: { "uniq": r() } },
+  ], selected: 5 }] };
+
+  let loadCount = 0;
+  function test_apptabs_only_progressCallback(aBrowser) {
+    loadCount++;
+
+    // We'll make sure that the loads we get come from pinned tabs or the
+    // the selected tab.
+
+    // get the tab
+    let tab;
+    for (let i = 0; i < window.gBrowser.tabs.length; i++) {
+      if (!tab && window.gBrowser.tabs[i].linkedBrowser == aBrowser)
+        tab = window.gBrowser.tabs[i];
+    }
+
+    ok(tab.pinned || gBrowser.selectedTab == tab,
+       "test_apptabs_only: load came from pinned or selected tab");
+
+    // We should get 4 loads: 3 app tabs + 1 normal selected tab
+    if (loadCount < 4)
+      return;
+
+    window.gBrowser.removeTabsProgressListener(progressListener);
+    runNextTest();
+  }
+
+  window.gBrowser.addTabsProgressListener(progressListener);
+  ss.setBrowserState(JSON.stringify(state));
+}
+
+
 function countTabs() {
   let needsRestore = 0,
       isRestoring = 0,