Layout Test http/tests/workers/service/postmessage-after-sw-process-crash.https.html...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Dec 2017 22:49:10 +0000 (22:49 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Dec 2017 22:49:10 +0000 (22:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180659

Reviewed by Youenn Fablet.

Source/WebKit:

Fix flaky crash due to capturing an IPC::DataReference in a lambda, which would point to invalid
memory when the lambda is called asynchronously.

* StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::postMessageToServiceWorkerFromClient):
(WebKit::WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker):

LayoutTests:

Rewrite test so that it is no longer flaky.

* http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt:
* http/tests/workers/service/resources/postmessage-after-sw-process-crash-worker.js: Added.
* http/tests/workers/service/resources/postmessage-after-sw-process-crash.js:

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

LayoutTests/ChangeLog
LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt
LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash-worker.js [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash.js
Source/WebKit/ChangeLog
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp

index 9e9e6ca..eeff771 100644 (file)
@@ -1,3 +1,16 @@
+2017-12-11  Chris Dumez  <cdumez@apple.com>
+
+        Layout Test http/tests/workers/service/postmessage-after-sw-process-crash.https.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=180659
+
+        Reviewed by Youenn Fablet.
+
+        Rewrite test so that it is no longer flaky.
+
+        * http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt:
+        * http/tests/workers/service/resources/postmessage-after-sw-process-crash-worker.js: Added.
+        * http/tests/workers/service/resources/postmessage-after-sw-process-crash.js:
+
 2017-12-11  David Quesada  <david_quesada@apple.com>
 
         Turn on ENABLE_APPLICATION_MANIFEST
index c10c281..7f70d05 100644 (file)
@@ -1,8 +1,5 @@
-* Sending 'Message 1' to Service Worker
-PASS: Client received message from service worker, origin: https://127.0.0.1:8443
-PASS: Service worker received message 'Message 1' from origin 'https://127.0.0.1:8443'
+* Sending State to Service Worker
+* Asking Service Worker if it received the state
 * Simulating Service Worker process crash
-* Sending 'Message 2' to Service Worker
-PASS: Client received message from service worker, origin: https://127.0.0.1:8443
-PASS: Service worker received message 'Message 2' from origin 'https://127.0.0.1:8443'
+PASS: service worker lost the state and responded the postMessage after process termination
 
diff --git a/LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash-worker.js b/LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash-worker.js
new file mode 100644 (file)
index 0000000..1c6ec81
--- /dev/null
@@ -0,0 +1,15 @@
+var client = null;
+let receivedState = false;
+
+self.addEventListener("message", (event) => {
+    if (event.data === "SetState") {
+        receivedState = true;
+        return;
+    }
+
+    if (event.data === "HasState") {
+        event.source.postMessage(receivedState);
+        return;
+    }
+});
+
index 4e5ed92..4f72164 100644 (file)
@@ -1,28 +1,46 @@
-var messageNumber = 1;
+let serviceWorkerHasReceivedState = false;
+let worker = null;
+let remainingAttempts = 1000; // We try for 10 seconds before timing out.
+
 navigator.serviceWorker.addEventListener("message", function(event) {
-    log("PASS: Client received message from service worker, origin: " + event.origin);
-    log(event.data);
-    if (messageNumber == 1) {
+    if (!serviceWorkerHasReceivedState) {
+        if (!event.data) {
+            log("FAIL: service worker did not receive the state");
+            finishSWTest();
+            return;
+        }
+        serviceWorkerHasReceivedState = true;
+
         log("* Simulating Service Worker process crash");
         testRunner.terminateServiceWorkerProcess();
-        setTimeout(function() {
-            log("* Sending 'Message 2' to Service Worker");
-            event.source.postMessage("Message 2");
-            messageNumber++;
-            handle = setTimeout(function() {
-                log("FAIL: Did not receive message from service worker process after the crash");
+
+        handle = setInterval(function() {
+            remainingAttempts--;
+            if (!remainingAttempts) {
+                log("FAIL: service worker did not respond after process termination");
+                clearInterval(handle);
                 finishSWTest();
-            }, 1000);
-        }, 0);
+                return;
+            }
+
+            worker.postMessage("HasState");
+        }, 10);
         return;
     }
-    if (messageNumber == 2) {
-        clearTimeout(handle);
-        finishSWTest();
-    }
+
+    // Worker still has the state, it was not terminated yet.
+    if (event.data === serviceWorkerHasReceivedState)
+        return;
+
+    log("PASS: service worker lost the state and responded the postMessage after process termination");
+    clearInterval(handle);
+    finishSWTest();
 });
 
-navigator.serviceWorker.register("resources/postmessage-echo-worker.js", { }).then(function(registration) {
-    log("* Sending 'Message 1' to Service Worker");
-    registration.installing.postMessage("Message 1");
+navigator.serviceWorker.register("resources/postmessage-after-sw-process-crash-worker.js", { }).then(function(registration) {
+    worker = registration.installing;
+    log("* Sending State to Service Worker");
+    worker.postMessage("SetState");
+    log("* Asking Service Worker if it received the state");
+    worker.postMessage("HasState");
 });
index e3b1619..71b2027 100644 (file)
@@ -1,3 +1,17 @@
+2017-12-11  Chris Dumez  <cdumez@apple.com>
+
+        Layout Test http/tests/workers/service/postmessage-after-sw-process-crash.https.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=180659
+
+        Reviewed by Youenn Fablet.
+
+        Fix flaky crash due to capturing an IPC::DataReference in a lambda, which would point to invalid
+        memory when the lambda is called asynchronously.
+
+        * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
+        (WebKit::WebSWServerConnection::postMessageToServiceWorkerFromClient):
+        (WebKit::WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker):
+
 2017-12-11  Brent Fulgham  <bfulgham@apple.com>
 
         [iOS] Don't import 'UIKit-apps.sb' to the WebContent process sandbox
index 74811f4..90c97b3 100644 (file)
@@ -130,7 +130,7 @@ void WebSWServerConnection::startFetch(uint64_t fetchIdentifier, ServiceWorkerId
 void WebSWServerConnection::postMessageToServiceWorkerFromClient(ServiceWorkerIdentifier destinationIdentifier, IPC::DataReference&& message, ServiceWorkerClientIdentifier sourceIdentifier, ServiceWorkerClientData&& sourceData)
 {
     // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
-    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceIdentifier, sourceData = WTFMove(sourceData)](bool success, auto& contextConnection) mutable {
+    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = message.vector(), sourceIdentifier, sourceData = WTFMove(sourceData)](bool success, auto& contextConnection) mutable {
         if (success)
             sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorkerFromClient { destinationIdentifier, message, sourceIdentifier, WTFMove(sourceData) });
     });
@@ -143,7 +143,7 @@ void WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker(ServiceW
         return;
 
     // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
-    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceData = sourceWorker->data()](bool success, auto& contextConnection) mutable {
+    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = message.vector(), sourceData = sourceWorker->data()](bool success, auto& contextConnection) mutable {
         if (!success)
             return;