Bug 1334642 - Cache luminance values for each toolbar in ToolbarIconColor. draft
authorSam Foster <sfoster@mozilla.com>
Fri, 17 Mar 2017 09:34:50 -0700
changeset 553909 cc569004766b4422f1bdac9cbb9b51c941355dfc
parent 553751 3364cc17988c013c36f2a8123315db2855393011
child 553910 4b9d207f8121325dd03826cafd9d75f2d31b0655
push id51821
push userbmo:sfoster@mozilla.com
push dateFri, 31 Mar 2017 00:07:56 +0000
bugs1334642
milestone55.0a1
Bug 1334642 - Cache luminance values for each toolbar in ToolbarIconColor. * Add ToolbarIconColor.reCalculate to do the inferFromText work from a clear luminance cache * Call reCalculate rather than inferFromText when lw-theme changes, when toolbars are shown/hidden * Track window active/inactive state in ToolbarIconColor and cache luminance values separately for each state * WIP - working on test check for unexpected reflows on window activation MozReview-Commit-ID: 3LIXdxpvkIX
browser/base/content/browser-tabsintitlebar.js
browser/base/content/browser.js
browser/base/content/test/general/browser_window_activestate_reflows.js
--- a/browser/base/content/browser-tabsintitlebar.js
+++ b/browser/base/content/browser-tabsintitlebar.js
@@ -242,17 +242,17 @@ var TabsInTitlebar = {
 
       // Reset the margins and padding that might have been modified:
       titlebarContent.style.marginTop = "";
       titlebarContent.style.marginBottom = "";
       titlebar.style.marginBottom = "";
       menubar.style.paddingBottom = "";
     }
 
-    ToolbarIconColor.inferFromText();
+    ToolbarIconColor.reCalculate();
     if (CustomizationHandler.isCustomizing()) {
       gCustomizeMode.updateLWTStyling();
     }
   },
 
   _sizePlaceholder(type, width) {
     Array.forEach(document.querySelectorAll(".titlebar-placeholder[type='" + type + "']"),
                   function(node) { node.width = width; });
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -5364,18 +5364,19 @@ function setToolbarVisibility(toolbar, i
     },
     bubbles: true
   };
   let event = new CustomEvent("toolbarvisibilitychange", eventParams);
   toolbar.dispatchEvent(event);
 
   PlacesToolbarHelper.init();
   BookmarkingUI.onToolbarVisibilityChange();
-  if (isVisible)
-    ToolbarIconColor.inferFromText();
+  if (isVisible) {
+    ToolbarIconColor.reCalculate();
+  }
 }
 
 var TabletModeUpdater = {
   init() {
     if (AppConstants.isPlatformAndVersionAtLeast("win", "10")) {
       this.update(WindowsUIUtils.inTabletMode);
       Services.obs.addObserver(this, "tablet-mode-change", false);
     }
@@ -8200,57 +8201,78 @@ var MousePosTracker = {
         listener.onMouseEnter();
     } else if (listener.onMouseLeave) {
       listener.onMouseLeave();
     }
   }
 };
 
 var ToolbarIconColor = {
+  _isActiveWindow: false,
   init() {
     this._initialized = true;
 
     window.addEventListener("activate", this);
     window.addEventListener("deactivate", this);
     Services.obs.addObserver(this, "lightweight-theme-styling-update", false);
 
     // If the window isn't active now, we assume that it has never been active
     // before and will soon become active such that inferFromText will be
     // called from the initial activate event.
-    if (Services.focus.activeWindow == window)
+    if (Services.focus.activeWindow == window) {
       this.inferFromText();
+    }
   },
 
   uninit() {
     this._initialized = false;
+    this._toolbarLuminances = null;
 
     window.removeEventListener("activate", this);
     window.removeEventListener("deactivate", this);
     Services.obs.removeObserver(this, "lightweight-theme-styling-update");
   },
 
   handleEvent(event) {
+    console.log("ToolbarIconColor.handleEvent: ", event.type);
     switch (event.type) {
       case "activate":
+        this._isActiveWindow = true;
+        this.inferFromText();
+        break;
       case "deactivate":
+        this._isActiveWindow = false;
         this.inferFromText();
         break;
     }
   },
 
   observe(aSubject, aTopic, aData) {
     switch (aTopic) {
       case "lightweight-theme-styling-update":
         // inferFromText needs to run after LightweightThemeConsumer.jsm's
         // lightweight-theme-styling-update observer.
-        setTimeout(() => { this.inferFromText(); }, 0);
+        setTimeout(() => {
+          // invalidate the cached luminance values on the toobars
+          this.reCalculate();
+        }, 0);
         break;
     }
   },
 
+  // a cache of luminance values for each toolbar
+  // to avoid unnecessary calls to getComputedStyle
+  _toolbarLuminances: null,
+
+  reCalculate: function() {
+    this._toolbarLuminances = null;
+    console.log('ToolbarIconColor.reCalculate, flushed _toolbarLuminances cache');
+    this.inferFromText();
+  },
+
   inferFromText() {
     if (!this._initialized)
       return;
 
     function parseRGB(aColorString) {
       let rgb = aColorString.match(/^rgba?\((\d+), (\d+), (\d+)/);
       rgb.shift();
       return rgb.map(x => parseInt(x));
@@ -8258,20 +8280,37 @@ var ToolbarIconColor = {
 
     let toolbarSelector = "#navigator-toolbox > toolbar:not([collapsed=true]):not(#addon-bar)";
     if (AppConstants.platform == "macosx")
       toolbarSelector += ":not([type=menubar])";
 
     // The getComputedStyle calls and setting the brighttext are separated in
     // two loops to avoid flushing layout and making it dirty repeatedly.
 
-    let luminances = new Map;
+    let cachedLuminances = this._toolbarLuminances || (
+      this._toolbarLuminances = new Map());
+    let luminances = new Map();
     for (let toolbar of document.querySelectorAll(toolbarSelector)) {
-      let [r, g, b] = parseRGB(getComputedStyle(toolbar).color);
-      let luminance = 0.2125 * r + 0.7154 * g + 0.0721 * b;
+      // toolbars *should* all have ids, but guard anyway to avoid blowing up
+      let cacheKey = toolbar.id && JSON.stringify({
+        id: toolbar.id,
+        active: this._isActiveWindow
+      });
+      let luminance = cacheKey && cachedLuminances.get(cacheKey);
+      if (isNaN(luminance)) {
+        console.log("ToolbarIconColor.inferFromText, no cached value for: ", cacheKey);
+        let [r, g, b] = parseRGB(getComputedStyle(toolbar).color);
+        luminance = 0.2125 * r + 0.7154 * g + 0.0721 * b;
+        if (cacheKey) {
+          console.log("ToolbarIconColor.inferFromText, caching: ", luminance);
+          cachedLuminances.set(cacheKey, luminance);
+        }
+      } else {
+        console.log("ToolbarIconColor.inferFromText, using cached value for: ", cacheKey);
+      }
       luminances.set(toolbar, luminance);
     }
 
     for (let [toolbar, luminance] of luminances) {
       if (luminance <= 110)
         toolbar.removeAttribute("brighttext");
       else
         toolbar.setAttribute("brighttext", "true");
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_window_activestate_reflows.js
@@ -0,0 +1,180 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+XPCOMUtils.defineLazyGetter(this, "docShell", () => {
+  return window.QueryInterface(Ci.nsIInterfaceRequestor)
+               .getInterface(Ci.nsIWebNavigation)
+               .QueryInterface(Ci.nsIDocShell);
+});
+
+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
+  "_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
+  "get_scrollPosition@chrome://global/content/bindings/scrollbox.xml|" +
+    "_fillTrailingGap@chrome://browser/content/tabbrowser.xml|" +
+    "_handleNewTab@chrome://browser/content/tabbrowser.xml|" +
+    "onxbltransitionend@chrome://browser/content/tabbrowser.xml|",
+
+  // SessionStore.getWindowDimensions()
+  "ssi_getWindowDimension@resource:///modules/sessionstore/SessionStore.jsm|" +
+    "ssi_updateWindowFeatures/<@resource:///modules/sessionstore/SessionStore.jsm|" +
+    "ssi_updateWindowFeatures@resource:///modules/sessionstore/SessionStore.jsm|" +
+    "ssi_collectWindowData@resource:///modules/sessionstore/SessionStore.jsm|",
+
+  // 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|",
+
+];
+const _Observer = {
+  reflow(start, end) {
+    console.log(name + ": handling reflow, stack: ", new Error().stack);
+    // Gather information about the current code path.
+    let path = (new Error().stack).split("\n").slice(1).map(line => {
+      return line.replace(/:\d+:\d+$/, "");
+    }).join("|");
+    let pathWithLineNumbers = (new Error().stack).split("\n").slice(1).join("|");
+
+    // Stack trace is empty. Reflow was triggered by native code.
+    if (path === "") {
+      console.log(name + ": reflow: empty path from stack");
+      return;
+    }
+
+    // Check if this is an expected reflow.
+    for (let stack of EXPECTED_REFLOWS) {
+      if (path.startsWith(stack)) {
+        ok(true, "expected uninterruptible reflow '" + stack + "'");
+        return;
+      }
+    }
+    ok(false, "unexpected uninterruptible reflow '" + pathWithLineNumbers + "'");
+  },
+
+  reflowInterruptible(start, end) {
+    // We're not interested in interruptible reflows.
+  },
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIReflowObserver,
+                                         Ci.nsISupportsWeakReference])
+};
+
+
+function setupWindowForTest(win, name) {
+  let observer = Object.create(_Observer);
+  win.addEventListener("activate", (evt) => {
+    console.log(name + ":window event: ", evt.type);
+  });
+  win.addEventListener("deactivate", (evt) => {
+    console.log(name + ":window event: ", evt.type);
+  });
+  let docShell = win.QueryInterface(Ci.nsIInterfaceRequestor)
+                  .getInterface(Ci.nsIWebNavigation)
+                  .QueryInterface(Ci.nsIDocShell);
+
+  let isObservingReflow
+  let wrapper = {
+    throwOnUnexpectedReflow() {
+      if (isObservingReflow) {
+        console.log(name + ": throwOnUnexpectedReflow, already observing reflow");
+        return;
+      }
+      console.log(name + ": addWeakReflowObserver");
+      docShell.addWeakReflowObserver(observer);
+      isObservingReflow = true;
+      console.log(name + ": /addWeakReflowObserver");
+    },
+    unObserveReflows() {
+      if (isObservingReflow) {
+        console.log(name + ": removeWeakReflowObserver");
+        docShell.removeWeakReflowObserver(observer);
+        isObservingReflow = false;
+        console.log(name + ": /removeWeakReflowObserver");
+      }
+    },
+    cleanUp() {
+      this.unObserveReflows();
+      return promiseWindowClosed(win);
+    }
+  };
+  return wrapper;
+}
+
+/*
+ * This test ensures that there are no unexpected
+ * uninterruptible reflows when toggling between 2 windows
+ */
+add_task(function*() {
+
+  // open and focus 2 windows
+  let win1 = yield promiseOpenAndLoadWindow();
+  console.log("focusing win1");
+  yield promiseWaitForFocus(win1);
+
+
+  console.log("opening win2");
+  let win2 = yield promiseOpenAndLoadWindow();
+  yield promiseWaitForFocus(win2);
+  // cache for win1 active and inactive states should be populated
+  // win2 active cache should be populated
+
+  let windowWrapper1 = setupWindowForTest(win1, "win1");
+  let windowWrapper2 = setupWindowForTest(win2, "win2");
+
+  // switch to 1st window, making 2nd window inactive for the first time
+  console.log("focusing win1");
+  console.log("throw for reflow stacks in results");
+  windowWrapper1.throwOnUnexpectedReflow();
+  yield promiseWaitForFocus(win1);
+  // reset
+  windowWrapper1.unObserveReflows();
+
+  // SimpleTest.ok(!windowWrapper1.stack,
+  //               "Unexpected reflow from activated window",
+  //               null,
+  //               windowWrapper1.stack);
+
+  // switch back to 2nd window,
+  // both 1st and 2nd window have seen these states before and shouldn't reflow
+  // yield promiseWaitForFocus(win2);
+  // windowWrapper1.throwOnUnexpectedReflow();
+  // windowWrapper2.throwOnUnexpectedReflow();
+
+  // // reset
+  // windowWrapper1.unObserveReflows();
+  // windowWrapper2.unObserveReflows();
+  // unhook the observers and clean up the windows
+  yield windowWrapper1.cleanUp();
+  yield windowWrapper2.cleanUp();
+});
+
+function waitForTransitionEnd() {
+  return new Promise(resolve => {
+    let tab = gBrowser.selectedTab;
+    tab.addEventListener("transitionend", function onEnd(event) {
+      if (event.propertyName === "max-width") {
+        tab.removeEventListener("transitionend", onEnd);
+        resolve();
+      }
+    });
+  });
+}