[testharnessreport.js] Only pay attention to the top-level completion…

… handler https://bugs.webkit.org/show_bug.cgi?id=259409 rdar://problem/112683033 Reviewed by Jonathan Bedard. Previously, we were considering the test as having to run to completion when any testharness.js completion handler first ran (which was sometimes that of the frame within the opened window; the nondeterminism here just adds flakiness to the already bad behaviour). Instead, only output anything for the top-level completion handler, as all results should be passed up to it. Note wptrunner with the WebDriver or Marionette executors only ever pays attention to the top-level completion handler (as they only pay attention to the top-level frame & window), thus they don't have any flakiness like this. Ideally testharness.js would have an API we could use for this; I've filed an RFC at web-platform-tests/rfcs#168 for this, but there's no reason not to do the simple fix ourselves, as at least for now this is a strict progression. (It would stop being a strict progression if at some point testharness.js started allowing fetch_tests_from_window() with a window with noopener.) It seems likely we have other tests that are marked as flaky because of it, but alas there's no easy way to find what tests will have been fixed by this. * LayoutTests/TestExpectations: * LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https-expected.txt: * LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html: * LayoutTests/imported/w3c/web-platform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/source/navigate-child-function-parent-then-fragment-expected.txt: * LayoutTests/platform/wk2/TestExpectations: * LayoutTests/resources/testharnessreport.js: Canonical link: https://commits.webkit.org/269483@main
gsnedders · Oct 18, 2023 · 5273909 · 5273909
1 parent b4cd98a
commit 5273909
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 11 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -6163,6 +6163,7 @@ imported/w3c/web-platform-tests/html/browsers/browsing-the-web/history-traversal
 imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/004.html [ Skip ]
 imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html [ Skip ]
 imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/navigate-cross-origin-iframe-to-same-url-with-fragment-fire-load-event.html [ Skip ]
+imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/source/navigate-child-function-parent-then-fragment.html [ Skip ]
 imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/004.html [ Skip ]
 imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/008.html [ Skip ]
 imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/combination_history_004.html [ Skip ]

diff --git a/...ity/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https-expected.txt b/...ity/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https-expected.txt
@@ -2,3 +2,6 @@ CONSOLE MESSAGE: PASS: Successfully retrieved image data.
 This test opens a window that loads an insecure image. We should upgrade this request and thereby avoid triggering a mixed content callback.
 
 
+
+PASS Verify that images have correct cross-origin behavior.
+
diff --git a/...ts/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html b/...ts/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html
@@ -1,14 +1,14 @@
 <!DOCTYPE html>
 <html>
+<head>
+<script src="/js-test-resources/testharness.js"></script>
+<script src="/js-test-resources/testharnessreport.js"></script>
 <body>
-<script>
-if (window.testRunner) {
-    testRunner.waitUntilDone();
-    testRunner.dumpAsText();
-}
-</script>
 <p>This test opens a window that loads an insecure image.  We should upgrade
 this request and thereby avoid triggering a mixed content callback.</p>
 <iframe src="https://127.0.0.1:8443/security/contentSecurityPolicy/upgrade-insecure-requests/resources/basic-upgrade-cors.https.html"></iframe>
+<script>
+fetch_tests_from_window(window.frames[0]);
+</script>
 </body>
 </html>
diff --git a/...atform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https-expected.txt b/...atform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https-expected.txt
@@ -1,7 +1,7 @@
 
-FAIL Partitioned cookies accessible on the top-level site they are created in via HTTP assert_equals: Expected __Host-pccookistore to be available on the top-level site it was created in expected true but got false
-FAIL Partitioned cookies accessible on the top-level site they are created in via DOM assert_equals: Expected __Host-pccookistore to be available on the top-level site it was created in expected true but got false
-FAIL Partitioned cookies accessible on the top-level site they are created in via CookieStore assert_equals: Expected __Host-pccookistore to be available on the top-level site it was created in expected true but got false
+FAIL Partitioned cookies accessible on the top-level site they are created in via HTTP assert_equals: Expected __Host-pccookiestore to be available on the top-level site it was created in expected true but got false
+FAIL Partitioned cookies accessible on the top-level site they are created in via DOM assert_equals: Expected __Host-pccookiestore to be available on the top-level site it was created in expected true but got false
+FAIL Partitioned cookies accessible on the top-level site they are created in via CookieStore assert_equals: Expected __Host-pccookiestore to be available on the top-level site it was created in expected true but got false
 PASS Cross-site window opened correctly
 FAIL Partitioned cookies are not accessible on a different top-level site via HTTP assert_equals: Expected __Host-pchttp to not be available on a different top-level site expected false but got true
 FAIL Partitioned cookies are not accessible on a different top-level site via DOM assert_equals: Expected __Host-pchttp to not be available on a different top-level site expected false but got true

diff --git a/...igating-across-documents/source/navigate-child-function-parent-then-fragment-expected.txt b/...igating-across-documents/source/navigate-child-function-parent-then-fragment-expected.txt
@@ -1 +1,9 @@
 
+
+Harness Error (TIMEOUT), message = null
+
+TIMEOUT
+  Set location from a parent, then do a fragment navigation from within the
+  frame.
+ Test timed out
+
diff --git a/LayoutTests/platform/wk2/TestExpectations b/LayoutTests/platform/wk2/TestExpectations
@@ -874,8 +874,6 @@ webkit.org/b/253533 imported/w3c/web-platform-tests/fetch/api/basic/status.h2.an
 webkit.org/b/253533 imported/w3c/web-platform-tests/fetch/api/basic/status.h2.any.worker.html [ Pass Failure ]
 webkit.org/b/253533 imported/w3c/web-platform-tests/xhr/status.h2.window.html [ Pass Failure ]
 
-webkit.org/b/259409 imported/w3c/web-platform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https.html [ Pass Failure ]
-
 webkit.org/b/259482 fast/media/managed-media-source-open-crash.html [ Pass Failure ]
 
 webkit.org/b/260640 [ Release arm64 ] editing/execCommand/apply-inline-style-to-element-with-no-renderer-crash.html [ Pass Failure ]

diff --git a/LayoutTests/resources/testharnessreport.js b/LayoutTests/resources/testharnessreport.js
@@ -10,6 +10,8 @@
  * parameters they are called with see testharness.js
  */
 
+(function(){
+
 // Setup for WebKit JavaScript tests
 if (self.testRunner) {
     testRunner.dumpAsText();
@@ -57,10 +59,20 @@ if (self.testRunner) {
     */
     setup({"output": false, "explicit_timeout": true});
 
+    // window.opener is a configurable property, so store it before we run anything.
+    const orig_opener = window.opener;
+
     /*  Using a callback function, test results will be added to the page in a
     *   manner that allows dumpAsText to produce readable test results
     */
     add_completion_callback(function (tests, harness_status) {
+        // Only pay attention to results at the top-level window.
+        // Ideally testharness.js would allow us to only attach a completion handler in this case:
+        // https://github.com/web-platform-tests/rfcs/pull/168
+        if (window !== window.top || (orig_opener !== null && orig_opener !== window)) {
+            return;
+        }
+
         var resultStr = "\n";
 
         // Sanitizes the given text for display in test results.
@@ -140,3 +152,5 @@ if (self.testRunner) {
         }, 0);
     });
 }
+
+})();
Original file line number	Diff line number	Diff line change
Expand Up		@@ -2,3 +2,6 @@ CONSOLE MESSAGE: PASS: Successfully retrieved image data.
		This test opens a window that loads an insecure image. We should upgrade this request and thereby avoid triggering a mixed content callback.



		PASS Verify that images have correct cross-origin behavior.