Bug 594644 - Return from Private Browsing Mode, Panorama forgets tab groups, until Panorama is manually re-launched r=dietrich a=blocking
authorIan Gilman <ian@iangilman.com>
Fri, 08 Oct 2010 15:59:24 -0700
changeset 60032 b06a94065ef0726698fe6c42c706c8ddbe601f84
parent 60031 2d849e2d302eaca2c777bbb13f96109e6f756e03
child 60033 ccfc9d2147038b5422ddce218b03092028aabe0e
push id17837
push userian@iangilman.com
push dateWed, 05 Jan 2011 20:59:13 +0000
treeherdermozilla-central@f453924d5fe1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdietrich, blocking
bugs594644
milestone2.0b9pre
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 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
browser/base/content/tabview/ui.js
browser/base/content/test/tabview/browser_tabview_privatebrowsing.js
--- a/browser/base/content/tabview/groupitems.js
+++ b/browser/base/content/tabview/groupitems.js
@@ -550,17 +550,17 @@ GroupItem.prototype = Utils.extend(new I
       });
     }
   },
 
   // ----------
   // 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) {
       iQ(this.container).remove();
       if (this.$undoContainer) {
         this.$undoContainer.remove();
         this.$undoContainer = null;
        }
@@ -806,19 +806,23 @@ GroupItem.prototype = Utils.extend(new I
 
   // ----------
   // Function: add
   // Adds an item to the groupItem.
   // Parameters:
   //
   //   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.
-  //   options - An object with optional settings for this call. Currently this includes dontArrange
-  //       and immediately
+  //   dropPos - An object with left and top properties referring to the 
+  //             location dropped at.  Optional.
+  //   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;
       var $el;
       if (a.isAnItem) {
         item = a;
         $el = iQ(a.container);
       } else {
@@ -926,18 +930,22 @@ GroupItem.prototype = Utils.extend(new I
 
   // ----------
   // Function: remove
   // Removes an item from the groupItem.
   // Parameters:
   //
   //   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
-  //             dontArrange and immediately
+  //   options - An optional object with settings for this call. See below.
+  //
+  // 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;
       var item;
 
       if (a.isAnItem) {
         item = a;
         $el = iQ(item.container);
@@ -983,21 +991,26 @@ GroupItem.prototype = Utils.extend(new I
     } catch(e) {
       Utils.log(e);
     }
   },
 
   // ----------
   // Function: removeAll
   // Removes all of the groupItem's children.
-  removeAll: function GroupItem_removeAll() {
-    var self = this;
-    var toRemove = this._children.concat();
+  // The optional "options" param is passed to each remove call. 
+  removeAll: function GroupItem_removeAll(options) {
+    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);
     });
   },
   
   // ----------
   // Handles error event for loading app tab's fav icon.
   _onAppTabError : function(event) {
     iQ(".appTabIcon", this.$appTabTray).each(function(icon) {
       let $icon = iQ(icon);
@@ -1722,24 +1735,24 @@ let GroupItems = {
   // Restores to stored state, creating groupItems as needed.
   // If no data, sets up blank slate (including "new tabs" groupItem).
   reconstitute: function GroupItems_reconstitute(groupItemsData, groupItemData) {
     try {
       let activeGroupId;
 
       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,
               immediately: true
             };
 
             new GroupItem([], Utils.extend({}, groupItem, options));
           }
@@ -1755,16 +1768,34 @@ let GroupItems = {
       this._inited = true;
       this._save(); // for nextID
     } catch(e) {
       Utils.log("error in recons: "+e);
     }
   },
 
   // ----------
+  // 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.
   groupItemStorageSanity: function GroupItems_groupItemStorageSanity(groupItemData) {
     // TODO: check everything
     // Bug 586555
     var sane = true;
     if (!Utils.isRect(groupItemData.bounds)) {
       Utils.log('GroupItems.groupItemStorageSanity: bad bounds', groupItemData.bounds);
--- a/browser/base/content/tabview/ui.js
+++ b/browser/base/content/tabview/ui.js
@@ -167,34 +167,31 @@ let UI = {
       }, false);
 
       // ___ setup key handlers
       this._setTabViewFrameKeyHandlers();
 
       // ___ add tab action handlers
       this._addTabActionHandlers();
 
-      // ___ Storage
-      GroupItems.pauseArrange();
+      // ___ groups
       GroupItems.init();
-
-      let firstTime = true;
-      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?
+      GroupItems.pauseArrange();
+      let hasGroupItemsData = GroupItems.load();
 
       // ___ 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
       if (this._pageBounds)
         this._resize(true);
       else
         this._pageBounds = Items.getPageBounds();
 
@@ -520,16 +517,20 @@ let UI = {
   _addTabActionHandlers: function UI__addTabActionHandlers() {
     var self = this;
 
     // session restore
     function srObserver(aSubject, aTopic, aData) {
       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;
       else if (self._privateBrowsing.transitionStage == 3) {
         if (self._privateBrowsing.transitionMode == "exit" &&
             self._privateBrowsing.wasInTabView)
           self.showTabView(false);
 
--- a/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js
+++ b/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js
@@ -31,64 +31,97 @@
  * 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 ***** */
 
+let contentWindow = null;
 let normalURLs = []; 
 let pbTabURL = "about:privatebrowsing";
+let groupTitles = [];
+let normalIteration = 0;
 
 let pb = Cc["@mozilla.org/privatebrowsing;1"].
          getService(Ci.nsIPrivateBrowsingService);
 
 // -----------
 function test() {
   waitForExplicitFinish();
 
-  // Establish initial state
-  is(gBrowser.tabs.length, 1, "we start with 1 tab");
-  
-  // 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();
-  });
+  // Go into Tab View
+  window.addEventListener("tabviewshown", onTabViewLoadedAndShown, false);
+  TabView.toggle();
 }
 
 // -----------
 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
-  togglePBAndThen(function() {
-    ok(!TabView.isVisible(), "Tab View is no longer visible");
-    verifyPB();
-    
-    // exit private browsing and make sure Tab View is shown again
+  // Establish initial state
+  contentWindow = document.getElementById("tab-view").contentWindow;  
+  verifyCleanState("start");
+  
+  // register a clean up for private browsing just in case
+  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");
-      verifyNormal();
+      ok(!TabView.isVisible(), "Tab View is no longer visible");
+      verifyPB();
       
-      // exit Tab View
-      window.addEventListener("tabviewhidden", onTabViewHidden, false);
-      TabView.toggle();
-    });  
+      // exit private browsing and make sure Tab View is shown again
+      togglePBAndThen(function() {
+        ok(TabView.isVisible(), "Tab View is visible again");
+        verifyNormal();
+        
+        // exit Tab View
+        window.addEventListener("tabviewhidden", onTabViewHidden, false);
+        TabView.toggle();
+      });  
+    });
   });
 }
 
 // -----------
 function onTabViewHidden() {
   window.removeEventListener("tabviewhidden", onTabViewHidden, false);
   ok(!TabView.isVisible(), "Tab View is not visible");
   
@@ -96,47 +129,68 @@ function onTabViewHidden() {
   togglePBAndThen(function() {
     ok(!TabView.isVisible(), "Tab View is still not visible");
     verifyPB();
     
     // turn private browsing back off
     togglePBAndThen(function() {
       verifyNormal();
       
-      // clean up
-      gBrowser.removeTab(gBrowser.tabs[1]);
-      
-      is(gBrowser.tabs.length, 1, "we finish with one tab");
-      ok(!pb.privateBrowsingEnabled, "we finish with private browsing off");
+      // end game
       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");
 }
 
 // ----------
 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;
-  is(count, 2, "we have 2 tabs in normal mode");
-
-  for (let a = 0; a < count; a++) {
-    let browser = gBrowser.tabs[a].linkedBrowser;
-    is(browser.currentURI.spec, normalURLs[a], "correct URL for normal mode");
+    let groupItem = contentWindow.GroupItems.groupItems[a];
+    is(groupItem.getTitle(), groupTitles[a], prefix + "correct group title");
+    
+    ok(tab.tabItem.parent == groupItem,
+        prefix + "tab " + a + " is in group " + a);
   }
 }
 
 // ----------
 function togglePBAndThen(callback) {
   function pbObserver(aSubject, aTopic, aData) {
     if (aTopic != "private-browsing-transition-complete")
       return;