1332321 - The *SKIPPABLE_*_INHERITED macros are footguns

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Boris Zbarsky [:bzbarsky]]

If class A inherits from B, A uses one of the *SKIPPABLE_*_INHERITED macros, and B does _not_ use a skippable macro, then A's skippability stuff will never get called.  That's because it's based on a boolean that only the least-derived CC participant gets to set.

So you can write all this nice skippability code and have it all be dead code and never even notice.

Comment 1

[Boris Zbarsky [:bzbarsky]]

One simple solution may be to just have a ctor for the participant that calls its parent participant ctor in a way that only skippable participants implement or something.

Another is to make all participant ctors take a skippability arg and propagate it up with skippable things adding in "true" and the default value always being false or something.  This would allow a skippable subclass of a non-skippable superclass and just invoke the canskip bits on the subclass.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Make all cycle collection participants explicitly say whether they need skippability themselves, or just want whatever skippability their descendants want

This way we can't end up with a situation in which an ancestor doesn't care about skippability but a descendant does and doesn't get it, because the ancestor just claimed no skippability was needed.

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 8829959 [details] [diff] [review]
Make all cycle collection participants explicitly say whether they need skippability themselves, or just want whatever skippability their descendants want

Review of attachment 8829959 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/226f342c26fb
Make all cycle collection participants explicitly say whether they need skippability themselves, or just want whatever skippability their descendants want. r=mccr8

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/226f342c26fb