Changeset 235570 in webkit

Timestamp:

Aug 31, 2018 2:48:18 PM (6 years ago)

Author:

Chris Dumez

Message:

[ 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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html (modified) (4 diffs)
• LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html (modified) (4 diffs)
• LayoutTests/http/tests/workers/service/resources/getClientIds-worker.js (moved) (moved from trunk/LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js) (1 diff)
• LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js (modified) (3 diffs)
• LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html (modified) (2 diffs)
• LayoutTests/platform/ios-wk2/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac-wk2/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/testing/Internals.cpp (modified) (1 diff)
• Source/WebCore/testing/Internals.h (modified) (1 diff)
• Source/WebCore/testing/Internals.idl (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html

@@ -14 +14 @@
 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;
     }
@@ -44 +55 @@
        setTimeout(function() {
          step = "SecondClientRestoredFromPageCache";
-         worker.postMessage("getClientCount");
+         worker.postMessage("getClientIds");
        }, 0);
     });
@@ -53 +64 @@
 
   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();
     }
@@ -63 +74 @@
 });
 
-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");
     };
 });

trunk/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html

@@ -12 +12 @@
 }
 
+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;
     }
@@ -37 +50 @@
        setTimeout(function() {
          step = "OnlyOneClientRemainsActive"
-         worker.postMessage("getClientCount");
+         worker.postMessage("getClientIds");
        }, 0);
     });
@@ -46 +59 @@
 
   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();
     }
@@ -56 +69 @@
 });
 
-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");
     };
 });

trunk/LayoutTests/http/tests/workers/service/resources/getClientIds-worker.js

@@ -1 +1 @@
 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);
     });
 });

trunk/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js

@@ -4 +4 @@
 }
 
-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;
     });
 }
@@ -27 +33 @@
 {
     try {
-        if (event.data !== "start") {
+        if (event.data.test !== "checkClientIsUncontrolled") {
             event.source.postMessage("FAIL: received unexpected message from client");
+            return;
+        }
+
+        const clientIdentifier = event.data.identifier;
+
+        let result = await checkClientNotInControlledClients(clientIdentifier);
+        if (result !== "PASS") {
+            event.source.postMessage(result);
             return;
         }
@@ -36 +50 @@
             if (tries)
                 await waitFor(50);
-            result = await matchAllPromise1();
+            result = await checkClientInUncontrolledClients(clientIdentifier);
         } while (result !== "PASS" && ++tries <= 200);
 
-        if (result !== "PASS") {
-            event.source.postMessage(result);
-            return;
-        }
-
-        tries = 0;
-        do {
-            if (tries)
-                await waitFor(50);
-            result = await matchAllPromise2();
-        } 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);

trunk/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html

@@ -28 +28 @@
 
 promise_test(async (test) => {
+    const serviceWorkerClientIdentifier = internals.serviceWorkerClientIdentifier(document);
     var promise = new Promise((resolve, reject) => {
         navigator.serviceWorker.addEventListener("message", test.step_func((event) => {
@@ -35 +36 @@
     });
 
-    activeWorker.postMessage("start");
+    activeWorker.postMessage({test: "checkClientIsUncontrolled", identifier: serviceWorkerClientIdentifier });
     await promise;
 }, "Test self.clients.matchAll");

trunk/LayoutTests/platform/ios-wk2/TestExpectations

@@ -1278 +1278 @@
 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 ]
 

trunk/LayoutTests/platform/mac-wk2/TestExpectations

@@ -832 +832 @@
 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 ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/testing/Internals.cpp

@@ -2329 +2329 @@
 }
 
+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)
 {

trunk/Source/WebCore/testing/Internals.h

@@ -372 +372 @@
     bool isDocumentAlive(uint64_t documentIdentifier) const;
 
+    String serviceWorkerClientIdentifier(const Document&) const;
+
     RefPtr<WindowProxy> openDummyInspectorFrontend(const String& url);
     void closeDummyInspectorFrontend();

trunk/Source/WebCore/testing/Internals.idl

@@ -625 +625 @@
     boolean isDocumentAlive(unsigned long long documentIdentifier);
 
+    DOMString serviceWorkerClientIdentifier(Document document);
+
     Promise<void> clearCacheStorageMemoryRepresentation();
     Promise<DOMString> cacheStorageEngineRepresentation();