Bug 1362866 - Rearrange tab focusing behaviour to avoid extra potential reflows. r=Felipe
authorMike Conley <mconley@mozilla.com>
Tue, 23 May 2017 13:08:11 -0400
changeset 360187 9a89e9597191495acfeaa83608adfe7ff75109a1
parent 360186 8b77d1448314a21d4f884dc2fc86fe6ed49ea1e0
child 360188 51a2c493c9492b16d0ba3f8c7c234a7d36bd3101
push id31871
push userryanvm@gmail.com
push dateTue, 23 May 2017 22:02:07 +0000
treeherdermozilla-central@545ffce30eac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersFelipe
bugs1362866
milestone55.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 1362866 - Rearrange tab focusing behaviour to avoid extra potential reflows. r=Felipe MozReview-Commit-ID: F1S179A1GH6
browser/base/content/tabbrowser.xml
browser/base/content/test/performance/browser_tabopen_reflows.js
browser/base/content/utilityOverlay.js
browser/components/customizableui/test/browser_editcontrols_update.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1241,36 +1241,20 @@
                   if (prompts.length) {
                     // NB: This code assumes that the beforeunload prompt
                     //     is the top-most prompt on the tab.
                     prompts[prompts.length - 1].abortPrompt();
                   }
                 });
               }
 
-              oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
-              if (this.isFindBarInitialized(oldTab)) {
-                let findBar = this.getFindBar(oldTab);
-                oldTab._findBarFocused = (!findBar.hidden &&
-                  findBar._findField.getAttribute("focused") == "true");
+              if (!gMultiProcessBrowser) {
+                this._adjustFocusBeforeTabSwitch(oldTab, this.mCurrentTab);
+                this._adjustFocusAfterTabSwitch(this.mCurrentTab);
               }
-
-              // If 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();
-              } else if (gMultiProcessBrowser && document.activeElement !== newBrowser) {
-                // Clear focus so that _adjustFocusAfterTabSwitch can detect if
-                // some element has been focused and respect that.
-                document.activeElement.blur();
-              }
-
-              if (!gMultiProcessBrowser)
-                this._adjustFocusAfterTabSwitch(this.mCurrentTab);
             }
 
             updateUserContextUIIndicator();
             gIdentityHandler.updateSharingIndicator();
 
             this.tabContainer._setPositionalAttributes();
 
             if (!gMultiProcessBrowser) {
@@ -1284,16 +1268,55 @@
             }
 
             if (!aForceUpdate)
               TelemetryStopwatch.finish("FX_TAB_SWITCH_UPDATE_MS");
           ]]>
         </body>
       </method>
 
+      <method name="_adjustFocusBeforeTabSwitch">
+        <parameter name="oldTab"/>
+        <parameter name="newTab"/>
+        <body><![CDATA[
+          if (this._previewMode) {
+            return;
+          }
+
+          let oldBrowser = oldTab.linkedBrowser;
+          let newBrowser = newTab.linkedBrowser;
+
+          oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
+
+          if (this.isFindBarInitialized(oldTab)) {
+            let findBar = this.getFindBar(oldTab);
+            oldTab._findBarFocused = (!findBar.hidden &&
+              findBar._findField.getAttribute("focused") == "true");
+          }
+
+          // If 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.
+            newTab.focus();
+          } else if (gMultiProcessBrowser && document.activeElement !== newBrowser) {
+
+            let keepFocusOnUrlBar = newBrowser &&
+                                    newBrowser._urlbarFocused &&
+                                    gURLBar &&
+                                    gURLBar.focused;
+            if (!keepFocusOnUrlBar) {
+              // Clear focus so that _adjustFocusAfterTabSwitch can detect if
+              // some element has been focused and respect that.
+              document.activeElement.blur();
+            }
+          }
+        ]]></body>
+      </method>
+
       <method name="_adjustFocusAfterTabSwitch">
         <parameter name="newTab"/>
         <body><![CDATA[
         // Don't steal focus from the tab bar.
         if (document.activeElement == newTab)
           return;
 
         let newBrowser = this.getBrowserForTab(newTab);
@@ -1309,22 +1332,25 @@
 
         // 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();
+          // If the user happened to type into the URL bar for this browser
+          // by the time we got here, focusing will cause the text to be
+          // selected which could cause them to overwrite what they've
+          // already typed in.
+          if (gURLBar.focused && newBrowser.userTypedValue) {
             return;
           }
 
-          if (isTabEmpty(this.mCurrentTab)) {
+          if (!window.fullScreen || isTabEmpty(this.mCurrentTab)) {
             focusAndSelectUrlBar();
             return;
           }
         }
 
         // Focus the find bar if it was previously focused for that tab.
         if (gFindBarInitialized && !gFindBar.hidden &&
             this.selectedTab._findBarFocused) {
@@ -1526,16 +1552,17 @@
             var aNoReferrer;
             var aUserContextId;
             var aSameProcessAsFrameLoader;
             var aOriginPrincipal;
             var aOpener;
             var aIsPrerendered;
             var aCreateLazyBrowser;
             var aNextTabParentId;
+            var aFocusUrlBar;
             if (arguments.length == 2 &&
                 typeof arguments[1] == "object" &&
                 !(arguments[1] instanceof Ci.nsIURI)) {
               let params = arguments[1];
               aTriggeringPrincipal      = params.triggeringPrincipal
               aReferrerURI              = params.referrerURI;
               aReferrerPolicy           = params.referrerPolicy;
               aCharset                  = params.charset;
@@ -1551,16 +1578,17 @@
               aNoReferrer               = params.noReferrer;
               aUserContextId            = params.userContextId;
               aSameProcessAsFrameLoader = params.sameProcessAsFrameLoader;
               aOriginPrincipal          = params.originPrincipal;
               aOpener                   = params.opener;
               aIsPrerendered            = params.isPrerendered;
               aCreateLazyBrowser        = params.createLazyBrowser;
               aNextTabParentId          = params.nextTabParentId;
+              aFocusUrlBar              = params.focusUrlBar;
             }
 
             var bgLoad = (aLoadInBackground != null) ? aLoadInBackground :
                          Services.prefs.getBoolPref("browser.tabs.loadInBackground");
             var owner = bgLoad ? null : this.selectedTab;
 
             var tab = this.addTab(aURI, {
                                   triggeringPrincipal: aTriggeringPrincipal,
@@ -1578,17 +1606,18 @@
                                   createLazyBrowser: aCreateLazyBrowser,
                                   preferredRemoteType: aPreferredRemoteType,
                                   noReferrer: aNoReferrer,
                                   userContextId: aUserContextId,
                                   originPrincipal: aOriginPrincipal,
                                   sameProcessAsFrameLoader: aSameProcessAsFrameLoader,
                                   opener: aOpener,
                                   isPrerendered: aIsPrerendered,
-                                  nextTabParentId: aNextTabParentId });
+                                  nextTabParentId: aNextTabParentId,
+                                  focusUrlBar: aFocusUrlBar });
             if (!bgLoad)
               this.selectedTab = tab;
 
             return tab;
          ]]>
         </body>
       </method>
 
@@ -2283,16 +2312,17 @@
             var aOriginPrincipal;
             var aDisallowInheritPrincipal;
             var aOpener;
             var aIsPrerendered;
             var aCreateLazyBrowser;
             var aSkipBackgroundNotify;
             var aNextTabParentId;
             var aNoInitialLabel;
+            var aFocusUrlBar;
             if (arguments.length == 2 &&
                 typeof arguments[1] == "object" &&
                 !(arguments[1] instanceof Ci.nsIURI)) {
               let params = arguments[1];
               aTriggeringPrincipal      = params.triggeringPrincipal;
               aReferrerURI              = params.referrerURI;
               aReferrerPolicy           = params.referrerPolicy;
               aCharset                  = params.charset;
@@ -2312,16 +2342,17 @@
               aOriginPrincipal          = params.originPrincipal;
               aDisallowInheritPrincipal = params.disallowInheritPrincipal;
               aOpener                   = params.opener;
               aIsPrerendered            = params.isPrerendered;
               aCreateLazyBrowser        = params.createLazyBrowser;
               aSkipBackgroundNotify     = params.skipBackgroundNotify;
               aNextTabParentId          = params.nextTabParentId;
               aNoInitialLabel           = params.noInitialLabel;
+              aFocusUrlBar              = params.focusUrlBar;
             }
 
             // if we're adding tabs, we're past interrupt mode, ditch the owner
             if (this.mCurrentTab.owner)
               this.mCurrentTab.owner = null;
 
             var t = document.createElementNS(NS_XUL, "tab");
 
@@ -2438,16 +2469,21 @@
                                         userContextId: aUserContextId,
                                         sameProcessAsFrameLoader: aSameProcessAsFrameLoader,
                                         opener: aOpener,
                                         isPrerendered: aIsPrerendered,
                                         nextTabParentId: aNextTabParentId });
             }
 
             t.linkedBrowser = b;
+
+            if (aFocusUrlBar) {
+              b._urlbarFocused = true;
+            }
+
             this._tabForBrowser.set(b, t);
             t.permanentKey = b.permanentKey;
             t._browserParams = { uriIsAboutBlank,
                                  remoteType,
                                  usingPreloadedContent };
 
             // If the caller opts in, create a lazy browser.
             if (aCreateLazyBrowser) {
@@ -4030,18 +4066,16 @@
               this.assert(this.lastVisibleTab === this.requestedTab);
               this.assert(this.minimized || this.getTabState(this.requestedTab) == this.STATE_LOADED);
 
               this.destroy();
 
               let toBrowser = this.requestedTab.linkedBrowser;
               toBrowser.setAttribute("primary", "true");
 
-              this.tabbrowser._adjustFocusAfterTabSwitch(this.requestedTab);
-
               let fromBrowser = this.originalTab.linkedBrowser;
               // 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.removeAttribute("primary");
               }
 
@@ -4123,16 +4157,17 @@
                 }
                 this.spinnerTab = showTab;
                 this.tabbrowser.setAttribute("pendingpaint", "true");
                 this.spinnerTab.linkedBrowser.setAttribute("pendingpaint", "true");
               }
 
               // Switch to the tab we've decided to make visible.
               if (this.visibleTab !== showTab) {
+                this.tabbrowser._adjustFocusBeforeTabSwitch(this.visibleTab, showTab);
                 this.visibleTab = showTab;
 
                 this.maybeVisibleTabs.add(showTab);
 
                 let tabs = this.tabbrowser.mTabBox.tabs;
                 let tabPanel = this.tabbrowser.mPanelContainer;
                 let showPanel = tabs.getRelatedElement(showTab);
                 let index = Array.indexOf(tabPanel.childNodes, showPanel);
--- a/browser/base/content/test/performance/browser_tabopen_reflows.js
+++ b/browser/base/content/test/performance/browser_tabopen_reflows.js
@@ -13,64 +13,34 @@
  * for tips on how to do that.
  */
 const EXPECTED_REFLOWS = [
   // selection change notification may cause querying the focused editor content
   // by IME and that will cause reflow.
   [
     "select@chrome://global/content/bindings/textbox.xml",
     "focusAndSelectUrlBar@chrome://browser/content/browser.js",
-    "openLinkIn@chrome://browser/content/utilityOverlay.js",
-    "openUILinkIn@chrome://browser/content/utilityOverlay.js",
-    "BrowserOpenTab@chrome://browser/content/browser.js",
+    "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
   ],
 
   // selection change notification may cause querying the focused editor content
   // by IME and that will cause reflow.
   [
     "select@chrome://global/content/bindings/textbox.xml",
     "focusAndSelectUrlBar@chrome://browser/content/browser.js",
-    "openLinkIn@chrome://browser/content/utilityOverlay.js",
-    "openUILinkIn@chrome://browser/content/utilityOverlay.js",
-    "BrowserOpenTab@chrome://browser/content/browser.js",
+    "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
   ],
 
   [
     "select@chrome://global/content/bindings/textbox.xml",
     "focusAndSelectUrlBar@chrome://browser/content/browser.js",
-    "openLinkIn@chrome://browser/content/utilityOverlay.js",
-    "openUILinkIn@chrome://browser/content/utilityOverlay.js",
-    "BrowserOpenTab@chrome://browser/content/browser.js",
-  ],
-
-  [
-    "openLinkIn@chrome://browser/content/utilityOverlay.js",
-    "openUILinkIn@chrome://browser/content/utilityOverlay.js",
-    "BrowserOpenTab@chrome://browser/content/browser.js",
-  ],
-
-  [
-    // switching focus in updateCurrentBrowser() causes reflows
     "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
-    "updateDisplay@chrome://browser/content/tabbrowser.xml",
-    "postActions@chrome://browser/content/tabbrowser.xml",
-    "requestTab@chrome://browser/content/tabbrowser.xml"
   ],
 ];
 
-if (!gMultiProcessBrowser) {
-  EXPECTED_REFLOWS.push(
-    [
-      "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
-      "updateCurrentBrowser@chrome://browser/content/tabbrowser.xml",
-      "onselect@chrome://browser/content/browser.xul",
-    ],
-  );
-}
-
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when opening new tabs.
  */
 add_task(async function() {
   // If we've got a preloaded browser, get rid of it so that it
   // doesn't interfere with the test if it's loading. We have to
   // do this before we disable preloading or changing the new tab
--- a/browser/base/content/utilityOverlay.js
+++ b/browser/base/content/utilityOverlay.js
@@ -391,16 +391,18 @@ function openLinkIn(url, where, params) 
     // 'where' is "tab" or "tabshifted", so we'll load the link in a new tab.
     loadInBackground = aInBackground;
     if (loadInBackground == null) {
       loadInBackground =
         aFromChrome ? false : getBoolPref("browser.tabs.loadInBackground");
     }
   }
 
+  let focusUrlBar = false;
+
   switch (where) {
   case "current":
     let flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE;
 
     if (aAllowThirdPartyFixup) {
       flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
       flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FIXUP_SCHEME_TYPOS;
     }
@@ -432,30 +434,33 @@ function openLinkIn(url, where, params) 
       postData: aPostData,
       userContextId: aUserContextId
     });
     break;
   case "tabshifted":
     loadInBackground = !loadInBackground;
     // fall through
   case "tab":
+    focusUrlBar = !loadInBackground && w.isBlankPageURL(url);
+
     let tabUsedForLoad = w.gBrowser.loadOneTab(url, {
       referrerURI: aReferrerURI,
       referrerPolicy: aReferrerPolicy,
       charset: aCharset,
       postData: aPostData,
       inBackground: loadInBackground,
       allowThirdPartyFixup: aAllowThirdPartyFixup,
       relatedToCurrent: aRelatedToCurrent,
       skipAnimation: aSkipTabAnimation,
       allowMixedContent: aAllowMixedContent,
       noReferrer: aNoReferrer,
       userContextId: aUserContextId,
       originPrincipal: aPrincipal,
       triggeringPrincipal: aTriggeringPrincipal,
+      focusUrlBar,
     });
     targetBrowser = tabUsedForLoad.linkedBrowser;
 
     if (params.frameOuterWindowID != undefined && w) {
       // Only notify it as a WebExtensions' webNavigation.onCreatedNavigationTarget
       // event if it contains the expected frameOuterWindowID params.
       // (e.g. we should not notify it as a onCreatedNavigationTarget if the user is
       // opening a new tab using the keyboard shortcut).
@@ -466,24 +471,20 @@ function openLinkIn(url, where, params) 
           sourceTabBrowser: w.gBrowser.selectedBrowser,
           sourceFrameOuterWindowID: params.frameOuterWindowID,
         },
       }, "webNavigation-createdNavigationTarget");
     }
     break;
   }
 
-  // Focus the content, but only if the browser used for the load is selected.
-  if (targetBrowser == w.gBrowser.selectedBrowser) {
+  if (!focusUrlBar && targetBrowser == w.gBrowser.selectedBrowser) {
+    // Focus the content, but only if the browser used for the load is selected.
     targetBrowser.focus();
   }
-
-  if (!loadInBackground && w.isBlankPageURL(url)) {
-    w.focusAndSelectUrlBar();
-  }
 }
 
 // Used as an onclick handler for UI elements with link-like behavior.
 // e.g. onclick="checkForMiddleClick(this, event);"
 function checkForMiddleClick(node, event) {
   // We should be using the disabled property here instead of the attribute,
   // but some elements that this function is used with don't support it (e.g.
   // menuitem).
--- a/browser/components/customizableui/test/browser_editcontrols_update.js
+++ b/browser/components/customizableui/test/browser_editcontrols_update.js
@@ -96,16 +96,21 @@ add_task(async function test_panelui_cus
   await startCustomizing();
   let navbar = document.getElementById("nav-bar").customizationTarget;
   simulateItemDrag(document.getElementById("edit-controls"), navbar);
   await endCustomizing();
 
   // updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790.
   updateEditUIVisibility();
 
+  // The URL bar may have been focused to begin with, which means
+  // that subsequent calls to focus it won't result in command
+  // updates, so we'll make sure to blur it.
+  gURLBar.blur();
+
   let overridePromise = expectCommandUpdate(1);
   gURLBar.select();
   gURLBar.focus();
   gURLBar.value = "other";
   await overridePromise;
   checkState(false, "Update when edit-controls on toolbar and focused");
 
   overridePromise = expectCommandUpdate(1);