Bug 83618 – fast/dom/inline-event-attributes-release.html randomly fails

• Component: DOM
• Status: RESOLVED FIXED

[Anders Carlsson]

fast/dom/inline-event-attributes-release.html is flakey on Mac and needs to be added to the Skipped list.

[Fujii Hironori]

Another test failure:

Buildbot: builder Apple-BigSur-Release-WK1-Tests build 11350: 251845@main
https://build.webkit.org/#/builders/48/builds/11350

--- /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-expected.txt
+++ /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-actual.txt
@@ -3,7 +3,7 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS afterCount - beforeCount is 0
+FAIL afterCount - beforeCount should be 0. Was 1.
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Fujii Hironori]

Another test failure:

Buildbot: builder Apple-Monterey-Release-WK2-Tests build 4424
251329@main
https://build.webkit.org/#/builders/365/builds/4424

--- /Volumes/Data/worker/Apple-Monterey-Release-WK2-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-expected.txt
+++ /Volumes/Data/worker/Apple-Monterey-Release-WK2-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-actual.txt
@@ -3,7 +3,7 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS afterCount - beforeCount is 0
+FAIL afterCount - beforeCount should be 0. Was 1.
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Fujii Hironori]

Created attachment 460694 [details]
Patch

[Fujii Hironori]

Another test failure:

Buildbot: builder Apple-BigSur-Release-WK1-Tests build 10954: 251198@main
https://build.webkit.org/#/builders/48/builds/10954

--- /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-expected.txt
+++ /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-actual.txt
@@ -3,7 +3,7 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS afterCount - beforeCount is 0
+FAIL afterCount - beforeCount should be 0. Was -110.
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Ryosuke Niwa]

Comment on attachment 460694 [details]
Patch

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

> LayoutTests/fast/dom/inline-event-attributes-release.html:27
> +if (delta < iterations / 10)

This is a very high factor. I suggest we use 2 or 3 instead.

[Fujii Hironori]

Comment on attachment 460694 [details]
Patch

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

>> LayoutTests/fast/dom/inline-event-attributes-release.html:27
>> +if (delta < iterations / 10)
> 
> This is a very high factor. I suggest we use 2 or 3 instead.

I retrieved the number from checkForNodeLeaks.
https://github.com/WebKit/WebKit/blob/e36b05647cc7b60f59a9c96872d4f562c9514e7c/LayoutTests/fast/dom/reference-cycle-leaks.html#L21

And, testDocumentIsNotLeaked passes if at least a single document is reclaimed.
https://github.com/WebKit/WebKit/blob/e36b05647cc7b60f59a9c96872d4f562c9514e7c/LayoutTests/resources/gc.js#L49

However, I agree with you. Will fix this test.

[Fujii Hironori]

Created attachment 460696 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 460696 [details]
Patch

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

> LayoutTests/fast/dom/inline-event-attributes-release.html:27
> +if (delta < 3)

Oh, no, I meant to say divide iterations by 3, not literal 3.
Requiring at least one document getting collected is okay too.
Delta to be less than 10 is too aggressive of a policy.

[Fujii Hironori]

Comment on attachment 460696 [details]
Patch

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

>> LayoutTests/fast/dom/inline-event-attributes-release.html:27
>> +if (delta < 3)
> 
> Oh, no, I meant to say divide iterations by 3, not literal 3.
> Requiring at least one document getting collected is okay too.
> Delta to be less than 10 is too aggressive of a policy.

Oh. Will fix.

[Fujii Hironori]

Created attachment 460701 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 460701 [details]
Patch

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

> LayoutTests/fast/dom/inline-event-attributes-release.html:26
> +if (delta < iterations / 3)

iterations / 2 might be better and this might be still flaky but we can try.

[Fujii Hironori]

Comment on attachment 460701 [details]
Patch

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

>> LayoutTests/fast/dom/inline-event-attributes-release.html:26
>> +if (delta < iterations / 3)
> 
> iterations / 2 might be better and this might be still flaky but we can try.

Why do you think it might be still flaky?
fast/dom/reference-cycle-leaks.html is using the condition "if (leaks < repetitions / 10)", but not flaky.

[Ryosuke Niwa]

(In reply to Fujii Hironori from comment #12)
> Comment on attachment 460701 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=460701&action=review
> 
> >> LayoutTests/fast/dom/inline-event-attributes-release.html:26
> >> +if (delta < iterations / 3)
> > 
> > iterations / 2 might be better and this might be still flaky but we can try.
> 
> Why do you think it might be still flaky?
> fast/dom/reference-cycle-leaks.html is using the condition "if (leaks <
> repetitions / 10)", but not flaky.

Because it's conservative GC. The assertion that more than half the nodes get collected is only probabilistically true.

[Fujii Hironori]

Created attachment 460705 [details]
Patch for landing

[Fujii Hironori]

I don't think the conservative GC can make so much false-positive in this test case.
However, it would happen in some cases, for example, if all created element objects were pushed into an array, and the array wasn't zero-filled after used.

[EWS]

Committed 252173@main (45cf55da6e5b): <https://commits.webkit.org/252173@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/96496965>