[Service Workers] Registration promise should be rejected when the service worker...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 Nov 2017 05:15:15 +0000 (05:15 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 Nov 2017 05:15:15 +0000 (05:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179566

Reviewed by Brady Eidson.

LayoutTests/imported/w3c:

Rebaseline WPT tests now that we properly reject registration promises when the worker script
fails to evaluate.

* web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/registration-attribute.https-expected.txt:
* web-platform-tests/service-workers/service-worker/navigation-redirect.https-expected.txt:
* web-platform-tests/service-workers/service-worker/redirected-response.https-expected.txt:
* web-platform-tests/service-workers/service-worker/update.https-expected.txt:

Source/WebCore:

Registration promise should be rejected when the service worker fails to start.

Though our code intended to do this, there were several issues:
- Our code failed to properly detect when the script failed to evaluate. This is
  because it relied on the exception message being non-null but it was always
  null when same origin due to a bug.
- Our scriptContextFailedToStart() handler failed to rejected the promise
  and finish the job as per:
  - https://w3c.github.io/ServiceWorker/#update-algorithm (step 9.7)

Test: http/tests/workers/service/worker-fails-to-start.html

* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::evaluate):
* workers/service/server/SWServerJobQueue.cpp:
(WebCore::SWServerJobQueue::scriptContextFailedToStart):

LayoutTests:

Add layout test coverage.

* TestExpectations: Mark test as flaky as the console lines sometimes change order.
* http/tests/workers/service/resources/worker-fails-to-start-worker.js: Added.
* http/tests/workers/service/worker-fails-to-start-expected.txt: Added.
* http/tests/workers/service/worker-fails-to-start.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224723 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/http/tests/workers/service/resources/worker-fails-to-start-worker.js [new file with mode: 0644]
LayoutTests/http/tests/workers/service/worker-fails-to-start-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/service/worker-fails-to-start.html [new file with mode: 0644]
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/registration-attribute.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redirect.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/redirected-response.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/update.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/WorkerScriptController.cpp
Source/WebCore/workers/service/server/SWServerJobQueue.cpp

index 8f502dc..0e92933 100644 (file)
@@ -1,3 +1,17 @@
+2017-11-10  Chris Dumez  <cdumez@apple.com>
+
+        [Service Workers] Registration promise should be rejected when the service worker fails to start
+        https://bugs.webkit.org/show_bug.cgi?id=179566
+
+        Reviewed by Brady Eidson.
+
+        Add layout test coverage.
+
+        * TestExpectations: Mark test as flaky as the console lines sometimes change order.
+        * http/tests/workers/service/resources/worker-fails-to-start-worker.js: Added.
+        * http/tests/workers/service/worker-fails-to-start-expected.txt: Added.
+        * http/tests/workers/service/worker-fails-to-start.html: Added.
+
 2017-11-10  Ryan Haddad  <ryanhaddad@apple.com>
 
         Update TestExpectations for imported/w3c/web-platform-tests/html/browsers/origin/relaxing-the-same-origin-restriction/document_domain_setter_null.tentative.html.
index e45d93d..19951eb 100644 (file)
@@ -184,6 +184,7 @@ imported/w3c/web-platform-tests/service-workers/service-worker/fetch-header-visi
 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-request-redirect.https.html [ Pass Failure ]
 imported/w3c/web-platform-tests/service-workers/service-worker/foreign-fetch-cors.https.html [ Pass Failure ]
 imported/w3c/web-platform-tests/service-workers/service-worker/performance-timeline.https.html [ Pass Failure ]
+imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redirect.https.html [ Pass Failure ]
 webkit.org/b/179452 imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html [ Pass Failure ]
 webkit.org/b/179194 imported/w3c/web-platform-tests/service-workers/service-worker/registration-mime-types.https.html [ Pass Failure ]
 imported/w3c/web-platform-tests/service-workers/service-worker/registration-script.https.html [ Pass Failure ]
diff --git a/LayoutTests/http/tests/workers/service/resources/worker-fails-to-start-worker.js b/LayoutTests/http/tests/workers/service/resources/worker-fails-to-start-worker.js
new file mode 100644 (file)
index 0000000..decbe1f
--- /dev/null
@@ -0,0 +1 @@
+let a = foo
diff --git a/LayoutTests/http/tests/workers/service/worker-fails-to-start-expected.txt b/LayoutTests/http/tests/workers/service/worker-fails-to-start-expected.txt
new file mode 100644 (file)
index 0000000..934baac
--- /dev/null
@@ -0,0 +1,2 @@
+PASS: Rejected the promise as expected with message: TypeError: ReferenceError: Can't find variable: foo
+
diff --git a/LayoutTests/http/tests/workers/service/worker-fails-to-start.html b/LayoutTests/http/tests/workers/service/worker-fails-to-start.html
new file mode 100644 (file)
index 0000000..12701ca
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<head>
+<script src="resources/sw-test-pre.js"></script>
+</head>
+<body>
+<script>
+navigator.serviceWorker.register("resources/worker-fails-to-start-worker.js", { }).then(function(registration) {
+    log("FAIL: Should have rejected the promise");
+    finishSWTest();
+}, function(e) {
+    log("PASS: Rejected the promise as expected with message: " + e);
+    finishSWTest();
+});
+</script>
+</body>
+</html>
index 4b16ee4..9a3343f 100644 (file)
@@ -1,5 +1,20 @@
 2017-11-10  Chris Dumez  <cdumez@apple.com>
 
+        [Service Workers] Registration promise should be rejected when the service worker fails to start
+        https://bugs.webkit.org/show_bug.cgi?id=179566
+
+        Reviewed by Brady Eidson.
+
+        Rebaseline WPT tests now that we properly reject registration promises when the worker script
+        fails to evaluate.
+
+        * web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/registration-attribute.https-expected.txt:
+        * web-platform-tests/service-workers/service-worker/navigation-redirect.https-expected.txt:
+        * web-platform-tests/service-workers/service-worker/redirected-response.https-expected.txt:
+        * web-platform-tests/service-workers/service-worker/update.https-expected.txt:
+
+2017-11-10  Chris Dumez  <cdumez@apple.com>
+
         [Service Workers] Implement better support for "Clear Registration" algorithm
         https://bugs.webkit.org/show_bug.cgi?id=179441
 
index 847f418..5b1d1a8 100644 (file)
@@ -1,4 +1,3 @@
 
-
-FAIL Verify registration attribute on ServiceWorkerGlobalScope assert_equals: Service Worker should respond to fetch expected "updatefound,install,statechange(installed),statechange(activating),activate,statechange(activated),fetch" but got "{\"error\": {\"message\": \"\", \"code\": 404}}"
+FAIL Verify registration attribute on ServiceWorkerGlobalScope assert_unreached: unregister and register should not fail: TypeError: null is not an object (evaluating 'self.registration.scope') Reached unreachable code
 
index cd75706..7af528d 100644 (file)
@@ -1,33 +1,34 @@
-CONSOLE MESSAGE: line 126: TypeError: resolve is not a function. (In 'resolve(e.data.result)', 'resolve' is undefined)
+CONSOLE MESSAGE: line 50: Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'registrations[0].unregister')
+CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: TypeError: null is not an object (evaluating 'self.registration.scope')
 
 
-FAIL Normal redirect to same-origin scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Normal redirect to other-origin scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fallbacked redirect to same-origin out-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fallbacked redirect to same-origin same-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fallbacked redirect to same-origin other-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fallbacked redirect to other-origin out-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fallbacked redirect to other-origin in-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-generated redirect to same-origin out-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-generated redirect to same-origin same-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-generated redirect to same-origin other-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-generated redirect to other-origin out-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-generated redirect to other-origin in-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fetched redirect to same-origin out-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fetched redirect to same-origin same-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fetched redirect to same-origin other-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fetched redirect to other-origin out-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL SW-fetched redirect to other-origin in-scope. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to same-origin out-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to same-origin same-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to same-origin other-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to other-origin out-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to other-origin in-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL No location redirect response. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to same-origin out-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to same-origin same-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to same-origin other-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to other-origin out-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Redirect to other-origin in-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL No location redirect response via Cache. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
+FAIL Normal redirect to same-origin scope. assert_unreached: unregister and register should not fail: TypeError: null is not an object (evaluating 'self.registration.scope') Reached unreachable code
+FAIL Normal redirect to other-origin scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fallbacked redirect to same-origin out-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fallbacked redirect to same-origin same-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fallbacked redirect to same-origin other-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fallbacked redirect to other-origin out-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fallbacked redirect to other-origin in-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-generated redirect to same-origin out-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-generated redirect to same-origin same-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-generated redirect to same-origin other-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-generated redirect to other-origin out-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-generated redirect to other-origin in-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fetched redirect to same-origin out-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fetched redirect to same-origin same-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fetched redirect to same-origin other-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fetched redirect to other-origin out-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL SW-fetched redirect to other-origin in-scope. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to same-origin out-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to same-origin same-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to same-origin other-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to other-origin out-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to other-origin in-scope with opaque redirect response. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL No location redirect response. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to same-origin out-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to same-origin same-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to same-origin other-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to other-origin out-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL Redirect to other-origin in-scope with opaque redirect response which is passed through Cache. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
+FAIL No location redirect response via Cache. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.active')"
 
index 2a1dc7d..a517181 100644 (file)
@@ -1,21 +1,20 @@
 
-FAIL initialize global state (service worker registration and caches) promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "follow", non-intercepted request, no server redirect promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "follow", non-intercepted request promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "manual", non-intercepted request promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "error", non-intercepted request promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "follow", no mode change, no server redirect promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "follow", no mode change promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "error", mode change: "follow" promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "manual", mode change: "follow" promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "follow", mode change: "manual" promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "error", mode change: "manual" promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "manual", no mode change promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "follow", generated redirect response promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "error", generated redirect response promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL mode: "manual", generated redirect response promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Fetch should follow the redirect response 20 times promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-FAIL Fetch should not follow the redirect response 21 times. promise_test: Unhandled rejection with value: object "NotSupportedError: Passing MessagePort objects to postMessage is not yet supported"
-PASS restore global state (service worker registration) 
-PASS restore global state (caches) 
+FAIL initialize global state (service worker registration and caches) assert_unreached: unregister and register should not fail: TypeError: null is not an object (evaluating 'self.registration.scope') Reached unreachable code
+FAIL mode: "follow", non-intercepted request, no server redirect promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "follow", non-intercepted request promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "manual", non-intercepted request promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "error", non-intercepted request promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "follow", no mode change, no server redirect promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "follow", no mode change promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "error", mode change: "follow" promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "manual", mode change: "follow" promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "follow", mode change: "manual" promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "error", mode change: "manual" promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "manual", no mode change promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "follow", generated redirect response promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "error", generated redirect response promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL mode: "manual", generated redirect response promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL Fetch should follow the redirect response 20 times promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL Fetch should not follow the redirect response 21 times. promise_test: Unhandled rejection with value: object "TypeError: undefined is not an object (evaluating 'registration.installing')"
+FAIL restore global state (service worker registration) undefined is not an object (evaluating 'registration.unregister')
 
index 3a6a2b7..60debfb 100644 (file)
@@ -1,3 +1,27 @@
+2017-11-10  Chris Dumez  <cdumez@apple.com>
+
+        [Service Workers] Registration promise should be rejected when the service worker fails to start
+        https://bugs.webkit.org/show_bug.cgi?id=179566
+
+        Reviewed by Brady Eidson.
+
+        Registration promise should be rejected when the service worker fails to start.
+
+        Though our code intended to do this, there were several issues:
+        - Our code failed to properly detect when the script failed to evaluate. This is
+          because it relied on the exception message being non-null but it was always
+          null when same origin due to a bug.
+        - Our scriptContextFailedToStart() handler failed to rejected the promise
+          and finish the job as per:
+          - https://w3c.github.io/ServiceWorker/#update-algorithm (step 9.7)
+
+        Test: http/tests/workers/service/worker-fails-to-start.html
+
+        * bindings/js/WorkerScriptController.cpp:
+        (WebCore::WorkerScriptController::evaluate):
+        * workers/service/server/SWServerJobQueue.cpp:
+        (WebCore::SWServerJobQueue::scriptContextFailedToStart):
+
 2017-11-10  Jer Noble  <jer.noble@apple.com>
 
         Unreviewed Win Debug build fix; add FourCC.cpp to the cmake source list for Windows.
index 41cbec0..c02aa12 100644 (file)
@@ -152,16 +152,15 @@ void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, NakedP
     }
 
     if (returnedException) {
-        String errorMessage;
+        String errorMessage = returnedException->value().toWTFString(exec);
         int lineNumber = 0;
         int columnNumber = 0;
         String sourceURL = sourceCode.url().string();
         JSC::Strong<JSC::Unknown> error;
-        if (m_workerGlobalScope->sanitizeScriptError(errorMessage, lineNumber, columnNumber, sourceURL, error, sourceCode.cachedScript())) {
-            if (returnedExceptionMessage)
-                *returnedExceptionMessage = errorMessage;
+        if (m_workerGlobalScope->sanitizeScriptError(errorMessage, lineNumber, columnNumber, sourceURL, error, sourceCode.cachedScript()))
             returnedException = JSC::Exception::create(vm, createError(exec, errorMessage.impl()));
-        }
+        if (returnedExceptionMessage)
+            *returnedExceptionMessage = errorMessage;
     }
 }
 
index c97a398..cec6290 100644 (file)
@@ -83,21 +83,22 @@ void SWServerJobQueue::scriptFetchFinished(SWServer::Connection& connection, con
     m_server.updateWorker(connection, m_registrationKey, job.scriptURL, result.script, WorkerType::Classic);
 }
 
-void SWServerJobQueue::scriptContextFailedToStart(SWServer::Connection&, ServiceWorkerIdentifier identifier, const String& message)
+// https://w3c.github.io/ServiceWorker/#update-algorithm
+void SWServerJobQueue::scriptContextFailedToStart(SWServer::Connection&, ServiceWorkerIdentifier, const String& message)
 {
+    // If an uncaught runtime script error occurs during the above step, then:
     auto* registration = m_server.getRegistration(m_registrationKey);
     ASSERT(registration);
 
-    // FIXME: Install has failed. Run the install failed substeps
-    // Run the Update Worker State algorithm passing registration's installing worker and redundant as the arguments.
-    // Run the Update Registration State algorithm passing registration, "installing" and null as the arguments.
+    // Invoke Reject Job Promise with job and TypeError.
+    m_server.rejectJob(firstJob(), { TypeError, message });
 
     // If newestWorker is null, invoke Clear Registration algorithm passing registration as its argument.
     if (!registration->getNewestWorker())
         clearRegistration(*registration);
 
-    UNUSED_PARAM(identifier);
-    UNUSED_PARAM(message);
+    // Invoke Finish Job with job and abort these steps.
+    finishCurrentJob();
 }
 
 void SWServerJobQueue::scriptContextStarted(SWServer::Connection& connection, ServiceWorkerIdentifier identifier)