Attachment 348674 Details for Bug 183705 – Patch

[patch] Patch

bug-183705-20180831144631.patch (text/plain), 18.34 KB, created by Chris Dumez on 2018-08-31 14:46:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-08-31 14:46:32 PDT

Size: 18.34 KB

patch

obsolete

>Subversion Revision: 235566
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 38be1efa13f8c48aa4910c3a6b3ae57acbac4eea..74088a47ac43b97c27c675b9e0df0b864e342690 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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  Youenn Fablet  <youenn@apple.com>
> 
>         WebAudioBufferList.cpp/.h is named as cocoa/WebAudioBufferList.cpp/.h
>diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
>index 49fc7c061f99b4b57bc7e0476e495fcebec50e80..2ae2da8c5d0cd6d181c6df52982722ab5a822b07 100644
>--- a/Source/WebCore/testing/Internals.cpp
>+++ b/Source/WebCore/testing/Internals.cpp
>@@ -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();
>diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
>index 8199ea4c67d24e4456cd7d3d2c5b4b0f732c20d9..40cdbef12acb2e162ffbd21f47ec099189c95d6e 100644
>--- a/Source/WebCore/testing/Internals.h
>+++ b/Source/WebCore/testing/Internals.h
>@@ -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);
>diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
>index 04f77ac7fc9c67c6b3279a3c2441e08ae40b041b..53e2c78eae28981e0ea197b4af3b0071aadbaf6d 100644
>--- a/Source/WebCore/testing/Internals.idl
>+++ b/Source/WebCore/testing/Internals.idl
>@@ -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);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 79d6cc866e8b67a8d0396c7878af8e97a55bee0e..0be75ac28761edcdd87dceb20c4c6ac3a6b0cc33 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -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  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Add margin box verification back now that Display::Box has non-computed horizontal margin.
>diff --git a/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html b/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
>index 21cacb6b4b8dd53f0d2cfc4a95db64795ac46800..62bd62941890f91696c9cb2322be226e59a576a0 100644
>--- a/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
>+++ b/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
>@@ -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>
>diff --git a/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html b/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
>index 2055dbbb4423ef3a38371301b9823db2b106f007..b10919ac73d072bf76cfa15c32a768c2c61e1274 100644
>--- a/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
>+++ b/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
>@@ -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>
>diff --git a/LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js b/LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js
>deleted file mode 100644
>index 8b115aca6b681dd737086deb75a7bb7c03cfc622..0000000000000000000000000000000000000000
>--- a/LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js
>+++ /dev/null
>@@ -1,6 +0,0 @@
>-self.addEventListener("message", (event) => {
>-    source = event.source;
>-    clients.matchAll({ includeUncontrolled : true }).then(function(clientList) {
>-        source.postMessage(clientList.length);
>-    });
>-});
>diff --git a/LayoutTests/http/tests/workers/service/resources/getClientIds-worker.js b/LayoutTests/http/tests/workers/service/resources/getClientIds-worker.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..99dc1cb8221d0df8df0ceec7f5fa6b1083d73b18
>--- /dev/null
>+++ b/LayoutTests/http/tests/workers/service/resources/getClientIds-worker.js
>@@ -0,0 +1,9 @@
>+self.addEventListener("message", (event) => {
>+    source = event.source;
>+    clients.matchAll({ includeUncontrolled : true }).then(function(clients) {
>+        let ids = [];
>+        for (let client of clients)
>+            ids.push(client.id);
>+        source.postMessage(ids);
>+    });
>+});
>diff --git a/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js b/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js
>index bc0da9857cfd3ca3fcdc8e42664acfa79f4fd8ad..bdba28286053f1fdad035d3933c28680117368c3 100644
>--- a/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js
>+++ b/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll-worker.js
>@@ -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);
>     }
>diff --git a/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html b/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html
>index 9af4d986548457bda49ef74fce5983fc145a7aaa..2e6dbf992d052d9f1e3e518dd7b0e851ef2000a7 100644
>--- a/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html
>+++ b/LayoutTests/http/tests/workers/service/serviceworkerclients-matchAll.https.html
>@@ -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>
>diff --git a/LayoutTests/platform/ios-wk2/TestExpectations b/LayoutTests/platform/ios-wk2/TestExpectations
>index cb19c7582bc29d4711149203d33c9c9293a00b63..ac0b0eb98143777d762bc2003aeffaa279fb51f6 100644
>--- a/LayoutTests/platform/ios-wk2/TestExpectations
>+++ b/LayoutTests/platform/ios-wk2/TestExpectations
>@@ -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
>diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
>index 8eebfdfff757c8c226aa1fc2b8f8c20c164395f2..bd20c85260aa2bb1c36c58ea2f01fc113a6fa2ed 100644
>--- a/LayoutTests/platform/mac-wk2/TestExpectations
>+++ b/LayoutTests/platform/mac-wk2/TestExpectations
>@@ -830,10 +830,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 ]

Actions: View | Formatted Diff | Diff

Attachments on bug 183705: 345463 | 345470 | 348671 | 348674