| Summary: | Layout Tests using internals.numberOfLiveNodes() and internals.numberOfLiveDocuments() are flaky | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||
| Component: | WebCore Misc. | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||
| Status: | RESOLVED INVALID | ||||||
| Severity: | Normal | CC: | ap, cdumez, rniwa, saam | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Fujii Hironori
2022-07-03 19:35:11 PDT
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 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 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? 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 Created attachment 460649 [details]
Patch
Chromium is using precise GC for tests. https://codereview.chromium.org/300783002 > 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.
(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 :( 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. |