Bug 1457213 support early startup for proxy api, r=aswan
authorShane Caraveo <scaraveo@mozilla.com>
Tue, 01 May 2018 18:46:07 -0500
changeset 472806 ec8911b8f9c7ea3fcb51a7957903503eb163ae30
parent 472805 1292058fb7248a32e4ad3062397c0c86defaaaab
child 472807 7a6c8a95fc9f4c087ef1172dbcaeb74e79631888
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1457213
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 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]