Bug 1319607: Make request resumption more resilient. r=mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Thu, 24 Nov 2016 13:56:22 -0800
changeset 324297 5b45f7938182ea8bf814ab0bb6373bcf803755b5
parent 324296 46a20ea6799d1e4141fbb567162b4b3dfccc2f39
child 324298 f8f4eaac1701107f794b48891bcca2c95d39d503
child 324312 762cd82916340a3803543cda64079fcdf5cd9dd6
push id30996
push userphilringnalda@gmail.com
push dateSat, 26 Nov 2016 05:26:31 +0000
treeherdermozilla-central@f8f4eaac1701 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1319607
milestone53.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 1319607: Make request resumption more resilient. r=mixedpuppy MozReview-Commit-ID: 8ObjJKDf3wb
toolkit/components/extensions/test/mochitest/test_ext_webrequest_suspend.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_suspend.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_suspend.html
@@ -59,11 +59,81 @@ add_task(function* test_suspend() {
 
   let headers = JSON.parse(yield result.text());
 
   is(headers.foo, "Bar", "Request header was correctly set on suspended request");
 
   yield extension.unload();
 });
 
+
+// Test that requests that were canceled while suspended for a blocking
+// listener are correctly resumed.
+add_task(function* test_error_resume() {
+  let chromeScript = SpecialPowers.loadChromeScript(() => {
+    const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+    Cu.import("resource://gre/modules/Services.jsm");
+
+    let observer = channel => {
+      if (channel instanceof Ci.nsIHttpChannel && channel.URI.spec === "http://example.com/") {
+        Services.obs.removeObserver(observer, "http-on-modify-request");
+
+        // Wait until the next tick to make sure this runs after WebRequest observers.
+        Promise.resolve().then(() => {
+          channel.cancel(Cr.NS_BINDING_ABORTED);
+        });
+      }
+    };
+
+    Services.obs.addObserver(observer, "http-on-modify-request", false);
+  });
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: [
+        "webRequest",
+        "webRequestBlocking",
+      ],
+    },
+
+    background() {
+      browser.webRequest.onBeforeSendHeaders.addListener(
+        details => {
+          browser.test.log(`onBeforeSendHeaders({url: ${details.url}})`);
+
+          if (details.url === "http://example.com/") {
+            browser.test.sendMessage("got-before-send-headers");
+          }
+        },
+        {urls: ["<all_urls>"]},
+        ["blocking"]);
+
+      browser.webRequest.onErrorOccurred.addListener(
+        details => {
+          browser.test.log(`onErrorOccurred({url: ${details.url}})`);
+
+          if (details.url === "http://example.com/") {
+            browser.test.sendMessage("got-error-occurred");
+          }
+        },
+        {urls: ["<all_urls>"]});
+    },
+  });
+
+  yield extension.startup();
+
+  try {
+    yield fetch("http://example.com/");
+    ok(false, "Fetch should have failed.");
+  } catch (e) {
+    ok(true, "Got expected error.");
+  }
+
+  yield extension.awaitMessage("got-before-send-headers");
+  yield extension.awaitMessage("got-error-occurred");
+
+  yield extension.unload();
+  chromeScript.destroy();
+});
+
 </script>
 </body>
 </html>
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -775,32 +775,28 @@ HttpObserverManager = {
         if (opts.requestHeaders && result.requestHeaders && requestHeaders) {
           requestHeaders.applyChanges(result.requestHeaders);
         }
 
         if (opts.responseHeaders && result.responseHeaders && responseHeaders) {
           responseHeaders.applyChanges(result.responseHeaders);
         }
       }
+
+      if (kind === "opening") {
+        yield this.runChannelListener(channel, loadContext, "modify");
+      } else if (kind === "modify") {
+        yield this.runChannelListener(channel, loadContext, "afterModify");
+      }
     } catch (e) {
       Cu.reportError(e);
     }
 
-    if (kind === "opening") {
-      return this.runChannelListener(channel, loadContext, "modify");
-    } else if (kind === "modify") {
-      return this.runChannelListener(channel, loadContext, "afterModify");
-    }
-
-    // Only resume the channel if either it was suspended by this call,
-    // and this callback is not part of the opening-modify-afterModify
-    // chain, or if this is an afterModify callback. This allows us to
-    // chain the aforementioned handlers without repeatedly suspending
-    // and resuming the request.
-    if (shouldResume || kind === "afterModify") {
+    // Only resume the channel if either it was suspended by this call.
+    if (shouldResume) {
       this.maybeResume(channel);
     }
   }),
 
   examine(channel, topic, data) {
     let loadContext = this.getLoadContext(channel);
 
     if (this.needTracing) {