Merge ServoGroupRuleRules into GroupRule

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

Trunk
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(3 attachments)

No description provided.
Assignee

Updated

a year ago
Assignee: nobody → xidorn+moz
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8962976 [details]
Bug 1449103 part 1 - Make Rule::mParentRule not necessarily a GroupRule.

https://reviewboard.mozilla.org/r/231816/#review237358

::: layout/style/Rule.h:97
(Diff revision 1)
>    }
>  
>    virtual void SetStyleSheet(StyleSheet* aSheet);
>  
> -  void SetParentRule(GroupRule* aRule) {
> +  void SetParentRule(Rule* aRule) {
>      // We don't reference count this up reference. The group rule

s/The group rule/the rule/.

Can we assert the kind of rule we have around matches our expectations?

Just to ensure nobody starts incorrectly calling this.

Or maybe add an overload for GroupRule and another for KeyframesRule, while keeping the member type Rule?
Attachment #8962976 - Flags: review?(emilio) → review+

Comment 5

a year ago
mozreview-review
Comment on attachment 8962977 [details]
Bug 1449103 part 2 - Make CSSKeyframesRule inherit from Rule directly.

https://reviewboard.mozilla.org/r/231818/#review237360
Attachment #8962977 - Flags: review?(emilio) → review+

Comment 6

a year ago
mozreview-review
Comment on attachment 8962978 [details]
Bug 1449103 part 3 - Merge ServoGroupRuleRules into GroupRule.

https://reviewboard.mozilla.org/r/231820/#review237362

Wow, this is much nicer. Only a single formatting nit because I didn't really have any other complaint, nice cleanup! :)

::: layout/style/GroupRule.h:56
(Diff revision 1)
> -  }
> -  Rule* GetStyleRuleAt(uint32_t aIndex) const {
> -    REDIRECT_TO_INNER(GetStyleRuleAt(aIndex))
> -  }
>  
> +  Rule* GetStyleRuleAt(int32_t aIndex) const {

I'd say "brace to the next line", but we're really inconsistent at this so your call :P
Attachment #8962978 - Flags: review?(emilio) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

a year ago
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/233d968614c0
part 1 - Make Rule::mParentRule not necessarily a GroupRule. r=emilio
https://hg.mozilla.org/integration/autoland/rev/17e8f60b41e8
part 2 - Make CSSKeyframesRule inherit from Rule directly. r=emilio
https://hg.mozilla.org/integration/autoland/rev/83d97a59a659
part 3 - Merge ServoGroupRuleRules into GroupRule. r=emilio
You need to log in before you can comment on or make changes to this bug.