Bug 242303 – Layout Tests using internals.numberOfLiveNodes() and internals.numberOfLiveDocuments() are flaky

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Fujii Hironori]

Layout Tests using internals.numberOfLiveNodes() are flaky

Bug 174180 – Layout test svg/animations/smil-leak-elements.svg is flaky
Bug 174218 – Layout Test svg/animations/smil-leak-list-property-instances.svg is flaky
Bug 175867 – Layout Test svg/animations/smil-leak-dynamically-added-element-instances.svg is flaky.
Bug 175886 – [macOS] Layout test svg/animations/smil-leak-element-instances.svg is flaky.
Bug 191664 – [ High Sierra+ ] Layout Test svg/animations/svglength-element-removed-crash.svg is flaky
Bug 214574 – [Mac] svg/animations/smil-leak-list-property-instances.svg is a flaky failure
Bug 214579 – [macOS WK2] svg/animations/smil-leak-element-instances.svg is flaky failing
Bug 242230 – REGRESSION(250469@main) svg/animations/animation-leak-list-property-instances.html is randomly failing

[Fujii Hironori]

Layout Tests using internals.numberOfLiveNodes():

css-typedom/attribute-style-map-should-not-leak-every-element.html
fast/dom/gc-dom-tree-lifetime.html
fast/dom/inline-event-attributes-release.html
fast/dom/reference-cycle-leaks.html
http/tests/paymentrequest/payment-response-reference-cycle-leak.https.html
intersection-observer/intersection-observer-callback-leak.html
svg/animations/animation-leak-list-property-instances.html
svg/animations/smil-leak-dynamically-added-element-instances.svg
svg/animations/smil-leak-element-instances-noBaseValRef.svg
svg/animations/smil-leak-element-instances.svg
svg/animations/smil-leak-elements.svg
svg/animations/smil-leak-list-property-instances.svg
svg/animations/svglength-element-removed-crash.svg

[Fujii Hironori]

The following test are using internals.numberOfLiveNodes(). But not flaky becuase they are checking loosely.

css-typedom/attribute-style-map-should-not-leak-every-element.html
fast/dom/gc-dom-tree-lifetime.html
fast/dom/reference-cycle-leaks.html
http/tests/paymentrequest/payment-response-reference-cycle-leak.https.html
intersection-observer/intersection-observer-callback-leak.html
svg/animations/smil-leak-element-instances-noBaseValRef.svg

[Fujii Hironori]

Internals::numberOfLiveNodes is counting live nodes of allDocuments.
https://github.com/WebKit/WebKit/blob/b01e40564c2149ac330006002babf439d1ecded4/Source/WebCore/testing/Internals.cpp#L2758

I guess they are counting documents of the previous test?

[Fujii Hironori]

Internals::numberOfLiveDocuments is also using Document::allDocuments().

following tests are using internals.numberOfLiveDocuments()

animations/leak-document-with-css-animation.html
editing/selection/leak-document-with-selection-inside.html
editing/selection/navigation-clears-editor-state.html
fast/dom/ImageDocument-world-leak.html
fast/workers/worker-document-leak.html
plugins/netscape-dom-access-and-reload.html
requestidlecallback/requestidlecallback-document-gc.html

Some of them are flaky.

Bug 146182 – editing/selection/leak-document-with-selection-inside.html is flaky
Bug 190890 – [macOS Release WK1] Layout Test editing/selection/navigation-clears-editor-state.html is a flaky failure
Bug 215177 – [ macOS iOS wk2 Release] editing/selection/navigation-clears-editor-state.html is a flaky failure
Bug 226598 – [MacOS & iOS] animations/leak-document-with-css-animation.html is a flaky failure

[Fujii Hironori]

Created attachment 460649 [details]
Patch

[Fujii Hironori]

Chromium is using precise GC for tests.
https://codereview.chromium.org/300783002

[Alexey Proskuryakov]

> I guess they are counting documents of the previous test?

I thought that each test was supposed to be written in a way that's resilient against that.

[Ryosuke Niwa]

(In reply to Alexey Proskuryakov from comment #7)
> > I guess they are counting documents of the previous test?
> 
> I thought that each test was supposed to be written in a way that's
> resilient against that.

That's the theory but perhaps some test authors didn't do that :(

[Fujii Hironori]

https://webkit.slack.com/archives/CU64U6FDW/p1656950876079959?thread_ts=1656921143.374209&cid=CU64U6FDW

> We don't have precise GC and they are not something switchable thing.
> Chromium is using precise GC for V8 and conservative GC for blink. And Chromium also cannot switch. The change is just making blink stack empty to make V8 handle everything, not switching something internally.
> Test needs to be written based on conservative GC's assumption. https://trac.webkit.org/changeset/261391/webkit

OK. I'm going to fix the tests.