| Summary: | ValidityState object should be the same on each access (JS object getting GC'd incorrectly) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
| Component: | Forms | Assignee: | Ryosuke Niwa <rniwa> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ahmad.saleem792, akeerthi, cdumez, darin, ggaren, michelangelo, rniwa, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
Geoff Garen pointed out our usual technique for this sort of thing. JSHTMLFormControlElement can mark JSValidityState. See JSSharedWorker::markChildren in JSSharedWorkerCustom.cpp. I am unable to reproduce this bug in Safari 15.5 on macOS 12.4 since now test case shows "PASS". Thanks! It also matches other browsers - Chrome Canary 104 and Firefox Nightly 103. Thanks! Created attachment 460139 [details]
test case
Sadly, this is not fixed yet, it's just that our garbage collection became less precise than it used to be. This still reproduces when I adjust the test to be more forceful with GC.
A relevant term of art here is "expando properties". We should make JSValidityState report its owner element as an opaque root, and make it reachable from the opaque root. More precisely, add the following extended attributes to IDL:
GenerateIsReachable=ImplOwnerNodeRoot,
GenerateAddOpaqueRoot=opaqueRootConcurrently
And then expose two member functions:
void* opaqueRootConcurrently() const;
Node* ownerNode() const;
both of which simply returns asHTMLElement().
That sounds right for how to fix. As for how to test we should have a test for the expando properties on this object in WPT, which already has some other expando property preservation tests. Pull request: https://github.com/WebKit/WebKit/pull/1460 Committed r295475 (251480@main): <https://commits.webkit.org/251480@main> Reviewed commits have been landed. Closing PR #1460 and removing active labels. |
Created attachment 46707 [details] test case Per HTML5, "The validity attribute must return a ValidityState object that represents the validity states of the element. This object is live, and the same object must be returned each time the element's validity attribute is retrieved." This means that a form element must prevent garbage collection of associated ValidityState wrapper, not just of its underlying DOM object.