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 415430 82d178d9966ddd126ec1cf3c0ff90173c9ad4a80
parent 415429 b6f7de464d7f6fc953aaa4022c6c48cf879fcb10
child 415431 5da0c34c96001035a846d5ac2c29dd9dfc8a3d2a
push id33895
push userrgurzau@mozilla.com
push dateWed, 25 Apr 2018 09:35:01 +0000
treeherdermozilla-central@e786f1edbbef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag, mixedpuppy
bugs1447551
milestone61.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 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) {