Bug 231260 - Add more GC tests for ResizeObserver
Summary: Add more GC tests for ResizeObserver
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-05 15:16 PDT by Chris Dumez
Modified: 2022-06-22 12:26 PDT (History)
3 users (show)

See Also:

Attachments
Patch (5.07 KB, patch)
2021-10-05 15:19 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (5.21 KB, patch)
2021-10-06 07:50 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (5.56 KB, patch)
2021-10-07 13:20 PDT, Chris Dumez 		rniwa: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.