Bug 1009628 - Part 2: Asynchronous tab switching for tabbrowser that waits on MozAfterRemotePaint to fire from a remote browser. r=dao,Enn.
☠☠ backed out by eb6854a81836 ☠ ☠
authorMike Conley <mconley@mozilla.com>
Sat, 02 Aug 2014 08:22:06 -0400
changeset 219653 3fb0e4bb67f52ce2ee62fa19788728e8cceceda0
parent 219652 249413f56629aa546da09374ab0710c53ff1f558
child 219654 0366dfc363406e449fc8b4a4c124c0766b4cfcd6
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao, Enn
bugs1009628
milestone34.0a1
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 1009628 - Part 2: Asynchronous tab switching for tabbrowser that waits on MozAfterRemotePaint to fire from a remote browser. r=dao,Enn.
browser/base/content/tabbrowser.css
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser_tabopen_reflows.js
layout/xul/nsDeckFrame.cpp
layout/xul/nsDeckFrame.h
--- a/browser/base/content/tabbrowser.css
+++ b/browser/base/content/tabbrowser.css
@@ -1,16 +1,20 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 .tabbrowser-tabbox {
   -moz-binding: url("chrome://browser/content/tabbrowser.xml#tabbrowser-tabbox");
 }
 
+.tabbrowser-tabpanels {
+  -moz-binding: url("chrome://browser/content/tabbrowser.xml#tabbrowser-tabpanels");
+}
+
 .tabbrowser-arrowscrollbox {
   -moz-binding: url("chrome://browser/content/tabbrowser.xml#tabbrowser-arrowscrollbox");
 }
 
 .tab-close-button {
   -moz-binding: url("chrome://browser/content/tabbrowser.xml#tabbrowser-close-tab-button");
 }
 
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1003,27 +1003,30 @@
 
             if (this._lastRelatedTab) {
               if (!this._lastRelatedTab.selected)
                 this._lastRelatedTab.owner = null;
               this._lastRelatedTab = null;
             }
 
             var oldBrowser = this.mCurrentBrowser;
-            oldBrowser.setAttribute("type", "content-targetable");
-            oldBrowser.docShellIsActive = false;
+
+            if (!gMultiProcessBrowser) {
+              oldBrowser.setAttribute("type", "content-targetable");
+              oldBrowser.docShellIsActive = false;
+              newBrowser.setAttribute("type", "content-primary");
+              newBrowser.docShellIsActive =
+                (window.windowState != window.STATE_MINIMIZED);
+            }
 
             var updateBlockedPopups = false;
             if ((oldBrowser.blockedPopups && !newBrowser.blockedPopups) ||
                 (!oldBrowser.blockedPopups && newBrowser.blockedPopups))
               updateBlockedPopups = true;
 
-            newBrowser.setAttribute("type", "content-primary");
-            newBrowser.docShellIsActive =
-              (window.windowState != window.STATE_MINIMIZED);
             this.mCurrentBrowser = newBrowser;
             this.mCurrentTab = this.tabContainer.selectedItem;
             this.showTab(this.mCurrentTab);
 
             var forwardButtonContainer = document.getElementById("urlbar-wrapper");
             if (forwardButtonContainer) {
               forwardButtonContainer.setAttribute("switchingtabs", "true");
               window.addEventListener("MozAfterPaint", function removeSwitchingtabsAttr() {
@@ -1129,91 +1132,110 @@
                 //   3. User returns to tab, presses "Leave page"
                 let promptBox = this.getTabModalPromptBox(oldBrowser);
                 let prompts = promptBox.listPrompts();
                 // NB: This code assumes that the beforeunload prompt
                 //     is the top-most prompt on the tab.
                 promptBox.removePrompt(prompts[prompts.length - 1]);
               }
 
-              // Adjust focus
-              oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
-              if (this.isFindBarInitialized(oldTab)) {
-                let findBar = this.getFindBar(oldTab);
-                oldTab._findBarFocused = (!findBar.hidden &&
-                  findBar._findField.getAttribute("focused") == "true");
-              }
-              do {
-                // When focus is in the tab bar, retain it there.
-                if (document.activeElement == oldTab) {
-                  // We need to explicitly focus the new tab, because
-                  // tabbox.xml does this only in some cases.
-                  this.mCurrentTab.focus();
-                  break;
-                }
-
-                // If there's a tabmodal prompt showing, focus it.
-                if (newBrowser.hasAttribute("tabmodalPromptShowing")) {
-                  let XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
-                  let prompts = newBrowser.parentNode.getElementsByTagNameNS(XUL_NS, "tabmodalprompt");
-                  let prompt = prompts[prompts.length - 1];
-                  prompt.Dialog.setDefaultFocus();
-                  break;
-                }
-
-                // Focus the location bar if it was previously focused for that tab.
-                // In full screen mode, only bother making the location bar visible
-                // if the tab is a blank one.
-                if (newBrowser._urlbarFocused && gURLBar) {
-
-                  // Explicitly close the popup if the URL bar retains focus
-                  gURLBar.closePopup();
-
-                  if (!window.fullScreen) {
-                    gURLBar.focus();
-                    break;
-                  } else if (isTabEmpty(this.mCurrentTab)) {
-                    focusAndSelectUrlBar();
-                    break;
-                  }
-                }
-
-                // Focus the find bar if it was previously focused for that tab.
-                if (gFindBarInitialized && !gFindBar.hidden &&
-                    this.selectedTab._findBarFocused) {
-                  gFindBar._findField.focus();
-                  break;
-                }
-
-                // Otherwise, focus the content area.
-                let fm = Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager);
-                let focusFlags = fm.FLAG_NOSCROLL;
-
-                if (!gMultiProcessBrowser) {
-                  let newFocusedElement = fm.getFocusedElementForWindow(window.content, true, {});
-
-                  // for anchors, use FLAG_SHOWRING so that it is clear what link was
-                  // last clicked when switching back to that tab
-                  if (newFocusedElement &&
-                      (newFocusedElement instanceof HTMLAnchorElement ||
-                       newFocusedElement.getAttributeNS("http://www.w3.org/1999/xlink", "type") == "simple"))
-                    focusFlags |= fm.FLAG_SHOWRING;
-                }
-                fm.setFocus(newBrowser, focusFlags);
-              } while (false);
+              if (!gMultiProcessBrowser)
+                this._adjustFocusAfterTabSwitch(this.mCurrentTab, oldTab);
             }
 
             this.tabContainer._setPositionalAttributes();
 
             if (!aForceUpdate)
               TelemetryStopwatch.finish("FX_TAB_SWITCH_UPDATE_MS");
           ]]>
         </body>
       </method>
 
+      <method name="_adjustFocusAfterTabSwitch">
+        <parameter name="newTab"/>
+        <parameter name="oldTab"/>
+        <body><![CDATA[
+        let newBrowser = this.getBrowserForTab(newTab);
+        let oldBrowser = this.getBrowserForTab(oldTab);
+
+        if (oldBrowser) {
+          oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
+          if (this.isFindBarInitialized(oldTab)) {
+            let findBar = this.getFindBar(oldTab);
+            oldTab._findBarFocused = (!findBar.hidden &&
+              findBar._findField.getAttribute("focused") == "true");
+          }
+        }
+
+        // When focus is in the tab bar, retain it there.
+        if (document.activeElement == oldTab) {
+          // We need to explicitly focus the new tab, because
+          // tabbox.xml does this only in some cases.
+          this.mCurrentTab.focus();
+          return;
+        }
+
+        // If there's a tabmodal prompt showing, focus it.
+        if (newBrowser.hasAttribute("tabmodalPromptShowing")) {
+          let XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
+          let prompts = newBrowser.parentNode.getElementsByTagNameNS(XUL_NS, "tabmodalprompt");
+          let prompt = prompts[prompts.length - 1];
+          prompt.Dialog.setDefaultFocus();
+          return;
+        }
+
+        // Focus the location bar if it was previously focused for that tab.
+        // In full screen mode, only bother making the location bar visible
+        // if the tab is a blank one.
+        if (newBrowser._urlbarFocused && gURLBar) {
+
+          // Explicitly close the popup if the URL bar retains focus
+          gURLBar.closePopup();
+
+          if (!window.fullScreen) {
+            gURLBar.focus();
+            return;
+          }
+
+          if (isTabEmpty(this.mCurrentTab)) {
+            focusAndSelectUrlBar();
+            return;
+          }
+        }
+
+        // Focus the find bar if it was previously focused for that tab.
+        if (gFindBarInitialized && !gFindBar.hidden &&
+            this.selectedTab._findBarFocused) {
+          gFindBar._findField.focus();
+          return;
+        }
+
+        // Otherwise, focus the content area. If we're not using remote tabs, we
+        // can focus the content area right away, since tab switching is synchronous.
+        // If we're using remote tabs, we have to wait until after we've finalized
+        // switching the tabs.
+
+        let fm = Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager);
+        let focusFlags = fm.FLAG_NOSCROLL;
+
+        if (!gMultiProcessBrowser) {
+          let newFocusedElement = fm.getFocusedElementForWindow(window.content, true, {});
+
+          // for anchors, use FLAG_SHOWRING so that it is clear what link was
+          // last clicked when switching back to that tab
+          if (newFocusedElement &&
+              (newFocusedElement instanceof HTMLAnchorElement ||
+               newFocusedElement.getAttributeNS("http://www.w3.org/1999/xlink", "type") == "simple"))
+            focusFlags |= fm.FLAG_SHOWRING;
+        }
+
+        fm.setFocus(newBrowser, focusFlags);
+        ]]></body>
+      </method>
+
       <method name="_tabAttrModified">
         <parameter name="aTab"/>
         <body><![CDATA[
           if (aTab.closing)
             return;
 
           // This event should be dispatched when any of these attributes change:
           // label, crop, busy, image, selected
@@ -2114,25 +2136,17 @@
             // Release the browser in case something is erroneously holding a
             // reference to the tab after its removal.
             aTab.linkedBrowser = null;
 
             // As the browser is removed, the removal of a dependent document can
             // cause the whole window to close. So at this point, it's possible
             // that the binding is destructed.
             if (this.mTabBox) {
-              let selectedPanel = this.mTabBox.selectedPanel;
-
               this.mPanelContainer.removeChild(panel);
-
-              // Under the hood, a selectedIndex attribute controls which panel
-              // is displayed. Removing a panel A which precedes the selected
-              // panel B makes selectedIndex point to the panel next to B. We
-              // need to explicitly preserve B as the selected panel.
-              this.mTabBox.selectedPanel = selectedPanel;
             }
 
             if (aCloseWindow)
               this._windowIsClosing = closeWindow(true, window.warnAboutClosingWindow);
           ]]>
         </body>
       </method>
 
@@ -3136,16 +3150,20 @@
           let remote = window.QueryInterface(Ci.nsIInterfaceRequestor)
             .getInterface(Ci.nsIWebNavigation)
             .QueryInterface(Ci.nsILoadContext)
             .useRemoteTabs;
           if (remote) {
             messageManager.addMessageListener("DOMTitleChanged", this);
             messageManager.addMessageListener("DOMWindowClose", this);
             messageManager.addMessageListener("contextmenu", this);
+
+            // If this window has remote tabs, switch to our tabpanels fork
+            // which does asynchronous tab switching.
+            this.mPanelContainer.classList.add("tabbrowser-tabpanels");
           }
           messageManager.addMessageListener("DOMWebNotificationClicked", this);
         ]]>
       </constructor>
 
       <method name="_generateUniquePanelID">
         <body><![CDATA[
           if (!this._uniquePanelIDCounter) {
@@ -3207,16 +3225,133 @@
           this.tabContainer.visible = aShow;
         </body>
       </method>
       <method name="getStripVisibility">
         <body>
           return this.tabContainer.visible;
         </body>
       </method>
+
+      <method name="_prepareForTabSwitch">
+        <parameter name="toTab"/>
+        <parameter name="fromTab"/>
+        <body><![CDATA[
+          const kTabSwitchTimeout = 300;
+          let toBrowser = this.getBrowserForTab(toTab);
+          let fromBrowser = fromTab ? this.getBrowserForTab(fromTab)
+                                    : null;
+
+          // We only want to wait for the MozAfterRemotePaint event if
+          // the tab we're switching to is a remote tab, and if the tab
+          // we're switching to isn't the one we've already got. The latter
+          // case can occur when closing tabs before the currently selected
+          // one.
+          let shouldWait = toBrowser.getAttribute("remote") == "true" &&
+                           toBrowser != fromBrowser;
+
+          let switchPromise;
+
+          if (shouldWait) {
+            let timeoutId;
+            let panels = this.mPanelContainer;
+
+            // Both the timeout and MozAfterPaint promises use this same
+            // logic to determine whether they should carry out the tab
+            // switch, or reject it outright.
+            let attemptTabSwitch = (aResolve, aReject) => {
+              if (this.selectedBrowser == toBrowser) {
+                aResolve();
+              } else {
+                // We switched away or closed the browser before we timed
+                // out. We reject, which will cancel the tab switch.
+                aReject();
+              }
+            };
+
+            let timeoutPromise = new Promise((aResolve, aReject) => {
+              timeoutId = setTimeout(() => {
+                attemptTabSwitch(aResolve, aReject);
+              }, kTabSwitchTimeout);
+            });
+
+            let paintPromise = new Promise((aResolve, aReject) => {
+              toBrowser.addEventListener("MozAfterRemotePaint", function onRemotePaint() {
+                toBrowser.removeEventListener("MozAfterRemotePaint", onRemotePaint);
+                clearTimeout(timeoutId);
+                attemptTabSwitch(aResolve, aReject);
+              });
+              toBrowser.QueryInterface(Ci.nsIFrameLoaderOwner)
+                       .frameLoader
+                       .sendRequestNotifyAfterRemotePaint();
+               // We need to activate the docShell on the tab we're switching
+               // to - otherwise, we won't initiate a remote paint request and
+               // therefore we won't get the MozAfterRemotePaint event that we're
+               // waiting for.
+               // Note that this happens, as we require, even if the timeout in the
+               // timeoutPromise triggers before the paintPromise even runs.
+               toBrowser.docShellIsActive = true;
+            });
+
+            switchPromise = Promise.race([paintPromise, timeoutPromise]);
+          } else {
+            // Activate the docShell on the tab we're switching to.
+            toBrowser.docShellIsActive = true;
+
+            // No need to wait - just resolve immediately to do the switch ASAP.
+            switchPromise = Promise.resolve();
+          }
+
+          return switchPromise;
+        ]]></body>
+      </method>
+
+      <method name="_deactivateContent">
+        <parameter name="tab"/>
+        <body><![CDATA[
+          // It's unlikely, yet possible, that while we were waiting
+          // to deactivate this tab, that something closed it and wiped
+          // out the browser. For example, during a tab switch, while waiting
+          // for the MozAfterRemotePaint event to fire, something closes the
+          // original tab that the user had selected. If that's the case, then
+          // there's nothing to deactivate.
+          let browser = this.getBrowserForTab(tab);
+          if (browser && this.selectedBrowser != browser) {
+            browser.docShellIsActive = false;
+          }
+        ]]></body>
+      </method>
+
+      <method name="_finalizeTabSwitch">
+        <parameter name="toTab"/>
+        <parameter name="fromTab"/>
+        <body><![CDATA[
+          this._adjustFocusAfterTabSwitch(toTab, fromTab);
+          this._deactivateContent(fromTab);
+
+          let toBrowser = this.getBrowserForTab(toTab);
+          toBrowser.setAttribute("type", "content-primary");
+
+          let fromBrowser = this.getBrowserForTab(fromTab);
+          // It's possible that the tab we're switching from closed
+          // before we were able to finalize, in which case, fromBrowser
+          // doesn't exist.
+          if (fromBrowser) {
+            fromBrowser.setAttribute("type", "content-targetable");
+          }
+        ]]></body>
+      </method>
+
+      <method name="_cancelTabSwitch">
+        <parameter name="toTab"/>
+        <body><![CDATA[
+          this._deactivateContent(toTab);
+        ]]></body>
+      </method>
+
       <property name="mContextTab" readonly="true"
                 onget="return TabContextMenu.contextTab;"/>
       <property name="mPrefs" readonly="true"
                 onget="return Services.prefs;"/>
       <property name="mTabContainer" readonly="true"
                 onget="return this.tabContainer;"/>
       <property name="mTabs" readonly="true"
                 onget="return this.tabs;"/>
@@ -5136,9 +5271,58 @@
             this.setAttribute("sizelimit", "true");
             this._calcMouseTargetRect();
           }
         </body>
       </method>
     </implementation>
   </binding>
 
+  <binding id="tabbrowser-tabpanels"
+           extends="chrome://global/content/bindings/tabbox.xml#tabpanels">
+    <implementation>
+      <field name="_selectedIndex">0</field>
+
+      <property name="selectedIndex">
+        <getter>
+        <![CDATA[
+          return this._selectedIndex;
+        ]]>
+        </getter>
+
+        <setter>
+        <![CDATA[
+          if (val < 0 || val >= this.childNodes.length)
+            return val;
+
+          let toTab = this.getRelatedElement(this.childNodes[val]);
+          let fromTab = this._selectedPanel ? this.getRelatedElement(this._selectedPanel)
+                                            : null;
+
+          let switchPromise = gBrowser._prepareForTabSwitch(toTab, fromTab);
+
+          var panel = this._selectedPanel;
+          this._selectedPanel = this.childNodes[val];
+          if (this._selectedPanel != panel) {
+            var event = document.createEvent("Events");
+            event.initEvent("select", true, true);
+            this.dispatchEvent(event);
+          }
+
+          this._selectedIndex = val;
+
+          switchPromise.then(() => {
+            this.setAttribute("selectedIndex", val);
+            gBrowser._finalizeTabSwitch(toTab, fromTab);
+          }, () => {
+            // If the promise rejected, that means we don't want to actually
+            // flip the deck, so we cancel the tab switch.
+            gBrowser._cancelTabSwitch(toTab);
+          });
+
+          return val;
+        ]]>
+        </setter>
+      </property>
+    </implementation>
+  </binding>
+
 </bindings>
--- a/browser/base/content/test/general/browser_tabopen_reflows.js
+++ b/browser/base/content/test/general/browser_tabopen_reflows.js
@@ -9,17 +9,18 @@ XPCOMUtils.defineLazyGetter(this, "docSh
 
 const EXPECTED_REFLOWS = [
   // tabbrowser.adjustTabstrip() call after tabopen animation has finished
   "adjustTabstrip@chrome://browser/content/tabbrowser.xml|" +
     "_handleNewTab@chrome://browser/content/tabbrowser.xml|" +
     "onxbltransitionend@chrome://browser/content/tabbrowser.xml|",
 
   // switching focus in updateCurrentBrowser() causes reflows
-  "updateCurrentBrowser@chrome://browser/content/tabbrowser.xml|" +
+  "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml|" +
+    "updateCurrentBrowser@chrome://browser/content/tabbrowser.xml|" +
     "onselect@chrome://browser/content/browser.xul|",
 
   // switching focus in openLinkIn() causes reflows
   "openLinkIn@chrome://browser/content/utilityOverlay.js|" +
     "openUILinkIn@chrome://browser/content/utilityOverlay.js|" +
     "BrowserOpenTab@chrome://browser/content/browser.js|",
 
   // accessing element.scrollPosition in _fillTrailingGap() flushes layout
--- a/layout/xul/nsDeckFrame.cpp
+++ b/layout/xul/nsDeckFrame.cpp
@@ -20,16 +20,17 @@
 #include "nsHTMLParts.h"
 #include "nsIPresShell.h"
 #include "nsCSSRendering.h"
 #include "nsViewManager.h"
 #include "nsBoxLayoutState.h"
 #include "nsStackLayout.h"
 #include "nsDisplayList.h"
 #include "nsContainerFrame.h"
+#include "nsContentUtils.h"
 
 #ifdef ACCESSIBILITY
 #include "nsAccessibilityService.h"
 #endif
 
 nsIFrame*
 NS_NewDeckFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 {
@@ -150,16 +151,44 @@ nsDeckFrame::BuildDisplayList(nsDisplayL
   // if a tab is hidden all its children are too.
   if (!StyleVisibility()->mVisible)
     return;
     
   nsBoxFrame::BuildDisplayList(aBuilder, aDirtyRect, aLists);
 }
 
 void
+nsDeckFrame::RemoveFrame(ChildListID aListID,
+                         nsIFrame* aOldFrame)
+{
+  nsIFrame* currentFrame = GetSelectedBox();
+  if (currentFrame &&
+      aOldFrame &&
+      currentFrame != aOldFrame) {
+    // If the frame we're removing is at an index that's less
+    // than mIndex, that means we're going to be shifting indexes
+    // by 1.
+    //
+    // We attempt to keep the same child displayed by automatically
+    // updating our internal notion of the current index.
+    int32_t removedIndex = mFrames.IndexOf(aOldFrame);
+    MOZ_ASSERT(removedIndex >= 0,
+               "A deck child was removed that was not in mFrames.");
+    if (removedIndex < mIndex) {
+      mIndex--;
+      // This is going to cause us to handle the index change in IndexedChanged,
+      // but since the new index will match mIndex, it's essentially a noop.
+      nsContentUtils::AddScriptRunner(new nsSetAttrRunnable(
+        mContent, nsGkAtoms::selectedIndex, mIndex));
+    }
+  }
+  nsBoxFrame::RemoveFrame(aListID, aOldFrame);
+}
+
+void
 nsDeckFrame::BuildDisplayListForChildren(nsDisplayListBuilder*   aBuilder,
                                          const nsRect&           aDirtyRect,
                                          const nsDisplayListSet& aLists)
 {
   // only paint the selected box
   nsIFrame* box = GetSelectedBox();
   if (!box)
     return;
--- a/layout/xul/nsDeckFrame.h
+++ b/layout/xul/nsDeckFrame.h
@@ -32,16 +32,19 @@ public:
                                     int32_t         aModType) MOZ_OVERRIDE;
 
   NS_IMETHOD DoLayout(nsBoxLayoutState& aState) MOZ_OVERRIDE;
 
   virtual void BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                                 const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists) MOZ_OVERRIDE;
 
+  virtual void RemoveFrame(ChildListID aListID,
+                           nsIFrame* aOldFrame) MOZ_OVERRIDE;
+
   virtual void BuildDisplayListForChildren(nsDisplayListBuilder*   aBuilder,
                                            const nsRect&           aDirtyRect,
                                            const nsDisplayListSet& aLists) MOZ_OVERRIDE;
                                          
   virtual void Init(nsIContent*       aContent,
                     nsContainerFrame* aParent,
                     nsIFrame*         aPrevInFlow) MOZ_OVERRIDE;