The code in nsComputedDOMStyle.cpp has too much copy & paste. It needs to use nsCSSPropList.h somehow.

I think there are a bunch of steps we could take here (which should probably be separate patches): 1. move the _LAYOUT variant of COMPUTED_STYLE_MAP_ENTRY into a property bit (defined in nsCSSProps.h and nsCSSPropList.h) 2. replace nsComputedDOMStyle::GetQueryablePropertyMap with something generated from nsCSSPropList.h 3. add a property bit that says "use a custom function" for computed style, and set it for all the properties. (It should actually be done in the style of CSS_PROPERTY_PARSE_PROPERTY_MASK, since it will end up being a value that takes up multiple bits.) 4, 5, 6, ...: one pattern at a time, replace common patterns for computed style with additional values in that property bit space, and remove the custom functions that can be replaced) The relevant code here is all in layout/style/, in the files nsComputedDOMStyle.{h,cpp}, nsCSSPropList.h, and nsCSSProps.h. It should be reasonably well covered by the mochitests in layout/style/test/.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=fdcbcd21e39e By the way, I presume it is fine to work on one part at a time and land each one individually?

(In reply to Birunthan Mohanathas [:poiru] from comment #3) > By the way, I presume it is fine to work on one part at a time and land each > one individually? Yes, that's great as long as all the intermediate states work correctly.

Comment on attachment 808163 [details] [diff] [review] Move the _LAYOUT variant of COMPUTED_STYLE_MAP_ENTRY into a property bit >+bool nsComputedDOMStyle::ComputedStyleMapEntry::IsLayoutFlushNeeded() const >+{ >+ return nsCSSProps::PropHasFlags(mProperty, CSS_PROPERTY_NEEDS_LAYOUT_FLUSH); >+} This should be inline (meaning, defined in the .h so that it gets inlined at callers). >+// This property is a layout property that needs a flush. >+#define CSS_PROPERTY_NEEDS_LAYOUT_FLUSH (1<<20) >+ Could you name this CSS_PROPERTY_GETCS_NEEDS_LAYOUT_FLUSH instead? (That's easy to do by search-replace of the entire patch.) And make the comment say: // This property's getComputedStyle implementation requires layout to be // flushed. r=dbaron with those changes, and thanks for the patch. The bits with the directional shorthands (margin-left, margin-right, padding-left, padding-right, etc.) might get interesting in the next patch, but seem correct in this patch.

(In reply to David Baron [:dbaron] (needinfo? me) from comment #5) > Comment on attachment 808163 [details] [diff] [review] Thanks, took care of those.

Hi Birunthan. Are you still actively working on this one? If not, may I take it? Regards, Gorman

(In reply to Gorman Ho from comment #9) > Hi Birunthan. Are you still actively working on this one? If not, may I take > it? Sure, it's yours!

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.

No assignee, updating the status.

There is no nsCSSPropList.h in latest source code of firefox :/

Redirecting to David who should know!

GETCS_NEEDS_LAYOUT_FLUSH is now a flag set in servo/components/style/properties/longhands/*.mako.rs.

I think there was still a lot more to do here -- many of the nsComputedDOMStyle::DoGet* functions are quite similar to each other -- but I'm OK with that happening in other bugs.

Eventually all the DoGet* functions should be replaced by calling into Servo code to serialize.