Bug 231260

Summary: Add more GC tests for ResizeObserver
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Tools / TestsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: rniwa, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=231235
Attachments:
Description Flags
Patch
none
Patch
none
Patch rniwa: review+

Description Chris Dumez 2021-10-05 15:16:50 PDT
Add more GC tests for ResizeObserver.
Comment 1 Chris Dumez 2021-10-05 15:19:30 PDT
Created attachment 440276 [details]
Patch
Comment 2 Chris Dumez 2021-10-06 07:50:23 PDT
Created attachment 440367 [details]
Patch
Comment 3 Chris Dumez 2021-10-07 13:20:14 PDT
Created attachment 440532 [details]
Patch
Comment 4 Yusuke Suzuki 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.
Comment 5 Chris Dumez 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.
Comment 6 Radar WebKit Bug Importer 2021-10-12 15:17:18 PDT
<rdar://problem/84168766>
Comment 7 Ryosuke Niwa 2022-06-21 17:06:21 PDT
Doesn't seem like commit queue is doing anything here??
Comment 8 Chris Dumez 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.
Comment 9 Chris Dumez 2022-06-22 08:39:41 PDT
Pull request: https://github.com/WebKit/WebKit/pull/1682
Comment 10 EWS 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.