Closed Bug 1472065 Opened Last year Closed Last year

simplify management of style sheet and parent rule pointers in css::Rule

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The style sheet and parent rule should be known at the time the Rule object is constructed, so I think it would be better to pass those in to the constructor rather than wait for SetStyleSheet / SetParentRule calls later.
Blocks: 1471807
Two things to note:

1. I moved the check we do when clearing out mSheet in group rules to avoid quadratic time behavior when unlinking to inside the function that would drop the reference (ServoCSSRuleList::DropSheetReference, renamed from SetStyleSheet).

2. I removed the mention of the direct style sheet owner of ServoCSSRuleList, since previously we would end up calling SetStyleSheet on it for non-top-level group rules anyway.
Comment on attachment 8988973 [details]
Bug 1472065 - Initialize mSheet and mParentRule in css::Rule's constructor.

https://reviewboard.mozilla.org/r/254092/#review260884

::: layout/style/Rule.h:132
(Diff revision 1)
>    bool IsKnownLive() const;
>  
> -  // This is sometimes null (e.g., for style attributes).
> -  StyleSheet* mSheet;
> -  // When the parent GroupRule is destroyed, it will call SetParentRule(nullptr)
> -  // on this object. (Through SetParentRuleReference);
> +  // mSheet should only ever be null when we create a synthetic CSSFontFaceRule
> +  // for an InspectorFontFace.
> +  //
> +  // mSheet and mParentRule will be cleared when the parent object is destroyed.

Probably better "when they are detached from the parent object, either because the rule is removed or the parent is destroyed."
Attachment #8988973 - Flags: review?(xidorn+moz) → review+
Pushed by cam@mcc.id.au:
https://hg.mozilla.org/integration/autoland/rev/8fb94a730b6c
Initialize mSheet and mParentRule in css::Rule's constructor. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/8fb94a730b6c
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.