Bug 230047 – [ macOS and iOS ] editing/deleting/forward-delete-crash.html is timing out

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

editing/deleting/forward-delete-crash.html

Is a flaky timeout (Almost consistent) on macOS and iOS simulator.

History: https://results.webkit.org/?suite=layout-tests&test=editing%2Fdeleting%2Fforward-delete-crash.html

Result page: https://build.webkit.org/results/Apple-BigSur-Debug-WK1-Tests/r282140%20(4040)/results.html

Diff:

--- /Volumes/Data/worker/bigsur-debug-tests-wk1/build/layout-test-results/editing/deleting/forward-delete-crash-expected.txt
+++ /Volumes/Data/worker/bigsur-debug-tests-wk1/build/layout-test-results/editing/deleting/forward-delete-crash-actual.txt
@@ -1 +1,5 @@
-Test passes if it does not crash.
+FAIL: Timed out waiting for notifyDone to be called
+:last-of-type { height: 1px; display: block; } @font-face { font-family: "Ahem"; src: url("../../resources/Ahem.ttf"); }
+if (window.testRunner) { window.testRunner.dumpAsText(); window.testRunner.waitUntilDone(); } onload = async () => { document.designMode = 'on'; let img0 = document.createElement('img'); img0.onerror = () => { document.execCommand('ForwardDelete'); setTimeout(function() { window.testRunner.notifyDone(); }, 0); document.write("Test passes if it does not crash."); }; let datalist0 = document.createElement('datalist'); document.head.appendChild(datalist0); document.head.appendChild(document.createElement('datalist')); img0.src = 'data:'; let embed0 = document.createElement('embed'); embed0.src = '#'; datalist0.appendChild(embed0); if (navigator.platform.indexOf('Mac') == 0 && window.caches) await caches.has('a'); else await document.fonts.load("80px Ahem"); img0.src = 'data:'; getSelection().selectAllChildren(datalist0); if (navigator.platform.indexOf('Mac') == 0 && window.caches) await caches.has('a'); else await document.fonts.load("80px Ahem"); document.execCommand('Delete'); };
+
+

Stderr:

2021-09-08 07:17:36.675 DumpRenderTree[64818:105897551] nil host used in call to allowsSpecificHTTPSCertificateForHost
2021-09-08 07:17:36.675 DumpRenderTree[64818:105897551] nil host used in call to allowsAnyHTTPSCertificateForHost:
2021-09-08 07:17:36.794 DumpRenderTree[64818:105897551] nil host used in call to allowsSpecificHTTPSCertificateForHost
2021-09-08 07:17:36.794 DumpRenderTree[64818:105897551] nil host used in call to allowsAnyHTTPSCertificateForHost:
2021-09-08 07:17:36.806 DumpRenderTree[64818:105897551] nil host used in call to allowsSpecificHTTPSCertificateForHost
2021-09-08 07:17:36.806 DumpRenderTree[64818:105897551] nil host used in call to allowsAnyHTTPSCertificateForHost:

[Radar WebKit Bug Importer]

<rdar://problem/82875247>

Seems the test has been timing out since it was added at https://trac.webkit.org/changeset/282074/webkit

The test is also showing up on EWS as a pre-existing test. Marked expectations to speed up EWS https://trac.webkit.org/changeset/282145/webkit

[Alexey Proskuryakov]

Why are we not reverting r282074, given that it included a broken test?

[Ryosuke Niwa]

(In reply to Alexey Proskuryakov from comment #4)
> Why are we not reverting r282074, given that it included a broken test?

I don't think there is any issue with the fix. The issue is with the test.

[Ryosuke Niwa]

(In reply to Ryosuke Niwa from comment #5)
> (In reply to Alexey Proskuryakov from comment #4)
> > Why are we not reverting r282074, given that it included a broken test?
> 
> I don't think there is any issue with the fix. The issue is with the test.

Also note that the test was passing on EWS.

[Rob Buis]

This test is not enough to trigger the original problem anymore on ToT (with the original fix reverted). Still bisecting, but perhaps d0b3b4b3becae0da071719c86df2450e283e43bb changed timing/behaviour here.

[Rob Buis]

(In reply to Rob Buis from comment #7)
> This test is not enough to trigger the original problem anymore on ToT (with
> the original fix reverted). Still bisecting, but perhaps
> d0b3b4b3becae0da071719c86df2450e283e43bb changed timing/behaviour here.

Indeed d0b3b4b3becae0da071719c86df2450e283e43bb is the one.

[Rob Buis]

Created attachment 444955 [details]
Patch

[Rob Buis]

(In reply to Rob Buis from comment #8)
> (In reply to Rob Buis from comment #7)
> > This test is not enough to trigger the original problem anymore on ToT (with
> > the original fix reverted). Still bisecting, but perhaps
> > d0b3b4b3becae0da071719c86df2450e283e43bb changed timing/behaviour here.
> 
> Indeed d0b3b4b3becae0da071719c86df2450e283e43bb is the one.

I should not have used the SHA, needed to bisect again, it is https://bugs.webkit.org/show_bug.cgi?id=229905 that changed behaviour so the test does not trigger the crash anymore.

[Rob Buis]

Created attachment 445861 [details]
Patch

[Darin Adler]

Comment on attachment 445861 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445861&action=review

> LayoutTests/editing/deleting/forward-delete-crash.html:40
> +    setTimeout(function() { document.write("Test passes if it does not crash."); window.testRunner.notifyDone(); }, 10);

Seems like using a duration here adds a race to the test. While I won’t insist on it, is there a way to solve the problem without a race?

[Rob Buis]

Created attachment 446161 [details]
Patch

[Rob Buis]

Comment on attachment 445861 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445861&action=review

>> LayoutTests/editing/deleting/forward-delete-crash.html:40
>> +    setTimeout(function() { document.write("Test passes if it does not crash."); window.testRunner.notifyDone(); }, 10);
> 
> Seems like using a duration here adds a race to the test. While I won’t insist on it, is there a way to solve the problem without a race?

I checked, indeed the 10ms wait is not needed.

[EWS]

Committed r286966 (245189@main): <https://commits.webkit.org/245189@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446161 [details].