Bug 1447551 Part 2: Convert webRequest to persistent events r=kmag,mixedpuppy
☠☠ backed out by 9d3390130f27 ☠ ☠
authorAndrew Swan <aswan@mozilla.com>
Fri, 20 Apr 2018 11:41:30 -0700
changeset 787746 82d178d9966ddd126ec1cf3c0ff90173c9ad4a80
parent 787745 b6f7de464d7f6fc953aaa4022c6c48cf879fcb10
child 787747 5da0c34c96001035a846d5ac2c29dd9dfc8a3d2a
push id107796
push userbmo:emilio@crisal.io
push dateWed, 25 Apr 2018 10:57:04 +0000
reviewerskmag, mixedpuppy
bugs1447551
milestone61.0a1
Bug 1447551 Part 2: Convert webRequest to persistent events r=kmag,mixedpuppy MozReview-Commit-ID: ANprpK8Kw5Q
toolkit/components/extensions/parent/ext-webRequest.js
toolkit/components/extensions/test/xpcshell/test_ext_webRequest_startup.js
toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/parent/ext-webRequest.js
+++ b/toolkit/components/extensions/parent/ext-webRequest.js
@@ -2,105 +2,129 @@
 
 // This file expects tabTracker to be defined in the global scope (e.g.
 // by ext-utils.js).
 /* global tabTracker */
 
 ChromeUtils.defineModuleGetter(this, "WebRequest",
                                "resource://gre/modules/WebRequest.jsm");
 
-// EventManager-like class specifically for WebRequest. Inherits from
-// EventManager. Takes care of converting |details| parameter
-// when invoking listeners.
-function WebRequestEventManager(context, eventName) {
-  let name = `webRequest.${eventName}`;
-  let register = (fire, filter, info) => {
-    let listener = data => {
-      let browserData = {tabId: -1, windowId: -1};
-      if (data.browser) {
-        browserData = tabTracker.getBrowserData(data.browser);
-      }
-      if (filter.tabId != null && browserData.tabId != filter.tabId) {
-        return;
-      }
-      if (filter.windowId != null && browserData.windowId != filter.windowId) {
-        return;
+// The guts of a WebRequest event handler.  Takes care of converting
+// |details| parameter when invoking listeners.
+function registerEvent(extension, eventName, fire, filter, info, tabParent = null) {
+  let listener = async data => {
+    let browserData = {tabId: -1, windowId: -1};
+    if (data.browser) {
+      browserData = tabTracker.getBrowserData(data.browser);
+    }
+    if (filter.tabId != null && browserData.tabId != filter.tabId) {
+      return;
+    }
+    if (filter.windowId != null && browserData.windowId != filter.windowId) {
+      return;
+    }
+
+    let event = data.serialize(eventName);
+    event.tabId = browserData.tabId;
+
+    if (data.registerTraceableChannel) {
+      if (fire.wakeup) {
+        await fire.wakeup();
       }
+      data.registerTraceableChannel(extension.policy, tabParent);
+    }
 
-      let event = data.serialize(eventName);
-      event.tabId = browserData.tabId;
+    return fire.sync(event);
+  };
 
-      return fire.sync(event);
-    };
+  let filter2 = {};
+  if (filter.urls) {
+    let perms = new MatchPatternSet([...extension.whiteListedHosts.patterns,
+                                     ...extension.optionalOrigins.patterns]);
+
+    filter2.urls = new MatchPatternSet(filter.urls);
 
-    let filter2 = {};
-    if (filter.urls) {
-      let perms = new MatchPatternSet([...context.extension.whiteListedHosts.patterns,
-                                       ...context.extension.optionalOrigins.patterns]);
+    if (!perms.overlapsAll(filter2.urls)) {
+      Cu.reportError("The webRequest.addListener filter doesn't overlap with host permissions.");
+    }
+  }
+  if (filter.types) {
+    filter2.types = filter.types;
+  }
+  if (filter.tabId) {
+    filter2.tabId = filter.tabId;
+  }
+  if (filter.windowId) {
+    filter2.windowId = filter.windowId;
+  }
 
-      filter2.urls = new MatchPatternSet(filter.urls);
+  let blockingAllowed = extension.hasPermission("webRequestBlocking");
 
-      if (!perms.overlapsAll(filter2.urls)) {
-        Cu.reportError("The webRequest.addListener filter doesn't overlap with host permissions.");
+  let info2 = [];
+  if (info) {
+    for (let desc of info) {
+      if (desc == "blocking" && !blockingAllowed) {
+        Cu.reportError("Using webRequest.addListener with the blocking option " +
+                       "requires the 'webRequestBlocking' permission.");
+      } else {
+        info2.push(desc);
       }
     }
-    if (filter.types) {
-      filter2.types = filter.types;
-    }
-    if (filter.tabId) {
-      filter2.tabId = filter.tabId;
-    }
-    if (filter.windowId) {
-      filter2.windowId = filter.windowId;
-    }
-
-    let blockingAllowed = context.extension.hasPermission("webRequestBlocking");
+  }
 
-    let info2 = [];
-    if (info) {
-      for (let desc of info) {
-        if (desc == "blocking" && !blockingAllowed) {
-          Cu.reportError("Using webRequest.addListener with the blocking option " +
-                         "requires the 'webRequestBlocking' permission.");
-        } else {
-          info2.push(desc);
-        }
-      }
-    }
-
-    let listenerDetails = {
-      addonId: context.extension.id,
-      extension: context.extension.policy,
-      blockingAllowed,
-      tabParent: context.xulBrowser.frameLoader.tabParent,
-    };
-
-    WebRequest[eventName].addListener(
-      listener, filter2, info2,
-      listenerDetails);
-    return () => {
-      WebRequest[eventName].removeListener(listener);
-    };
+  let listenerDetails = {
+    addonId: extension.id,
+    extension: extension.policy,
+    blockingAllowed,
   };
 
-  return new EventManager({context, name, register}).api();
+  WebRequest[eventName].addListener(
+    listener, filter2, info2,
+    listenerDetails);
+
+  return {
+    unregister: () => { WebRequest[eventName].removeListener(listener); },
+    convert(_fire, context) {
+      fire = _fire;
+      tabParent = context.xulBrowser.frameLoader.tabParent;
+    },
+  };
+}
+
+function makeWebRequestEvent(context, name) {
+  return new EventManager({
+    context,
+    name: `webRequest.${name}`,
+    persistent: {
+      module: "webRequest",
+      event: name,
+    },
+    register: (fire, filter, info) => {
+      return registerEvent(context.extension, name, fire, filter, info,
+                           context.xulBrowser.frameLoader.tabParent).unregister;
+    },
+  }).api();
 }
 
 this.webRequest = class extends ExtensionAPI {
+  primeListener(extension, event, fire, params) {
+    return registerEvent(extension, event, fire, ...params);
+  }
+
   getAPI(context) {
     return {
       webRequest: {
-        onBeforeRequest: WebRequestEventManager(context, "onBeforeRequest"),
-        onBeforeSendHeaders: WebRequestEventManager(context, "onBeforeSendHeaders"),
-        onSendHeaders: WebRequestEventManager(context, "onSendHeaders"),
-        onHeadersReceived: WebRequestEventManager(context, "onHeadersReceived"),
-        onAuthRequired: WebRequestEventManager(context, "onAuthRequired"),
-        onBeforeRedirect: WebRequestEventManager(context, "onBeforeRedirect"),
-        onResponseStarted: WebRequestEventManager(context, "onResponseStarted"),
-        onErrorOccurred: WebRequestEventManager(context, "onErrorOccurred"),
-        onCompleted: WebRequestEventManager(context, "onCompleted"),
+        onBeforeRequest: makeWebRequestEvent(context, "onBeforeRequest"),
+        onBeforeSendHeaders: makeWebRequestEvent(context, "onBeforeSendHeaders"),
+        onSendHeaders: makeWebRequestEvent(context, "onSendHeaders"),
+        onHeadersReceived: makeWebRequestEvent(context, "onHeadersReceived"),
+        onAuthRequired: makeWebRequestEvent(context, "onAuthRequired"),
+        onBeforeRedirect: makeWebRequestEvent(context, "onBeforeRedirect"),
+        onResponseStarted: makeWebRequestEvent(context, "onResponseStarted"),
+        onErrorOccurred: makeWebRequestEvent(context, "onErrorOccurred"),
+        onCompleted: makeWebRequestEvent(context, "onCompleted"),
         handlerBehaviorChanged: function() {
           // TODO: Flush all caches.
         },
       },
     };
   }
 };
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_startup.js
@@ -0,0 +1,172 @@
+"use strict";
+
+PromiseTestUtils.whitelistRejectionsGlobally(/Message manager disconnected/);
+
+AddonTestUtils.init(this);
+AddonTestUtils.overrideCertDB();
+AddonTestUtils.createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "43");
+
+let {
+  promiseRestartManager,
+  promiseShutdownManager,
+  promiseStartupManager,
+} = AddonTestUtils;
+
+const server = createHttpServer({hosts: ["example.com"]});
+server.registerDirectory("/data/", do_get_file("data"));
+
+Services.prefs.setBoolPref("extensions.webextensions.background-delayed-startup", true);
+
+function trackEvents(wrapper) {
+  let events = new Map();
+  for (let event of ["background-page-event", "start-background-page"]) {
+    events.set(event, false);
+    wrapper.extension.once(event, () => events.set(event, true));
+  }
+  return events;
+}
+
+// Test that a non-blocking listener during startup does not immediately
+// start the background page, but the event is queued until the background
+// page is started.
+add_task(async function() {
+  await promiseStartupManager();
+
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "permanent",
+    manifest: {
+      permissions: ["webRequest", "http://example.com/"],
+    },
+
+    background() {
+      browser.webRequest.onBeforeRequest.addListener(details => {
+        browser.test.sendMessage("saw-request");
+      }, {urls: ["http://example.com/data/file_sample.html"]});
+    },
+  });
+
+  await extension.startup();
+
+  await promiseRestartManager(false);
+
+  let events = trackEvents(extension);
+
+  await ExtensionTestUtils.fetch("http://example.com/",
+                                 "http://example.com/data/file_sample.html");
+
+  equal(events.get("background-page-event"), true,
+        "Should have gotten a background page event");
+  equal(events.get("start-background-page"), false,
+        "Background page should not be started");
+
+  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
+  await new Promise(executeSoon);
+
+  equal(events.get("start-background-page"), true,
+        "Should have gotten start-background-page event");
+
+  await extension.awaitMessage("saw-request");
+  ok(true, "Background page loaded and received webRequest event");
+
+  await extension.unload();
+
+  await promiseShutdownManager();
+});
+
+// Tests that filters are handled properly: if we have a blocking listener
+// with a filter, a request that does not match the filter does not get
+// suspended and does not start the background page.
+add_task(async function() {
+  await promiseStartupManager();
+
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "permanent",
+    manifest: {
+      permissions: ["webRequest", "webRequestBlocking",
+                    "http://test1.example.com/"],
+    },
+
+    background() {
+      browser.webRequest.onBeforeRequest.addListener(details => {
+        browser.test.fail("Listener should not have been called");
+      }, {urls: ["http://test1.example.com/*"]}, ["blocking"]);
+
+      browser.test.sendMessage("ready");
+    },
+  });
+
+  await extension.startup();
+  await extension.awaitMessage("ready");
+
+  await promiseRestartManager(false);
+
+  let events = trackEvents(extension);
+
+  await ExtensionTestUtils.fetch("http://example.com/",
+                                 "http://example.com/data/file_sample.html");
+
+  equal(events.get("background-page-event"), false,
+        "Should not have gotten a background page event");
+
+  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
+  await new Promise(executeSoon);
+
+  equal(events.get("start-background-page"), false,
+        "Should not have tried to start background page yet");
+
+  Services.obs.notifyObservers(null, "sessionstore-windows-restored");
+  await extension.awaitMessage("ready");
+
+  await extension.unload();
+  await promiseShutdownManager();
+});
+
+// Test that a block listener that uses filterResponseData() works
+// properly (i.e., that the delayed call to registerTraceableChannel
+// works properly).
+add_task(async function() {
+  const DATA = `<!DOCTYPE html>
+<html>
+<body>
+  <h1>This is a modified page</h1>
+</body>
+</html>`;
+
+  function background(data) {
+    browser.webRequest.onBeforeRequest.addListener(details => {
+      let filter = browser.webRequest.filterResponseData(details.requestId);
+      filter.onstop = () => {
+        let encoded = new TextEncoder("utf-8").encode(data);
+        filter.write(encoded);
+        filter.close();
+      };
+    }, {urls: ["http://example.com/data/file_sample.html"]}, ["blocking"]);
+  }
+
+  await promiseStartupManager();
+
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "permanent",
+    manifest: {
+      permissions: ["webRequest", "webRequestBlocking", "http://example.com/"],
+    },
+
+    background: `(${background})(${uneval(DATA)})`,
+  });
+
+  await extension.startup();
+
+  await promiseRestartManager(false);
+
+  let dataPromise = ExtensionTestUtils.fetch("http://example.com/",
+                                             "http://example.com/data/file_sample.html");
+
+  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
+  let data = await dataPromise;
+
+  equal(data, DATA, "Stream filter was properly installed for a load during startup");
+
+  await extension.unload();
+  await promiseShutdownManager();
+});
+
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
@@ -109,16 +109,17 @@ skip-if = os == 'android' # Bug 1258975 
 skip-if = os == "android"
 [test_ext_unload_frame.js]
 skip-if = true # Too frequent intermittent failures
 [test_ext_webRequest_auth.js]
 [test_ext_webRequest_filterResponseData.js]
 [test_ext_webRequest_permission.js]
 [test_ext_webRequest_responseBody.js]
 [test_ext_webRequest_set_cookie.js]
+[test_ext_webRequest_startup.js]
 [test_ext_webRequest_suspend.js]
 [test_ext_webRequest_webSocket.js]
 [test_ext_xhr_capabilities.js]
 [test_native_manifests.js]
 subprocess = true
 skip-if = os == "android"
 [test_ext_permissions.js]
 skip-if = os == "android" # Bug 1350559
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -751,17 +751,19 @@ HttpObserverManager = {
           if (this.STATUS_TYPES.has(kind)) {
             commonData.statusCode = channel.statusCode;
             commonData.statusLine = channel.statusLine;
           }
         }
         let data = Object.create(commonData);
 
         if (registerFilter && opts.blocking && opts.extension) {
-          channel.registerTraceableChannel(opts.extension, opts.tabParent);
+          data.registerTraceableChannel = (extension, tabParent) => {
+            channel.registerTraceableChannel(extension, tabParent);
+          };
         }
 
         if (opts.requestHeaders) {
           requestHeaders = requestHeaders || new RequestHeaderChanger(channel);
           data.requestHeaders = requestHeaders.toArray();
         }
 
         if (opts.responseHeaders) {