Bug 1303798: Correctly propagate addListener errors from WebRequest.jsm to extension. r=mixedpuppy
☠☠ backed out by 7e604b351f4f ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Sun, 06 Nov 2016 18:08:46 -0800
changeset 321565 045ce2ca1b7d9e0f997391819daae1f6f87280cc
parent 321564 e388e453977c7bf257d87e97ff003c43f44b7c0f
child 321566 143f26b923d021c80b9103c7de17f446bd32b19e
push id83647
push userkwierso@gmail.com
push dateTue, 08 Nov 2016 22:08:41 +0000
treeherdermozilla-inbound@1d0b02250149 [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
@@ -47,16 +47,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);
@@ -369,17 +375,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";
@@ -1749,17 +1755,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);
@@ -2237,16 +2247,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
@@ -38,16 +38,18 @@
   target="_blank"
   >
 <input type="text" name="textInput" value="value1">
 <input type="text" name="textInput" value="value2">
 </form>
 <script>
 "use strict";
 
+var requestBodySupported = true;
+
 let files, testFile, blob, file, uploads;
 add_task(function* test_setup() {
   files = yield new Promise(resolve => {
     SpecialPowers.createFiles([{name: "testFile.pdf", data: "Not really a PDF file :)", "type": "application/x-pdf"}], (result) => {
       resolve(result);
     });
   });
   testFile = files[0];
@@ -70,17 +72,17 @@ function background() {
   let events = {
     "onBeforeRequest":  [{urls: ["<all_urls>"]}, ["blocking", "requestBody"]],
     "onCompleted":      [{urls: ["<all_urls>"]}, ["responseHeaders"]],
   };
 
   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;