Bug 1570715 - Treat (deprecation) warnings as errors r=rpl

Add new preference `extensions.webextensions.warnings-as-errors` that
defaults to `true` in tests. Tests that expect warnings are modified
to briefly flip the pref for the specific part of the test that needs
an exception.

As part of the refactor, log entries for schema entries that contain
`"onError": "warn"` will now be prefixed by "Warning" instead of
"Error", to be consistent with the change from bug 1495908.

Differential Revision: https://phabricator.services.mozilla.com/D40548

--HG--
extra : moz-landing-system : lando

browser/components/extensions/test/browser/browser_ext_browserAction_simple.js

@@ -45,7 +45,9 @@ add_task(async function() {
     ]);
   });
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   // Do this a few times to make sure the pop-up is reloaded each time.
   for (let i = 0; i < 3; i++) {

browser/components/extensions/test/browser/browser_ext_commands_onCommand.js

@@ -238,8 +238,9 @@ add_task(async function test_user_defined_commands() {
       },
     ]);
   });
-
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   await extension.awaitMessage("ready");
 
   async function runTest(window) {
@@ -323,7 +324,11 @@ add_task(async function test_user_defined_commands() {
     incognitoOverride: "spanning",
     background,
   });
+
+  // unrecognized_property in manifest triggers warning.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   await extension.awaitMessage("ready");
   keysetID = `ext-keyset-id-${makeWidgetId(extension.id)}`;
 

browser/components/extensions/test/browser/browser_ext_pageAction_simple.js

@@ -55,7 +55,9 @@ add_task(async function test_pageAction_basic() {
     ]);
   });
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   await extension.awaitMessage("page-action-shown");
 
   let elem = await getPageActionButton(extension);

devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_warnings.js

@@ -14,6 +14,7 @@ add_task(async function() {
   const { document, tab, window } = await openAboutDebugging();
   await selectThisFirefoxPage(document, window.AboutDebugging.store);
 
+  await pushPref("extensions.webextensions.warnings-as-errors", false);
   await installTemporaryExtensionFromXPI(
     {
       id: EXTENSION_ID,
@@ -25,6 +26,7 @@ add_task(async function() {
     },
     document
   );
+  await SpecialPowers.popPrefEnv();
 
   info("Wait until a debug target item appears");
   await waitUntil(() => findDebugTargetByText(EXTENSION_NAME, document));

devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_message_close.js

@@ -54,6 +54,7 @@ async function testCloseMessageWithButton(warningMessage, doc) {
 }
 
 async function installExtensionWithWarning(doc) {
+  await pushPref("extensions.webextensions.warnings-as-errors", false);
   await installTemporaryExtensionFromXPI(
     {
       id: EXTENSION_ID,
@@ -65,6 +66,7 @@ async function installExtensionWithWarning(doc) {
     },
     doc
   );
+  await SpecialPowers.popPrefEnv();
 
   info("Wait until a debug target item appears");
   await waitUntil(() => findDebugTargetByText(EXTENSION_NAME, doc));

testing/mochitest/tests/SimpleTest/ExtensionTestUtils.js

@@ -138,4 +138,18 @@ ExtensionTestUtils.loadExtension = function(ext)
 
   SimpleTest.info(`Extension loaded`);
   return extension;
-}
+};
+
+ExtensionTestUtils.failOnSchemaWarnings = (warningsAsErrors = true) => {
+  let prefName = "extensions.webextensions.warnings-as-errors";
+  SpecialPowers.setBoolPref(prefName, warningsAsErrors);
+  if (!warningsAsErrors) {
+    let registerCleanup;
+    if (typeof registerCleanupFunction != "undefined") {
+      registerCleanup = registerCleanupFunction;
+    } else {
+      registerCleanup = SimpleTest.registerCleanupFunction.bind(SimpleTest);
+    }
+    registerCleanup(() => SpecialPowers.setBoolPref(prefName, true));
+  }
+};

testing/profiles/common/user.js

@@ -41,6 +41,8 @@ user_pref("extensions.legacy.enabled", true);
 user_pref("extensions.update.enabled", false);
 // Prevent network access for recommendations by default. The payload is {"results":[]}.
 user_pref("extensions.getAddons.discovery.api_url", "data:;base64,eyJyZXN1bHRzIjpbXX0%3D");
+// Treat WebExtension API/schema warnings as errors.
+user_pref("extensions.webextensions.warnings-as-errors", true);
 // Disable useragent updates.
 user_pref("general.useragent.updates.enabled", false);
 // Ensure WR doesn't get enabled in tests unless we do it explicitly with the MOZ_WEBRENDER envvar.

testing/profiles/xpcshell/user.js

@@ -5,6 +5,8 @@ user_pref("app.normandy.api_url", "https://%(server)s/selfsupport-dummy/");
 user_pref("browser.safebrowsing.downloads.remote.url", "https://%(server)s/safebrowsing-dummy");
 user_pref("browser.search.geoip.url", "https://%(server)s/geoip-dummy");
 user_pref("extensions.systemAddon.update.url", "http://%(server)s/dummy-system-addons.xml");
+// Treat WebExtension API/schema warnings as errors.
+user_pref("extensions.webextensions.warnings-as-errors", true);
 // Always use network provider for geolocation tests
 // so we bypass the OSX dialog raised by the corelocation provider
 user_pref("geo.provider.testing", true);

toolkit/components/extensions/ExtensionXPCShellUtils.jsm

@@ -955,6 +955,16 @@ var ExtensionTestUtils = {
     return new ExternallyInstalledWrapper(this.currentScope, id);
   },
 
+  failOnSchemaWarnings(warningsAsErrors = true) {
+    let prefName = "extensions.webextensions.warnings-as-errors";
+    Services.prefs.setBoolPref(prefName, warningsAsErrors);
+    if (!warningsAsErrors) {
+      this.currentScope.registerCleanupFunction(() => {
+        Services.prefs.setBoolPref(prefName, true);
+      });
+    }
+  },
+
   get remoteContentScripts() {
     return REMOTE_CONTENT_SCRIPTS;
   },

toolkit/components/extensions/Schemas.jsm

@@ -50,6 +50,13 @@ XPCOMUtils.defineLazyGetter(
   () => ExtensionParent.StartupCache
 );
 
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "treatWarningsAsErrors",
+  "extensions.webextensions.warnings-as-errors",
+  false
+);
+
 var EXPORTED_SYMBOLS = ["SchemaRoot", "Schemas"];
 
 const KEY_CONTENT_SCHEMAS = "extensions-framework/schemas/content";
@@ -1193,7 +1200,31 @@ class Entry {
       }
     }
 
-    context.logError(context.makeError(message, { warning: true }));
+    this.logWarning(context, message);
+  }
+
+  /**
+   * @param {Context} context
+   * @param {string} warningMessage
+   */
+  logWarning(context, warningMessage) {
+    let error = context.makeError(warningMessage, { warning: true });
+    context.logError(error);
+
+    if (treatWarningsAsErrors) {
+      // This pref is false by default, and true by default in tests to
+      // discourage the use of deprecated APIs in our unit tests.
+      // If a warning is an expected part of a test, temporarily set the pref
+      // to false, e.g. with the ExtensionTestUtils.failOnSchemaWarnings helper.
+      Services.console.logStringMessage(
+        "Treating warning as error because the preference " +
+          "extensions.webextensions.warnings-as-errors is set to true"
+      );
+      if (typeof error === "string") {
+        error = new Error(error);
+      }
+      throw error;
+    }
   }
 
   /**
@@ -1874,7 +1905,7 @@ class ObjectType extends Type {
 
     if (error) {
       if (onError == "warn") {
-        context.logError(forceString(error.error));
+        this.logWarning(context, forceString(error.error));
       } else if (onError != "ignore") {
         throw error;
       }
@@ -2172,7 +2203,7 @@ class ArrayType extends Type {
       );
       if (element.error) {
         if (this.onError == "warn") {
-          context.logError(forceString(element.error));
+          this.logWarning(context, forceString(element.error));
         } else if (this.onError != "ignore") {
           return element;
         }

toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js

@@ -4,6 +4,9 @@ const DEFAULT_THEME_BG_COLOR = "rgb(255, 255, 255)";
 const DEFAULT_THEME_TEXT_COLOR = "rgb(0, 0, 0)";
 
 add_task(async function test_deprecated_LWT_properties_ignored() {
+  // This test uses deprecated theme properties, so warnings are expected.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
+
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       theme: {

toolkit/components/extensions/test/mochitest/test_chrome_ext_contentscript_unrecognizedprop_warning.html

@@ -60,7 +60,9 @@ add_task(async function test_contentscript() {
     }]);
   });
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   window.open(`${BASE}/file_sample.html`);
 

toolkit/components/extensions/test/xpcshell/test_ext_eventpage_warning.js

@@ -1,6 +1,8 @@
 "use strict";
 
 AddonTestUtils.init(this);
+// This test expects and checks deprecation warnings.
+ExtensionTestUtils.failOnSchemaWarnings(false);
 
 function createEventPageExtension(eventPage) {
   return ExtensionTestUtils.loadExtension({

toolkit/components/extensions/test/xpcshell/test_ext_manifest.js

@@ -75,7 +75,9 @@ add_task(async function test_manifest_warnings_on_unexpected_props() {
     },
   });
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   // Retrieve the warning message collected by the Extension class
   // packagingWarning method.

toolkit/components/extensions/test/xpcshell/test_ext_manifest_content_security_policy.js

@@ -15,9 +15,11 @@ add_task(async function test_manifest_csp() {
     "Should have the expected poilcy string"
   );
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   normalized = await ExtensionTestUtils.normalizeManifest({
     content_security_policy: "object-src 'none'",
   });
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   equal(normalized.error, undefined, "Should not have an error");
 

toolkit/components/extensions/test/xpcshell/test_ext_manifest_themes.js

@@ -29,5 +29,7 @@ async function test_theme_property(property) {
 add_task(async function test_manifest_themes() {
   await test_theme_property("images");
   await test_theme_property("colors");
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await test_theme_property("unrecognized_key");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 });

toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js

@@ -18,7 +18,10 @@ const DUMMY_APP_NAME = "Dummy brandName";
 
 async function getManifestPermissions(extensionData) {
   let extension = ExtensionTestCommon.generate(extensionData);
+  // Some tests contain invalid permissions; ignore the warnings about their invalidity.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.loadManifest();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   return extension.manifestPermissions;
 }
 

toolkit/components/extensions/test/xpcshell/test_ext_proxy_auth.js

@@ -115,8 +115,11 @@ add_task(async function test_webRequest_auth_proxy() {
 
   let handlingExt = getExtension(background);
 
+  // proxy.register is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await handlingExt.startup();
   await handlingExt.awaitMessage("pac-ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   authManager.clearAll();
 
@@ -157,8 +160,11 @@ add_task(async function test_webRequest_auth_proxy_system() {
 
   let handlingExt = getExtension(background);
 
+  // proxy.register is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await handlingExt.startup();
   await handlingExt.awaitMessage("pac-ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   function fetch(url) {
     return new Promise((resolve, reject) => {

toolkit/components/extensions/test/xpcshell/test_ext_proxy_socks.js

@@ -543,8 +543,11 @@ add_task(async function test_webRequest_socks_proxy() {
     },
   });
 
+  // proxy.register is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await handlingExt.startup();
   await handlingExt.awaitMessage("pac-ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   let contentPage = await ExtensionTestUtils.loadContentPage(
     `http://localhost/`

toolkit/components/extensions/test/xpcshell/test_ext_schema.js

@@ -33,6 +33,22 @@ add_task(async function testEmptySchema() {
   await extension.unload();
 });
 
+add_task(async function test_warnings_as_errors() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: { unrecognized_property_that_should_be_treated_as_a_warning: 1 },
+  });
+
+  // Tests should be run with extensions.webextensions.warnings-as-errors=true
+  // by default, and prevent extensions with manifest warnings from loading.
+  await Assert.rejects(
+    extension.startup(),
+    /unrecognized_property_that_should_be_treated_as_a_warning/,
+    "extension with invalid manifest should not load if warnings-as-errors=true"
+  );
+  // When ExtensionTestUtils.failOnSchemaWarnings(false) is called, startup is
+  // expected to succeed, as shown by the next "testUnknownProperties" test.
+});
+
 add_task(async function testUnknownProperties() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
@@ -45,7 +61,9 @@ add_task(async function testUnknownProperties() {
   });
 
   let { messages } = await promiseConsoleOutput(async () => {
+    ExtensionTestUtils.failOnSchemaWarnings(false);
     await extension.startup();
+    ExtensionTestUtils.failOnSchemaWarnings(true);
   });
 
   AddonTestUtils.checkMessages(messages, {

toolkit/components/extensions/test/xpcshell/test_ext_schemas.js

@@ -951,7 +951,9 @@ add_task(async function() {
   ]);
   checkErrors([]);
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   root.testing.errors({ default: "0123", ignore: "0123", warn: "x123" });
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   verify("call", "testing", "errors", [
     { default: "0123", ignore: "0123", warn: null },
   ]);
@@ -1252,6 +1254,9 @@ let deprecatedJson = [
 ];
 
 add_task(async function testDeprecation() {
+  // This whole test expects deprecation warnings.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
+
   let url = "data:," + JSON.stringify(deprecatedJson);
   Schemas._rootSchema = null;
   await Schemas.load(url);
@@ -1306,6 +1311,14 @@ add_task(async function testDeprecation() {
 
   root.deprecated.onDeprecated.hasListener(() => {});
   checkErrors(["This event does not work"]);
+
+  ExtensionTestUtils.failOnSchemaWarnings(true);
+
+  Assert.throws(
+    () => root.deprecated.onDeprecated.hasListener(() => {}),
+    /This event does not work/,
+    "Deprecation warning with extensions.webextensions.warnings-as-errors=true"
+  );
 });
 
 let choicesJson = [

toolkit/components/extensions/test/xpcshell/test_ext_unknown_permissions.js

@@ -1,5 +1,8 @@
 "use strict";
 
+// This test expects and checks warnings for unknown permissions.
+ExtensionTestUtils.failOnSchemaWarnings(false);
+
 add_task(async function test_unknown_permissions() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {

toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js

@@ -401,7 +401,9 @@ add_task(async function test_cached_userScript_on_document_start() {
     },
   });
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   await extension.awaitMessage("user-script-registered");
 
   let url = `${BASE_URL}/file_sample.html`;

toolkit/components/extensions/test/xpcshell/test_proxy_incognito.js

@@ -133,8 +133,11 @@ add_task(async function test_incognito_proxy_register_access() {
       }),
     },
   });
+  // proxy.register is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
   await extension.awaitMessage("ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   // This extension will succeed if it gets a request
   let pb_extension = ExtensionTestUtils.loadExtension({
@@ -161,8 +164,11 @@ add_task(async function test_incognito_proxy_register_access() {
       }),
     },
   });
+  // proxy.register is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await pb_extension.startup();
   await pb_extension.awaitMessage("ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   let finished = pb_extension.awaitFinish("success");
   let contentPage = await ExtensionTestUtils.loadContentPage(

toolkit/components/extensions/test/xpcshell/test_proxy_scripts.js

@@ -57,8 +57,11 @@ async function testProxyScript(script, expected = {}) {
   };
 
   let extension = ExtensionTestUtils.loadExtension(extensionData);
+  // proxy.register and proxy.onProxyError are deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
   await extension.awaitMessage("ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   let errorWait = extension.awaitMessage("proxy-error-received");
 
@@ -82,8 +85,11 @@ async function testProxyScript(script, expected = {}) {
       "Error should include stack trace"
     );
   }
+  // proxy.unregister is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   extension.sendMessage("unregister-proxy-script");
   await extension.awaitFinish("proxy");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   await extension.unload();
 }
 
@@ -162,8 +168,11 @@ async function getExtension(proxyResult) {
     },
   };
   let extension = ExtensionTestUtils.loadExtension(extensionData);
+  // proxy.register is deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
   await extension.awaitMessage("ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
   return extension;
 }
 

toolkit/components/extensions/test/xpcshell/test_proxy_scripts_deprecated.js

@@ -1,6 +1,8 @@
 "use strict";
 
 AddonTestUtils.init(this);
+// This test expects and checks deprecation messages.
+ExtensionTestUtils.failOnSchemaWarnings(false);
 
 add_task(async function test_proxy_deprecation_messages() {
   let extension = ExtensionTestUtils.loadExtension({

toolkit/components/extensions/test/xpcshell/test_proxy_scripts_results.js

@@ -51,8 +51,11 @@ add_task(async function setup() {
     },
   };
   extension = ExtensionTestUtils.loadExtension(extensionData);
+  // proxy.register and proxy.onProxyError are deprecated - bug 1443259.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
   await extension.awaitMessage("ready");
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 });
 
 async function setupProxyScript(proxy) {

toolkit/mozapps/extensions/test/xpcshell/test_webextension.js

@@ -388,6 +388,7 @@ add_task(async function developerEmpty() {
 });
 
 add_task(async function authorNotString() {
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   for (let author of [{}, [], 42]) {
     let addon = await promiseInstallWebExtension({
       manifest: {
@@ -404,6 +405,7 @@ add_task(async function authorNotString() {
 
     await addon.uninstall();
   }
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 });
 
 add_task(async function testThemeExtension() {
@@ -434,6 +436,7 @@ add_task(async function testThemeExtension() {
   await addon.uninstall();
 
   // Also test one without a proper 'theme' section.
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   addon = await promiseInstallWebExtension({
     manifest: {
       author: "Some author",
@@ -443,6 +446,7 @@ add_task(async function testThemeExtension() {
       theme: null,
     },
   });
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   checkAddon(ID, addon, {
     type: "extension",

toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js

@@ -654,7 +654,9 @@ add_task(async function test_non_gecko_bss_install() {
     manifest,
     useAddonManager: "temporary",
   });
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   await extension.startup();
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   const addon = await promiseAddonByID(ID);
   notEqual(addon, null, "Add-on is installed");