1408300 - make nsComputedDOMStyle use Servo code for property serialization

Status: NEW

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

Description

[Cameron McCormack (:heycam)]

There is a lot of serialization code in nsComputedDOMStyle and it would be nice if we could use Servo's code for this.

One complication is that the current code is all written in terms of the old DOM Level 2 Style OM, i.e. each serialization method returns a CSSValue object, in case it needs to be returned from getPropertyCSSValue.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

It seems to me that nsComputedDOMStyle is pretty much decoupled from other parts of style system. It has its own serialization code from style structs. It may call into nsCSSValue somewhere, but we cannot remove nsCSSValue anyway. So I guess it probably isn't really a blocker for removing the old style system.

Comment 2

[Cameron McCormack (:heycam)]

Maybe the dependency should be the other way around.  But still, since there is a lot of code in there, I wonder if we will need to recover that space for the Android apk file.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

There is lots of code, but that's lots of code in Servo side which we may not be using at the moment as well (assuming linkers correctly strips those ToCss for computed value which we don't call, but that may not be true).

Comment 4

[Chris Peterson [:cpeterson]]

We hope to ship Stylo for Android in Firefox 59, so it would be nice to know whether sharing this code would reduce the Android APK file size.

Comment 5

[Cameron McCormack (:heycam)]

As an approximation, the total size of all of the nsComputedDOMStyle::DoGetXXX functions is about 86 KiB.

Comment 6

[Cameron McCormack (:heycam)]

Ah, that was a non-opt build.  In an opt build, it's around 53 KiB.

Comment 7

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Here's the current state of it. Some of the failures look bogus tests that rely on calc() order not as in the spec. There are a few special-cases like flex min-* which I haven't accounted for.

Then there's URLs and colors, which we need to really resolve, which is kind of annoying...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2e860a31f7f209da06ade2be0bde61176bb5ff0

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

All the dependencies are fixed, is there anything left to do here?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Yes, most of the work actually :).

I have a patch locally, but I'm not sure what's the best way to land it, so I punted on it for a bit, probably should spend some time to enable it property by property and fixing some tests while at it.