Bug 1457213 support early startup for proxy api, r?aswan draft
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 27 Apr 2018 13:26:39 -0500
changeset 789162 74eff7c2867d2407cb43b27e3b77161745445da3
parent 789020 d2d518b1f8730eb61554df7179ef9a2aeed4d843
push id108205
push usermixedpuppy@gmail.com
push dateFri, 27 Apr 2018 18:27:10 +0000
reviewersaswan
bugs1457213
milestone61.0a1
Bug 1457213 support early startup for proxy api, r?aswan MozReview-Commit-ID: kMf8ig4U2S
toolkit/components/extensions/ProxyScriptContext.jsm
toolkit/components/extensions/parent/ext-proxy.js
toolkit/components/extensions/test/xpcshell/test_ext_proxy_startup.js
toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
--- a/toolkit/components/extensions/ProxyScriptContext.jsm
+++ b/toolkit/components/extensions/ProxyScriptContext.jsm
@@ -238,18 +238,19 @@ function normalizeFilter(filter) {
   if (!filter) {
     filter = {};
   }
 
   return {urls: filter.urls || null, types: filter.types || null};
 }
 
 class ProxyChannelFilter {
-  constructor(context, listener, filter, extraInfoSpec) {
+  constructor(context, extension, listener, filter, extraInfoSpec) {
     this.context = context;
+    this.extension = extension;
     this.filter = normalizeFilter(filter);
     this.listener = listener;
     this.extraInfoSpec = extraInfoSpec || [];
 
     ProxyService.registerChannelFilter(
       this /* nsIProtocolProxyChannelFilter aFilter */,
       0 /* unsigned long aPosition */
     );
@@ -305,17 +306,17 @@ class ProxyChannelFilter {
       let {filter} = this;
       if (filter.tabId != null && browserData.tabId !== filter.tabId) {
         return;
       }
       if (filter.windowId != null && browserData.windowId !== filter.windowId) {
         return;
       }
 
-      if (wrapper.matches(filter, this.context.extension.policy, {isProxy: true})) {
+      if (wrapper.matches(filter, this.extension.policy, {isProxy: true})) {
         let data = this.getRequestData(wrapper, {tabId: browserData.tabId});
 
         let ret = await this.listener(data);
         if (ret == null) {
           // If ret undefined or null, fall through to the `finally` block to apply the proxy result.
           proxyInfo = ret;
           return;
         }
@@ -326,18 +327,24 @@ class ProxyChannelFilter {
         }
         // We allow the call to return either a single proxyInfo or an array of proxyInfo.
         if (!Array.isArray(ret)) {
           ret = [ret];
         }
         proxyInfo = ProxyInfoData.createProxyInfoFromData(ret, defaultProxyInfo);
       }
     } catch (e) {
+      // We need to normalize errors to dispatch them to the extension handler.  If
+      // we have not started up yet, we'll just log those to the console.
+      if (!this.context) {
+        this.extension.logError(`proxy-error before extension startup: ${e}`);
+        return;
+      }
       let error = this.context.normalizeError(e);
-      this.context.extension.emit("proxy-error", {
+      this.extension.emit("proxy-error", {
         message: error.message,
         fileName: error.fileName,
         lineNumber: error.lineNumber,
         stack: error.stack,
       });
     } finally {
       // We must call onProxyFilterResult.  proxyInfo may be null or nsIProxyInfo.
       // defaultProxyInfo will be null unless a prior proxy handler has set something.
--- a/toolkit/components/extensions/parent/ext-proxy.js
+++ b/toolkit/components/extensions/parent/ext-proxy.js
@@ -73,63 +73,65 @@ ExtensionPreferencesManager.addSetting("
         prefs[`network.proxy.${prop}_port`] = undefined;
       }
     }
 
     return prefs;
   },
 });
 
-// EventManager-like class specifically for Proxy filters. Inherits from
-// EventManager. Takes care of converting |details| parameter
-// when invoking listeners.
-class ProxyFilterEventManager extends EventManager {
-  constructor(context, eventName) {
-    let name = `proxy.${eventName}`;
-    let register = (fire, filterProps, extraInfoSpec = []) => {
-      let listener = (data) => {
-        return fire.sync(data);
-      };
+function registerProxyFilterEvent(context, extension, fire, filterProps, extraInfoSpec = []) {
+  let listener = (data) => {
+    return fire.sync(data);
+  };
 
-      let filter = {...filterProps};
-      if (filter.urls) {
-        let perms = new MatchPatternSet([...context.extension.whiteListedHosts.patterns,
-                                         ...context.extension.optionalOrigins.patterns]);
+  let filter = {...filterProps};
+  if (filter.urls) {
+    let perms = new MatchPatternSet([...extension.whiteListedHosts.patterns,
+                                     ...extension.optionalOrigins.patterns]);
+
+    filter.urls = new MatchPatternSet(filter.urls);
 
-        filter.urls = new MatchPatternSet(filter.urls);
-
-        if (!perms.overlapsAll(filter.urls)) {
-          throw new context.cloneScope.Error("The proxy.addListener filter doesn't overlap with host permissions.");
-        }
-      }
+    if (!perms.overlapsAll(filter.urls)) {
+      Cu.reportError("The proxy.onRequest filter doesn't overlap with host permissions.");
+    }
+  }
 
-      let proxyFilter = new ProxyChannelFilter(context, listener, filter, extraInfoSpec);
-      return () => {
-        proxyFilter.destroy();
-      };
-    };
-
-    super({context, name, register});
-  }
+  let proxyFilter = new ProxyChannelFilter(context, extension, listener, filter, extraInfoSpec);
+  return {
+    unregister: () => { proxyFilter.destroy(); },
+    convert(_fire, _context) {
+      fire = _fire;
+      proxyFilter.context = _context;
+    },
+  };
 }
 
 this.proxy = class extends ExtensionAPI {
   onShutdown() {
     let {extension} = this;
 
     let proxyScriptContext = proxyScriptContextMap.get(extension);
     if (proxyScriptContext) {
       proxyScriptContext.unload();
       proxyScriptContextMap.delete(extension);
     }
   }
 
+  primeListener(extension, event, fire, params) {
+    if (event === "onRequest") {
+      return registerProxyFilterEvent(undefined, extension, fire, ...params);
+    }
+  }
+
   getAPI(context) {
     let {extension} = context;
 
+    // Leaving as non-persistent.  By itself it's not useful since proxy-error
+    // is emitted from the proxy filter.
     let onError = new EventManager({
       context,
       name: "proxy.onError",
       register: fire => {
         let listener = (name, error) => {
           fire.async(error);
         };
         extension.on("proxy-error", listener);
@@ -157,17 +159,27 @@ this.proxy = class extends ExtensionAPI 
             proxyScriptContextMap.delete(extension);
           }
         },
 
         registerProxyScript(url) {
           this.register(url);
         },
 
-        onRequest: new ProxyFilterEventManager(context, "onRequest").api(),
+        onRequest: new EventManager({
+          context,
+          name: `proxy.onRequest`,
+          persistent: {
+            module: "proxy",
+            event: "onRequest",
+          },
+          register: (fire, filter, info) => {
+            return registerProxyFilterEvent(context, context.extension, fire, filter, info).unregister;
+          },
+        }).api(),
 
         onError,
 
         // TODO Bug 1388619 deprecate onProxyError.
         onProxyError: onError,
 
         settings: Object.assign(
           getSettingsAPI(
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_startup.js
@@ -0,0 +1,76 @@
+"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 promiseExtensionEvent(wrapper, event) {
+  return new Promise(resolve => {
+    wrapper.extension.once(event, resolve);
+  });
+}
+
+// Test that a proxy 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 test_proxy_startup() {
+  await promiseStartupManager();
+
+  function background(proxyInfo) {
+    browser.proxy.onRequest.addListener(details => {
+      browser.test.sendMessage("saw-request");
+      return proxyInfo;
+    }, {urls: ["http://example.com/data/file_sample.html"]});
+  }
+
+  let proxyInfo = {host: server.identity.primaryHost, port: server.identity.primaryPort, type: "http"};
+
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "permanent",
+    manifest: {
+      permissions: ["proxy", "http://example.com/"],
+    },
+    background: `(${background})(${JSON.stringify(proxyInfo)})`,
+  });
+
+  await extension.startup();
+
+  await promiseRestartManager(false);
+  await extension.awaitStartup();
+
+  let request = ExtensionTestUtils.fetch("http://example.com/",
+                                         "http://example.com/data/file_sample.html");
+
+  await promiseExtensionEvent(extension, "background-page-event");
+  ok(true, "Should have gotten a background page event");
+  let startup = promiseExtensionEvent(extension, "start-background-page");
+
+  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
+  await new Promise(executeSoon);
+
+  await startup;
+  ok(true, "Should have gotten start-background-page event");
+
+  await extension.awaitMessage("saw-request");
+  ok(true, "Background page loaded and received proxy.onRequest event");
+
+  await request;
+  ok(true, "Request completed");
+
+  await extension.unload();
+
+  await promiseShutdownManager();
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
@@ -67,16 +67,17 @@ skip-if = true # This test no longer tes
 [test_ext_privacy_update.js]
 [test_ext_proxy_auth.js]
 [test_ext_proxy_config.js]
 [test_ext_proxy_onauthrequired.js]
 [test_ext_proxy_settings.js]
 skip-if = os == "android" # proxy settings are not supported on android
 [test_ext_proxy_socks.js]
 [test_ext_proxy_speculative.js]
+[test_ext_proxy_startup.js]
 [test_ext_redirects.js]
 [test_ext_runtime_connect_no_receiver.js]
 [test_ext_runtime_getBrowserInfo.js]
 [test_ext_runtime_getPlatformInfo.js]
 [test_ext_runtime_id.js]
 [test_ext_runtime_onInstalled_and_onStartup.js]
 skip-if = true # bug 1315829
 [test_ext_runtime_sendMessage.js]