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 395397 a9e408c24cd2a4685507b77bb7c2933eafdea815
parent 395396 12a4f938b5aaad367488612fa205315929b2ee45
child 395398 97181d67a50b4b78ee340b6eb441698bc6c1acd6
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, gchang
bugs1342255
milestone54.0a2
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
@@ -1571,17 +1571,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
@@ -60784,16 +60784,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": [
     [
      {}
@@ -118793,16 +118798,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.html": [
     [
      "/service-workers/service-worker/fetch-event-within-sw.html",
      {}
     ]
    ],
    "service-workers/service-worker/fetch-event.https.html": [
     [
@@ -139575,17 +139586,17 @@
    "1329850363c327533f50e509c6a48f6e4b1ed4bb",
    "testharness"
   ],
   "beacon/headers/header-referrer-same-origin.html": [
    "9701f2f0a83c6eeefe781d7de2c0cdbcff38b58e",
    "testharness"
   ],
   "beacon/headers/header-referrer-strict-origin-when-cross-origin.https.html": [
-   "79b4a278f0e35646cfdffeebf8f0523e2772bc9b",
+   "295ef746c475fca0ae8b492375a48948b4ea19c3",
    "testharness"
   ],
   "beacon/headers/header-referrer-strict-origin.https.html": [
    "295ef746c475fca0ae8b492375a48948b4ea19c3",
    "testharness"
   ],
   "beacon/headers/header-referrer-unsafe-url.https.html": [
    "a7b6e697be165124ed5d6846335c8d3a38ee98f5",
@@ -164850,17 +164861,17 @@
   "geolocation-API/getCurrentPosition_permission_allow.html": [
    "695f80f5a06279b3a0bdd137e6a402da66a5eeee",
    "testharness"
   ],
   "geolocation-API/getCurrentPosition_permission_deny-manual.html": [
    "44b2d8846c79ddf7eb8cb3ab76d8899b7e783fad",
    "manual"
   ],
-  "geolocation-API/getCurrentPosition_permission_deny.https.html": [
+  "geolocation-API/getCurrentPosition_permission_deny.html": [
    "28939dd8e719ba66497a814edd1f4500ad348e95",
    "testharness"
   ],
   "geolocation-API/interfaces.html": [
    "c5e300b504b6bf75818fbe79728c87b086ccce3d",
    "testharness"
   ],
   "geolocation-API/support.js": [
@@ -176707,17 +176718,17 @@
    "d869799718137671a2eacc323aa26ea4364e845f",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/textfieldselection-setRangeText.html": [
    "0caf2b08ccc5a35578291af8f5adaf7de9537d66",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/textfieldselection-setSelectionRange.html": [
-   "ffcef015b49fd156cc529117509f0ae0a38234bd",
+   "462049246a2ef3e66c22017ec6ad362e07b467e6",
    "testharness"
   ],
   "html/semantics/forms/the-button-element/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "html/semantics/forms/the-button-element/button-activate-frame.html": [
    "a3dd367a8f7eb9158b1271fcf95bf98b60fe0c9b",
@@ -179475,17 +179486,17 @@
    "0f882828f5cc321992ab9265fe632e55c6d6760b",
    "testharness"
   ],
   "html/webappapis/scripting/events/event-handler-onresize.html": [
    "917a10cd8a60b916eb4e2ad7bb7cb1ae657335a8",
    "testharness"
   ],
   "html/webappapis/scripting/events/event-handler-processing-algorithm.html": [
-   "9a1fa2065ba742d6ab945065d65bdc0f60783d94",
+   "a7c163d53eb559ea710527cace404ed88e9c4d0a",
    "testharness"
   ],
   "html/webappapis/scripting/events/event-handler-spec-example.html": [
    "8d4d0bfdf447591695ac134cd243277ea2326c84",
    "testharness"
   ],
   "html/webappapis/scripting/events/eventhandler-cancellation.html": [
    "491fd73a4ec8812cb8dc1ee01e34a7ff2a07ffb3",
@@ -199879,21 +199890,21 @@
    "974fc8372db8f06f87919d35be48f922166d6652",
    "testharness"
   ],
   "service-workers/service-worker/appcache-ordering-main.https.html": [
    "a71f51cde17f9d209750877dfbe1bacd26412ab3",
    "testharness"
   ],
   "service-workers/service-worker/claim-affect-other-registration.https.html": [
-   "79b4a278f0e35646cfdffeebf8f0523e2772bc9b",
+   "96ffb6f3376a5fa73abd405e123d019d8cac694d",
    "testharness"
   ],
   "service-workers/service-worker/claim-fetch.https.html": [
-   "92d1733358db62804567a642ef405ba74edadf66",
+   "adec77bbeec2ec9ea7da3aba94375bc92e1b4f6f",
    "testharness"
   ],
   "service-workers/service-worker/claim-not-using-registration.https.html": [
    "72da19e038b6ab32ea04a0e91d117bc67d25a301",
    "testharness"
   ],
   "service-workers/service-worker/claim-using-registration.https.html": [
    "fb56cc3ae802669bb7898e76ac55e75ba6ac1441",
@@ -199982,16 +199993,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.html": [
    "6bdea01ca619a894d07364a0485f717b46afe585",
    "manual"
   ],
   "service-workers/service-worker/fetch-event-within-sw.html": [
    "0dfff8289762988423eb8fda40ef47884c243427",
    "testharness"
   ],
@@ -200730,16 +200745,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"
   ],
@@ -206039,17 +206058,17 @@
    "0d244bffe67ef57be68aad99f1cbc7440ff80e27",
    "support"
   ],
   "webdriver/actions/support/test_actions_wdspec.html": [
    "63b5de5ab6c7a00717663a18c3b4d79857ee2136",
    "support"
   ],
   "webdriver/conftest.py": [
-   "9a5d2c122d912af2d53478a3f83ac9da676ba531",
+   "8815d31591ba21a28d392bfc4c989b6f2255529e",
    "wdspec"
   ],
   "webdriver/contexts.py": [
    "cef7ae3987fa61d0b17c616e35c6066ce1e4af83",
    "wdspec"
   ],
   "webdriver/interface.html": [
    "d783d0dd370f58b264ef238d8da5cd8601dc3c7f",
@@ -206059,17 +206078,17 @@
    "2216ea3b518ec6b1beef54ce2580b5e62c2841a0",
    "wdspec"
   ],
   "webdriver/util/__init__.py": [
    "8910ee7d68dfff68460731ea37eb0d406d07862d",
    "support"
   ],
   "webdriver/util/cleanup.py": [
-   "b7e2e0707925c0d331fe943612764d677f87ce90",
+   "0e82dfb4d90006b5878a7438b01ef2731771118d",
    "wdspec"
   ],
   "webdriver/util/http_request.py": [
    "01c4b525c27f77d253c75031a9cee3f17aca8df0",
    "wdspec"
   ],
   "webgl/OWNERS": [
    "34855ff53b22857e0085833768f19f7304e6a75e",
@@ -217079,17 +217098,17 @@
    "a4b04414ea26f04d70970247420f3f256b7631e3",
    "testharness"
   ],
   "webstorage/storage_setitem.html": [
    "74fb62c4369f7929bb9387d8fadb1341e30eb90f",
    "testharness"
   ],
   "webstorage/storage_string_conversion.html": [
-   "3d8b227e0884935392f66d00c9d09e0f0d4aadc5",
+   "33c748773a518af29ea2ef8cbbc859fe988fb88a",
    "testharness"
   ],
   "webstorage/storage_supported_property_names.html": [
    "19fd5ede17b9c7e9d234e855eb7dd7fe343bc73e",
    "testharness"
   ],
   "webvr/OWNERS": [
    "adb04954bffbb33f29864668db95f5c5367d6a05",
@@ -219191,17 +219210,17 @@
    "2126e9f28c1fbb6ea721d25d3ac70022639f5597",
    "reftest"
   ],
   "webvtt/webvtt-api-for-browsers/vttcue-interface/align.html": [
    "49a9bf76b1a8d907e48ca95d3e8d3b0afc306566",
    "testharness"
   ],
   "webvtt/webvtt-api-for-browsers/vttcue-interface/getCueAsHTML.html": [
-   "79c03cfbda74f1047bd88886fa6ab44ce0c9cded",
+   "5187200a7d7e2ea96627aaea69189098d3bf5013",
    "testharness"
   ],
   "webvtt/webvtt-api-for-browsers/vttcue-interface/line.html": [
    "f76ca4b827ed42faa1c46e6a8a6a4dc66938f6e3",
    "testharness"
   ],
   "webvtt/webvtt-api-for-browsers/vttcue-interface/snapToLines.html": [
    "365542b9a68ddb679408e5b9df16ff77027e125a",
@@ -219283,17 +219302,17 @@
    "cfaa1c59bdfefe7682fd73152e2d4a706a415aaa",
    "support"
   ],
   "webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/ids.vtt": [
    "e94efc2cea87f5ebaba3d7a46bdfe1f74cd556ef",
    "support"
   ],
   "webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt": [
-   "a5bfb88a0066da230fbf05f0cf9d200f73c0bb12",
+   "ba3848383a2197647a9c34c52150991ecb87f22a",
    "support"
   ],
   "webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/no-signature.vtt": [
    "71ffec71e6a8923027309dd41c591302ac880550",
    "support"
   ],
   "webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/nulls.vtt": [
    "140e16000f93f8565c28c2fd12a638afcc6bbf05",
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");
+  });