Still need to get the test finished to cover only providing some of the icons.

These are the two webextensions I've been testing with. One defines only one icon and the other defines all.

Comment on attachment 8842979 [details] Bug 1343921 - Implement support for custom icons through the Theming API. https://reviewboard.mozilla.org/r/116708/#review118482 This looks great! ::: browser/components/extensions/ext-theme.js:151 (Diff revision 2) > + for (let icon of Object.keys(icons)) { > + let val = icons[icon]; > + if (!val) { > + continue; > + } > + if (Object.keys(ICONS_TO_VARIABLE_MAP).includes(icon)) { Would it be better to store the call to Object.keys(ICONS_TO_VARIABLE_MAP) outside of the loop? ::: browser/components/extensions/ext-theme.js:153 (Diff revision 2) > + if (!val) { > + continue; > + } > + if (Object.keys(ICONS_TO_VARIABLE_MAP).includes(icon)) { > + if (!this.lwtStyles.icons) { > + this.lwtStyles.icons = {}; Could we just move this to the constructor? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:5 (Diff revision 2) > +"use strict"; > + > +const ENCODED_IMAGE_DATA = "PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHZpZXdCb3g9IjAgMCA2NCA2NCIgZW5hYmxlLWJhY2tncm91bmQ9Im5ldyAwIDAgNjQgNjQiPjxwYXRoIGQ9Im01NS45IDMyLjFsLTIyLjctMTQuOWMwIDAgMTIuOS0xNy40IDE5LjQtMTQuOSAzLjEgMS4xIDUuNCAyNS4xIDMuMyAyOS44IiBmaWxsPSIjM2U0MzQ3Ii8+PHBhdGggZD0ibTU0LjkgMzMuOWwtOS00LjFjMCAwLTUuMy0xNCA2LjEtMjQuMSAyLjQgMiA1LjEgMjUgMi45IDI4LjIiIGZpbGw9IiNmZmYiLz48cGF0aCBkPSJtOC4xIDMyLjFsMjIuNi0xNC45YzAgMC0xMi45LTE3LjQtMTkuNC0xNC45LTMgMS4xLTUuMyAyNS4xLTMuMiAyOS44IiBmaWxsPSIjM2U0MzQ3Ii8+PHBhdGggZD0ibTkuMSAzMy45bDktNC4xYzAgMCA1LjMtMTQtNi4xLTI0LjEtMi40IDItNS4xIDI1LTIuOSAyOC4yIiBmaWxsPSIjZmZmIi8+PHBhdGggZD0iTTMyLDEzQzE4LjksMTMsMiwzMy42LDIsNDUuNEMyMC41LDQ1LjQsMTkuNyw2MiwzMiw2MnMxMS41LTE2LjYsMzAtMTYuNkM2MiwzMy42LDQ1LjEsMTMsMzIsMTN6IiBmaWxsPSIjZmY4NzM2Ii8+PGcgZmlsbD0iI2ZmZiI+PHBhdGggZD0iTTMyLDU2LjJjMCw1LjEsOS42LDQuMiw5LjUtMi45YzYuNy05LjQsMTkuOS04LjcsMTkuOS04LjdDMzkuNiwzMi40LDMyLDU2LjIsMzIsNTYuMnoiLz48cGF0aCBkPSJNMzIsNTYuMmMwLDUuMS05LjYsNC4yLTkuNS0yLjlDMTUuOCw0NCwyLjYsNDQuNywyLjYsNDQuN0MyNC40LDMyLjQsMzIsNTYuMiwzMiw1Ni4yeiIvPjwvZz48ZyBmaWxsPSIjZmY4NzM2Ij48cGF0aCBkPSJtNTMuNCAxOC41Yy00IC43LTQuOSA2LjMtNC45IDYuM2w2IDUuM2MtMi4zLTUuOS0xLjEtMTEuNi0xLjEtMTEuNiIvPjxwYXRoIGQ9Im01MS4xIDEzLjVjLTQuNCAzLjktNS4xIDguNy01LjEgOC43bDYgNS4zYy0yLjQtNS44LS45LTE0LS45LTE0Ii8+PHBhdGggZD0ibTEwLjYgMTguNWM0IC43IDQuOSA2LjMgNC45IDYuM2wtNiA1LjNjMi4zLTUuOSAxLjEtMTEuNiAxLjEtMTEuNiIvPjxwYXRoIGQ9Im0xMi45IDEzLjVjNC40IDMuOSA1LjEgOC43IDUuMSA4LjdsLTYgNS4zYzIuNC01LjguOS0xNCAuOS0xNCIvPjwvZz48cGF0aCBkPSJtNTIuOCAzMS4xYy01LjctMS44LTEwLjktMy40LTEzLjguOS0yLjQgMy43LjcgOS40LjcgOS40IDExLjIgMS4yIDEzLjEtMTAuMyAxMy4xLTEwLjMiIGZpbGw9IiMzZTQzNDciLz48ZWxsaXBzZSBjeD0iNDMiIGN5PSIzNi4zIiByeD0iNC4yIiByeT0iNC4xIiBmaWxsPSIjZDVmZjgzIi8+PGcgZmlsbD0iIzNlNDM0NyI+PGVsbGlwc2UgY3g9IjQzIiBjeT0iMzYuMyIgcng9IjIuNyIgcnk9IjIuNyIvPjxwYXRoIGQ9Im0xMS4yIDMxLjFjNS43LTEuOCAxMC45LTMuNCAxMy43LjkgMi40IDMuNy0uNyA5LjQtLjcgOS40LTExLjEgMS4yLTEzLTEwLjMtMTMtMTAuMyIvPjwvZz48ZWxsaXBzZSBjeD0iMjEiIGN5PSIzNi4zIiByeD0iNC4yIiByeT0iNC4xIiBmaWxsPSIjZDVmZjgzIi8+PGcgZmlsbD0iIzNlNDM0NyI+PGVsbGlwc2UgY3g9IjIxIiBjeT0iMzYuMyIgcng9IjIuNyIgcnk9IjIuNyIvPjxwYXRoIGQ9Im00MS4yIDQ3LjljLS43LTIuMy0xLjgtNC40LTMtNi41IDEuMSAyLjEgMiA0LjMgMi41IDYuNi41IDIuMy43IDQuNyAwIDYuOC0uNCAxLTEgMi0xLjggMi42LS44LjYtMS44IDEtMi43IDEtLjkgMC0xLjktLjMtMi41LTEtLjYtLjctLjktMS42LS44LTIuNmwtLjkuMmgtLjljMCAxLS4yIDEuOS0uOCAyLjYtLjYuNy0xLjUgMS0yLjUgMS0uOSAwLTEuOS0uNC0yLjctMS0uOC0uNi0xLjQtMS42LTEuOC0yLjYtLjgtMi4xLS42LTQuNiAwLTYuOC41LTIuMyAxLjUtNC41IDIuNS02LjYtMS4yIDItMi4zIDQuMS0zIDYuNS0uNyAyLjMtMS4xIDQuOC0uNCA3LjMuMyAxLjIgMSAyLjQgMS45IDMuMy45LjkgMi4xIDEuNCAzLjQgMS41IDEuMi4xIDIuNi0uMiAzLjctMS4yLjMtLjIuNS0uNS43LS44LjIuMy40LjYuNy44IDEgMSAyLjQgMS4zIDMuNyAxLjIgMS4zLS4xIDIuNC0uNyAzLjQtMS41LjktLjkgMS42LTIgMS45LTMuMy41LTIuNi4xLTUuMi0uNi03LjUiLz48cGF0aCBkPSJtMzcuNiA1MC4zYy0xLjEtMS4xLTQuNS0xLjItNS42LTEuMi0xIDAtNC41LjEtNS42IDEuMi0uOC44LS4yIDIuOCAxLjkgNC41IDEuMyAxLjEgMi42IDEuNCAzLjYgMS40IDEgMCAyLjMtLjMgMy42LTEuNCAyLjMtMS43IDIuOS0zLjcgMi4xLTQuNSIvPjwvZz48L3N2Zz4="; > + > +function shouldButtonHaveCustomStyling(selector, verifyFn, icon, message) { The title of this function seems to suggest it should return true if the button should have custom styling and false otherwise. Could you maybe rename it to something like `verifyButtonProperties` and add jsdoc? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:8 (Diff revision 2) > +const ENCODED_IMAGE_DATA = "PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHZpZXdCb3g9IjAgMCA2NCA2NCIgZW5hYmxlLWJhY2tncm91bmQ9Im5ldyAwIDAgNjQgNjQiPjxwYXRoIGQ9Im01NS45IDMyLjFsLTIyLjctMTQuOWMwIDAgMTIuOS0xNy40IDE5LjQtMTQuOSAzLjEgMS4xIDUuNCAyNS4xIDMuMyAyOS44IiBmaWxsPSIjM2U0MzQ3Ii8+PHBhdGggZD0ibTU0LjkgMzMuOWwtOS00LjFjMCAwLTUuMy0xNCA2LjEtMjQuMSAyLjQgMiA1LjEgMjUgMi45IDI4LjIiIGZpbGw9IiNmZmYiLz48cGF0aCBkPSJtOC4xIDMyLjFsMjIuNi0xNC45YzAgMC0xMi45LTE3LjQtMTkuNC0xNC45LTMgMS4xLTUuMyAyNS4xLTMuMiAyOS44IiBmaWxsPSIjM2U0MzQ3Ii8+PHBhdGggZD0ibTkuMSAzMy45bDktNC4xYzAgMCA1LjMtMTQtNi4xLTI0LjEtMi40IDItNS4xIDI1LTIuOSAyOC4yIiBmaWxsPSIjZmZmIi8+PHBhdGggZD0iTTMyLDEzQzE4LjksMTMsMiwzMy42LDIsNDUuNEMyMC41LDQ1LjQsMTkuNyw2MiwzMiw2MnMxMS41LTE2LjYsMzAtMTYuNkM2MiwzMy42LDQ1LjEsMTMsMzIsMTN6IiBmaWxsPSIjZmY4NzM2Ii8+PGcgZmlsbD0iI2ZmZiI+PHBhdGggZD0iTTMyLDU2LjJjMCw1LjEsOS42LDQuMiw5LjUtMi45YzYuNy05LjQsMTkuOS04LjcsMTkuOS04LjdDMzkuNiwzMi40LDMyLDU2LjIsMzIsNTYuMnoiLz48cGF0aCBkPSJNMzIsNTYuMmMwLDUuMS05LjYsNC4yLTkuNS0yLjlDMTUuOCw0NCwyLjYsNDQuNywyLjYsNDQuN0MyNC40LDMyLjQsMzIsNTYuMiwzMiw1Ni4yeiIvPjwvZz48ZyBmaWxsPSIjZmY4NzM2Ij48cGF0aCBkPSJtNTMuNCAxOC41Yy00IC43LTQuOSA2LjMtNC45IDYuM2w2IDUuM2MtMi4zLTUuOS0xLjEtMTEuNi0xLjEtMTEuNiIvPjxwYXRoIGQ9Im01MS4xIDEzLjVjLTQuNCAzLjktNS4xIDguNy01LjEgOC43bDYgNS4zYy0yLjQtNS44LS45LTE0LS45LTE0Ii8+PHBhdGggZD0ibTEwLjYgMTguNWM0IC43IDQuOSA2LjMgNC45IDYuM2wtNiA1LjNjMi4zLTUuOSAxLjEtMTEuNiAxLjEtMTEuNiIvPjxwYXRoIGQ9Im0xMi45IDEzLjVjNC40IDMuOSA1LjEgOC43IDUuMSA4LjdsLTYgNS4zYzIuNC01LjguOS0xNCAuOS0xNCIvPjwvZz48cGF0aCBkPSJtNTIuOCAzMS4xYy01LjctMS44LTEwLjktMy40LTEzLjguOS0yLjQgMy43LjcgOS40LjcgOS40IDExLjIgMS4yIDEzLjEtMTAuMyAxMy4xLTEwLjMiIGZpbGw9IiMzZTQzNDciLz48ZWxsaXBzZSBjeD0iNDMiIGN5PSIzNi4zIiByeD0iNC4yIiByeT0iNC4xIiBmaWxsPSIjZDVmZjgzIi8+PGcgZmlsbD0iIzNlNDM0NyI+PGVsbGlwc2UgY3g9IjQzIiBjeT0iMzYuMyIgcng9IjIuNyIgcnk9IjIuNyIvPjxwYXRoIGQ9Im0xMS4yIDMxLjFjNS43LTEuOCAxMC45LTMuNCAxMy43LjkgMi40IDMuNy0uNyA5LjQtLjcgOS40LTExLjEgMS4yLTEzLTEwLjMtMTMtMTAuMyIvPjwvZz48ZWxsaXBzZSBjeD0iMjEiIGN5PSIzNi4zIiByeD0iNC4yIiByeT0iNC4xIiBmaWxsPSIjZDVmZjgzIi8+PGcgZmlsbD0iIzNlNDM0NyI+PGVsbGlwc2UgY3g9IjIxIiBjeT0iMzYuMyIgcng9IjIuNyIgcnk9IjIuNyIvPjxwYXRoIGQ9Im00MS4yIDQ3LjljLS43LTIuMy0xLjgtNC40LTMtNi41IDEuMSAyLjEgMiA0LjMgMi41IDYuNi41IDIuMy43IDQuNyAwIDYuOC0uNCAxLTEgMi0xLjggMi42LS44LjYtMS44IDEtMi43IDEtLjkgMC0xLjktLjMtMi41LTEtLjYtLjctLjktMS42LS44LTIuNmwtLjkuMmgtLjljMCAxLS4yIDEuOS0uOCAyLjYtLjYuNy0xLjUgMS0yLjUgMS0uOSAwLTEuOS0uNC0yLjctMS0uOC0uNi0xLjQtMS42LTEuOC0yLjYtLjgtMi4xLS42LTQuNiAwLTYuOC41LTIuMyAxLjUtNC41IDIuNS02LjYtMS4yIDItMi4zIDQuMS0zIDYuNS0uNyAyLjMtMS4xIDQuOC0uNCA3LjMuMyAxLjIgMSAyLjQgMS45IDMuMy45LjkgMi4xIDEuNCAzLjQgMS41IDEuMi4xIDIuNi0uMiAzLjctMS4yLjMtLjIuNS0uNS43LS44LjIuMy40LjYuNy44IDEgMSAyLjQgMS4zIDMuNyAxLjIgMS4zLS4xIDIuNC0uNyAzLjQtMS41LjktLjkgMS42LTIgMS45LTMuMy41LTIuNi4xLTUuMi0uNi03LjUiLz48cGF0aCBkPSJtMzcuNiA1MC4zYy0xLjEtMS4xLTQuNS0xLjItNS42LTEuMi0xIDAtNC41LjEtNS42IDEuMi0uOC44LS4yIDIuOCAxLjkgNC41IDEuMyAxLjEgMi42IDEuNCAzLjYgMS40IDEgMCAyLjMtLjMgMy42LTEuNCAyLjMtMS43IDIuOS0zLjcgMi4xLTQuNSIvPjwvZz48L3N2Zz4="; > + > +function shouldButtonHaveCustomStyling(selector, verifyFn, icon, message) { > + try { > + let element; > + if (selector == "#bookmarks-menu-button > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon") { Could you add a comment explaining why this selector needs to be different from all the others? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:28 (Diff revision 2) > +function verifyButtonWithoutCustomStyling(selector, icon, message) { > + shouldButtonHaveCustomStyling(selector, (result, message) => ok(!result, message), icon, message); > +} I think it's a bit confusing having the same parameter name `message` for the function and the callback you are passing in. You might not need to pass the message to `shouldButtonHaveCustomStyling`. function verifyButtonWithoutCustomStyling(selector, icon, message) { shouldButtonHaveCustomStyling(selector, result => ok(!result, message), icon); } And for the function below: function verifyButtonWithCustomStyling(selector, icon, message) { shouldButtonHaveCustomStyling(selector, result => ok(result, message), icon); } Would it work to rewrite functions like this? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:61 (Diff revision 2) > + }; > + let files = { > + "fox.svg": imageBufferFromDataURI(ENCODED_IMAGE_DATA), > + }; > + > + let buttons = [ Perhaps something like `const ICON_INFO`? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:67 (Diff revision 2) > + ["bookmark_menu", "#bookmarks-menu-button > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon"], > + ["downloads", "#downloads-button", "downloads-button"], > + ["home", "#home-button", "home-button"], > + ["app_menu", "#PanelUI-menu-button"], > + ["cut", "#cut-button", "edit-controls"], > + ["copy", "#copy-button"], Could you add a comment explaining how this array is structured? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:139 (Diff revision 2) > + } finally { > + CustomizableUI.reset(); > + window.restore(); > + } Would it work to have a teardown task at the end instead of wrapping all of this in a try/catch? If so I think that would be a little cleaner. Something like: add_task(function* teardown() { CustomizableUI.reset(); window.restore(); });

Comment on attachment 8842979 [details] Bug 1343921 - Implement support for custom icons through the Theming API. https://reviewboard.mozilla.org/r/116708/#review118686 This is looking great so far :-) I'm curious to hear what you think about the suggestion I made below. Something else is to think about is the rebase you'll have to do today: where will the icons and their definitions live? Ideally somewhere in browser/ but where and how? Perhaps Matt knows? ::: browser/components/extensions/ext-theme.js:43 (Diff revision 3) > + "subscribe": "--subscribe-icon", > + "text_encoding": "--text_encoding-icon", > + "email_link": "--email_link-icon", > + "forget": "--forget-icon", > + "pocket": "--pocket-icon", > +}; It looks like you don't need this map at all - just an array (or `Set([...])`) called `ICON_NAMES` ::: browser/components/extensions/ext-theme.js:149 (Diff revision 3) > } > } > > + loadIcons(icons) { > + let iconNames = Object.keys(ICONS_TO_VARIABLE_MAP); > + for (let icon of Object.keys(icons)) { nit: `Object.getOwnPropertyNames(icons)` ::: browser/components/extensions/ext-theme.js:154 (Diff revision 3) > + for (let icon of Object.keys(icons)) { > + let val = icons[icon]; > + if (!val) { > + continue; > + } > + if (iconNames.includes(icon)) { ```js if (!val || !ICON_NAMES.has(icon)) continue; ``` ::: browser/components/extensions/ext-theme.js:155 (Diff revision 3) > + let val = icons[icon]; > + if (!val) { > + continue; > + } > + if (iconNames.includes(icon)) { > + let variableName = ICONS_TO_VARIABLE_MAP[icon]; ```js let cssVar = `--${icon}-icon`; ```

Comment on attachment 8843057 [details] Bug 1343921 - Put the icon support behind a pref so it can be enabled once we are satisfied with the performance. https://reviewboard.mozilla.org/r/116808/#review118698 Not a - per sé, but MozReview will get the least unhappy this way...

Comment on attachment 8842979 [details] Bug 1343921 - Implement support for custom icons through the Theming API. https://reviewboard.mozilla.org/r/116708/#review119212 At first I was a bit on edge about stuffing the icon setting logic in LightweightThemeConsumer.jsm, but now I'm actually really happy about it! We'll investigate the Category Manager approach later on, if that becomes necessary. I am interested in the performance implications of using CSS string matching operators, but that's also for later. ::: toolkit/components/extensions/ext-theme.js:11 (Diff revision 4) > "resource://gre/modules/Preferences.jsm"); > > // WeakMap[Extension -> Theme] > let themeMap = new WeakMap(); > > +const ICONS = Preferences.get("extensions.webextensions.themes.icons.buttons", nit: I think you can leave this one a single line. ::: toolkit/components/extensions/ext-theme.js:117 (Diff revision 4) > } > } > } > > + loadIcons(icons) { > + if (!Preferences.get("extensions.webextensions.themes.icons.enabled")) { I forgot why we needed this to be behind a pref?

(In reply to Mike de Boer [:mikedeboer] from comment #14) > > + loadIcons(icons) { > > + if (!Preferences.get("extensions.webextensions.themes.icons.enabled")) { > > I forgot why we needed this to be behind a pref? We moved it behind a pref while we research the performance implications. This way we can enable the API before we enable the icon support, only enabling the icon support once we are satisfied with the performance characteristics.

Comment on attachment 8842979 [details] Bug 1343921 - Implement support for custom icons through the Theming API. https://reviewboard.mozilla.org/r/116708/#review119280 Nice! ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:14 (Diff revision 4) > + * within the DOM. > + * @param {function} resultModifierFn Used to invert the result, if needed. > + * @param {string} message The message that is printed to the console > + * by the verifyFn. > + */ > +function verifyButtonProperties(selector, resultModifierFn, message) { Nit: instead of taking a `resultModifierFn`, could you just have this take a boolean `shouldHaveCustomStyling` and use that to modify how you call `ok`? ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:140 (Diff revision 4) > + for (let button of ICON_INFO) { > + let iconInfo = icons.find(arr => arr[0] == button[0]); > + if (iconInfo[1]) { > + verifyButtonWithCustomStyling(button[1], > + `The ${button[1]} should have it's icon customized in the toolbar`); > + } else { > + verifyButtonWithoutCustomStyling(button[1], > + `The ${button[1]} should not have it's icon customized in the toolbar`); > + } > + } Could you move this to a separate function since it's basically repeated below. ::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:140 (Diff revision 4) > + for (let button of ICON_INFO) { > + let iconInfo = icons.find(arr => arr[0] == button[0]); > + if (iconInfo[1]) { > + verifyButtonWithCustomStyling(button[1], > + `The ${button[1]} should have it's icon customized in the toolbar`); > + } else { > + verifyButtonWithoutCustomStyling(button[1], > + `The ${button[1]} should not have it's icon customized in the toolbar`); > + } > + } Could you move this to a separate function? It looks like it's repeated again below and the for loop above could also potentially be moved.

Gr, sorry about the redundant comment.

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1666512ef081 Implement support for custom icons through the Theming API. r=mattw,mikedeboer

Backed out for failing browser_ext_themes_chromeparity.js and browser_ext_themes_icons.js: https://hg.mozilla.org/integration/autoland/rev/25f5d0648983899de6f0bed21f3c3c72e09e4867 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1666512ef0813b231b82a713075e09b614143f6f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log browser_ext_themes_chromeparity.js: https://treeherder.mozilla.org/logviewer.html#?job_id=81975878&repo=autoland [task 2017-03-06T21:00:17.531310Z] 21:00:17 INFO - TEST-START | toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js [task 2017-03-06T21:00:18.090502Z] 21:00:18 INFO - TEST-INFO | started process screentopng [task 2017-03-06T21:00:19.242000Z] 21:00:19 INFO - TEST-INFO | screentopng: exit 0 [task 2017-03-06T21:00:19.242337Z] 21:00:19 INFO - Buffered messages logged at 21:00:17 [task 2017-03-06T21:00:19.242532Z] 21:00:19 INFO - Entering test bound setup [task 2017-03-06T21:00:19.242721Z] 21:00:19 INFO - Leaving test bound setup [task 2017-03-06T21:00:19.242929Z] 21:00:19 INFO - Entering test bound test_support_theme_frame [task 2017-03-06T21:00:19.243097Z] 21:00:19 INFO - Extension loaded [task 2017-03-06T21:00:19.243294Z] 21:00:19 INFO - Buffered messages logged at 21:00:18 [task 2017-03-06T21:00:19.245934Z] 21:00:19 INFO - Console message: [JavaScript Warning: "Property contained reference to invalid variable. Error in parsing value for ‘background-image’. Falling back to ‘initial’." {file: "chrome://browser/content/browser.css" line: 20 column: 664 source: " var(--lwt-header-image) "}] [task 2017-03-06T21:00:19.249308Z] 21:00:19 INFO - Console message: [JavaScript Warning: "Property contained reference to invalid variable. Error in parsing value for ‘background-image’. Falling back to ‘initial’." {file: "chrome://browser/skin/browser.css" line: 2972 column: 95855 source: " linear-gradient(transparent 2px, rgba(255,255,255,.4) 2px, rgba(255,255,255,.4)), var(--lwt-header-image)"}] [task 2017-03-06T21:00:19.256097Z] 21:00:19 INFO - Console message: [JavaScript Warning: "Property contained reference to invalid variable. Error in parsing value for ‘background-image’. Falling back to ‘initial’." {file: "chrome://browser/skin/browser.css" line: 2980 column: 96285 source: " var(--lwt-header-image)"}] [task 2017-03-06T21:00:19.258493Z] 21:00:19 INFO - Console message: [JavaScript Warning: "Property contained reference to invalid variable. Error in parsing value for ‘background-image’. Falling back to ‘initial’." {file: "chrome://browser/skin/browser.css" line: 2972 column: 95855 source: " linear-gradient(transparent 2px, rgba(255,255,255,.4) 2px, rgba(255,255,255,.4)), var(--lwt-header-image)"}] [task 2017-03-06T21:00:19.260669Z] 21:00:19 INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js | LWT attribute should be set - true == true - [task 2017-03-06T21:00:19.268149Z] 21:00:19 INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js | LWT text color attribute should be set - "bright" == "bright" - [task 2017-03-06T21:00:19.269792Z] 21:00:19 INFO - Buffered messages finished [task 2017-03-06T21:00:19.274656Z] 21:00:19 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js | The backgroundImage should use face.png. Actual value is: none - false == true - JS frame :: chrome://mochitests/content/browser/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js :: test_support_theme_frame :: line 45 [task 2017-03-06T21:00:19.278517Z] 21:00:19 INFO - Stack trace: [task 2017-03-06T21:00:19.279460Z] 21:00:19 INFO - chrome://mochitests/content/browser/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js:test_support_theme_frame:45 [task 2017-03-06T21:00:19.280312Z] 21:00:19 INFO - process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23 [task 2017-03-06T21:00:19.283674Z] 21:00:19 INFO - walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7 [task 2017-03-06T21:00:19.285313Z] 21:00:19 INFO - Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11 [task 2017-03-06T21:00:19.287047Z] 21:00:19 INFO - schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7 [task 2017-03-06T21:00:19.289722Z] 21:00:19 INFO - completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7 [task 2017-03-06T21:00:19.291206Z] 21:00:19 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:324:15 [task 2017-03-06T21:00:19.292806Z] 21:00:19 INFO - promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7 [task 2017-03-06T21:00:19.294454Z] 21:00:19 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15 [task 2017-03-06T21:00:19.296238Z] 21:00:19 INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3 [task 2017-03-06T21:00:19.299052Z] 21:00:19 INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14 [task 2017-03-06T21:00:19.300537Z] 21:00:19 INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12 [task 2017-03-06T21:00:19.302018Z] 21:00:19 INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16 [task 2017-03-06T21:00:19.303695Z] 21:00:19 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15 [task 2017-03-06T21:00:19.307543Z] 21:00:19 INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3 [task 2017-03-06T21:00:19.309148Z] 21:00:19 INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14 [task 2017-03-06T21:00:19.310720Z] 21:00:19 INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12 [task 2017-03-06T21:00:19.312391Z] 21:00:19 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:735:9 [task 2017-03-06T21:00:19.313930Z] 21:00:19 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7 [task 2017-03-06T21:00:19.315501Z] 21:00:19 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:791:59 Failure log browser_ext_themes_icons.js: https://treeherder.mozilla.org/logviewer.html#?job_id=81976291&repo=autoland [task 2017-03-06T21:10:30.943022Z] 21:10:30 INFO - Console message: [JavaScript Warning: "Property contained reference to invalid variable. Error in parsing value for ‘background-image’. Falling back to ‘initial’." {file: "chrome://browser/skin/browser.css" line: 2972 column: 95855 source: " linear-gradient(transparent 2px, rgba(255,255,255,.4) 2px, rgba(255,255,255,.4)), var(--lwt-header-image)"}] [task 2017-03-06T21:10:30.946082Z] 21:10:30 INFO - listStyleImage for fox.svg is none [task 2017-03-06T21:10:30.947924Z] 21:10:30 INFO - Buffered messages finished [task 2017-03-06T21:10:30.949988Z] 21:10:30 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_themes_icons.js | The #back-button should have it's icon customized in the toolbar - Got false, expected true [task 2017-03-06T21:10:30.955515Z] 21:10:30 INFO - Stack trace: [task 2017-03-06T21:10:30.956890Z] 21:10:30 INFO - chrome://mochikit/content/browser-test.js:test_is:911 [task 2017-03-06T21:10:30.958625Z] 21:10:30 INFO - chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_themes_icons.js:verifyButtonProperties:35 [task 2017-03-06T21:10:30.960265Z] 21:10:30 INFO - chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_themes_icons.js:verifyButtonWithCustomStyling:62 [task 2017-03-06T21:10:30.962025Z] 21:10:30 INFO - chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_themes_icons.js:checkButtons:79 [task 2017-03-06T21:10:30.964945Z] 21:10:30 INFO - chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_themes_icons.js:runTestWithIcons:164 [task 2017-03-06T21:10:30.971131Z] 21:10:30 INFO - process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23 [task 2017-03-06T21:10:30.972776Z] 21:10:30 INFO - walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7 [task 2017-03-06T21:10:30.974314Z] 21:10:30 INFO - Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11 [task 2017-03-06T21:10:30.978779Z] 21:10:30 INFO - schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7 [task 2017-03-06T21:10:30.980746Z] 21:10:30 INFO - completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7 [task 2017-03-06T21:10:30.983979Z] 21:10:30 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:324:15 [task 2017-03-06T21:10:30.988853Z] 21:10:30 INFO - promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7 [task 2017-03-06T21:10:30.989234Z] 21:10:30 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15 [task 2017-03-06T21:10:30.989577Z] 21:10:30 INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3 [task 2017-03-06T21:10:30.991392Z] 21:10:30 INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14 [task 2017-03-06T21:10:30.994038Z] 21:10:30 INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12 [task 2017-03-06T21:10:30.996885Z] 21:10:30 INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16 [task 2017-03-06T21:10:30.999552Z] 21:10:30 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15 [task 2017-03-06T21:10:31.002141Z] 21:10:30 INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3 [task 2017-03-06T21:10:31.003777Z] 21:10:30 INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14 [task 2017-03-06T21:10:31.008549Z] 21:10:30 INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12 [task 2017-03-06T21:10:31.011478Z] 21:10:30 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:735:9 [task 2017-03-06T21:10:31.014379Z] 21:10:30 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7 [task 2017-03-06T21:10:31.018119Z] 21:10:30 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:791:59

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e4f9dfc7fee5 Implement support for custom icons through the Theming API. r=mattw,mikedeboer