Bug 1457213 support early startup for proxy api, r?aswan draft
authorShane Caraveo <scaraveo@mozilla.com>
Tue, 01 May 2018 18:46:07 -0500
changeset 790316 d2a2ff367a6597f742428d8f240a8e322cc32510
parent 790147 d2a4720d1c334b64d88a51678758c27ba8f03c89
push id108494
push usermixedpuppy@gmail.com
push dateTue, 01 May 2018 23:46:37 +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,133 @@
+"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;
+
+let nonProxiedRequests = 0;
+const nonProxiedServer = createHttpServer({hosts: ["example.com"]});
+nonProxiedServer.registerPathHandler("/", (request, response) => {
+  nonProxiedRequests++;
+  response.setStatusLine(request.httpVersion, 200, "OK");
+  response.write("ok");
+});
+
+// No hosts defined to avoid proxy filter setup.
+let proxiedRequests = 0;
+const server = createHttpServer();
+server.identity.add("http", "proxied.example.com", 80);
+server.registerPathHandler("/", (request, response) => {
+  proxiedRequests++;
+  response.setStatusLine(request.httpVersion, 200, "OK");
+  response.write("ok");
+});
+
+Services.prefs.setBoolPref("extensions.webextensions.background-delayed-startup", true);
+
+function promiseExtensionEvent(wrapper, event) {
+  return new Promise(resolve => {
+    wrapper.extension.once(event, resolve);
+  });
+}
+
+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 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 => {
+      // ignore speculative requests
+      if (details.type == "xmlhttprequest") {
+        browser.test.sendMessage("saw-request");
+      }
+      return proxyInfo;
+    }, {urls: ["<all_urls>"]});
+  }
+
+  let proxyInfo = {host: server.identity.primaryHost, port: server.identity.primaryPort, type: "http"};
+
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "permanent",
+    manifest: {
+      permissions: ["proxy", "http://proxied.example.com/*"],
+    },
+    background: `(${background})(${JSON.stringify(proxyInfo)})`,
+  });
+
+  await extension.startup();
+
+  // Initial requests to test the proxy and non-proxied servers.
+  await Promise.all([
+    extension.awaitMessage("saw-request"),
+    ExtensionTestUtils.fetch("http://proxied.example.com/?a=0"),
+  ]);
+  equal(1, proxiedRequests, "proxied request ok");
+  equal(0, nonProxiedRequests, "non proxied request ok");
+
+  await ExtensionTestUtils.fetch("http://example.com/?a=0");
+  equal(1, proxiedRequests, "proxied request ok");
+  equal(1, nonProxiedRequests, "non proxied request ok");
+
+  await promiseRestartManager(false);
+  await extension.awaitStartup();
+
+  let events = trackEvents(extension);
+
+  // Initiate a non-proxied request to make sure the startup listeners are using
+  // the extensions filters/etc.
+  await ExtensionTestUtils.fetch("http://example.com/?a=1");
+  equal(1, proxiedRequests, "proxied request ok");
+  equal(2, nonProxiedRequests, "non proxied request ok");
+
+  equal(events.get("background-page-event"), false,
+        "Should not have gotten a background page event");
+
+  // Make a request that the extension will proxy once it is started.
+  let request = Promise.all([
+    extension.awaitMessage("saw-request"),
+    ExtensionTestUtils.fetch("http://proxied.example.com/?a=1"),
+  ]);
+
+  await promiseExtensionEvent(extension, "background-page-event");
+  equal(events.get("background-page-event"), true,
+        "Should have gotten a background page event");
+
+  // Test the background page startup.
+  equal(events.get("start-background-page"), false,
+        "Should have gotten a background page event");
+
+  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
+  await new Promise(executeSoon);
+
+  equal(events.get("start-background-page"), true,
+        "Should have gotten a background page event");
+
+  // Verify our proxied request finishes properly and that the
+  // request was not handled via our non-proxied server.
+  await request;
+  equal(2, proxiedRequests, "proxied request ok");
+  equal(2, nonProxiedRequests, "non proxied requests ok");
+
+  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]