[ WK2 ] http/tests/workers/service/client-*-page-cache.html LayoutTests are flaky
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 Aug 2018 21:48:18 +0000 (21:48 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 Aug 2018 21:48:18 +0000 (21:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183705
<rdar://problem/42440606>

Reviewed by Youenn Fablet.

Source/WebCore:

Add internals.serviceWorkerClientIdentifier() utility function so that a layout test can get the
service worker client identifier of a document.

* testing/Internals.cpp:
(WebCore::Internals::serviceWorkerClientIdentifier const):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Update Layout tests to not rely on the total number of clients as this is flaky. Instead, check for specific client
identifiers to see if they are present or not.

* http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html:
* http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html:
* http/tests/workers/service/resources/getClientIds-worker.js: Renamed from LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js.
(event.then):
* http/tests/workers/service/serviceworkerclients-matchAll-worker.js:
(async.doTestAfterMessage):
* http/tests/workers/service/serviceworkerclients-matchAll.https.html:

* platform/ios-wk2/TestExpectations:
* platform/mac-wk2/TestExpectations:
Unskip tests as they should no longer be flaky.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
LayoutTests/http/tests/workers/service/resources/getClientIds-worker.js [moved from LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js with 51% similarity]
LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js
LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 3147864..9972e44 100644 (file)
@@ -1,3 +1,26 @@
+2018-08-31  Chris Dumez  <cdumez@apple.com>
+
+        [ WK2 ] http/tests/workers/service/client-*-page-cache.html LayoutTests are flaky
+        https://bugs.webkit.org/show_bug.cgi?id=183705
+        <rdar://problem/42440606>
+
+        Reviewed by Youenn Fablet.
+
+        Update Layout tests to not rely on the total number of clients as this is flaky. Instead, check for specific client
+        identifiers to see if they are present or not.
+
+        * http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html:
+        * http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html:
+        * http/tests/workers/service/resources/getClientIds-worker.js: Renamed from LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js.
+        (event.then):
+        * http/tests/workers/service/serviceworkerclients-matchAll-worker.js:
+        (async.doTestAfterMessage):
+        * http/tests/workers/service/serviceworkerclients-matchAll.https.html:
+
+        * platform/ios-wk2/TestExpectations:
+        * platform/mac-wk2/TestExpectations:
+        Unskip tests as they should no longer be flaky.
+
 2018-08-31  John Wilander  <wilander@apple.com>
 
         Storage Access API: Maintain access through same-site navigations
index 21cacb6..62bd629 100644 (file)
@@ -13,15 +13,26 @@ if (window.testRunner) {
 
 let tries = 0;
 
+let expectedClientIdentifiers = [];
+
+function containsExpectedClients(clientIdentifiers)
+{
+    for (let expectedIdentifier of expectedClientIdentifiers) {
+        if (!clientIdentifiers.includes(expectedIdentifier))
+            return false;
+    }
+    return true;
+}
+
 navigator.serviceWorker.addEventListener("message", function(event) {
   if (step == "BothClientsInitiallyActive") {
-    if (event.data != 2) {
+    if (!containsExpectedClients(event.data)) {
       if (++tries > 20) {
-          log("FAIL: Wrong initial number of clients: " + event.data);
+          log("FAIL: Wrong initial number of clients");
           finishSWTest();
           return;
       }
-      worker.postMessage("getClientCount");
+      worker.postMessage("getClientIds");
       return;
     }
     log("PASS: service worker has initially 2 clients");
@@ -43,7 +54,7 @@ navigator.serviceWorker.addEventListener("message", function(event) {
        }
        setTimeout(function() {
          step = "SecondClientRestoredFromPageCache";
-         worker.postMessage("getClientCount");
+         worker.postMessage("getClientIds");
        }, 0);
     });
 
@@ -52,8 +63,8 @@ navigator.serviceWorker.addEventListener("message", function(event) {
   }
 
   if (step == "SecondClientRestoredFromPageCache") {
-    if (event.data != 2) {
-      log("FAIL: Wrong number of clients after one client was restored from page cache: " + event.data);
+    if (!containsExpectedClients(event.data)) {
+      log("FAIL: Wrong number of clients after one client was restored from page cache");
       finishSWTest();
     }
 
@@ -62,12 +73,15 @@ navigator.serviceWorker.addEventListener("message", function(event) {
   }
 });
 
-navigator.serviceWorker.register("resources/getClientCount-worker.js", { }).then(function(registration) {
+navigator.serviceWorker.register("resources/getClientIds-worker.js", { }).then(function(registration) {
+    expectedClientIdentifiers.push(internals.serviceWorkerClientIdentifier(document));
+
     worker = registration.installing;
     otherWindow = open("other_resources/test.html");
     otherWindow.onload = function() {
+      expectedClientIdentifiers.push(internals.serviceWorkerClientIdentifier(otherWindow.document));
       step = "BothClientsInitiallyActive"
-      worker.postMessage("getClientCount");
+      worker.postMessage("getClientIds");
     };
 });
 </script>
index 2055dbb..b10919a 100644 (file)
@@ -11,17 +11,30 @@ if (window.testRunner) {
   testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
 }
 
+let topClientIdentifier = null;
+let windowClientIdentifier = null;
+
 let tries = 0;
 
+function containsBothClients(clientIdentifiers)
+{
+    return clientIdentifiers.includes(topClientIdentifier) && clientIdentifiers.includes(windowClientIdentifier);
+}
+
+function containsOnlyTopClient(clientIdentifiers)
+{
+    return clientIdentifiers.includes(topClientIdentifier) && !clientIdentifiers.includes(windowClientIdentifier);
+}
+
 navigator.serviceWorker.addEventListener("message", function(event) {
   if (step == "BothClientsInitiallyActive") {
-    if (event.data != 2) {
+    if (!containsBothClients(event.data)) {
       if (++tries > 20) {
-          log("FAIL: Wrong initial number of clients: " + event.data);
+          log("FAIL: Wrong initial number of clients");
           finishSWTest();
           return;
       }
-      worker.postMessage("getClientCount");
+      worker.postMessage("getClientIds");
       return;
     }
     log("PASS: service worker has initially 2 clients");
@@ -36,7 +49,7 @@ navigator.serviceWorker.addEventListener("message", function(event) {
 
        setTimeout(function() {
          step = "OnlyOneClientRemainsActive"
-         worker.postMessage("getClientCount");
+         worker.postMessage("getClientIds");
        }, 0);
     });
 
@@ -45,8 +58,8 @@ navigator.serviceWorker.addEventListener("message", function(event) {
   }
 
   if (step == "OnlyOneClientRemainsActive") {
-    if (event.data != 1) {
-      log("FAIL: Wrong number of clients after one client entered page cache: " + event.data);
+    if (!containsOnlyTopClient(event.data)) {
+      log("FAIL: Wrong number of clients after one client entered page cache");
       finishSWTest();
     }
 
@@ -55,12 +68,15 @@ navigator.serviceWorker.addEventListener("message", function(event) {
   }
 });
 
-navigator.serviceWorker.register("resources/getClientCount-worker.js", { }).then(function(registration) {
+navigator.serviceWorker.register("resources/getClientIds-worker.js", { }).then(function(registration) {
+    topClientIdentifier = internals.serviceWorkerClientIdentifier(document);
+
     worker = registration.installing;
     otherWindow = open("other_resources/test.html");
     otherWindow.onload = function() {
+      windowClientIdentifier = internals.serviceWorkerClientIdentifier(otherWindow.document);
       step = "BothClientsInitiallyActive"
-      worker.postMessage("getClientCount");
+      worker.postMessage("getClientIds");
     };
 });
 </script>
@@ -1,6 +1,9 @@
 self.addEventListener("message", (event) => {
     source = event.source;
-    clients.matchAll({ includeUncontrolled : true }).then(function(clientList) {
-        source.postMessage(clientList.length);
+    clients.matchAll({ includeUncontrolled : true }).then(function(clients) {
+        let ids = [];
+        for (let client of clients)
+            ids.push(client.id);
+        source.postMessage(ids);
     });
 });
index bc0da98..bdba282 100644 (file)
@@ -3,58 +3,56 @@ function waitFor(duration)
     return new Promise((resolve) => setTimeout(resolve, duration));
 }
 
-function matchAllPromise1()
+function checkClientNotInControlledClients(clientIdentifier)
 {
     return self.clients.matchAll().then((clients) => {
-        return clients.length === 0 ? "PASS" : "FAIL: expected no matched client, got " + clients.length;
+        for (let client of clients) {
+            if (client.id == clientIdentifier)
+                return "FAIL: Client should not have matched";
+        }
+        return "PASS";
     }, (e) => {
-        return "FAIL: matchAll 1 rejected with " + e;
+        return "FAIL: First matchAll() request rejected with " + e;
     });
 }
 
-var matchedClients;
-function matchAllPromise2()
+function checkClientInUncontrolledClients(clientIdentifier)
 {
-    return self.clients.matchAll({ includeUncontrolled : true }).then((c) => {
-        matchedClients = c;
-        return matchedClients.length === 1 ? "PASS" : "FAIL: expected one matched client, got " + matchedClients.length;
+    return self.clients.matchAll({ includeUncontrolled : true }).then((clients) => {
+        for (let client of clients) {
+            if (client.id == clientIdentifier)
+                return "PASS";
+        }
+        return "FAIL: Client should have matched with includeUncontrolled set to true";
     }, (e) => {
-        return "FAIL: matchAll 2 rejected with " + e;
+        return "FAIL: Second matchAll() request rejected with " + e;
     });
 }
 
 async function doTestAfterMessage(event)
 {
     try {
-        if (event.data !== "start") {
+        if (event.data.test !== "checkClientIsUncontrolled") {
             event.source.postMessage("FAIL: received unexpected message from client");
             return;
         }
 
-        let tries = 0;
-        do {
-            if (tries)
-                await waitFor(50);
-            result = await matchAllPromise1();
-        } while (result !== "PASS" && ++tries <= 200);
+        const clientIdentifier = event.data.identifier;
 
+        let result = await checkClientNotInControlledClients(clientIdentifier);
         if (result !== "PASS") {
             event.source.postMessage(result);
             return;
         }
 
-        tries = 0;
+        let tries = 0;
         do {
             if (tries)
                 await waitFor(50);
-            result = await matchAllPromise2();
+            result = await checkClientInUncontrolledClients(clientIdentifier);
         } while (result !== "PASS" && ++tries <= 200);
 
-        if (result !== "PASS") {
-            event.source.postMessage(result);
-            return;
-        }
-        event.source.postMessage("PASS");
+        event.source.postMessage(result);
     } catch (e) {
         event.source.postMessage("FAIL: received exception " + e);
     }
index 9af4d98..2e6dbf9 100644 (file)
@@ -27,6 +27,7 @@ promise_test(async (test) => {
 }, "Setup worker");
 
 promise_test(async (test) => {
+    const serviceWorkerClientIdentifier = internals.serviceWorkerClientIdentifier(document);
     var promise = new Promise((resolve, reject) => {
         navigator.serviceWorker.addEventListener("message", test.step_func((event) => {
             assert_equals(event.data, "PASS");
@@ -34,7 +35,7 @@ promise_test(async (test) => {
         }));
     });
 
-    activeWorker.postMessage("start");
+    activeWorker.postMessage({test: "checkClientIsUncontrolled", identifier: serviceWorkerClientIdentifier });
     await promise;
 }, "Test self.clients.matchAll");
 </script>
index cb19c75..ac0b0eb 100644 (file)
@@ -1277,10 +1277,6 @@ webkit.org/b/184245 http/tests/workers/service/service-worker-cache-api.https.ht
 
 webkit.org/b/184783 compositing/ios/overflow-scroll-touch-tiles.html [ Pass Failure ]
 
-webkit.org/b/183705 http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html [ Pass Failure ]
-webkit.org/b/183705 http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html [ Pass Failure ]
-webkit.org/b/183705 http/tests/workers/service/serviceworkerclients-matchAll.https.html [ Pass Failure ]
-
 webkit.org/b/179853 [ Debug ] imported/blink/fast/text/international-iteration-simple-text.html [ Pass Failure ]
 
 # skip manual payment-request tests
index 0b24e8f..e1fefc2 100644 (file)
@@ -831,10 +831,6 @@ webkit.org/b/186362 [ Release ] http/tests/resourceLoadStatistics/prevalent-reso
 
 webkit.org/b/186425 [ Debug ] inspector/console/webcore-logging.html [ Pass Failure ]
 
-webkit.org/b/183705 http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html [ Pass Failure ]
-webkit.org/b/183705 http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html [ Pass Failure ]
-webkit.org/b/183705 http/tests/workers/service/serviceworkerclients-matchAll.https.html [ Pass Failure ]
-
 webkit.org/b/187658 http/tests/security/bypassing-cors-checks-for-extension-urls.html [ Pass Failure ]
 
 [ Mojave+ ] fast/canvas/webgl/context-update-on-display-configuration.html [ Pass ]
index 98f8147..74c70b5 100644 (file)
@@ -1,3 +1,19 @@
+2018-08-31  Chris Dumez  <cdumez@apple.com>
+
+        [ WK2 ] http/tests/workers/service/client-*-page-cache.html LayoutTests are flaky
+        https://bugs.webkit.org/show_bug.cgi?id=183705
+        <rdar://problem/42440606>
+
+        Reviewed by Youenn Fablet.
+
+        Add internals.serviceWorkerClientIdentifier() utility function so that a layout test can get the
+        service worker client identifier of a document.
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::serviceWorkerClientIdentifier const):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2018-08-31  John Wilander  <wilander@apple.com>
 
         Storage Access API: Maintain access through same-site navigations
index 49fc7c0..2ae2da8 100644 (file)
@@ -2328,6 +2328,16 @@ bool Internals::isDocumentAlive(uint64_t documentIdentifier) const
     return Document::allDocumentsMap().contains(makeObjectIdentifier<DocumentIdentifierType>(documentIdentifier));
 }
 
+String Internals::serviceWorkerClientIdentifier(const Document& document) const
+{
+#if ENABLE(SERVICE_WORKER)
+    return ServiceWorkerClientIdentifier { ServiceWorkerProvider::singleton().serviceWorkerConnectionForSession(document.sessionID()).serverConnectionIdentifier(), document.identifier() }.toString();
+#else
+    UNUSED_PARAM(document);
+    return String();
+#endif
+}
+
 RefPtr<WindowProxy> Internals::openDummyInspectorFrontend(const String& url)
 {
     auto* inspectedPage = contextDocument()->frame()->page();
index 8199ea4..40cdbef 100644 (file)
@@ -371,6 +371,8 @@ public:
     uint64_t documentIdentifier(const Document&) const;
     bool isDocumentAlive(uint64_t documentIdentifier) const;
 
+    String serviceWorkerClientIdentifier(const Document&) const;
+
     RefPtr<WindowProxy> openDummyInspectorFrontend(const String& url);
     void closeDummyInspectorFrontend();
     ExceptionOr<void> setInspectorIsUnderTest(bool);
index 04f77ac..53e2c78 100644 (file)
@@ -624,6 +624,8 @@ enum CompositingPolicy {
     unsigned long long documentIdentifier(Document document);
     boolean isDocumentAlive(unsigned long long documentIdentifier);
 
+    DOMString serviceWorkerClientIdentifier(Document document);
+
     Promise<void> clearCacheStorageMemoryRepresentation();
     Promise<DOMString> cacheStorageEngineRepresentation();
     void setResponseSizeWithPadding(FetchResponse response, unsigned long long size);