Bug 594644 - Return from Private Browsing Mode, Panorama forgets tab groups, until Panorama is manually re-launched r=dietrich a=blocking

browser/base/content/tabview/groupitems.js

@@ -555,7 +555,7 @@ GroupItem.prototype = Utils.extend(new Item(), new Subscribable(), {
   // Function: close
   // Closes the groupItem, removing (but not closing) all of its children.
   close: function GroupItem_close() {
-    this.removeAll();
+    this.removeAll({dontClose: true});
     GroupItems.unregister(this);
 
     if (this.hidden) {
@@ -811,9 +811,13 @@ GroupItem.prototype = Utils.extend(new Item(), new Subscribable(), {
   //
   //   a - The item to add. Can be an <Item>, a DOM element or an iQ object.
   //       The latter two must refer to the container of an <Item>.
-  //   dropPos - An object with left and top properties referring to the location dropped at.  Optional.
+  //   dropPos - An object with left and top properties referring to the 
-  //   options - An object with optional settings for this call. Currently this includes dontArrange
+  //             location dropped at.  Optional.
-  //       and immediately
+  //   options - An optional object with settings for this call. See below.
+  //
+  // Possible options:
+  //   dontArrange - Don't rearrange the children for the new item
+  //   immediately - Don't animate
   add: function GroupItem_add(a, dropPos, options) {
     try {
       var item;
@@ -931,8 +935,12 @@ GroupItem.prototype = Utils.extend(new Item(), new Subscribable(), {
   //
   //   a - The item to remove. Can be an <Item>, a DOM element or an iQ object.
   //       The latter two must refer to the container of an <Item>.
-  //   options - An object with optional settings for this call. Currently this includes
+  //   options - An optional object with settings for this call. See below.
-  //             dontArrange and immediately
+  //
+  // Possible options: 
+  //   dontArrange - don't rearrange the remaining items
+  //   dontClose - don't close the group even if it normally would
+  //   immediately - don't animate
   remove: function GroupItem_remove(a, options) {
     try {
       var $el;
@@ -988,11 +996,16 @@ GroupItem.prototype = Utils.extend(new Item(), new Subscribable(), {
   // ----------
   // Function: removeAll
   // Removes all of the groupItem's children.
-  removeAll: function GroupItem_removeAll() {
+  // The optional "options" param is passed to each remove call. 
-    var self = this;
+  removeAll: function GroupItem_removeAll(options) {
-    var toRemove = this._children.concat();
+    let self = this;
+    let newOptions = {dontArrange: true};
+    if (options)
+      Utils.extend(newOptions, options);
+      
+    let toRemove = this._children.concat();
     toRemove.forEach(function(child) {
-      self.remove(child, {dontArrange: true});
+      self.remove(child, newOptions);
     });
   },
   
@@ -1727,14 +1740,14 @@ let GroupItems = {
 
       if (groupItemsData) {
         if (groupItemsData.nextID)
-          this.nextID = groupItemsData.nextID;
+          this.nextID = Math.max(this.nextID, groupItemsData.nextID);
         if (groupItemsData.activeGroupId)
           activeGroupId = groupItemsData.activeGroupId;
       }
 
       if (groupItemData) {
         for (var id in groupItemData) {
-          var groupItem = groupItemData[id];
+          let groupItem = groupItemData[id];
           if (this.groupItemStorageSanity(groupItem)) {
             var options = {
               dontPush: true,
@@ -1759,6 +1772,24 @@ let GroupItems = {
     }
   },
 
+  // ----------
+  // Function: load
+  // Loads the storage data for groups. 
+  // Returns true if there was global group data.
+  load: function GroupItems_load() {
+    var toClose = this.groupItems.concat();
+    toClose.forEach(function(groupItem) {
+      groupItem.close();
+    });
+
+    let groupItemsData = Storage.readGroupItemsData(gWindow);
+    let groupItemData = Storage.readGroupItemData(gWindow);
+    this.reconstitute(groupItemsData, groupItemData);
+    this.killNewTabGroup(); // temporary?
+    
+    return (groupItemsData && !Utils.isEmptyObject(groupItemsData));
+  },
+
   // ----------
   // Function: groupItemStorageSanity
   // Given persistent storage data for a groupItem, returns true if it appears to not be damaged.

browser/base/content/tabview/ui.js

@@ -172,24 +172,21 @@ let UI = {
       // ___ add tab action handlers
       this._addTabActionHandlers();
 
-      // ___ Storage
+      // ___ groups
-      GroupItems.pauseArrange();
       GroupItems.init();
-
+      GroupItems.pauseArrange();
-      let firstTime = true;
+      let hasGroupItemsData = GroupItems.load();
-      if (gPrefBranch.prefHasUserValue("experienced_first_run"))
-        firstTime = !gPrefBranch.getBoolPref("experienced_first_run");
-      let groupItemsData = Storage.readGroupItemsData(gWindow);
-      let groupItemData = Storage.readGroupItemData(gWindow);
-      GroupItems.reconstitute(groupItemsData, groupItemData);
-      GroupItems.killNewTabGroup(); // temporary?
 
       // ___ tabs
       TabItems.init();
       TabItems.pausePainting();
 
       // if first time in Panorama or no group data:
-      if (firstTime || !groupItemsData || Utils.isEmptyObject(groupItemsData))
+      let firstTime = true;
+      if (gPrefBranch.prefHasUserValue("experienced_first_run"))
+        firstTime = !gPrefBranch.getBoolPref("experienced_first_run");
+
+      if (firstTime || !hasGroupItemsData)
         this.reset(firstTime);
 
       // ___ resizing
@@ -525,6 +522,10 @@ let UI = {
       if (aTopic != "sessionstore-browser-state-restored")
         return;
         
+      let hasGroupItemsData = GroupItems.load();
+      if (!hasGroupItemsData)
+        self.reset(false);
+        
       // if we're transitioning into/out of private browsing, update appropriately
       if (self._privateBrowsing.transitionStage == 1)
         self._privateBrowsing.transitionStage = 2;

browser/base/content/test/tabview/browser_tabview_privatebrowsing.js

@@ -36,8 +36,11 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+let contentWindow = null;
 let normalURLs = []; 
 let pbTabURL = "about:privatebrowsing";
+let groupTitles = [];
+let normalIteration = 0;
 
 let pb = Cc["@mozilla.org/privatebrowsing;1"].
          getService(Ci.nsIPrivateBrowsingService);
@@ -46,23 +49,9 @@ let pb = Cc["@mozilla.org/privatebrowsing;1"].
 function test() {
   waitForExplicitFinish();
 
-  // Establish initial state
+  // Go into Tab View
-  is(gBrowser.tabs.length, 1, "we start with 1 tab");
+  window.addEventListener("tabviewshown", onTabViewLoadedAndShown, false);
-  
+  TabView.toggle();
-  // Create a second tab
-  gBrowser.addTab("about:robots");
-  is(gBrowser.tabs.length, 2, "we now have 2 tabs");
-  
-  afterAllTabsLoaded(function() {
-    // Get normal tab urls
-    for (let a = 0; a < gBrowser.tabs.length; a++) {
-      normalURLs.push(gBrowser.tabs[a].linkedBrowser.currentURI.spec);
-    }
-  
-    // Go into Tab View
-    window.addEventListener("tabviewshown", onTabViewLoadedAndShown, false);
-    TabView.toggle();
-  });
 }
 
 // -----------
@@ -70,20 +59,64 @@ function onTabViewLoadedAndShown() {
   window.removeEventListener("tabviewshown", onTabViewLoadedAndShown, false);
   ok(TabView.isVisible(), "Tab View is visible");
 
-  // go into private browsing and make sure Tab View becomes hidden
+  // Establish initial state
-  togglePBAndThen(function() {
+  contentWindow = document.getElementById("tab-view").contentWindow;  
-    ok(!TabView.isVisible(), "Tab View is no longer visible");
+  verifyCleanState("start");
-    verifyPB();
+  
-    
+  // register a clean up for private browsing just in case
-    // exit private browsing and make sure Tab View is shown again
+  registerCleanupFunction(function() {
+    pb.privateBrowsingEnabled = false;
+  });
+  
+  // create a group
+  let box = new contentWindow.Rect(20, 20, 180, 180);
+  let groupItem = new contentWindow.GroupItem([], {bounds: box, title: "test1"});
+  let id = groupItem.id; 
+  is(contentWindow.GroupItems.groupItems.length, 2, "we now have two groups");
+  registerCleanupFunction(function() {
+    contentWindow.GroupItems.groupItem(id).close();
+  });
+  
+  // make it the active group so new tabs will be added to it
+  contentWindow.GroupItems.setActiveGroupItem(groupItem);
+  
+  // collect the group titles
+  let count = contentWindow.GroupItems.groupItems.length;
+  for (let a = 0; a < count; a++) {
+    let gi = contentWindow.GroupItems.groupItems[a];
+    groupTitles[a] = gi.getTitle();
+  }
+  
+  // Create a second tab
+  gBrowser.addTab("about:robots");
+  is(gBrowser.tabs.length, 2, "we now have 2 tabs");
+  registerCleanupFunction(function() {
+    gBrowser.removeTab(gBrowser.tabs[1]);
+  });
+
+  afterAllTabsLoaded(function() {
+    // Get normal tab urls
+    for (let a = 0; a < gBrowser.tabs.length; a++)
+      normalURLs.push(gBrowser.tabs[a].linkedBrowser.currentURI.spec);
+
+    // verify that we're all set up for our test
+    verifyNormal();
+
+    // go into private browsing and make sure Tab View becomes hidden
     togglePBAndThen(function() {
-      ok(TabView.isVisible(), "Tab View is visible again");
+      ok(!TabView.isVisible(), "Tab View is no longer visible");
-      verifyNormal();
+      verifyPB();
       
-      // exit Tab View
+      // exit private browsing and make sure Tab View is shown again
-      window.addEventListener("tabviewhidden", onTabViewHidden, false);
+      togglePBAndThen(function() {
-      TabView.toggle();
+        ok(TabView.isVisible(), "Tab View is visible again");
-    });  
+        verifyNormal();
+        
+        // exit Tab View
+        window.addEventListener("tabviewhidden", onTabViewHidden, false);
+        TabView.toggle();
+      });  
+    });
   });
 }
 
@@ -101,22 +134,31 @@ function onTabViewHidden() {
     togglePBAndThen(function() {
       verifyNormal();
       
-      // clean up
+      // end game
-      gBrowser.removeTab(gBrowser.tabs[1]);
-      
-      is(gBrowser.tabs.length, 1, "we finish with one tab");
-      ok(!pb.privateBrowsingEnabled, "we finish with private browsing off");
       ok(!TabView.isVisible(), "we finish with Tab View not visible");
-    
+      registerCleanupFunction(verifyCleanState); // verify after all cleanups
       finish();
     });
   });
 }
 
+// ----------
+function verifyCleanState(mode) {
+  let prefix = "we " + (mode || "finish") + " with ";
+  is(gBrowser.tabs.length, 1, prefix + "one tab");
+  is(contentWindow.GroupItems.groupItems.length, 1, prefix + "1 group");
+  ok(gBrowser.tabs[0].tabItem.parent == contentWindow.GroupItems.groupItems[0], 
+      "the tab is in the group");
+  ok(!pb.privateBrowsingEnabled, prefix + "private browsing off");
+}
+
 // ----------
 function verifyPB() {
   ok(pb.privateBrowsingEnabled == true, "private browsing is on");
   is(gBrowser.tabs.length, 1, "we have 1 tab in private browsing");
+  is(contentWindow.GroupItems.groupItems.length, 1, "we have 1 group in private browsing");
+  ok(gBrowser.tabs[0].tabItem.parent == contentWindow.GroupItems.groupItems[0], 
+      "the tab is in the group");
 
   let browser = gBrowser.tabs[0].linkedBrowser;
   is(browser.currentURI.spec, pbTabURL, "correct URL for private browsing");
@@ -124,14 +166,26 @@ function verifyPB() {
 
 // ----------
 function verifyNormal() {
-  ok(pb.privateBrowsingEnabled == false, "private browsing is off");
+  let prefix = "verify normal " + normalIteration + ": "; 
+  normalIteration++;
+  
+  ok(pb.privateBrowsingEnabled == false, prefix + "private browsing is off");
+  
+  let tabCount = gBrowser.tabs.length;
+  let groupCount = contentWindow.GroupItems.groupItems.length;
+  is(tabCount, 2, prefix + "we have 2 tabs");
+  is(groupCount, 2, prefix + "we have 2 groups");
+  ok(tabCount == groupCount, prefix + "same number of tabs as groups"); 
+  for (let a = 0; a < tabCount; a++) {
+    let tab = gBrowser.tabs[a];
+    is(tab.linkedBrowser.currentURI.spec, normalURLs[a],
+        prefix + "correct URL");
 
-  let count = gBrowser.tabs.length;
+    let groupItem = contentWindow.GroupItems.groupItems[a];
-  is(count, 2, "we have 2 tabs in normal mode");
+    is(groupItem.getTitle(), groupTitles[a], prefix + "correct group title");
-
+    
-  for (let a = 0; a < count; a++) {
+    ok(tab.tabItem.parent == groupItem,
-    let browser = gBrowser.tabs[a].linkedBrowser;
+        prefix + "tab " + a + " is in group " + a);
-    is(browser.currentURI.spec, normalURLs[a], "correct URL for normal mode");
   }
 }