Bug 1303798: Correctly propagate addListener errors from WebRequest.jsm to extension. r?mixedpuppy draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 06 Nov 2016 18:08:46 -0800
changeset 434622 99be810b505d5508a1e2d5a38dde3dd6ce09e339
parent 434621 61cf0a1a5186ae72d46d468ccb25492b1cb71734
child 536076 6a0a42bd12b29ce09829daa51c711269dcf4028d
push id34782
push usermaglione.k@gmail.com
push dateMon, 07 Nov 2016 02:09:27 +0000
reviewersmixedpuppy
bugs1303798
milestone52.0a1
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
@@ -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;