RESOLVED FIXED 231260
Add more GC tests for ResizeObserver
https://bugs.webkit.org/show_bug.cgi?id=231260
Summary Add more GC tests for ResizeObserver
Chris Dumez
Reported 2021-10-05 15:16:50 PDT
Add more GC tests for ResizeObserver.
Attachments
Patch (5.07 KB, patch)
2021-10-05 15:19 PDT, Chris Dumez
no flags
Patch (5.21 KB, patch)
2021-10-06 07:50 PDT, Chris Dumez
no flags
Patch (5.56 KB, patch)
2021-10-07 13:20 PDT, Chris Dumez
rniwa: review+
Chris Dumez
Comment 1 2021-10-05 15:19:30 PDT
Chris Dumez
Comment 2 2021-10-06 07:50:23 PDT
Chris Dumez
Comment 3 2021-10-07 13:20:14 PDT
Yusuke Suzuki
Comment 4 2021-10-07 22:54:06 PDT
Can you ensure that GC tests are written in a form like that (https://trac.webkit.org/changeset/261391/webkit)? Testing *one* object's liveness is bad pattern because of conservative GC. Let's test many objects and at least one is dead.
Chris Dumez
Comment 5 2021-10-08 08:04:15 PDT
(In reply to Yusuke Suzuki from comment #4) > Can you ensure that GC tests are written in a form like that > (https://trac.webkit.org/changeset/261391/webkit)? > > Testing *one* object's liveness is bad pattern because of conservative GC. > Let's test many objects and at least one is dead. This is not a test to make sure that a particular wrapper is GC'd (e.g. a leak test). Therefore, I don't see the issue with using a single wrapper. In other words, if we had a bug, the test would at least flakily fail. If we don't have a bug, the test will consistently pass.
Radar WebKit Bug Importer
Comment 6 2021-10-12 15:17:18 PDT
Ryosuke Niwa
Comment 7 2022-06-21 17:06:21 PDT
Doesn't seem like commit queue is doing anything here??
Chris Dumez
Comment 8 2022-06-21 18:29:58 PDT
Comment on attachment 440532 [details] Patch Probably because it is the format is too old now. No more ChangeLog.
Chris Dumez
Comment 9 2022-06-22 08:39:41 PDT
EWS
Comment 10 2022-06-22 12:26:29 PDT
Committed r295744 (251749@main): <https://commits.webkit.org/251749@main> Reviewed commits have been landed. Closing PR #1682 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.