Changeset 266984 in webkit


Ignore:
Timestamp:
Sep 12, 2020 4:44:27 PM (4 years ago)
Author:
Darin Adler
Message:

REGRESSION (r266817): ASSERTION FAILED: injectedBundle.isTestRunning() on imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html
​https://bugs.webkit.org/show_bug.cgi?id=216440

[GTK] imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html is flaky
​https://bugs.webkit.org/show_bug.cgi?id=210375

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

  • web-platform-tests/service-workers/service-worker/resources/svg-target-reftest-frame.html: Added.
  • web-platform-tests/service-workers/service-worker/resources/svg-target-reftest-001-frame.html: Added.

Somehow these files were missed in the web-platform-tests import, and they are both used by the
svg-target-reftest.https.html test.

  • web-platform-tests/service-workers/service-worker/svg-target-reftest.https-expected.html:

Updated the path in this expected result file. The test importer script should have done this;
not sure why it did not.

Tools:

Did some hardening of the reftest-wait mechanism in WebKitTestRunner.
We can later do the same in DumpRenderTree if the same problem comes up there,
but at this time I don't have a test that reproduces the issue there so not
changing it for now. Don't want to add speculative code without testing it.

  • WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:

(WTR::hasRefTestWaitAttribute): Changed to take a WKBundlePageRef instead of
a InjectedBundlePage&.
(WTR::dumpAfterWaitAttributeIsRemoved): Changed to take a WKBundlePageRef.
Added code to handle cases where an explicit dump occurs while we are waiting
for the attribute to be removed. Identify the page with the WKBundlePageRef,
which can be retained/released.
(WTR::InjectedBundlePage::frameDidChangeLocation): Check page for null
instead of indirectly checking by asking if the pageCount is 0. Also pass
a WKBundlePageRef in to dumpAfterWaitAttributeIsRemoved.

LayoutTests:

legacy WebKit and the symptom is a hang waiting for the reftest-wait attribute
to be removed. The failure is not new.

  • platform/gtk/TestExpectations: Removed expectation that this same test will

be flaky. The addition of reftest-wait support and the subresource along with
the bug fix to WebKitTestRunner should leave this passing and non-flaky on GTK.
Feel free to add this back if that proves wrong.

  • platform/wk2/TestExpectations: Expect a pass for this test. It's working fine

on modern WebKit.

Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r266978 r266984  
     12020-09-12  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r266817): ASSERTION FAILED: injectedBundle.isTestRunning() on imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html
     4        https://bugs.webkit.org/show_bug.cgi?id=216440
     5
     6        [GTK] imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html is flaky
     7        https://bugs.webkit.org/show_bug.cgi?id=210375
     8
     9        Reviewed by Sam Weinig.
     10
     11        * TestExpectations: Skip svg-target-reftest.https.html because it fails in
     12        legacy WebKit and the symptom is a hang waiting for the reftest-wait attribute
     13        to be removed. The failure is not new.
     14
     15        * platform/gtk/TestExpectations: Removed expectation that this same test will
     16        be flaky. The addition of reftest-wait support and the subresource along with
     17        the bug fix to WebKitTestRunner should leave this passing and non-flaky on GTK.
     18        Feel free to add this back if that proves wrong.
     19
     20        * platform/wk2/TestExpectations: Expect a pass for this test. It's working fine
     21        on modern WebKit.
     22
    1232020-09-12  Zalan Bujtas  <zalan@apple.com>
    224

  • trunk/LayoutTests/TestExpectations

    r266976 r266984  
    27912791webkit.org/b/182177 imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/errorhandling.html [ Pass Failure ]
    27922792
    2793 imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html [ ImageOnlyFailure ]
     2793# This fails with Legacy WebKit, and the symptom is a hang so we need to skip it. Skipped here and un-skipped in platform/wk2/TestExpectations.
     2794imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html [ Skip ]
    27942795
    27952796webkit.org/b/90980 fast/forms/textarea/textarea-state-restore.html [ Pass Timeout ]

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r266976 r266984  
     12020-09-12  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r266817): ASSERTION FAILED: injectedBundle.isTestRunning() on imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html
     4        https://bugs.webkit.org/show_bug.cgi?id=216440
     5
     6        [GTK] imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html is flaky
     7        https://bugs.webkit.org/show_bug.cgi?id=210375
     8
     9        Reviewed by Sam Weinig.
     10
     11        * web-platform-tests/service-workers/service-worker/resources/svg-target-reftest-frame.html: Added.
     12        * web-platform-tests/service-workers/service-worker/resources/svg-target-reftest-001-frame.html: Added.
     13        Somehow these files were missed in the web-platform-tests import, and they are both used by the
     14        svg-target-reftest.https.html test.
     15
     16        * web-platform-tests/service-workers/service-worker/svg-target-reftest.https-expected.html:
     17        Updated the path in this expected result file. The test importer script should have done this;
     18        not sure why it did not.
     19
    1202020-09-12  Rob Buis  <rbuis@igalia.com>
    221

  • trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https-expected.html

    r246363 r266984  
    33<title>Green svg box reference file</title>
    44<p>Pass if you see a green box below.</p>
    5 <iframe src="svg-target-reftest-001-frame.html">
     5<iframe src="resources/svg-target-reftest-001-frame.html">

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r266974 r266984  
    20632063webkit.org/b/210370 [ Release ] media/track/text-track-cue-is-reachable.html [ Pass Crash ]
    20642064
    2065 webkit.org/b/210375 imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html [ ImageOnlyFailure Pass ]
    2066 
    20672065webkit.org/b/210390 storage/domstorage/sessionstorage/blocked-file-access.html [ Pass Crash ]
    20682066webkit.org/b/210390 streams/clone-array-buffer.html [ Pass Crash ]

  • trunk/LayoutTests/platform/wk2/TestExpectations

    r266929 r266984  
    789789media/video-supports-fullscreen.html [ Pass ]
    790790
     791imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html [ Pass ]
     792
    791793### END OF (5) Progressions, expected successes that are expected failures in WebKit1.
    792794########################################

  • trunk/Tools/ChangeLog

    r266975 r266984  
     12020-09-12  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r266817): ASSERTION FAILED: injectedBundle.isTestRunning() on imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html
     4        https://bugs.webkit.org/show_bug.cgi?id=216440
     5
     6        [GTK] imported/w3c/web-platform-tests/service-workers/service-worker/svg-target-reftest.https.html is flaky
     7        https://bugs.webkit.org/show_bug.cgi?id=210375
     8
     9        Reviewed by Sam Weinig.
     10
     11        Did some hardening of the reftest-wait mechanism in WebKitTestRunner.
     12        We can later do the same in DumpRenderTree if the same problem comes up there,
     13        but at this time I don't have a test that reproduces the issue there so not
     14        changing it for now. Don't want to add speculative code without testing it.
     15
     16        * WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
     17        (WTR::hasRefTestWaitAttribute): Changed to take a WKBundlePageRef instead of
     18        a InjectedBundlePage&.
     19        (WTR::dumpAfterWaitAttributeIsRemoved): Changed to take a WKBundlePageRef.
     20        Added code to handle cases where an explicit dump occurs while we are waiting
     21        for the attribute to be removed. Identify the page with the WKBundlePageRef,
     22        which can be retained/released.
     23        (WTR::InjectedBundlePage::frameDidChangeLocation): Check page for null
     24        instead of indirectly checking by asking if the pageCount is 0. Also pass
     25        a WKBundlePageRef in to dumpAfterWaitAttributeIsRemoved.
     26
    1272020-09-12  Carlos Garcia Campos  <cgarcia@igalia.com>
    228

  • trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp

    r266974 r266984  
    19561956#endif
    19571957
    1958 static bool hasRefTestWaitAttribute(InjectedBundlePage& page)
    1959 {
    1960     auto frame = WKBundlePageGetMainFrame(page.page());
     1958static bool hasRefTestWaitAttribute(WKBundlePageRef page)
     1959{
     1960    auto frame = WKBundlePageGetMainFrame(page);
    19611961    return frame && hasRefTestWaitAttribute(WKBundleFrameGetJavaScriptContext(frame));
    19621962}
    19631963
    1964 static void dumpAfterWaitAttributeIsRemoved(void* = nullptr)
    1965 {
    1966     auto page = InjectedBundle::singleton().page();
    1967     if (!page)
    1968         return;
    1969     if (hasRefTestWaitAttribute(*page)) {
    1970         // Use a 1ms delay between tries to give some time for other lower priority run loop sources to be run.
    1971         RunLoop::current().dispatchAfter(1_ms, [] {
    1972             if (auto page = InjectedBundle::singleton().page()) {
    1973                 WKBundlePageCallAfterTasksAndTimers(page->page(), dumpAfterWaitAttributeIsRemoved, nullptr);
    1974         }});
    1975     } else
    1976         page->dump();
     1964static void dumpAfterWaitAttributeIsRemoved(WKBundlePageRef page)
     1965{
     1966    if (hasRefTestWaitAttribute(page)) {
     1967        WKRetain(page);
     1968        // Use a 1ms interval between tries to allow lower priority run loop sources with zero delays to run.
     1969        RunLoop::current().dispatchAfter(1_ms, [page] {
     1970            WKBundlePageCallAfterTasksAndTimers(page, [] (void* typelessPage) {
     1971                auto page = static_cast<WKBundlePageRef>(typelessPage);
     1972                dumpAfterWaitAttributeIsRemoved(page);
     1973                WKRelease(page);
     1974            }, const_cast<OpaqueWKBundlePage*>(page));
     1975        });
     1976        return;
     1977    }
     1978
     1979    if (auto& bundle = InjectedBundle::singleton(); bundle.isTestRunning()) {
     1980        if (auto currentPage = bundle.page(); currentPage && currentPage->page() == page)
     1981            currentPage->dump();
     1982    }
    19771983}
    19781984
    … …  
    19931999    }
    19942000
    1995     if (!injectedBundle.pageCount()) {
     2001    auto page = InjectedBundle::singleton().page();
     2002    if (!page) {
    19962003        injectedBundle.done();
    19972004        return;
    19982005    }
    19992006
    2000     dumpAfterWaitAttributeIsRemoved();
     2007    dumpAfterWaitAttributeIsRemoved(page->page());
    20012008}
    20022009
Note: See TracChangeset for help on using the changeset viewer.