Closed Bug 1467536 Opened Last year Closed Last year

Add infrastructure to serialize properties with Servo.

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Though I won't enable all of them at the same time.
Comment on attachment 8984245 [details]
Bug 1467536: Add a Servo API to get the serialized style of a property.

https://reviewboard.mozilla.org/r/250024/#review256424

::: layout/style/ServoBindingList.h:823
(Diff revision 1)
> +SERVO_BINDING_FUNC(Servo_GetPropertyValue, void,
> +                   ComputedStyleBorrowed computed_values,
> +                   nsCSSPropertyID property, nsAString* value)
> +

Functions on both sides of this function are all for custom properties, so maybe you want to move it before the previous, of after all the custom property ones.

::: servo/components/style/properties/properties.mako.rs:2631
(Diff revision 1)
> +        // We'd need to get a concept of ~resolved value, which may not be worth
> +        // it.

I guess it may be useful to add an attribute for properties which doesn't use computed value as resolved value. Probably something like `resolved_value` and have it defaulted to `computed`.

There are several types of non-computed-value resolved value can be present as a value of this attribute, e.g. "color", "height", "top", "transform", etc.

Then we can handle those cases separately here. Probably for layout-dependent types like "height", "top", and "transform", we should just `unreachable!` here.

This way it should be easier to audit from the spec's point of view.

What do you think?

::: servo/components/style/properties/properties.mako.rs:2649
(Diff revision 1)
> +                    % endif
> +
> +                % if prop.predefined_type == "Color":
> +                let value = self.resolve_color(value);
> +                % elif prop.name == "caret-color":
> +                let value = self.resolve_caret_color(value);

`caret-color` isn't the only case of `ColorOrAuto`. We have experimental support for `scrollbar-*-color`, but those require widget info.

Maybe it is easier to just have all `ColorOrAuto` redirected to C++ code, since used value of `auto` can change in the future (e.g. `caret-color: auto` may derive a color from background rather than using currentcolor).
Attachment #8984245 - Flags: review?(xidorn+moz)
Comment on attachment 8984245 [details]
Bug 1467536: Add a Servo API to get the serialized style of a property.

https://reviewboard.mozilla.org/r/250024/#review256424

> I guess it may be useful to add an attribute for properties which doesn't use computed value as resolved value. Probably something like `resolved_value` and have it defaulted to `computed`.
> 
> There are several types of non-computed-value resolved value can be present as a value of this attribute, e.g. "color", "height", "top", "transform", etc.
> 
> Then we can handle those cases separately here. Probably for layout-dependent types like "height", "top", and "transform", we should just `unreachable!` here.
> 
> This way it should be easier to audit from the spec's point of view.
> 
> What do you think?

Maybe we can get rid of `GETCS_NEEDS_LAYOUT_FLUSH` from flag and derive it from this new attribute.
Comment on attachment 8984246 [details]
Bug 1467536: Add CssPropFlags::SerializedByServo and use it on some simple properties.

https://reviewboard.mozilla.org/r/250026/#review256434
Attachment #8984246 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8984247 [details]
Bug 1467536: Split GetPropertyValue from GetPropertyCSSValueWithoutWarning.

https://reviewboard.mozilla.org/r/250028/#review256436

::: layout/style/nsComputedDOMStyle.cpp:439
(Diff revision 1)
>  {
>    aReturn.Truncate();
>  
> -  ErrorResult error;
> -  RefPtr<CSSValue> val =
> -    GetPropertyCSSValueWithoutWarning(aPropertyName, error);
> +  nsCSSPropertyID prop =
> +    nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent);
> +

Feels like you are folding `GetPropertyCSSValueWithoutWarning` into `GetPropertyValue`?

If so, maybe have a separate commit do the folding (and remove `GetPropertyCSSValueWithoutWarning`), then add the extra changes. That would make it easier to review I think.
Attachment #8984247 - Flags: review?(xidorn+moz)
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> Comment on attachment 8984245 [details]
> Bug 1467536: Add a Servo API to get the serialized style of a property.
> 
> https://reviewboard.mozilla.org/r/250024/#review256424
> 
> ::: layout/style/ServoBindingList.h:823
> (Diff revision 1)
> > +SERVO_BINDING_FUNC(Servo_GetPropertyValue, void,
> > +                   ComputedStyleBorrowed computed_values,
> > +                   nsCSSPropertyID property, nsAString* value)
> > +
> 
> Functions on both sides of this function are all for custom properties, so
> maybe you want to move it before the previous, of after all the custom
> property ones.
> 
> ::: servo/components/style/properties/properties.mako.rs:2631
> (Diff revision 1)
> > +        // We'd need to get a concept of ~resolved value, which may not be worth
> > +        // it.
> 
> I guess it may be useful to add an attribute for properties which doesn't
> use computed value as resolved value. Probably something like
> `resolved_value` and have it defaulted to `computed`.

I don't know what the value of that would be where the resolved value is not the computed one should be. I'd use something like: `computed_value_is_resolved=True` (and specify it to false everywhere else).

But I think that's a bit overkill, because we want to handle colors here, for example, but not more complex properties with layout info.

So far the ones I'm aware we don't want to handle is:

 * Layout dependent.
 * scrolbar colors -> I'd really love for them to be just `auto`, seems like a super-easy fingerprinting vector, but oh well.

The rest we should be able to handle here.

For now I've added an assertion that the property is not layout-dependent.

> ::: servo/components/style/properties/properties.mako.rs:2649
> (Diff revision 1)
> > +                    % endif
> > +
> > +                % if prop.predefined_type == "Color":
> > +                let value = self.resolve_color(value);
> > +                % elif prop.name == "caret-color":
> > +                let value = self.resolve_caret_color(value);
> 
> `caret-color` isn't the only case of `ColorOrAuto`. We have experimental
> support for `scrollbar-*-color`, but those require widget info.
> 
> Maybe it is easier to just have all `ColorOrAuto` redirected to C++ code,
> since used value of `auto` can change in the future (e.g. `caret-color:
> auto` may derive a color from background rather than using currentcolor).

Yes, my second patch for this does that, I plan to do all this incrementally since it's hard to audit / review / land otherwise.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #7)
> Comment on attachment 8984247 [details]
> Bug 1467536: Serialize properties flagged as SerializedByServo from
> getComputedStyle.
> 
> https://reviewboard.mozilla.org/r/250028/#review256436
> 
> ::: layout/style/nsComputedDOMStyle.cpp:439
> (Diff revision 1)
> >  {
> >    aReturn.Truncate();
> >  
> > -  ErrorResult error;
> > -  RefPtr<CSSValue> val =
> > -    GetPropertyCSSValueWithoutWarning(aPropertyName, error);
> > +  nsCSSPropertyID prop =
> > +    nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent);
> > +
> 
> Feels like you are folding `GetPropertyCSSValueWithoutWarning` into
> `GetPropertyValue`?
> 
> If so, maybe have a separate commit do the folding (and remove
> `GetPropertyCSSValueWithoutWarning`), then add the extra changes. That would
> make it easier to review I think.

Alright, I'll try to split it out into two patches.
Assignee: nobody → emilio
Comment on attachment 8984245 [details]
Bug 1467536: Add a Servo API to get the serialized style of a property.

https://reviewboard.mozilla.org/r/250024/#review256524

::: servo/components/style/properties/properties.mako.rs:2648
(Diff revision 2)
> +                % elif prop.name == "caret-color":
> +                let value = self.resolve_caret_color(value);

Per discussion in IRC, let's get rid of this special case and keep it in C++ side.

::: servo/components/style/properties/properties.mako.rs:2671
(Diff revision 2)
> +    /// Resolves a `ColorOrAuto` value, using `currentcolor` for `auto`.
> +    pub fn resolve_caret_color(&self, color: computed::ColorOrAuto) -> RGBA {
> +        self.resolve_color(match color {
> +            Either::First(color) => color,
> +            Either::Second(_auto) => computed::Color::currentcolor(),
> +        })
> +    }

So this can be removed.
Attachment #8984245 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8984369 [details]
Bug 1467536: Serialize properties flagged as SerializedByServo from getComputedStyle.

https://reviewboard.mozilla.org/r/250186/#review256526
Attachment #8984369 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8984247 [details]
Bug 1467536: Split GetPropertyValue from GetPropertyCSSValueWithoutWarning.

https://reviewboard.mozilla.org/r/250028/#review256530

::: layout/style/nsComputedDOMStyle.cpp:454
(Diff revision 3)
> +  UpdateCurrentStyleSources(layoutFlushIsNeeded);
> +  if (!mComputedStyle) {
> +    return NS_ERROR_NOT_AVAILABLE;
> +  }
> +
> +  auto cleanup = mozilla::MakeScopeExit([&] {

This lambda only needs to capture `this`. It would probably be better to just write `[this]` rather than `[&]`.

::: layout/style/nsComputedDOMStyle.cpp
(Diff revision 3)
> -    return nullptr;
> -  }
> -
> -  RefPtr<CSSValue> val;
> -  if (prop == eCSSPropertyExtra_variable) {
> -    val = DoGetCustomProperty(aPropertyName);

You can remove `DoGetCustomProperty` as well now.
Attachment #8984247 - Flags: review?(xidorn+moz) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/086eaeb69efa
Add a Servo API to get the serialized style of a property. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/13b38c2d3251
Add CssPropFlags::SerializedByServo and use it on some simple properties. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/697892c54d63
Split GetPropertyValue from GetPropertyCSSValueWithoutWarning. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e1cee0e0a48
Serialize properties flagged as SerializedByServo from getComputedStyle. r=xidorn
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b664f1fdec85
followup: Use a more specific capture for the lambda. r=xidorn
We should be able to remove quite a bit of functions in nsComputedDOMStyle as well as keyword tables now, which should mitigate the bloat to some extent.
This failure started perma failing with this push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4e1cee0e0a48d3e85bd3c20342354c95c5039d77&filter-searchStr=OS%20X%2010.10%20opt%20Mochitests%20with%20e10s%20test-macosx64%2Fopt-mochitest-e10s-5%20M-e10s(5)&selectedJob=182511164

It was incorrectly starred as Bug 1458170 and it was merged around on other branches.

Pushed the backout to try and it was green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=413c81c1599085187e83ff58d809ca0b09d5fd69&selectedJob=182636633

Backed out for permafailing on layout/style/test/test_bug418986-2.html

Backout link: https://hg.mozilla.org/mozilla-central/rev/35bd1a865ff5262e05f2e421d499e8c42a5df2de

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4e1cee0e0a48d3e85bd3c20342354c95c5039d77&filter-searchStr=OS%20X%2010.10%20opt%20Mochitests%20with%20e10s%20test-macosx64%2Fopt-mochitest-e10s-5%20M-e10s(5)&selectedJob=182511164

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=182635776&repo=mozilla-central&lineNumber=1564

Log snippet: 

23:34:03     INFO - TEST-START | layout/style/test/test_bug418986-2.html
23:34:03     INFO - TEST-INFO | started process screencapture
23:34:03     INFO - TEST-INFO | screencapture: exit 0
23:34:03     INFO - <snipped 31 output lines - if you need more context, please use SimpleTest.requestCompleteLog() in your test>
23:34:03     INFO - Buffered messages logged at 23:34:03
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-mac-graphite-theme' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-end-backward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-end-forward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-start-backward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-start-forward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-thumb-proportional' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-touch-enabled' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-compositor' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-default-theme' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-glass' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-available' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-minimize-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-maximize-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-close-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-osx-font-smoothing 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query 'device-aspect-ratio:1600/1200' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query 'device-height:1200px' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query 'device-width:1600px' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query 'height:300px' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query 'resolution:0.999dppx,1.001dppx' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query 'width:500px' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query '-moz-device-pixel-ratio:1' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Media query '-moz-device-orientation:landscape' in picture should match. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected color:8 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected color-index:0 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected aspect-ratio:500/300 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected device-aspect-ratio:500/300 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected device-height:300px 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected device-width:500px 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected grid:0 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected height:300px 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected monochrome:0 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected orientation:landscape 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected resolution:96dpi 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected resolution:1dppx 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected width:500px 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected -moz-device-pixel-ratio:1 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | Expected -moz-device-orientation:landscape 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-mac-graphite-theme should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-scrollbar-end-backward should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-scrollbar-end-forward should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-scrollbar-start-backward should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-scrollbar-start-forward should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-scrollbar-thumb-proportional should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-touch-enabled should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-windows-compositor should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-windows-default-theme should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-windows-glass should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-gtk-csd-available should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-gtk-csd-minimize-button should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-gtk-csd-maximize-button should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | -moz-gtk-csd-close-button should not exist. 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'device-aspect-ratio' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'device-height' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'device-width' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'height' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'resolution' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'width' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-device-pixel-ratio' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-device-orientation' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-mac-graphite-theme' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-end-backward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-end-forward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-start-backward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-start-forward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-thumb-proportional' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-touch-enabled' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-compositor' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-default-theme' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-glass' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-available' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-minimize-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-maximize-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-close-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'color' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'aspect-ratio' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'device-aspect-ratio' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'device-height' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'device-width' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'height' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'orientation' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'resolution' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'resolution' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for 'width' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-device-pixel-ratio' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-device-orientation' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-mac-graphite-theme' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-end-backward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-end-forward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-start-backward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-start-forward' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-scrollbar-thumb-proportional' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-touch-enabled' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-compositor' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-default-theme' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-windows-glass' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-available' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-minimize-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-maximize-button' 
23:34:03     INFO - TEST-PASS | layout/style/test/test_bug418986-2.html | CSS for '-moz-gtk-csd-close-button' 
23:34:03     INFO - Buffered messages finished
23:34:03     INFO - TEST-UNEXPECTED-FAIL | layout/style/test/test_bug418986-2.html | -moz-osx-font-smoothing - got "auto", expected ""
Flags: needinfo?(emilio)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla62 → ---
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #26)
> It looks like this regressed .text by ~320k [1].
> 
> [1]
> https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-inbound,
> 1686772,1,2&highlightedRevisions=3e1da01d4878

Yes, this is ~expected, but allows us to get rid of more (manually-written) code in the longer term.
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0e2de23f91f
Add a Servo API to get the serialized style of a property. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/b090688d3f71
Add CssPropFlags::SerializedByServo and use it on some simple properties. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/52b6bc85dcea
Split GetPropertyValue from GetPropertyCSSValueWithoutWarning. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/c32b85f2b391
Serialize properties flagged as SerializedByServo from getComputedStyle. r=xidorn
Thanks Raul.

I think we should just not expose that property, but for now I've removed them from the properties that get serialized with Servo.

It's not clear to me how useful it is to avoid exposing it in GetComputedStyle, but still show the property as enumerable (just return the empty string), and honor it (which is observable via canvas and what not)...

But anyway that's not the point of this bug.
Flags: needinfo?(emilio)
You need to log in before you can comment on or make changes to this bug.