Backed out changeset 91a30d78c074 (bug 1347207) for browser_ext_themes_experiment.js test failures
authorAndreea Pavel <apavel@mozilla.com>
Thu, 09 Aug 2018 14:46:38 +0300
changeset 485875 911fa6559859562c137ae7aac506838e5326cce2
parent 485874 91a30d78c074af8d15dcab9d6069dbaacffc8615
child 485876 959260497f7c24b1d01d495b3bad5057649b7f95
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1347207
milestone63.0a1
backs out91a30d78c074af8d15dcab9d6069dbaacffc8615
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
Backed out changeset 91a30d78c074 (bug 1347207) for browser_ext_themes_experiment.js test failures
toolkit/components/extensions/parent/ext-theme.js
toolkit/components/extensions/schemas/theme.json
toolkit/components/extensions/test/browser/browser.ini
toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js
toolkit/modules/LightweightThemeConsumer.jsm
--- a/toolkit/components/extensions/parent/ext-theme.js
+++ b/toolkit/components/extensions/parent/ext-theme.js
@@ -1,11 +1,11 @@
 "use strict";
 
-/* global windowTracker, EventManager, EventEmitter, AddonManager */
+/* global windowTracker, EventManager, EventEmitter */
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 ChromeUtils.defineModuleGetter(this, "LightweightThemeManager",
                                "resource://gre/modules/LightweightThemeManager.jsm");
 
 var {
   getWinUtils,
@@ -31,43 +31,25 @@ let windowOverrides = new Map();
  */
 class Theme {
   /**
    * Creates a theme instance.
    *
    * @param {string} extension Extension that created the theme.
    * @param {Integer} windowId The windowId where the theme is applied.
    */
-  constructor({extension, details, windowId, experiment}) {
+  constructor({extension, details, windowId}) {
     this.extension = extension;
     this.details = details;
     this.windowId = windowId;
 
     this.lwtStyles = {
       icons: {},
     };
 
-    if (experiment) {
-      const canRunExperiment = !AppConstants.MOZ_ALLOW_LEGACY_EXTENSIONS &&
-        Services.prefs.getBoolPref("extensions.legacy.enabled");
-      if (canRunExperiment) {
-        this.lwtStyles.experimental = {
-          colors: {},
-          images: {},
-          properties: {},
-        };
-        const {baseURI} = this.extension;
-        if (experiment.stylesheet) {
-          experiment.stylesheet = baseURI.resolve(experiment.stylesheet);
-        }
-        this.experiment = experiment;
-      } else {
-        throw new Error("This extension is not allowed to run theme experiments");
-      }
-    }
     this.load();
   }
 
   /**
    * Loads a theme by reading the properties from the extension's manifest.
    * This method will override any currently applied theme.
    *
    * @param {Object} details Theme part of the manifest. Supported
@@ -101,19 +83,16 @@ class Theme {
         getWinUtils(windowTracker.getWindow(this.windowId)).outerWindowID;
       windowOverrides.set(this.windowId, this);
     } else {
       windowOverrides.clear();
       defaultTheme = this;
     }
     onUpdatedEmitter.emit("theme-updated", this.details, this.windowId);
 
-    if (this.experiment) {
-      lwtData.experiment = this.experiment;
-    }
     LightweightThemeManager.fallbackThemeData = this.lwtStyles;
     Services.obs.notifyObservers(null,
                                  "lightweight-theme-styling-update",
                                  JSON.stringify(lwtData));
   }
 
   /**
    * Helper method for loading colors found in the extension's manifest.
@@ -179,23 +158,17 @@ class Theme {
         case "popup_text":
         case "popup_border":
         case "popup_highlight":
         case "popup_highlight_text":
         case "ntp_background":
         case "ntp_text":
           this.lwtStyles[color] = cssColor;
           break;
-        default:
-          if (this.experiment && this.experiment.colors && color in this.experiment.colors) {
-            this.lwtStyles.experimental.colors[color] = cssColor;
-          }
-          break;
       }
-
     }
   }
 
   /**
    * Helper method for loading images found in the extension's manifest.
    *
    * @param {Object} images Dictionary mapping image properties to values.
    */
@@ -216,22 +189,16 @@ class Theme {
           break;
         }
         case "headerURL":
         case "theme_frame": {
           let resolvedURL = baseURI.resolve(val);
           this.lwtStyles.headerURL = resolvedURL;
           break;
         }
-        default: {
-          if (this.experiment && this.experiment.images && image in this.experiment.images) {
-            this.lwtStyles.experimental.images[image] = baseURI.resolve(val);
-          }
-          break;
-        }
       }
     }
   }
 
   /**
    * Helper method for loading icons found in the extension's manifest.
    *
    * @param {Object} icons Dictionary mapping icon properties to extension URLs.
@@ -313,22 +280,16 @@ class Theme {
             tiling.push("no-repeat");
           }
           for (let i = 0, l = this.lwtStyles.additionalBackgrounds.length; i < l; ++i) {
             tiling.push(val[i] || "no-repeat");
           }
           this.lwtStyles.backgroundsTiling = tiling.join(",");
           break;
         }
-        default: {
-          if (this.experiment && this.experiment.properties && property in this.experiment.properties) {
-            this.lwtStyles.experimental.properties[property] = val;
-          }
-          break;
-        }
       }
     }
   }
 
   static unload(windowId) {
     let lwtData = {
       theme: null,
     };
@@ -348,22 +309,20 @@ class Theme {
                                  JSON.stringify(lwtData));
   }
 }
 
 this.theme = class extends ExtensionAPI {
   onManifestEntry(entryName) {
     let {extension} = this;
     let {manifest} = extension;
-    let {theme, theme_experiment} = manifest;
 
     defaultTheme = new Theme({
       extension,
-      details: theme,
-      experiment: theme_experiment,
+      details: manifest.theme,
     });
   }
 
   onShutdown(reason) {
     if (reason === "APP_SHUTDOWN") {
       return;
     }
 
@@ -402,17 +361,16 @@ this.theme = class extends ExtensionAPI 
               return Promise.reject(`Invalid window ID: ${windowId}`);
             }
           }
 
           new Theme({
             extension,
             details,
             windowId,
-            experiment: this.extension.manifest.theme_experiment,
           });
         },
         reset: (windowId) => {
           if (windowId) {
             const browserWindow = windowTracker.getWindow(windowId, context);
             if (!browserWindow) {
               return Promise.reject(`Invalid window ID: ${windowId}`);
             }
--- a/toolkit/components/extensions/schemas/theme.json
+++ b/toolkit/components/extensions/schemas/theme.json
@@ -37,47 +37,16 @@
             "maxItems": 4,
             "items": {
               "type": "number"
             }
           }
         ]
       },
       {
-        "id": "ThemeExperiment",
-        "type": "object",
-        "properties": {
-          "stylesheet": {
-            "optional": true,
-            "$ref": "ExtensionURL"
-          },
-          "images": {
-            "type": "object",
-            "optional": true,
-            "additionalProperties": {
-              "type": "string"
-            }
-          },
-          "colors": {
-            "type": "object",
-            "optional": true,
-            "additionalProperties": {
-              "type": "string"
-            }
-          },
-          "properties": {
-            "type": "object",
-            "optional": true,
-            "additionalProperties": {
-              "type": "string"
-            }
-          }
-        }
-      },
-      {
         "id": "ThemeType",
         "type": "object",
         "properties": {
           "images": {
             "type": "object",
             "optional": true,
             "properties": {
               "additional_backgrounds": {
@@ -606,39 +575,26 @@
         "description": "Contents of manifest.json for a static theme",
         "$import": "manifest.ManifestBase",
         "properties": {
           "theme": {
             "$ref": "ThemeType"
           },
           "default_locale": {
             "type": "string",
-            "optional": true
-          },
-          "theme_experiment": {
-            "$ref": "ThemeExperiment",
-            "optional": true
+            "optional": "true"
           },
           "icons": {
             "type": "object",
             "optional": true,
             "patternProperties": {
               "^[1-9]\\d*$": { "type": "string" }
             }
           }
         }
-      },
-      {
-        "$extend": "WebExtensionManifest",
-        "properties": {
-          "theme_experiment": {
-            "$ref": "ThemeExperiment",
-            "optional": true
-          }
-        }
       }
     ]
   },
   {
     "namespace": "theme",
     "description": "The theme API allows customizing of visual elements of the browser.",
     "types": [
       {
--- a/toolkit/components/extensions/test/browser/browser.ini
+++ b/toolkit/components/extensions/test/browser/browser.ini
@@ -4,17 +4,16 @@ support-files =
 
 [browser_ext_management_themes.js]
 skip-if = verify
 [browser_ext_themes_alpha_accentcolor.js]
 [browser_ext_themes_chromeparity.js]
 [browser_ext_themes_dynamic_getCurrent.js]
 [browser_ext_themes_dynamic_onUpdated.js]
 [browser_ext_themes_dynamic_updates.js]
-[browser_ext_themes_experiment.js]
 [browser_ext_themes_getCurrent_differentExt.js]
 [browser_ext_themes_lwtsupport.js]
 [browser_ext_themes_multiple_backgrounds.js]
 [browser_ext_themes_ntp_colors.js]
 [browser_ext_themes_ntp_colors_perwindow.js]
 [browser_ext_themes_persistence.js]
 [browser_ext_themes_separators.js]
 [browser_ext_themes_static_onUpdated.js]
deleted file mode 100644
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js
+++ /dev/null
@@ -1,217 +0,0 @@
-"use strict";
-
-// This test checks whether the theme experiments work
-
-add_task(async function test_experiment_static_theme() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      theme: {
-        colors: {
-          some_color_property: "#ff00ff",
-        },
-        images: {
-          some_image_property: "background.jpg",
-        },
-        properties: {
-          some_random_property: "no-repeat",
-        }
-      },
-      theme_experiment: {
-        colors: {
-          some_color_property: "--some-color-property",
-        },
-        images: {
-          some_image_property: "--some-image-property",
-        },
-        properties: {
-          some_random_property: "--some-random-property",
-        }
-      }
-    },
-  });
-
-  const root = window.document.documentElement;
-
-  is(root.style.getPropertyValue("--some-color-property"), "",
-     "Color property should be unset");
-  is(root.style.getPropertyValue("--some-image-property"), "",
-     "Image property should be unset");
-  is(root.style.getPropertyValue("--some-random-property"), "",
-     "Generic Property should be unset.");
-
-  await extension.startup();
-
-  is(root.style.getPropertyValue("--some-color-property"), hexToCSS("#ff00ff"),
-     "Color property should be parsed and set.");
-  ok(root.style.getPropertyValue("--some-image-property").startsWith("url("),
-     "Image property should be parsed.");
-  ok(root.style.getPropertyValue("--some-image-property").endsWith("background.jpg)"),
-     "Image property should be set.");
-  is(root.style.getPropertyValue("--some-random-property"), "no-repeat",
-     "Generic Property should be set.");
-
-  await extension.unload();
-
-  is(root.style.getPropertyValue("--some-color-property"), "",
-     "Color property should be unset");
-  is(root.style.getPropertyValue("--some-image-property"), "",
-     "Image property should be unset");
-  is(root.style.getPropertyValue("--some-random-property"), "",
-     "Generic Property should be unset.");
-});
-
-add_task(async function test_experiment_dynamic_theme() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      permissions: ["theme"],
-      theme_experiment: {
-        colors: {
-          some_color_property: "--some-color-property",
-        },
-        images: {
-          some_image_property: "--some-image-property",
-        },
-        properties: {
-          some_random_property: "--some-random-property",
-        }
-      }
-    },
-    background() {
-      const theme = {
-        colors: {
-          some_color_property: "#ff00ff",
-        },
-        images: {
-          some_image_property: "background.jpg",
-        },
-        properties: {
-          some_random_property: "no-repeat",
-        }
-      };
-      browser.test.onMessage.addListener((msg) => {
-        if (msg === "update-theme") {
-          browser.theme.update(theme).then(() => {
-            browser.test.sendMessage("theme-updated");
-          });
-        } else {
-          browser.theme.reset().then(() => {
-            browser.test.sendMessage("theme-reset");
-          });
-        }
-      });
-    },
-  }); 
-  
-  await extension.startup();
-
-  const root = window.document.documentElement;
-
-  is(root.style.getPropertyValue("--some-color-property"), "",
-     "Color property should be unset");
-  is(root.style.getPropertyValue("--some-image-property"), "",
-     "Image property should be unset");
-  is(root.style.getPropertyValue("--some-random-property"), "",
-     "Generic Property should be unset.");
-
-  extension.sendMessage("update-theme");
-  await extension.awaitMessage("theme-updated");
-
-  is(root.style.getPropertyValue("--some-color-property"), hexToCSS("#ff00ff"),
-     "Color property should be parsed and set.");
-  ok(root.style.getPropertyValue("--some-image-property").startsWith("url("),
-     "Image property should be parsed.");
-  ok(root.style.getPropertyValue("--some-image-property").endsWith("background.jpg)"),
-     "Image property should be set.");
-  is(root.style.getPropertyValue("--some-random-property"), "no-repeat",
-     "Generic Property should be set.");
-
-
-  extension.sendMessage("reset-theme");
-  await extension.awaitMessage("theme-reset");
-
-  is(root.style.getPropertyValue("--some-color-property"), "",
-     "Color property should be unset");
-  is(root.style.getPropertyValue("--some-image-property"), "",
-     "Image property should be unset");
-  is(root.style.getPropertyValue("--some-random-property"), "",
-     "Generic Property should be unset.");
-
-  extension.sendMessage("update-theme");
-  await extension.awaitMessage("theme-updated");
-
-  is(root.style.getPropertyValue("--some-color-property"), hexToCSS("#ff00ff"),
-     "Color property should be parsed and set.");
-  ok(root.style.getPropertyValue("--some-image-property").startsWith("url("),
-     "Image property should be parsed.");
-  ok(root.style.getPropertyValue("--some-image-property").endsWith("background.jpg)"),
-     "Image property should be set.");
-  is(root.style.getPropertyValue("--some-random-property"), "no-repeat",
-     "Generic Property should be set.");
-
-  await extension.unload();
-
-  is(root.style.getPropertyValue("--some-color-property"), "",
-     "Color property should be unset");
-  is(root.style.getPropertyValue("--some-image-property"), "",
-     "Image property should be unset");
-  is(root.style.getPropertyValue("--some-random-property"), "",
-     "Generic Property should be unset.");
-});
-
-add_task(async function test_experiment_stylesheet() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      theme: { 
-        colors: {
-          menu_button_background: "#ff00ff",
-        },
-      },
-      theme_experiment: {
-        stylesheet: "experiment.css",
-        colors: {
-          menu_button_background: "--menu-button-background",
-        },
-      },
-    },
-    files: {
-      "experiment.css": `#PanelUI-menu-button {
-        background-color: var(--menu-button-background);
-        fill: white;
-      }`,
-    }
-  });
-
-  const root = window.document.documentElement;
-  const menuButton = document.getElementById("PanelUI-menu-button");
-  const computedStyle = window.getComputedStyle(menuButton);
-  const expectedColor = hexToCSS("#ff00ff");
-  const expectedFill = hexToCSS("#ffffff");
-
-  is(root.style.getPropertyValue("--menu-button-background"), "",
-     "Variable should be unset");
-  isnot(computedStyle.backgroundColor, expectedColor,
-        "Menu button should not have custom background");
-  isnot(computedStyle.fill, expectedFill,
-        "Menu button should not have stylesheet fill");
-
-  await extension.startup();
-
-  // Wait for stylesheet load.
-  await BrowserTestUtils.waitForCondition(() => computedStyle.fill === expectedFill);
-
-  is(root.style.getPropertyValue("--menu-button-background"), expectedColor,
-     "Variable should be parsed and set.");
-  is(computedStyle.backgroundColor, expectedColor,
-     "Menu button should be have correct background");
-  is(computedStyle.fill, expectedFill,
-     "Menu button should be have correct fill");
-
-  await extension.unload();
-
-  is(root.style.getPropertyValue("--menu-button-background"), "",
-     "Variable should be unset");
-  isnot(computedStyle.backgroundColor, expectedColor,
-        "Menu button should not have custom background");
-  isnot(computedStyle.fill, expectedFill,
-        "Menu button should not have stylesheet fill");
-});
--- a/toolkit/modules/LightweightThemeConsumer.jsm
+++ b/toolkit/modules/LightweightThemeConsumer.jsm
@@ -132,24 +132,24 @@ LightweightThemeConsumer.prototype = {
   _active: false,
 
   observe(aSubject, aTopic, aData) {
     if (aTopic != "lightweight-theme-styling-update")
       return;
 
     let parsedData = JSON.parse(aData);
     if (!parsedData) {
-      parsedData = { theme: null, experiment: null };
+      parsedData = { theme: null };
     }
 
     if (parsedData.window && parsedData.window !== this._winId) {
       return;
     }
 
-    this._update(parsedData.theme, parsedData.experiment);
+    this._update(parsedData.theme);
   },
 
   handleEvent(aEvent) {
     switch (aEvent.type) {
       case "resolutionchange":
         if (this._active) {
           this._update(this._lastData);
         }
@@ -158,120 +158,67 @@ LightweightThemeConsumer.prototype = {
         Services.obs.removeObserver(this, "lightweight-theme-styling-update");
         Services.ppmm.sharedData.delete(`theme/${this._winId}`);
         this._win.removeEventListener("resolutionchange", this);
         this._win = this._doc = null;
         break;
     }
   },
 
-  _update(theme, experiment) {
-    this._lastData = theme;
-    if (theme) {
-      theme = LightweightThemeImageOptimizer.optimize(theme, this._win.screen);
+  _update(aData) {
+    this._lastData = aData;
+    if (aData) {
+      aData = LightweightThemeImageOptimizer.optimize(aData, this._win.screen);
     }
 
-    let active = this._active = theme && theme.id !== DEFAULT_THEME_ID;
+    let active = this._active = aData && aData.id !== DEFAULT_THEME_ID;
 
-    if (!theme) {
-      theme = {};
+    if (!aData) {
+      aData = {};
     }
 
     let root = this._doc.documentElement;
 
-    if (active && theme.headerURL) {
+    if (active && aData.headerURL) {
       root.setAttribute("lwtheme-image", "true");
     } else {
       root.removeAttribute("lwtheme-image");
     }
 
-    if (active && theme.icons) {
-      let activeIcons = Object.keys(theme.icons).join(" ");
+    if (active && aData.icons) {
+      let activeIcons = Object.keys(aData.icons).join(" ");
       root.setAttribute("lwthemeicons", activeIcons);
     } else {
       root.removeAttribute("lwthemeicons");
     }
 
     for (let icon of ICONS) {
-      let value = theme.icons ? theme.icons[`--${icon}-icon`] : null;
+      let value = aData.icons ? aData.icons[`--${icon}-icon`] : null;
       _setImage(root, active, `--${icon}-icon`, value);
     }
 
-    this._setExperiment(active, experiment, theme.experimental);
-    _setImage(root, active, "--lwt-header-image", theme.headerURL);
-    _setImage(root, active, "--lwt-footer-image", theme.footerURL);
-    _setImage(root, active, "--lwt-additional-images", theme.additionalBackgrounds);
-    _setProperties(root, active, theme);
+    _setImage(root, active, "--lwt-header-image", aData.headerURL);
+    _setImage(root, active, "--lwt-footer-image", aData.footerURL);
+    _setImage(root, active, "--lwt-additional-images", aData.additionalBackgrounds);
+    _setProperties(root, active, aData);
 
     if (active) {
       root.setAttribute("lwtheme", "true");
     } else {
       root.removeAttribute("lwtheme");
       root.removeAttribute("lwthemetextcolor");
     }
 
-    if (active && theme.footerURL)
+    if (active && aData.footerURL)
       root.setAttribute("lwthemefooter", "true");
     else
       root.removeAttribute("lwthemefooter");
 
-    let contentThemeData = _getContentProperties(this._doc, active, theme);
+    let contentThemeData = _getContentProperties(this._doc, active, aData);
     Services.ppmm.sharedData.set(`theme/${this._winId}`, contentThemeData);
-  },
-
-  _setExperiment(active, experiment, properties) {
-    const root = this._doc.documentElement;
-    if (this._lastExperimentData) {
-      const { stylesheet, usedVariables } = this._lastExperimentData;
-      if (stylesheet) {
-        stylesheet.remove();
-      }
-      if (usedVariables) {
-        for (const variable of usedVariables) {
-          _setProperty(root, false, variable);
-        }
-      }
-    }
-    if (active && experiment) {
-      this._lastExperimentData = {};
-      if (experiment.stylesheet) {
-        /* Stylesheet URLs are validated using WebExtension schemas */
-        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;
-        let stylesheet = this._doc.createProcessingInstruction("xml-stylesheet",
-          stylesheetAttr);
-        this._doc.insertBefore(stylesheet, root);
-        this._lastExperimentData.stylesheet = stylesheet;
-      }
-      let usedVariables = [];
-      if (properties.colors) {
-        for (const property in properties.colors) {
-          const cssVariable = experiment.colors[property];
-          const value = _sanitizeCSSColor(root.ownerDocument, properties.colors[property]);
-          _setProperty(root, active, cssVariable, value);
-          usedVariables.push(cssVariable);
-        }
-      }
-      if (properties.images) {
-        for (const property in properties.images) {
-          const cssVariable = experiment.images[property];
-          _setProperty(root, active, cssVariable, `url(${properties.images[property]})`);
-          usedVariables.push(cssVariable);
-        }
-      }
-      if (properties.properties) {
-        for (const property in properties.properties) {
-          const cssVariable = experiment.properties[property];
-          _setProperty(root, active, cssVariable, properties.properties[property]);
-          usedVariables.push(cssVariable);
-        }
-      }
-      this._lastExperimentData.usedVariables = usedVariables;
-    } else {
-      this._lastExperimentData = null;
-    }
   }
 };
 
 function _getContentProperties(doc, active, data) {
   if (!active) {
     return {};
   }
   let properties = {};