Bug 1342255 - Fix crash in respondWith resolved callback. Don't reset interception if the sw throws after .respondWith(). r=asuth
authorCatalin Badea <catalin.badea392@gmail.com>
Fri, 03 Mar 2017 01:51:06 +0000
changeset 348474 9ce518ccb46660d1fdb53a7e175bdb1092f70052
parent 348473 324a8b9f44f4c1cc71022fa2e9279526374c5bfe
child 348475 a13df489797929050b31913e62d3403d971e6af3
push id31526
push userkwierso@gmail.com
push dateTue, 21 Mar 2017 01:20:02 +0000
treeherdermozilla-central@5fe5dcf1c10a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1342255
milestone55.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 1342255 - Fix crash in respondWith resolved callback. Don't reset interception if the sw throws after .respondWith(). r=asuth
dom/workers/ServiceWorkerEvents.cpp
dom/workers/ServiceWorkerPrivate.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/service-workers/service-worker/fetch-event-throws-after-respond-with.https.html
testing/web-platform/tests/service-workers/service-worker/resources/respond-then-throw-worker.js
--- a/dom/workers/ServiceWorkerEvents.cpp
+++ b/dom/workers/ServiceWorkerEvents.cpp
@@ -653,17 +653,17 @@ RespondWithHandler::ResolvedCallback(JSC
   nsCOMPtr<nsIInputStream> body;
   ir->GetUnfilteredBody(getter_AddRefs(body));
   // Errors and redirects may not have a body.
   if (body) {
     response->SetBodyUsed();
 
     nsCOMPtr<nsIOutputStream> responseBody;
     rv = mInterceptedChannel->GetResponseBody(getter_AddRefs(responseBody));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
+    if (NS_WARN_IF(NS_FAILED(rv)) || !responseBody) {
       return;
     }
 
     const uint32_t kCopySegmentSize = 4096;
 
     // Depending on how the Response passed to .respondWith() was created, we may
     // get a non-buffered input stream.  In addition, in some configurations the
     // destination channel's output stream can be unbuffered.  We wrap the output
--- a/dom/workers/ServiceWorkerPrivate.cpp
+++ b/dom/workers/ServiceWorkerPrivate.cpp
@@ -1575,17 +1575,18 @@ private:
     }
 
     event->PostInit(mInterceptedChannel, mRegistration, mScriptSpec);
     event->SetTrusted(true);
 
     nsresult rv2 =
       DispatchExtendableEventOnWorkerScope(aCx, aWorkerPrivate->GlobalScope(),
                                            event, nullptr);
-    if (NS_WARN_IF(NS_FAILED(rv2)) || !event->WaitToRespond()) {
+    if ((NS_WARN_IF(NS_FAILED(rv2)) && rv2 != NS_ERROR_XPC_JS_THREW_EXCEPTION) ||
+        !event->WaitToRespond()) {
       nsCOMPtr<nsIRunnable> runnable;
       MOZ_ASSERT(!aWorkerPrivate->UsesSystemPrincipal(),
                  "We don't support system-principal serviceworkers");
       if (event->DefaultPrevented(CallerType::NonSystem)) {
         runnable = new CancelChannelRunnable(mInterceptedChannel,
                                              mRegistration,
                                              NS_ERROR_INTERCEPTION_FAILED);
       } else {
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -61201,16 +61201,21 @@
      {}
     ]
    ],
    "service-workers/service-worker/resources/resource-timing-worker.js": [
     [
      {}
     ]
    ],
+   "service-workers/service-worker/resources/respond-then-throw-worker.js": [
+    [
+     {}
+    ]
+   ],
    "service-workers/service-worker/resources/service-worker-csp-worker.py": [
     [
      {}
     ]
    ],
    "service-workers/service-worker/resources/shared-worker-controlled.js": [
     [
      {}
@@ -119853,16 +119858,22 @@
     ]
    ],
    "service-workers/service-worker/fetch-event-respond-with-stops-propagation.https.html": [
     [
      "/service-workers/service-worker/fetch-event-respond-with-stops-propagation.https.html",
      {}
     ]
    ],
+   "service-workers/service-worker/fetch-event-throws-after-respond-with.https.html": [
+    [
+     "/service-workers/service-worker/fetch-event-throws-after-respond-with.https.html",
+     {}
+    ]
+   ],
    "service-workers/service-worker/fetch-event-within-sw.https.html": [
     [
      "/service-workers/service-worker/fetch-event-within-sw.https.html",
      {}
     ]
    ],
    "service-workers/service-worker/fetch-event.https.html": [
     [
@@ -130987,17 +130998,17 @@
    "94689a9853ab8d0ddcd85b2a8db4b6f2c9045b4a",
    "support"
   ],
   "./lint": [
    "c40fe0907c52a1dd8c20222cf931cf7ae911993b",
    "support"
   ],
   "./lint.whitelist": [
-   "2bf4ed33e508857e92d2fcf5efac381a60e91bc6",
+   "e055247bfc62fbe467bbc2812f5f9d0791225748",
    "support"
   ],
   "./manifest": [
    "a6fa6f29100b87d7bfb47672d91002d880e14c1e",
    "support"
   ],
   "./serve": [
    "aa28dcfdd87f967544e051978bbccf9f3ce9c097",
@@ -141119,17 +141130,17 @@
    "295ef746c475fca0ae8b492375a48948b4ea19c3",
    "testharness"
   ],
   "beacon/headers/header-referrer-unsafe-url.https.html": [
    "a7b6e697be165124ed5d6846335c8d3a38ee98f5",
    "testharness"
   ],
   "beacon/headers/header-referrer.js": [
-   "1836174ce84714c39333a4cf863ec994ed70ff93",
+   "8185d2b31fbf67a573444d3c8f828f96422526f5",
    "support"
   ],
   "beacon/resources/inspect-header.py": [
    "e70503e7fb71617b9be631d5f2a9e73cacd83e3f",
    "support"
   ],
   "bluetooth/bluetooth-helpers.js": [
    "9794b578f1c5c08126fc10653e4beed1f1721d0c",
@@ -178435,17 +178446,17 @@
    "629f3682421060ea18e9046a637402212be1b1fd",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/selection-not-application.html": [
    "3763f117f8973ca9a994354ccbf22cb7114ece7a",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/selection-start-end.html": [
-   "755fb11ec3d9440d3883ec3e2820a9e77fc144ae",
+   "e38a79075e27780327f49e7ae9cadd2558165eac",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/selection-value-interactions.html": [
    "0f93258e5237c49fa24efe5180409e48721e8025",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/selection.html": [
    "7f3969423e86313ec20846c84f8deecc95048b82",
@@ -181979,17 +181990,17 @@
    "c832afb4826fd43d2fcc4f5e3fd6a773a6ee35f0",
    "testharness"
   ],
   "infrastructure/reftest-wait-ref.html": [
    "62552ac6981fc0c7aca84983569717eb2990a31e",
    "support"
   ],
   "infrastructure/reftest-wait.html": [
-   "2ce5c2dc6d3f9f96cbd8dd527802cc9835df56cf",
+   "1a291b68cdf6edcfc28a2ff22e294e8e8ebc0c42",
    "reftest"
   ],
   "innerText/getter-tests.js": [
    "3b33df08dce92cf6720bbbfa3881ccd04c9ab658",
    "support"
   ],
   "innerText/getter.html": [
    "644b5b18359d26db9cd4d037b075ca016e2337b5",
@@ -201970,16 +201981,20 @@
   "service-workers/service-worker/fetch-event-redirect.https.html": [
    "e322139b14149fe9b3f3aff76a9af8a58437e715",
    "testharness"
   ],
   "service-workers/service-worker/fetch-event-respond-with-stops-propagation.https.html": [
    "2feaa5022ee31fb980f97075d932b0d87d6efe75",
    "testharness"
   ],
+  "service-workers/service-worker/fetch-event-throws-after-respond-with.https.html": [
+   "41a64740d1d56a839f0888837a56c2b74cab0c77",
+   "testharness"
+  ],
   "service-workers/service-worker/fetch-event-within-sw-manual.https.html": [
    "6bdea01ca619a894d07364a0485f717b46afe585",
    "manual"
   ],
   "service-workers/service-worker/fetch-event-within-sw.https.html": [
    "0dfff8289762988423eb8fda40ef47884c243427",
    "testharness"
   ],
@@ -202734,16 +202749,20 @@
   "service-workers/service-worker/resources/resource-timing-iframe.html": [
    "0fc85a13c536aad45a87fb073fdcebf371d4476d",
    "support"
   ],
   "service-workers/service-worker/resources/resource-timing-worker.js": [
    "ee7888f2e145700cf2590b6d6de9bab39088a979",
    "support"
   ],
+  "service-workers/service-worker/resources/respond-then-throw-worker.js": [
+   "d57215bcad8a3966175930642dfd34281b11aeff",
+   "support"
+  ],
   "service-workers/service-worker/resources/service-worker-csp-worker.py": [
    "f7c6bb3ba222dc35a09ef806a7c6d145339f9eb2",
    "support"
   ],
   "service-workers/service-worker/resources/shared-worker-controlled.js": [
    "9ae937c2e8499991432caa4587eb68fe2998f503",
    "support"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/service-workers/service-worker/fetch-event-throws-after-respond-with.https.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title></title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<script src="resources/test-helpers.sub.js"></script>
+<script>
+
+promise_test(function(t) {
+    var scope = 'resources/fetch-event-throws-after-respond-with-iframe.html';
+    var workerscript = 'resources/respond-then-throw-worker.js';
+    var iframe;
+    return service_worker_unregister_and_register(t, workerscript, scope)
+      .then(function(reg) {
+          return wait_for_state(t, reg.installing, 'activated')
+            .then(() => reg.active);
+        })
+      .then(function(worker) {
+          var channel = new MessageChannel();
+          channel.port1.onmessage = function(e) {
+              assert_equals(e.data, 'SYNC', ' Should receive sync message.');
+              channel.port1.postMessage('ACK');
+            }
+          worker.postMessage({port: channel.port2}, [channel.port2]);
+          // The iframe will only be loaded after the sync is completed.
+          return with_iframe(scope);
+        })
+      .then(function(frame) {
+        assert_true(frame.contentDocument.body.innerHTML.includes("intercepted"));
+        service_worker_unregister_and_done(t, scope);
+      })
+  }, 'Fetch event handler throws after a successful respondWith()');
+
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/service-workers/service-worker/resources/respond-then-throw-worker.js
@@ -0,0 +1,40 @@
+var syncport = null;
+
+self.addEventListener('message', function(e) {
+  if ('port' in e.data) {
+    if (syncport) {
+      syncport(e.data.port);
+    } else {
+      syncport = e.data.port;
+    }
+  }
+});
+
+function sync() {
+  return new Promise(function(resolve) {
+      if (syncport) {
+        resolve(syncport);
+      } else {
+        syncport = resolve;
+      }
+    }).then(function(port) {
+      port.postMessage('SYNC');
+      return new Promise(function(resolve) {
+          port.onmessage = function(e) {
+            if (e.data === 'ACK') {
+              resolve();
+            }
+          }
+        });
+    });
+}
+
+
+self.addEventListener('fetch', function(event) {
+    // In Firefox the result would depend on a race between fetch handling
+    // and exception handling code. On the assumption that this might be a common
+    // design error, we explicitly allow the exception to be handled first.
+    event.respondWith(sync().then(() => new Response('intercepted')));
+
+    throw("error");
+  });