Bug 586693 - Do we still need to marshal browser events? [r=dolske a=blocking2.0]
authorRaymond Lee <raymond@raysquare.com>
Mon, 16 Aug 2010 12:46:22 +0800
changeset 50675 a15f73b8d43181b393586d97ec74dbd156830175
parent 50674 639e221ffc5567dff7c89ac3223e97e4b9b50de6
child 50676 7ad51f9290e83ae0c67903d779a76becf37154ea
push idunknown
push userunknown
push dateunknown
reviewersdolske, blocking2.0
bugs586693, 587029, 586552
milestone2.0b4pre
Bug 586693 - Do we still need to marshal browser events? [r=dolske a=blocking2.0] * Removed some timeouts and fixed some broken user interactions that fixes various other bugs and test failures. Bug 587029 - Tab Candy : closing last tab of a group leads to an isolated tab Bug 586552 - GroupItem.newTab feedback should be immediate * Init TabItems before handling firstrun tab grouping * Removed _stopZoomPreparation related code since we are not using it anymore. * Fixed the issue related to using move to other group feature. The moved tab is still visible in the tab bar after moving it to other group.
browser/base/content/tabview/groupitems.js
browser/base/content/tabview/tabitems.js
browser/base/content/tabview/ui.js
--- a/browser/base/content/tabview/groupitems.js
+++ b/browser/base/content/tabview/groupitems.js
@@ -651,21 +651,16 @@ window.GroupItem.prototype = Utils.exten
         if (item.tab == gBrowser.selectedTab)
           GroupItems.setActiveGroupItem(this);
       }
 
       if (!options.dontArrange) {
         this.arrange();
       }
       UI.setReorderTabsOnHide(this);
-
-      if (this._nextNewTabCallback) {
-        this._nextNewTabCallback.apply(this, [item])
-        this._nextNewTabCallback = null;
-      }
     } catch(e) {
       Utils.log('GroupItem.add error', e);
     }
   },
 
   // ----------
   // Function: remove
   // Removes an item from the groupItem.
@@ -1184,69 +1179,51 @@ window.GroupItem.prototype = Utils.exten
 
   // ----------
   // Function: newTab
   // Creates a new tab within this groupItem.
   newTab: function(url) {
     GroupItems.setActiveGroupItem(this);
     let newTab = gBrowser.loadOneTab(url || "about:blank", {inBackground: true});
 
-    var self = this;
-    var doNextTab = function(tab) {
-      var groupItem = GroupItems.getActiveGroupItem();
+    // TabItems will have handled the new tab and added the tabItem property
+    let newItem = newTab.tabItem;
 
-      iQ(tab.container).css({opacity: 0});
-      var $anim = iQ("<div>")
-        .addClass('newTabAnimatee')
-        .css({
-          top: tab.bounds.top+5,
-          left: tab.bounds.left+5,
-          width: tab.bounds.width-10,
-          height: tab.bounds.height-10,
-          zIndex: 999,
-          opacity: 0
-        })
-        .appendTo("body")
-        .animate({
-          opacity: 1.0
-        }, {
-          duration: 500,
-          complete: function() {
-            $anim.animate({
-              top: 0,
-              left: 0,
-              width: window.innerWidth,
-              height: window.innerHeight
-            }, {
-              duration: 270,
-              complete: function() {
-                iQ(tab.container).css({opacity: 1});
-                newTab.tabItem.zoomIn(!url);
-                $anim.remove();
-                // We need a timeout here so that there is a chance for the
-                // new tab to get made! Otherwise it won't appear in the list
-                // of the groupItem's tab.
-                // TODO: This is probably a terrible hack that sets up a race
-                // condition. We need a better solution.
-                // Bug 586551
-                setTimeout(function() {
-                  self._sendToSubscribers("tabAdded", { groupItemId: self.id });
-                }, 1);
-              }
-            });
-          }
-        });
-    }
-
-    // TODO: Because this happens as a callback, there is
-    // sometimes a long delay before the animation occurs.
-    // We need to fix this--immediate response to a users
-    // actions is necessary for a good user experience.
-    // Bug 586552
-    self.onNextNewTab(doNextTab);
+    var self = this;
+    iQ(newItem.container).css({opacity: 0});
+    let $anim = iQ("<div>")
+      .addClass("newTabAnimatee")
+      .css({
+        top: newItem.bounds.top + 5,
+        left: newItem.bounds.left + 5,
+        width: newItem.bounds.width - 10,
+        height: newItem.bounds.height - 10,
+        zIndex: 999,
+        opacity: 0
+      })
+      .appendTo("body")
+      .animate({opacity: 1}, {
+        duration: 500,
+        complete: function() {
+          $anim.animate({
+            top: 0,
+            left: 0,
+            width: window.innerWidth,
+            height: window.innerHeight
+          }, {
+            duration: 270,
+            complete: function() {
+              iQ(newItem.container).css({opacity: 1});
+              newItem.zoomIn(!url);
+              $anim.remove();
+              self._sendToSubscribers("tabAdded", {groupItemId: self.id});
+            }
+          });
+        }
+      });
   },
 
   // ----------
   // Function: reorderTabItemsBasedOnTabOrder
   // Reorders the tabs in a groupItem based on the arrangment of the tabs
   // shown in the tab bar. It does it by sorting the children
   // of the groupItem by the positions of their respective tabs in the
   // tab bar.
@@ -1315,29 +1292,16 @@ window.GroupItem.prototype = Utils.exten
     return this._children[index];
   },
 
   // ----------
   // Function: getChildren
   // Returns all children.
   getChildren: function() {
     return this._children;
-  },
-
-  // ---------
-  // Function: onNextNewTab
-  // Sets up a one-time handler that gets called the next time a
-  // tab is added to the groupItem.
-  //
-  // Parameters:
-  //  callback - the one-time callback that is fired when the next
-  //             time a tab is added to a groupItem; it gets passed the
-  //             new tab
-  onNextNewTab: function(callback) {
-    this._nextNewTabCallback = callback;
   }
 });
 
 // ##########
 // Class: GroupItems
 // Singelton for managing all <GroupItem>s.
 window.GroupItems = {
   groupItems: [],
@@ -1752,16 +1716,17 @@ window.GroupItems = {
       let listLength = list.length;
 
       if (listLength > 1) {
         let index = list.indexOf(tab);
         if (index == 0 || (index + 1) < listLength)
           gBrowser.selectTabAtIndex(index + 1);
         else
           gBrowser.selectTabAtIndex(index - 1);
+        shouldUpdateTabBar = true;
       } else {
         shouldShowTabView = true;
       }
     } else
       shouldUpdateTabBar = true
 
     // remove tab item from a groupItem
     if (tab.tabItem.parent)
--- a/browser/base/content/tabview/tabitems.js
+++ b/browser/base/content/tabview/tabitems.js
@@ -679,38 +679,32 @@ window.TabItems = {
     Utils.assert(window.AllTabs, "AllTabs must be initialized first");
     var self = this;
 
     // When a tab is opened, create the TabItem
     this._eventListeners["open"] = function(tab) {
       if (tab.ownerDocument.defaultView != gWindow)
         return;
 
-      setTimeout(function() { // Marshal event from chrome thread to DOM thread
-        self.link(tab);
-      }, 1);
+      self.link(tab);
     }
     // When a tab's content is loaded, show the canvas and hide the cached data
     // if necessary.
     this._eventListeners["attrModified"] = function(tab) {
       if (tab.ownerDocument.defaultView != gWindow)
         return;
 
-      setTimeout(function() { // Marshal event from chrome thread to DOM thread
-        self.update(tab);
-      }, 1);
+      self.update(tab);
     }
     // When a tab is closed, unlink.
     this._eventListeners["close"] = function(tab) {
       if (tab.ownerDocument.defaultView != gWindow)
         return;
 
-      setTimeout(function() { // Marshal event from chrome thread to DOM thread
-        self.unlink(tab);
-      }, 1);
+      self.unlink(tab);
     }
     for (let name in this._eventListeners) {
       AllTabs.register(name, this._eventListeners[name]);
     }
 
     // For each tab, create the link.
     AllTabs.tabs.forEach(function(tab) {
       if (tab.ownerDocument.defaultView != gWindow)
--- a/browser/base/content/tabview/ui.js
+++ b/browser/base/content/tabview/ui.js
@@ -61,20 +61,16 @@ var UIManager = {
   // Variable: _closedLastVisibleTab
   // If true, the last visible tab has just been closed in the tab strip.
   _closedLastVisibleTab : false,
 
   // Variable: _closedSelectedTabInTabView
   // If true, a select tab has just been closed in TabView.
   _closedSelectedTabInTabView : false,
 
-  // Variable: _stopZoomPreparation
-  // If true, prevent the next zoom preparation.
-  _stopZoomPreparation : false,
-
   // Variable: _reorderTabItemsOnShow
   // Keeps track of the <GroupItem>s which their tab items' tabs have been moved
   // and re-orders the tab items when switching to TabView.
   _reorderTabItemsOnShow : [],
 
   // Variable: _reorderTabsOnHide
   // Keeps track of the <GroupItem>s which their tab items have been moved in
   // TabView UI and re-orders the tabs when switcing back to main browser.
@@ -153,16 +149,20 @@ var UIManager = {
       GroupItems.init();
 
       var groupItemsData = Storage.readGroupItemsData(gWindow);
       var firstTime = !groupItemsData || Utils.isEmptyObject(groupItemsData);
       var groupItemData = Storage.readGroupItemData(gWindow);
       GroupItems.reconstitute(groupItemsData, groupItemData);
       GroupItems.killNewTabGroup(); // temporary?
 
+      // ___ tabs
+      TabItems.init();
+      TabItems.pausePainting();
+
       if (firstTime) {
         var padding = 10;
         var infoWidth = 350;
         var infoHeight = 350;
         var pageBounds = Items.getPageBounds();
         pageBounds.inset(padding, padding);
 
         // ___ make a fresh groupItem
@@ -198,20 +198,16 @@ var UIManager = {
 
         box.left = box.right + padding;
         box.width = infoWidth;
         box.height = infoHeight;
         var infoItem = new InfoItem(box);
         infoItem.html(html);
       }
 
-      // ___ tabs
-      TabItems.init();
-      TabItems.pausePainting();
-
       // ___ resizing
       if (this._pageBounds)
         this._resize(true);
       else
         this._pageBounds = Items.getPageBounds();
 
       iQ(window).resize(function() {
         self._resize();
@@ -435,37 +431,27 @@ var UIManager = {
               (groupItem == null && gBrowser.visibleTabs.length == 1)) {
             // for the tab focus event to pick up.
             self._closedLastVisibleTab = true;
             // remove the zoom prep.
             if (tab && tab.tabItem)
               tab.tabItem.setZoomPrep(false);
             self.showTabView();
           }
-          // ToDo: When running unit tests, everything happens so quick so
-          // new tabs might be added after a tab is closing. Therefore, this
-          // hack is used. We should look for a better solution.
-          setTimeout(function() { // Marshal event from chrome thread to DOM thread
-            if ((groupItem && groupItem._children.length > 0) ||
-              (groupItem == null && gBrowser.visibleTabs.length > 0))
-              self.hideTabView();
-          }, 1);
         }
       }
     });
 
     AllTabs.register("move", function(tab) {
       if (tab.ownerDocument.defaultView != gWindow)
         return;
 
-      setTimeout(function() { // Marshal event from chrome thread to DOM thread
-        var activeGroupItem = GroupItems.getActiveGroupItem();
-        if (activeGroupItem)
-          self.setReorderTabItemsOnShow(activeGroupItem);
-      }, 1);
+      let activeGroupItem = GroupItems.getActiveGroupItem();
+      if (activeGroupItem)
+        self.setReorderTabItemsOnShow(activeGroupItem);
     });
 
     AllTabs.register("select", function(tab) {
       if (tab.ownerDocument.defaultView != gWindow)
         return;
 
       self.tabOnFocus(tab);
     });
@@ -492,64 +478,50 @@ var UIManager = {
     // selected tab, show chrome.
     if (this._isTabViewVisible())
       this.hideTabView();
 
     // reset these vars, just in case.
     this._closedLastVisibleTab = false;
     this._closedSelectedTabInTabView = false;
 
-    setTimeout(function() { // Marshal event from chrome thread to DOM thread
-      // this value is true when TabView is open at browser startup.
-      if (self._stopZoomPreparation) {
-        self._stopZoomPreparation = false;
-        if (focusTab && focusTab.tabItem)
-          self.setActiveTab(focusTab.tabItem);
-        return;
+    // have things have changed while we were in timeout?
+    if (focusTab != self._currentTab)
+      return;
+
+    let newItem = null;
+    if (focusTab && focusTab.tabItem) {
+      newItem = focusTab.tabItem;
+      if (newItem.parent)
+        GroupItems.setActiveGroupItem(newItem.parent);
+      else {
+        GroupItems.setActiveGroupItem(null);
+        GroupItems.setActiveOrphanTab(newItem);
       }
-
-      if (focusTab != self._currentTab) {
-        // things have changed while we were in timeout
-        return;
-      }
-
-      var visibleTabCount = gBrowser.visibleTabs.length;
+      GroupItems.updateTabBar();
+    }
 
-      var newItem = null;
-      if (focusTab && focusTab.tabItem) {
-        newItem = focusTab.tabItem;
-        if (newItem.parent)
-          GroupItems.setActiveGroupItem(newItem.parent);
-        else {
-          GroupItems.setActiveGroupItem(null);
-          GroupItems.setActiveOrphanTab(newItem);
-        }
-        GroupItems.updateTabBar();
-      }
+    // ___ prepare for when we return to TabView
+    let oldItem = null;
+    if (currentTab && currentTab.tabItem)
+      oldItem = currentTab.tabItem;
+
+    if (newItem != oldItem) {
+      if (oldItem)
+        oldItem.setZoomPrep(false);
 
-      // ___ prepare for when we return to TabView
-      var oldItem = null;
-      if (currentTab && currentTab.tabItem)
-        oldItem = currentTab.tabItem;
-
-      if (newItem != oldItem) {
-        if (oldItem)
-          oldItem.setZoomPrep(false);
-
-        // if the last visible tab is removed, don't set zoom prep because
-        // we shoud be in the TabView interface.
-        if (visibleTabCount > 0 && newItem && !self._isTabViewVisible())
-          newItem.setZoomPrep(true);
-      } else {
-        // the tab is already focused so the new and old items are the
-        // same.
-        if (oldItem)
-          oldItem.setZoomPrep(!self._isTabViewVisible());
-      }
-    }, 1);
+      // if the last visible tab is removed, don't set zoom prep because
+      // we should be in the TabView interface.
+      let visibleTabCount = gBrowser.visibleTabs.length;
+      if (visibleTabCount > 0 && newItem && !self._isTabViewVisible())
+        newItem.setZoomPrep(true);
+    }
+    // the tab is already focused so the new and old items are the same.
+    else if (oldItem)
+      oldItem.setZoomPrep(!self._isTabViewVisible());
   },
 
   // ----------
   // Function: setReorderTabsOnHide
   // Sets the groupItem which the tab items' tabs should be re-ordered when
   // switching to the main browser UI.
   // Parameters:
   //   groupItem - the groupItem which would be used for re-ordering tabs.