Bug 1334642 - Cache luminance values for each toolbar in ToolbarIconColor. r=dao r=mconley draft
authorSam Foster <sfoster@mozilla.com>
Thu, 06 Apr 2017 10:50:09 -0700
changeset 565866 4837dcd736eb613923a8b3be677d8e863a659525
parent 565261 a34919b3d942cfd4f0737d432742b0ffa9929389
child 625160 d9db44e8bf4c05565148222b18982cd532730e9f
push id55047
push userbmo:sfoster@mozilla.com
push dateThu, 20 Apr 2017 16:33:51 +0000
reviewersdao, mconley
bugs1334642
milestone55.0a1
Bug 1334642 - Cache luminance values for each toolbar in ToolbarIconColor. r=dao r=mconley * Track window states: active, fullscreen and tabsintitlebar for each window * Use toolbar.id and window state to store and retrieve values from cache * Note: As each window has its own ToolbarIconColor object, the cache is not currently shared across windows * inferFromText callers pass in a reason and associated value, which is used to update the state we track, and potentially clear out the cache * Create new windows test directory for browser-window-specific tests like this * Test for the ToolbarIconColor changes to avoid sync style flushes when windows activate/deactivate MozReview-Commit-ID: JDJ3RtL4Lge
browser/base/content/browser-fullScreenAndPointerLock.js
browser/base/content/browser-tabsintitlebar.js
browser/base/content/browser.js
browser/base/content/test/windows/.eslintrc.js
browser/base/content/test/windows/browser.ini
browser/base/content/test/windows/browser_toolbariconcolor_restyles.js
browser/base/moz.build
--- a/browser/base/content/browser-fullScreenAndPointerLock.js
+++ b/browser/base/content/browser-fullScreenAndPointerLock.js
@@ -631,17 +631,18 @@ var FullScreen = {
         if (el.hasAttribute("saved-context")) {
           el.setAttribute("context", el.getAttribute("saved-context"));
           el.removeAttribute("saved-context");
         }
         el.removeAttribute("inFullscreen");
       }
     }
 
-    ToolbarIconColor.inferFromText();
+    ToolbarIconColor.inferFromText("fullscreen", aEnterFS);
+
 
     // For Lion fullscreen, all fullscreen controls are hidden, don't
     // bother to touch them. If we don't stop here, the following code
     // could cause the native fullscreen button be shown unexpectedly.
     // See bug 1165570.
     if (this.useLionFullScreen) {
       return;
     }
--- a/browser/base/content/browser-tabsintitlebar.js
+++ b/browser/base/content/browser-tabsintitlebar.js
@@ -242,17 +242,18 @@ 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.inferFromText("tabsintitlebar", TabsInTitlebar.enabled);
+
     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
@@ -5403,18 +5403,16 @@ function setToolbarVisibility(toolbar, i
     },
     bubbles: true
   };
   let event = new CustomEvent("toolbarvisibilitychange", eventParams);
   toolbar.dispatchEvent(event);
 
   PlacesToolbarHelper.init();
   BookmarkingUI.onToolbarVisibilityChange();
-  if (isVisible)
-    ToolbarIconColor.inferFromText();
 }
 
 var TabletModeUpdater = {
   init() {
     if (AppConstants.isPlatformAndVersionAtLeast("win", "10")) {
       this.update(WindowsUIUtils.inTabletMode);
       Services.obs.addObserver(this, "tablet-mode-change");
     }
@@ -7681,17 +7679,17 @@ var gIdentityHandler = {
     }
     stateLabel.textContent = SitePermissions.getCurrentStateLabel(state, scope);
 
     let button = document.createElement("button");
     button.setAttribute("class", "identity-popup-permission-remove-button");
     let tooltiptext = gNavigatorBundle.getString("permissions.remove.tooltip");
     button.setAttribute("tooltiptext", tooltiptext);
     button.addEventListener("command", () => {
-	  let browser = gBrowser.selectedBrowser;
+      let browser = gBrowser.selectedBrowser;
       // Only resize the window if the reload hint was previously hidden.
       this._handleHeightChange(() => this._permissionList.removeChild(container),
                                this._permissionReloadHint.hasAttribute("hidden"));
       if (aPermission.inUse &&
           ["camera", "microphone", "screen"].includes(aPermission.id)) {
         let windowId = this._sharingState.windowId;
         if (aPermission.id == "screen") {
           windowId = "screen:" + windowId;
@@ -8248,78 +8246,123 @@ var MousePosTracker = {
         listener.onMouseEnter();
     } else if (listener.onMouseLeave) {
       listener.onMouseLeave();
     }
   }
 };
 
 var ToolbarIconColor = {
+  _windowState: {
+    "active": false,
+    "fullscreen": false,
+    "tabsintitlebar": false
+  },
   init() {
     this._initialized = true;
 
     window.addEventListener("activate", this);
     window.addEventListener("deactivate", this);
+    window.addEventListener("toolbarvisibilitychange", this);
     Services.obs.addObserver(this, "lightweight-theme-styling-update");
 
     // 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)
-      this.inferFromText();
+    if (Services.focus.activeWindow == window) {
+      this.inferFromText("activate");
+    }
   },
 
   uninit() {
     this._initialized = false;
 
     window.removeEventListener("activate", this);
     window.removeEventListener("deactivate", this);
+    window.removeEventListener("toolbarvisibilitychange", this);
     Services.obs.removeObserver(this, "lightweight-theme-styling-update");
   },
 
   handleEvent(event) {
     switch (event.type) {
-      case "activate":
+      case "activate":  // falls through
       case "deactivate":
-        this.inferFromText();
+        this.inferFromText(event.type);
+        break;
+      case "toolbarvisibilitychange":
+        this.inferFromText(event.type, event.visible);
         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(() => {
+          this.inferFromText(aTopic);
+        }, 0);
         break;
     }
   },
 
-  inferFromText() {
+  // a cache of luminance values for each toolbar
+  // to avoid unnecessary calls to getComputedStyle
+  _toolbarLuminanceCache: new Map(),
+
+  inferFromText(reason, reasonValue) {
     if (!this._initialized)
       return;
-
     function parseRGB(aColorString) {
       let rgb = aColorString.match(/^rgba?\((\d+), (\d+), (\d+)/);
       rgb.shift();
       return rgb.map(x => parseInt(x));
     }
 
+    switch (reason) {
+      case "activate": // falls through
+      case "deactivate":
+        this._windowState.active = (reason === "activate");
+        break;
+      case "fullscreen":
+        this._windowState.fullscreen = reasonValue;
+        break;
+      case "lightweight-theme-styling-update":
+        // theme change, we'll need to recalculate all color values
+        this._toolbarLuminanceCache.clear();
+        break;
+      case "toolbarvisibilitychange":
+        // toolbar changes dont require reset of the cached color values
+        break;
+      case "tabsintitlebar":
+        this._windowState.tabsintitlebar = reasonValue;
+        break;
+    }
+
     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._toolbarLuminanceCache;
+    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 && toolbar.id + JSON.stringify(this._windowState);
+      // lookup cached luminance value for this toolbar in this window state
+      let luminance = cacheKey && cachedLuminances.get(cacheKey);
+      if (isNaN(luminance)) {
+        let [r, g, b] = parseRGB(getComputedStyle(toolbar).color);
+        luminance = 0.2125 * r + 0.7154 * g + 0.0721 * b;
+        if (cacheKey) {
+          cachedLuminances.set(cacheKey, luminance);
+        }
+      }
       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/windows/.eslintrc.js
@@ -0,0 +1,7 @@
+"use strict";
+
+module.exports = {
+  "extends": [
+    "plugin:mozilla/browser-test"
+  ]
+};
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/windows/browser.ini
@@ -0,0 +1,2 @@
+[DEFAULT]
+[browser_toolbariconcolor_restyles.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/windows/browser_toolbariconcolor_restyles.js
@@ -0,0 +1,43 @@
+"use strict";
+
+/**
+ * Ensure redundant style flushes are not triggered when switching between windows
+ */
+add_task(function* test_toolbar_element_restyles_on_activation() {
+  let restyles = {
+    win1:  {},
+    win2:  {}
+  };
+
+  // create a window and snapshot the elementsStyled
+  let win1 = yield BrowserTestUtils.openNewBrowserWindow();
+  yield new Promise(resolve => waitForFocus(resolve, win1));
+
+  // create a 2nd window and snapshot the elementsStyled
+  let win2 = yield BrowserTestUtils.openNewBrowserWindow();
+  yield new Promise(resolve => waitForFocus(resolve, win2));
+
+  let utils1 = SpecialPowers.getDOMWindowUtils(win1);
+  restyles.win1.initial = utils1.elementsRestyled;
+
+  let utils2 = SpecialPowers.getDOMWindowUtils(win2);
+  restyles.win2.initial = utils2.elementsRestyled;
+
+  // switch back to 1st window, and snapshot elementsStyled
+  win1.focus();
+  restyles.win1.activate = utils1.elementsRestyled;
+  restyles.win2.deactivate = utils2.elementsRestyled;
+
+  // switch back to 2nd window, and snapshot elementsStyled
+  win2.focus();
+  restyles.win2.activate = utils2.elementsRestyled;
+  restyles.win1.deactivate = utils1.elementsRestyled;
+
+  is(restyles.win1.activate - restyles.win1.deactivate, 0,
+      "No elements restyled when re-activating/deactivating a window");
+  is(restyles.win2.activate - restyles.win2.deactivate, 0,
+      "No elements restyled when re-activating/deactivating a window");
+
+  yield BrowserTestUtils.closeWindow(win1);
+  yield BrowserTestUtils.closeWindow(win2);
+});
--- a/browser/base/moz.build
+++ b/browser/base/moz.build
@@ -30,16 +30,17 @@ BROWSER_CHROME_MANIFESTS += [
     'content/test/social/browser.ini',
     'content/test/static/browser.ini',
     'content/test/tabcrashed/browser.ini',
     'content/test/tabPrompts/browser.ini',
     'content/test/tabs/browser.ini',
     'content/test/urlbar/browser.ini',
     'content/test/webextensions/browser.ini',
     'content/test/webrtc/browser.ini',
+    'content/test/windows/browser.ini',
 ]
 
 if CONFIG['MOZ_UPDATER']:
     BROWSER_CHROME_MANIFESTS += ['content/test/appUpdate/browser.ini']
 
 DEFINES['MOZ_APP_VERSION'] = CONFIG['MOZ_APP_VERSION']
 DEFINES['MOZ_APP_VERSION_DISPLAY'] = CONFIG['MOZ_APP_VERSION_DISPLAY']