Bug 1303798: Correctly propagate addListener errors from WebRequest.jsm to extension. r=mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Sun, 06 Nov 2016 18:08:46 -0800
changeset 321576 8c2ad6366c6be9fce02428ca22b973be8d9d295b
parent 321575 24d97dc514d4b8fe608e9aee7527615c1e613606
child 321577 564ab238f96d7d64c8100e0b7b234fb71d21a19e
push id30931
push userkwierso@gmail.com
push dateTue, 08 Nov 2016 21:58:36 +0000
treeherdermozilla-central@783356f1476e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1303798
milestone52.0a1
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
Bug 1303798: Correctly propagate addListener errors from WebRequest.jsm to extension. r=mixedpuppy MozReview-Commit-ID: BTTNaaAdBs5
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -45,16 +45,22 @@ function getConsole() {
   return new ConsoleAPI({
     maxLogLevelPref: "extensions.webextensions.log.level",
     prefix: "WebExtensions",
   });
 }
 
 XPCOMUtils.defineLazyGetter(this, "console", getConsole);
 
+/**
+ * An Error subclass for which complete error messages are always passed
+ * to extensions, rather than being interpreted as an unknown error.
+ */
+class ExtensionError extends Error {}
+
 function filterStack(error) {
   return String(error.stack).replace(/(^.*(Task\.jsm|Promise-backend\.js).*\n)+/gm, "<Promise Chain>\n");
 }
 
 // Run a function and report exceptions.
 function runSafeSyncWithoutClone(f, ...args) {
   try {
     return f(...args);
@@ -363,17 +369,17 @@ class BaseContext {
    * @param {Error|object} error
    * @returns {Error}
    */
   normalizeError(error) {
     if (error instanceof this.cloneScope.Error) {
       return error;
     }
     let message;
-    if (instanceOf(error, "Object")) {
+    if (instanceOf(error, "Object") || error instanceof ExtensionError) {
       message = error.message;
     } else if (typeof error == "object" &&
         this.principal.subsumes(Cu.getObjectPrincipal(error))) {
       message = error.message;
     } else {
       Cu.reportError(error);
     }
     message = message || "An unexpected error occurred";
@@ -1743,17 +1749,21 @@ class LocalAPIImplementation extends Sch
     return this.pathObj[this.name];
   }
 
   setProperty(value) {
     this.pathObj[this.name] = value;
   }
 
   addListener(listener, args) {
-    this.pathObj[this.name].addListener.call(null, listener, ...args);
+    try {
+      this.pathObj[this.name].addListener.call(null, listener, ...args);
+    } catch (e) {
+      throw this.context.normalizeError(e);
+    }
   }
 
   hasListener(listener) {
     return this.pathObj[this.name].hasListener.call(null, listener);
   }
 
   removeListener(listener) {
     this.pathObj[this.name].removeListener.call(null, listener);
@@ -2231,16 +2241,17 @@ this.ExtensionUtils = {
   runSafeSyncWithoutClone,
   runSafeWithoutClone,
   stylesheetMap,
   BaseContext,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
   EventManager,
+  ExtensionError,
   IconDetails,
   LocalAPIImplementation,
   LocaleData,
   Messenger,
   Port,
   PlatformInfo,
   SchemaAPIInterface,
   SingletonEventManager,
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html
@@ -67,20 +67,22 @@ add_task(function* test_setup() {
 });
 
 function background() {
   let events = {
     "onBeforeRequest":  [{urls: ["<all_urls>"]}, ["blocking", "requestBody"]],
     "onCompleted":      [{urls: ["<all_urls>"]}, ["responseHeaders"]],
   };
 
+  let requestBodySupported = true;
+
   function onUpload(details) {
     let url = new URL(details.url);
     let upload = url.searchParams.get("upload");
-    if (!upload) {
+    if (!upload || !requestBodySupported) {
       return;
     }
     let requestBody = details.requestBody;
     browser.test.log(`onUpload ${details.url} ${JSON.stringify(details.requestBody)}`);
     browser.test.assertTrue(!!requestBody, `Intercepted upload ${details.url} #${details.requestId} ${upload} have a requestBody`);
     if (!requestBody) {
       return;
     }
@@ -108,22 +110,28 @@ function background() {
   }
 
   for (let [name, args] of Object.entries(events)) {
     try {
       browser.test.log(`adding listener for ${name}`);
       browser.webRequest[name].addListener(
         listener.bind(null, name), ...args);
     } catch (e) {
+      browser.test.assertTrue(/\brequestBody\b/.test(e.message),
+                              "Request body is unsupported");
+
       // requestBody is disabled in release builds
       if (!/\brequestBody\b/.test(e.message)) {
-        args.pop();
-        browser.webRequest[name].addListener(
-          listener.bind(null, name), ...args);
+        throw e;
       }
+
+      requestBodySupported = false;
+      args.pop();
+      browser.webRequest[name].addListener(
+        listener.bind(null, name), ...args);
     }
   }
 }
 
 add_task(function* test_xhr_forms() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: [
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -17,21 +17,25 @@ const {nsIHttpActivityObserver, nsISocke
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
                                   "resource://gre/modules/AppConstants.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserUtils",
                                   "resource://gre/modules/BrowserUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ExtensionUtils",
+                                  "resource://gre/modules/ExtensionUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequestCommon",
                                   "resource://gre/modules/WebRequestCommon.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequestUpload",
                                   "resource://gre/modules/WebRequestUpload.jsm");
 
+XPCOMUtils.defineLazyGetter(this, "ExtensionError", () => ExtensionUtils.ExtensionError);
+
 function attachToChannel(channel, key, data) {
   if (channel instanceof Ci.nsIWritablePropertyBag2) {
     let wrapper = {wrappedJSObject: data};
     channel.setPropertyAsInterface(key, wrapper);
   }
   return data;
 }
 
@@ -75,17 +79,17 @@ function parseFilter(filter) {
   // FIXME: Support windowId filtering.
   return {urls: filter.urls || null, types: filter.types || null};
 }
 
 function parseExtra(extra, allowed = []) {
   if (extra) {
     for (let ex of extra) {
       if (allowed.indexOf(ex) == -1) {
-        throw new Error(`Invalid option ${ex}`);
+        throw new ExtensionError(`Invalid option ${ex}`);
       }
     }
   }
 
   let result = {};
   for (let al of allowed) {
     if (extra && extra.indexOf(al) != -1) {
       result[al] = true;