33733 2010-01-15 13:15:54 -0800 ValidityState object should be the same on each access (JS object getting GC'd incorrectly) 2022-06-11 15:34:14 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 ap rniwa ahmad.saleem792 akeerthi cdumez darin ggaren michelangelo rniwa webkit-bug-importer oldest_to_newest 181346 0 46707 ap 2010-01-15 13:15:54 -0800 Created attachment 46707 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. 181552 1 darin 2010-01-16 13:31:28 -0800 Geoff Garen pointed out our usual technique for this sort of thing. JSHTMLFormControlElement can mark JSValidityState. See JSSharedWorker::markChildren in JSSharedWorkerCustom.cpp. 1874835 2 ahmad.saleem792 2022-06-08 15:36:39 -0700 I am unable to reproduce this bug in Safari 15.5 on macOS 12.4 since now test case shows "PASS". Thanks! 1874836 3 ahmad.saleem792 2022-06-08 15:37:35 -0700 It also matches other browsers - Chrome Canary 104 and Firefox Nightly 103. Thanks! 1875045 4 460139 ap 2022-06-09 10:55:12 -0700 Created attachment 460139 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. 1875082 5 darin 2022-06-09 13:11:28 -0700 A relevant term of art here is "expando properties". 1875360 6 rniwa 2022-06-10 20:41:51 -0700 We should make JSValidityState report its owner element as an opaque root, and make it reachable from the opaque root. 1875363 7 rniwa 2022-06-10 20:48:25 -0700 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(). 1875392 8 darin 2022-06-11 09:41:06 -0700 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. 1875399 9 rniwa 2022-06-11 13:09:24 -0700 Pull request: https://github.com/WebKit/WebKit/pull/1460 1875403 10 ews-feeder 2022-06-11 15:33:34 -0700 Committed r295475 (251480@main): <https://commits.webkit.org/251480@main> Reviewed commits have been landed. Closing PR #1460 and removing active labels. 1875404 11 webkit-bug-importer 2022-06-11 15:34:14 -0700 <rdar://problem/94897805> 46707 2010-01-15 13:15:54 -0800 2022-06-09 10:55:12 -0700 test case validity-object.html text/html 496 ap PGJvZHk+CjxzZWxlY3QgaWQ9c2VsPjwvc2VsZWN0Pgo8c2NyaXB0PgpmdW5jdGlvbiBnYygpCnsK ICAgIGlmICh3aW5kb3cuR0NDb250cm9sbGVyKQogICAgICAgIHJldHVybiBHQ0NvbnRyb2xsZXIu Y29sbGVjdCgpOwoKICAgIGZvciAodmFyIGkgPSAwOyBpIDwgMTAwMDA7IGkrKykgeyAvLyA+IGZv cmNlIGdhcmJhZ2UgY29sbGVjdGlvbiAoRkYgcmVxdWlyZXMgYWJvdXQgOUsgYWxsb2NhdGlvbnMg YmVmb3JlIGEgY29sbGVjdCkKICAgICAgICB2YXIgcyA9IG5ldyBTdHJpbmcoIiIpOwogICAgfQp9 Cgpkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgic2VsIikudmFsaWRpdHkuZm9vID0gIlBBU1MiOwoK c2V0VGltZW91dChmdW5jdGlvbigpIHsKICAgIGdjKCk7CiAgICBkb2N1bWVudC53cml0ZSgiU2hv dWxkIHNheSBQQVNTOiAiICsgZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoInNlbCIpLnZhbGlkaXR5 LmZvbyArICI8cD4iKTsKfSwgMCk7Cjwvc2NyaXB0Pgo8L2JvZHk+Cg== 460139 2022-06-09 10:55:12 -0700 2022-06-09 10:55:12 -0700 test case validity-object.html text/html 565 ap PGJvZHk+CjxzZWxlY3QgaWQ9c2VsPjwvc2VsZWN0Pgo8c2NyaXB0PgpmdW5jdGlvbiBnYygpCnsK ICAgIGlmICh3aW5kb3cuR0NDb250cm9sbGVyKQogICAgICAgIHJldHVybiBHQ0NvbnRyb2xsZXIu Y29sbGVjdCgpOwoKICAgIGZvciAodmFyIGkgPSAwOyBpIDwgMTAwMDAwMDsgaSsrKSB7IC8vID4g Zm9yY2UgZ2FyYmFnZSBjb2xsZWN0aW9uIChGRiByZXF1aXJlcyBhYm91dCA5SyBhbGxvY2F0aW9u cyBiZWZvcmUgYSBjb2xsZWN0KQogICAgICAgIHZhciBzID0gbmV3IFN0cmluZygiIik7CiAgICB9 Cn0KCmRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJzZWwiKS52YWxpZGl0eS5mb28gPSAiUEFTUyI7 CiAgICBnYygpOwoKc2V0VGltZW91dChmdW5jdGlvbigpIHsKICAgIGdjKCk7CiAgICBzZXRUaW1l b3V0KGZ1bmN0aW9uKCkgewogICAgICAgIGdjKCk7CiAgICAgICAgZG9jdW1lbnQud3JpdGUoIlNo b3VsZCBzYXkgUEFTUzogIiArIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJzZWwiKS52YWxpZGl0 eS5mb28gKyAiPHA+Iik7CiAgICB9LCAwKTsKfSwgMCk7Cjwvc2NyaXB0Pgo8L2JvZHk+Cg==