Closed
Bug 1338461
Opened 7 years ago
Closed 7 years ago
stylo: use eRestyle_StyleAttribute rather than converting it to eRestyle_Self/Subtree
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: heycam, Assigned: heycam)
References
Details
Attachments
(1 file, 1 obsolete file)
Now that we support processing STYLE_ATTRIBUTE restyle hints on the Servo side, we can pass through eRestyle_StyleAttribute to Servo_NoteExplicitHints without converting it to eRestyle_Self/Subtree.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8835942 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3dd9260d84e357b65f27a91199d408b2d7c477d&group_state=expanded
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8835942 [details] Align RESTYLE_STYLE_ATTRIBUTE value with eRestyle_StyleAttribute. https://reviewboard.mozilla.org/r/111492/#review112930 ::: servo/components/style/restyle_hints.rs:49 (Diff revision 1) > const RESTYLE_LATER_SIBLINGS = 0x08, > > /// Don't re-run selector-matching on the element, only the style > /// attribute has changed, and this change didn't have any other > /// dependencies. > - const RESTYLE_STYLE_ATTRIBUTE = 0x10, > + const RESTYLE_STYLE_ATTRIBUTE = 0x80, So, this makes me a bit nervous. Please add a comment above RestyleHint that the bit positions matter for gecko compat. I think we should also add some static assertions in RestyleHint::from below that: (1) Each RestyleHint corresponds to the appropriate value according to bindgen, and that: (2) The union of all the bits we check is equal to RestyleHint::all() (to prevent unintentionally allowing garbage bits from Gecko if somebody adds a new restle hint).
Attachment #8835942 -
Flags: review?(bobbyholley) → review+
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8835943 [details] Bug 1338461 - Pass eRestyle_StyleAttribute through to Servo_NoteExplicitHints. https://reviewboard.mozilla.org/r/111494/#review112932
Attachment #8835943 -
Flags: review?(bobbyholley) → review+
Assignee | ||
Comment 6•7 years ago
|
||
> I think we should also add some static assertions in RestyleHint::from below
> that:
> (1) Each RestyleHint corresponds to the appropriate value according to
> bindgen, and that:
> (2) The union of all the bits we check is equal to RestyleHint::all() (to
> prevent unintentionally allowing garbage bits from Gecko if somebody adds a
> new restle hint).
I found existing assertions in tests/unit/stylo/sanity_checks.rs so I will add them there and point to them from the comment.
Assignee | ||
Comment 7•7 years ago
|
||
https://github.com/servo/servo/pull/15523
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8835942 -
Attachment is obsolete: true
Comment 9•7 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #6) > > I think we should also add some static assertions in RestyleHint::from below > > that: > > (1) Each RestyleHint corresponds to the appropriate value according to > > bindgen, and that: > > (2) The union of all the bits we check is equal to RestyleHint::all() (to > > prevent unintentionally allowing garbage bits from Gecko if somebody adds a > > new restle hint). > > I found existing assertions in tests/unit/stylo/sanity_checks.rs so I will > add them there and point to them from the comment. Would you mind moving those to restyle_hints.rs? In the current world those unit tests will run against the checked-in bindings, which may be arbitrarily out of date. I think it's much better to catch these bugs as soon as the enum on either side is changed.
Flags: needinfo?(cam)
Assignee | ||
Comment 10•7 years ago
|
||
OK, moving those in https://github.com/servo/servo/pull/15538.
Flags: needinfo?(cam)
Assignee | ||
Comment 11•7 years ago
|
||
Interestingly, I get some test failures from this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9105655834fc
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Comment 12•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9b3ea102ed73731599555ba51b7038af43dca5c
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1ac14d8a4ca4 Pass eRestyle_StyleAttribute through to Servo_NoteExplicitHints. r=bholley
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1ac14d8a4ca4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•