From 5273909547b13c75ce88e806b773737baa363c9d Mon Sep 17 00:00:00 2001 From: Sam Sneddon Date: Wed, 18 Oct 2023 12:26:12 -0700 Subject: [PATCH] [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 https://github.com/web-platform-tests/rfcs/pull/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 --- LayoutTests/TestExpectations | 1 + .../basic-upgrade-cors.https-expected.txt | 3 +++ .../basic-upgrade-cors.https.html | 12 ++++++------ ...artitioned-cookies.tentative.https-expected.txt | 6 +++--- ...hild-function-parent-then-fragment-expected.txt | 8 ++++++++ LayoutTests/platform/wk2/TestExpectations | 2 -- LayoutTests/resources/testharnessreport.js | 14 ++++++++++++++ 7 files changed, 35 insertions(+), 11 deletions(-) diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations index 9e64fc769dfc4..c0d265f1af904 100644 --- 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/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https-expected.txt index e0b1343300772..5e4adaa75df78 100644 --- a/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https-expected.txt +++ b/LayoutTests/http/tests/security/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/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html b/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html index 28d29ed9f353f..66cf092b0ebb6 100644 --- a/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html +++ b/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html @@ -1,14 +1,14 @@ + + + -

This test opens a window that loads an insecure image. We should upgrade this request and thereby avoid triggering a mixed content callback.

+ diff --git a/LayoutTests/imported/w3c/web-platform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https-expected.txt index 3e74c04bba812..6b691cf489e0e 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/cookies/partitioned-cookies/partitioned-cookies.tentative.https-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-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/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/source/navigate-child-function-parent-then-fragment-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/source/navigate-child-function-parent-then-fragment-expected.txt index 8b137891791fe..654313ae2394d 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/source/navigate-child-function-parent-then-fragment-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-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 index 0b939f7c84937..76f9c8df9d927 100644 --- 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 index bdfe562a472d4..ba88f46f7c1dd 100644 --- 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); }); } + +})();