Bug 1254204: Part 2 - Allow suspending requests by returning Promises from blocking request listeners. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 08 Nov 2016 21:35:43 -0800
changeset 435756 b7800bc6e407a8c262f73efcfa5dc7ad99d36a31
parent 435755 59cc3ae008e6cda22051c2766723c2e9b152b540
child 435758 963f981367100b3f7b5b62e42042ac906a711d6c
push id35116
push usermaglione.k@gmail.com
push dateWed, 09 Nov 2016 05:38:17 +0000
reviewersaswan
bugs1254204
milestone52.0a1
Bug 1254204: Part 2 - Allow suspending requests by returning Promises from blocking request listeners. r?aswan MozReview-Commit-ID: B1ekz7WJ9kU
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/redirection.sjs
toolkit/components/extensions/test/mochitest/return_headers.sjs
toolkit/components/extensions/test/mochitest/test_ext_webrequest.html
toolkit/components/extensions/test/mochitest/test_ext_webrequest_suspend.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -30,16 +30,17 @@ support-files =
   file_script_redirect.js
   file_script_xhr.js
   file_sample.html
   redirection.sjs
   file_privilege_escalation.html
   file_ext_test_api_injection.js
   file_permission_xhr.html
   file_teardown_test.js
+  return_headers.sjs
   webrequest_worker.js
 tags = webextensions
 
 [test_clipboard.html]
 # skip-if = # disabled test case with_permission_allow_copy, see inline comment.
 [test_ext_inIncognitoContext_window.html]
 skip-if = os == 'android' # Android does not currently support windows.
 [test_ext_geturl.html]
@@ -91,16 +92,18 @@ skip-if = (os == 'android') # Android do
 [test_ext_i18n.html]
 skip-if = (os == 'android') # Bug 1258975 on android.
 [test_ext_web_accessible_resources.html]
 skip-if = (os == 'android') # Bug 1258975 on android.
 [test_ext_webrequest.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
 [test_ext_webrequest_background_events.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
+[test_ext_webrequest_suspend.html]
+skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
 [test_ext_webrequest_upload.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
 [test_ext_webnavigation.html]
 skip-if = os == 'android' # port.sender.tab is undefined on Android (bug 1258975).
 [test_ext_webnavigation_filters.html]
 skip-if = os == 'android' # port.sender.tab is undefined on Android (bug 1258975).
 [test_ext_window_postMessage.html]
 [test_ext_subframes_privileges.html]
--- a/toolkit/components/extensions/test/mochitest/redirection.sjs
+++ b/toolkit/components/extensions/test/mochitest/redirection.sjs
@@ -1,4 +1,4 @@
 function handleRequest(aRequest, aResponse) {
   aResponse.setStatusLine(aRequest.httpVersion, 302);
   aResponse.setHeader("Location", "./dummy_page.html");
-}
\ No newline at end of file
+}
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/return_headers.sjs
@@ -0,0 +1,18 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+function handleRequest(request, response) {
+  response.setHeader("Content-Type", "text/plain", false);
+
+  let headers = {};
+  // Why on earth...
+  let enumerator = request.headers;
+  while (enumerator.hasMoreElements()) {
+    let header = enumerator.getNext().data;
+    headers[header.toLowerCase()] = request.getHeader(header);
+  }
+
+  response.write(JSON.stringify(headers));
+}
+
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest.html
@@ -247,16 +247,17 @@ function backgroundScript() {
     if (!/^https?:/.test(details.url)) {
       return;
     }
 
     let headers = details[`${phase}Headers`];
     browser.test.assertTrue(Array.isArray(headers), `valid ${phase}Headers array`);
 
     let {added, modified, deleted} = testHeaders[phase];
+    browser.test.log(`... ${uneval(added)} ${uneval(headers)}`);
     for (let name in added) {
       browser.test.assertTrue(headers.some(h => h.name.toLowerCase() === name.toLowerCase() && h.value === added[name]), `header ${name} correctly injected in ${phase}Headers`);
     }
 
     let modifiedAny = false;
     browser.test.log(`HEADERS ${JSON.stringify(headers)}`);
     for (let header of headers.filter(h => h.name in modified)) {
       let {name, value} = header;
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_suspend.html
@@ -0,0 +1,53 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for simple WebExtension</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(function* () {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: [
+        "webRequest",
+        "webRequestBlocking",
+      ],
+    },
+
+    background() {
+      browser.webRequest.onBeforeSendHeaders.addListener(details => {
+        let requestHeaders = details.requestHeaders.concat({name: "Foo", value: "Bar"});
+
+        return new Promise(resolve => {
+          setTimeout(resolve, 500);
+        }).then(() => {
+          return {requestHeaders};
+        });
+      },
+      {urls: ["<all_urls>"]},
+      ["blocking", "requestHeaders"]);
+    },
+  });
+
+  yield extension.startup();
+
+  let result = yield fetch(SimpleTest.getTestFileURL("return_headers.sjs"));
+
+  let headers = JSON.parse(yield result.text());
+
+  is(headers.foo, "Bar", "Request header was correctly set on suspended request");
+
+  yield extension.unload();
+});
+
+</script>
+</body>
+</html>
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -10,18 +10,19 @@ const EXPORTED_SYMBOLS = ["WebRequest"];
 
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 const {nsIHttpActivityObserver, nsISocketTransport} = Ci;
 
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
                                   "resource://gre/modules/AppConstants.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserUtils",
                                   "resource://gre/modules/BrowserUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionUtils",
                                   "resource://gre/modules/ExtensionUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequestCommon",
@@ -110,16 +111,20 @@ function mergeStatus(data, channel, even
     // NS_ERROR_NOT_AVAILABLE might be thrown if it's an internal redirect, happening before
     // any actual HTTP traffic. Otherwise, let's report.
     if (event !== "onRedirect" || e.result !== Cr.NS_ERROR_NOT_AVAILABLE) {
       Cu.reportError(`webRequest Error: ${e} trying to merge status in ${event}@${channel.name}`);
     }
   }
 }
 
+function isThenable(value) {
+  return value && typeof value === "object" && typeof value.then === "function";
+}
+
 class HeaderChanger {
   constructor(channel) {
     this.channel = channel;
 
     this.originalHeaders = new Map();
     this.visitHeaders((name, value) => {
       this.originalHeaders.set(name.toLowerCase(), value);
     });
@@ -482,17 +487,19 @@ HttpObserverManager = {
       }
     }
   },
 
   observe(subject, topic, data) {
     let channel = subject.QueryInterface(Ci.nsIHttpChannel);
     switch (topic) {
       case "http-on-modify-request":
-        this.modify(channel, topic, data);
+        let loadContext = this.getLoadContext(channel);
+
+        this.runChannelListener(channel, loadContext, "opening");
         break;
       case "http-on-examine-cached-response":
       case "http-on-examine-merged-response":
         getData(channel).fromCache = true;
         // falls through
       case "http-on-examine-response":
         this.examine(channel, topic, data);
         break;
@@ -556,21 +563,21 @@ HttpObserverManager = {
     return errorData;
   },
 
   runChannelListener(channel, loadContext, kind, extraData = null) {
     if (this.activityInitialized) {
       let channelData = getData(channel);
       if (kind === "onError") {
         if (channelData.errorNotified) {
-          return false;
+          return;
         }
         channelData.errorNotified = true;
       } else if (this.errorCheck(channel, loadContext, channelData)) {
-        return false;
+        return;
       }
     }
     let listeners = this.listeners[kind];
     let browser = loadContext ? loadContext.topFrameElement : null;
     let loadInfo = channel.loadInfo;
     let policyType = loadInfo ?
                      loadInfo.externalContentPolicyType :
                      Ci.nsIContentPolicy.TYPE_OTHER;
@@ -674,53 +681,81 @@ HttpObserverManager = {
         if (result && typeof result === "object" && opts.blocking) {
           handlerResults.push({opts, result});
         }
       } catch (e) {
         Cu.reportError(e);
       }
     }
 
-    for (let {opts, result} of handlerResults) {
-      if (result.cancel) {
-        channel.cancel(Cr.NS_ERROR_ABORT);
-        this.errorCheck(channel, loadContext);
-        return false;
-      }
+    this.applyChanges(kind, channel, loadContext, handlerResults, requestHeaders, responseHeaders);
+  },
+
+  applyChanges: Task.async(function* (kind, channel, loadContext, handlerResults, requestHeaders, responseHeaders) {
+    let asyncHandlers = handlerResults.filter(({result}) => isThenable(result));
+    let isAsync = asyncHandlers.length > 0;
 
-      // FIXME: This should only be available in some phases.
-      if (result.redirectUrl) {
-        try {
-          channel.redirectTo(BrowserUtils.makeURI(result.redirectUrl));
-          return false;
-        } catch (e) {
-          Cu.reportError(e);
+    try {
+      if (isAsync) {
+        channel.suspend();
+
+        for (let value of asyncHandlers) {
+          try {
+            value.result = yield value.result;
+          } catch (e) {
+            Cu.reportError(e);
+            value.result = {};
+          }
         }
       }
 
-      if (opts.requestHeaders && result.requestHeaders) {
-        requestHeaders.applyChanges(result.requestHeaders);
-      }
+      for (let {opts, result} of handlerResults) {
+        if (result.cancel) {
+          channel.cancel(Cr.NS_ERROR_ABORT);
+
+          this.errorCheck(channel, loadContext);
+          return;
+        }
+
+        // FIXME: This should only be available in some phases.
+        if (result.redirectUrl) {
+          try {
+            if (isAsync) {
+              channel.resume();
+            }
 
-      if (opts.responseHeaders && result.responseHeaders) {
-        responseHeaders.applyChanges(result.responseHeaders);
+            channel.redirectTo(BrowserUtils.makeURI(result.redirectUrl));
+            return;
+          } catch (e) {
+            Cu.reportError(e);
+          }
+        }
+
+        if (opts.requestHeaders && result.requestHeaders) {
+          requestHeaders.applyChanges(result.requestHeaders);
+        }
+
+        if (opts.responseHeaders && result.responseHeaders) {
+          responseHeaders.applyChanges(result.responseHeaders);
+        }
       }
+    } catch (e) {
+      Cu.reportError(e);
     }
 
-    return true;
-  },
-
-  modify(channel, topic, data) {
-    let loadContext = this.getLoadContext(channel);
+    if (isAsync) {
+      channel.resume();
+    }
 
-    if (this.runChannelListener(channel, loadContext, "opening") &&
-        this.runChannelListener(channel, loadContext, "modify")) {
-      this.runChannelListener(channel, loadContext, "afterModify");
+    if (kind === "opening") {
+      return this.runChannelListener(channel, loadContext, "modify");
+    } else if (kind === "modify") {
+      return this.runChannelListener(channel, loadContext, "afterModify");
     }
-  },
+  }),
 
   examine(channel, topic, data) {
     let loadContext = this.getLoadContext(channel);
 
     if (this.needTracing) {
       if (channel instanceof Ci.nsITraceableChannel) {
         let responseStatus = channel.responseStatus;
         // skip redirections, https://bugzilla.mozilla.org/show_bug.cgi?id=728901#c8