Generate some more flags from Servo side

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

Trunk
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(4 attachments)

This flag is for properties which needs layout flush for getComputedStyle (i.e. resolved value depends on reflow).

There is no flag in Servo side property database which contains this, but we can add one in PropertyFlags.
Assignee: nobody → xidorn+moz
Summary: Generate CSS_PROPERTY_GETCS_NEEDS_LAYOUT_FLUSH flag from Servo side → Generate some more flags from Servo side
Comment on attachment 8969604 [details]
Bug 1454830 part 1 - Adjust GETCS_NEEDS_LAYOUT_FLUSH flag in nsCSSPropList.h.

https://reviewboard.mozilla.org/r/238374/#review244180

Looks good, thanks for the commit message :)
Attachment #8969604 - Flags: review?(emilio) → review+
Comment on attachment 8969605 [details]
Bug 1454830 part 2 - Add GETCS_NEEDS_LAYOUT_FLUSH flag in Servo side and propagate it to ServoCSSPropList.h.

https://reviewboard.mozilla.org/r/238376/#review244182

::: servo/components/style/properties/longhand/box.mako.rs:436
(Diff revision 1)
>                            gecko_pref="layout.css.individual-transform.enabled",
>                            spec="https://drafts.csswg.org/css-transforms-2/#individual-transforms",
>                            servo_restyle_damage = "reflow_out_of_flow")}
>  
> -${helpers.predefined_type("translate", "Translate",
> +${helpers.predefined_type(
> +    "translate", "Translate",

nit: If while you're here you want to put the type on its own line that'd be nice :)

::: servo/components/style/properties/longhand/position.mako.rs:25
(Diff revision 1)
> +    )}
>  % endfor
>  // offset-* logical properties, map to "top" / "left" / "bottom" / "right"
>  % for side in LOGICAL_SIDES:
> -    ${helpers.predefined_type("offset-%s" % side, "LengthOrPercentageOrAuto",
> +    ${helpers.predefined_type(
> +        "offset-%s" % side, "LengthOrPercentageOrAuto",

ditto here and above.

::: servo/components/style/properties/longhand/ui.mako.rs:52
(Diff revision 1)
>                            animation_value_type="ComputedValue",
>                            spec="None (Nonstandard internal property)")}
>  
>  // TODO(bug 1419695) This should be hidden from content.
> -${helpers.predefined_type("-moz-window-transform", "Transform",
> +${helpers.predefined_type(
> +    "-moz-window-transform", "Transform",

ditto.

::: servo/components/style/properties/properties.mako.rs:790
(Diff revision 1)
> +        /* The following flags are currently not used in Rust code, they
> +         * only need to be listed in corresponding properties so that
> +         * they can be checked in the C++ side via ServoCSSPropList.h. */
> +        /// This property's getComputedStyle implementation requires layout
> +        /// to be flushed.
> +        const GETCS_NEEDS_LAYOUT_FLUSH = 0;

Maybe we should still make it `1 << 20` for consistency?
Attachment #8969605 - Flags: review?(emilio) → review+
Comment on attachment 8969606 [details]
Bug 1454830 part 3 - Add CAN_ANIMATE_ON_COMPOSITOR in Servo side and propagate it to ServoCSSPropList.h.

https://reviewboard.mozilla.org/r/238378/#review244184
Attachment #8969606 - Flags: review?(emilio) → review+
Comment on attachment 8969607 [details]
Bug 1454830 part 4 - Remove CSS_PROPERTY_VALUE_LIST_USES_COMMAS flag.

https://reviewboard.mozilla.org/r/238380/#review244186
Attachment #8969607 - Flags: review?(emilio) → review+
Comment on attachment 8969605 [details]
Bug 1454830 part 2 - Add GETCS_NEEDS_LAYOUT_FLUSH flag in Servo side and propagate it to ServoCSSPropList.h.

https://reviewboard.mozilla.org/r/238376/#review244182

> Maybe we should still make it `1 << 20` for consistency?

For consistency with what? Wouldn't this trigger warning or error that the number is out of range?
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #9)
> Comment on attachment 8969605 [details]
> Bug 1454830 part 2 - Add GETCS_NEEDS_LAYOUT_FLUSH flag in Servo side and
> propagate it to ServoCSSPropList.h.
> 
> https://reviewboard.mozilla.org/r/238376/#review244182
> 
> > Maybe we should still make it `1 << 20` for consistency?
> 
> For consistency with what? Wouldn't this trigger warning or error that the
> number is out of range?

I meant so they had the same values as the c++ bits, but doesn't matter much I guess.
Yeah, as far as we are not using them across the boundary, I don't think it really makes much sense to have them sync.

Also, C++ bits are going to change. Actually after part 4 here, we have been able to pack them into 8 bits, but I'd rather do that after I remove some more bits...
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67f2bf174fff
part 1 - Adjust GETCS_NEEDS_LAYOUT_FLUSH flag in nsCSSPropList.h. r=emilio
https://hg.mozilla.org/integration/autoland/rev/8e58913be805
part 2 - Add GETCS_NEEDS_LAYOUT_FLUSH flag in Servo side and propagate it to ServoCSSPropList.h. r=emilio
https://hg.mozilla.org/integration/autoland/rev/cd367de31d71
part 3 - Add CAN_ANIMATE_ON_COMPOSITOR in Servo side and propagate it to ServoCSSPropList.h. r=emilio
https://hg.mozilla.org/integration/autoland/rev/83921643241a
part 4 - Remove CSS_PROPERTY_VALUE_LIST_USES_COMMAS flag. r=emilio
You need to log in before you can comment on or make changes to this bug.