Bug 1342255 - Fix crash in respondWith resolved callback. Don't reset interception if the sw throws after .respondWith(). r=asuth, a=gchang
authorCatalin Badea <catalin.badea392@gmail.com>
Fri, 03 Mar 2017 01:51:06 +0000
changeset 379233 fcb9802d4df0f57363a860f721c5086305b9843a
parent 379232 b75f4332a4b1ff3bafa69674cd98bcd982bc2004
child 379234 991a3145ada237eaf59d41b1bf83bab617966907
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, gchang
bugs1342255
milestone53.0
Bug 1342255 - Fix crash in respondWith resolved callback. Don't reset interception if the sw throws after .respondWith(). r=asuth, a=gchang
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
@@ -1615,17 +1615,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
@@ -60071,16 +60071,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": [
     [
      {}
@@ -116907,16 +116912,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.https.html": [
     [
      "/service-workers/service-worker/fetch-event.https.html",
      {}
     ]
    ],
    "service-workers/service-worker/fetch-frame-resource.https.html": [
     [
@@ -196158,16 +196169,20 @@
   "service-workers/service-worker/fetch-event-redirect.https.html": [
    "5d4efd3504c37f5b728eef4941266b3d1d8842e7",
    "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.https.html": [
    "9197ebb50014905e02645c5dc71d467be0e7604c",
    "testharness"
   ],
   "service-workers/service-worker/fetch-frame-resource.https.html": [
    "e5996d90c4c383eda2947cc2bc39e734448b3bde",
    "testharness"
   ],
@@ -196790,16 +196805,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": [
    "f91ff42d4a88961aeb4c29cce61db8ad32261f4e",
    "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");
+  });