Bug 1445737: Ensure we only update tabsInTitlebar once during browser startup. r?dao draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 16 Mar 2018 15:51:30 +0100
changeset 768597 82bea62c1c01e410541fd6ffe1ba6d3e65afd318
parent 768509 f207b26a44cdfef6d46327f72835731508ef7be3
child 768598 aad5cbc5b0a06a40eeecba027eab83151a4b3207
push id102924
push userbmo:emilio@crisal.io
push dateFri, 16 Mar 2018 15:33:06 +0000
reviewersdao
bugs1445737
milestone61.0a1
Bug 1445737: Ensure we only update tabsInTitlebar once during browser startup. r?dao Two fixes here: updateAppearance really depends on the tabbrowser.xml updateVisibility(), so make that dependency explicit and remove the event handling from tabbrowser. After the previous patch, TabsInTitlebar.init() runs much earlier, which means that the initialization check that bails out from update() is not taken. Turns out that this is definitely not an edge case, and that tons of initialization code all over gBrowser.onDOMContentLoaded call updateAppearance(true), causing tons of extra invalidation and reflows that aren't really needed. Ensure we only do the work once. MozReview-Commit-ID: 2E9b12aBast
browser/base/content/browser-tabsintitlebar.js
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
--- a/browser/base/content/browser-tabsintitlebar.js
+++ b/browser/base/content/browser-tabsintitlebar.js
@@ -3,19 +3,16 @@
  * 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/. */
 
 // Note: the file browser-tabsintitlebar-stub.js is used instead of
 // this one on platforms which don't have CAN_DRAW_IN_TITLEBAR defined.
 
 var TabsInTitlebar = {
   init() {
-    if (this._initialized) {
-      return;
-    }
     this._readPref();
     Services.prefs.addObserver(this._prefName, this);
 
     // Always disable on unsupported GTK versions.
     if (AppConstants.MOZ_WIDGET_TOOLKIT == "gtk3") {
       this.allowedBy("gtk", window.matchMedia("(-moz-gtk-csd-available)"));
     }
 
@@ -41,17 +38,17 @@ var TabsInTitlebar = {
       if (aArea == CustomizableUI.AREA_TABSTRIP || aArea == CustomizableUI.AREA_MENUBAR)
         this._update(true);
     };
     CustomizableUI.addListener(this);
 
     addEventListener("resolutionchange", this, false);
 
     this._initialized = true;
-    this._update();
+    this._update(true, true);
   },
 
   allowedBy(condition, allow) {
     if (allow) {
       if (condition in this._disallowed) {
         delete this._disallowed[condition];
         this._update(true);
       }
@@ -75,46 +72,55 @@ var TabsInTitlebar = {
   },
 
   handleEvent(aEvent) {
     if (aEvent.type == "resolutionchange" && aEvent.target == window) {
       this._update(true);
     }
   },
 
+  onDOMContentLoaded() {
+    this._domLoaded = true;
+    this._update(true);
+  },
+
   _onMenuMutate(aMutations) {
     for (let mutation of aMutations) {
       if (mutation.attributeName == "inactive" ||
           mutation.attributeName == "autohide") {
         TabsInTitlebar._update(true);
         return;
       }
     }
   },
 
   _initialized: false,
   _disallowed: {},
   _prefName: "browser.tabs.drawInTitlebar",
   _lastSizeMode: null,
+  _domLoaded: false,
 
   _readPref() {
     this.allowedBy("pref",
                    Services.prefs.getBoolPref(this._prefName));
   },
 
-  _update(aForce = false) {
+  _update(aForce = false, aFromInit = false) {
     let $ = id => document.getElementById(id);
     let rect = ele => ele.getBoundingClientRect();
     let verticalMargins = cstyle => parseFloat(cstyle.marginBottom) + parseFloat(cstyle.marginTop);
 
     if (window.fullScreen)
       return;
 
-    // In some edgecases it is possible for this to fire before we've initialized.
-    if (!this._initialized) {
+    // We want to do this from initialization anyway, so that the
+    // "chromemargin" attributes are all correctly setup, but we don't want to
+    // do this before the DOM loads again, to prevent spurious reflows that
+    // will be superseded by the one on onDOMContentLoaded.
+    if (!this._domLoaded && !aFromInit) {
       return;
     }
 
     if (!aForce) {
       // _update is called on resize events, because the window is not ready
       // after sizemode events. However, we only care about the event when the
       // sizemode is different from the last time we updated the appearance of
       // the tabs in the titlebar.
@@ -264,17 +270,16 @@ var TabsInTitlebar = {
   },
 
   _sizePlaceholder(type, width) {
     Array.forEach(document.querySelectorAll(".titlebar-placeholder[type='" + type + "']"),
                   function(node) { node.style.width = width + "px"; });
   },
 
   uninit() {
-    this._initialized = false;
     removeEventListener("resolutionchange", this);
     Services.prefs.removeObserver(this._prefName, this);
     this._menuObserver.disconnect();
     CustomizableUI.removeListener(this);
   }
 };
 
 function updateTitlebarDisplay() {
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1292,16 +1292,18 @@ var gBrowserInit = {
       if (uriToLoad == "about:home") {
         gBrowser.setIcon(gBrowser.selectedTab, "chrome://branding/content/icon32.png");
       } else if (uriToLoad == "about:privatebrowsing") {
         gBrowser.setIcon(gBrowser.selectedTab, "chrome://browser/skin/privatebrowsing/favicon.svg");
       }
     });
 
     this._setInitialFocus();
+    gBrowser.tabContainer.updateVisibility();
+    TabsInTitlebar.onDOMContentLoaded();
   },
 
   onLoad() {
     gBrowser.addEventListener("DOMUpdateBlockedPopups", gPopupBlockerObserver);
 
     Services.obs.addObserver(gPluginHandler.NPAPIPluginCrashed, "plugin-crashed");
 
     window.addEventListener("AppCommand", HandleAppCommandEvent, true);
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -119,17 +119,16 @@
               "emptyPrivateTabTitle" : "emptyTabTitle";
           this.emptyTabTitle = gTabBrowserBundle.GetStringFromName("tabs." + strId);
 
           var tab = this.firstChild;
           tab.label = this.emptyTabTitle;
           tab.setAttribute("onerror", "this.removeAttribute('image');");
 
           window.addEventListener("resize", this);
-          window.addEventListener("DOMContentLoaded", this);
 
           Services.prefs.addObserver("privacy.userContext", this);
           this.observe(null, "nsPref:changed", "privacy.userContext.enabled");
 
           XPCOMUtils.defineLazyPreferenceGetter(this, "_tabMinWidthPref",
             "browser.tabs.tabMinWidth", null,
             (pref, prevValue, newValue) => this._tabMinWidth = newValue,
             newValue => {
@@ -749,19 +748,16 @@
           this._handleTabSelect();
         ]]></body>
       </method>
 
       <method name="handleEvent">
         <parameter name="aEvent"/>
         <body><![CDATA[
           switch (aEvent.type) {
-            case "DOMContentLoaded":
-              this.updateVisibility();
-              break;
             case "resize":
               if (aEvent.target != window)
                 break;
 
               TabsInTitlebar.updateAppearance();
               this._updateCloseButtons();
               this._handleTabSelect(true);
               this.updateSessionRestoreVisibility();