Bug 1394914 - toolbars no longer need to wait for xul overlays to load before initializing CustomizableUI icons (avoid sidebar icon flickering), r=Gijs.
authorFlorian Quèze <florian@queze.net>
Tue, 19 Dec 2017 23:40:40 +0100
changeset 448765 6cff7451155d9d1bcdcfad795bb79ec8b42a6d06
parent 448764 03e5401d560c1a9864bcdff7c4e835fa28575afc
child 448766 ce42f336f82c02fc7edf902699cdd276001f497d
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1394914
milestone59.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 1394914 - toolbars no longer need to wait for xul overlays to load before initializing CustomizableUI icons (avoid sidebar icon flickering), r=Gijs.
browser/base/content/test/performance/browser_startup_flicker.js
browser/base/content/test/performance/browser_windowopen_flicker.js
browser/base/content/test/performance/browser_windowopen_reflows.js
browser/components/customizableui/content/toolbar.xml
--- a/browser/base/content/test/performance/browser_startup_flicker.js
+++ b/browser/base/content/test/performance/browser_startup_flicker.js
@@ -43,23 +43,16 @@ add_task(async function() {
         {name: "bug 1403648 - urlbar down arrow shouldn't flicker",
          condition: r => // 5x9px area, sometimes less at the end of the opacity transition
                          inRange(r.h, 3, 5) && inRange(r.w, 7, 9) &&
                          inRange(r.y1, 40, 80) && // in the toolbar
                          // at ~80% of the window width
                          inRange(r.x1, width * .75, width * .9)
         },
 
-        {name: "bug 1394914 - sidebar toolbar icon should be visible at first paint",
-         condition: r => r.h == 13 && inRange(r.w, 14, 16) && // icon size
-                         inRange(r.y1, 40, 80) && // in the toolbar
-                         // near the right end of screen
-                         inRange(r.x1, width - 100, width - 50)
-        },
-
         {name: "bug 1403648 - urlbar should be focused at first paint",
          condition: r => inRange(r.y2, 60, 80) && // in the toolbar
                          // taking 50% to 75% of the window width
                          inRange(r.w, width * .5, width * .75) &&
                          // starting at 15 to 25% of the window width
                          inRange(r.x1, width * .15, width * .25)
         },
 
--- a/browser/base/content/test/performance/browser_windowopen_flicker.js
+++ b/browser/base/content/test/performance/browser_windowopen_flicker.js
@@ -93,23 +93,16 @@ add_task(async function() {
         {name: "bug 1403648 - urlbar down arrow shouldn't flicker",
          condition: r => // 5x9px area, sometimes less at the end of the opacity transition
                          inRange(r.h, 3, 5) && inRange(r.w, 7, 9) &&
                          inRange(r.y1, 40, 80) && // in the toolbar
                          // at ~80% of the window width
                          inRange(r.x1, width * .75, width * .9)
         },
 
-        {name: "bug 1394914 - sidebar toolbar icon should be visible at first paint",
-         condition: r => r.h == 13 && inRange(r.w, 14, 16) && // icon size
-                         inRange(r.y1, 40, 80) && // in the toolbar
-                         // near the right end of screen
-                         inRange(r.x1, width - 100, width - 50)
-        },
-
         {name: "bug 1403648 - urlbar should be focused at first paint",
          condition: r => inRange(r.y2, 60, 80) && // in the toolbar
                          // taking 50% to 75% of the window width
                          inRange(r.w, width * .5, width * .75) &&
                          // starting at 15 to 25% of the window width
                          inRange(r.x1, width * .15, width * .25)
         },
 
--- a/browser/base/content/test/performance/browser_windowopen_reflows.js
+++ b/browser/base/content/test/performance/browser_windowopen_reflows.js
@@ -7,17 +7,27 @@
  * WHOA THERE: We should never be adding new things to EXPECTED_REFLOWS. This
  * is a whitelist that should slowly go away as we improve the performance of
  * the front-end. Instead of adding more reflows to the whitelist, you should
  * be modifying your code to avoid the reflow.
  *
  * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
  * for tips on how to do that.
  */
-const EXPECTED_REFLOWS = [];
+const EXPECTED_REFLOWS = [
+  {
+    stack: [
+      "onOverflow@resource:///modules/CustomizableUI.jsm",
+      "init@resource:///modules/CustomizableUI.jsm",
+      "observe@resource:///modules/CustomizableUI.jsm",
+      "_delayedStartup@chrome://browser/content/browser.js",
+    ],
+    times: 2, // This number should only ever go down - never up.
+  },
+];
 
 if (Services.appinfo.OS == "WINNT") {
   EXPECTED_REFLOWS.push(
     {
       stack: [
         "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
         "_update@chrome://browser/content/browser-tabsintitlebar.js",
         "init@chrome://browser/content/browser-tabsintitlebar.js",
--- a/browser/components/customizableui/content/toolbar.xml
+++ b/browser/components/customizableui/content/toolbar.xml
@@ -13,43 +13,24 @@
       <stylesheet src="chrome://global/skin/toolbar.css"/>
     </resources>
     <implementation>
       <field name="overflowedDuringConstruction">null</field>
 
       <constructor><![CDATA[
           let scope = {};
           Cu.import("resource:///modules/CustomizableUI.jsm", scope);
+          let CustomizableUI = scope.CustomizableUI;
           // Add an early overflow event listener that will mark if the
           // toolbar overflowed during construction.
-          if (scope.CustomizableUI.isAreaOverflowable(this.id)) {
+          if (CustomizableUI.isAreaOverflowable(this.id)) {
             this.addEventListener("overflow", this);
             this.addEventListener("underflow", this);
           }
 
-          if (document.readyState == "complete") {
-            this._init();
-          } else {
-            // Need to wait until XUL overlays are loaded. See bug 554279.
-            let self = this;
-            document.addEventListener("readystatechange", function onReadyStateChange() {
-              if (document.readyState != "complete")
-                return;
-              document.removeEventListener("readystatechange", onReadyStateChange);
-              self._init();
-            });
-          }
-      ]]></constructor>
-
-      <method name="_init">
-        <body><![CDATA[
-          let scope = {};
-          Cu.import("resource:///modules/CustomizableUI.jsm", scope);
-          let CustomizableUI = scope.CustomizableUI;
-
           // Bug 989289: Forcibly set the now unsupported "mode" and "iconsize"
           // attributes, just in case they accidentally get restored from
           // persistence from a user that's been upgrading and downgrading.
           if (CustomizableUI.isBuiltinToolbar(this.id)) {
             const kAttributes = new Map([["mode", "icons"], ["iconsize", "small"]]);
             for (let [attribute, value] of kAttributes) {
               if (this.getAttribute(attribute) != value) {
                 this.setAttribute(attribute, value);
@@ -78,18 +59,17 @@
             }
           }
 
           // pass the current set of children for comparison with placements:
           let children = Array.from(this.childNodes)
                               .filter(node => node.getAttribute("skipintoolbarset") != "true" && node.id)
                               .map(node => node.id);
           CustomizableUI.registerToolbarNode(this, children);
-        ]]></body>
-      </method>
+      ]]></constructor>
 
       <method name="handleEvent">
         <parameter name="aEvent"/>
         <body><![CDATA[
           if (aEvent.type == "overflow" && aEvent.detail > 0) {
             if (this.overflowable && this.overflowable.initialized) {
               this.overflowable.onOverflow(aEvent);
             } else {