408933 - get rid of nsStyleStruct base type

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We should get rid of the nsStyleStruct base class and just make all the style structs simple structs that don't inherit from anything (and use void* where we currently use nsStyleStruct*).  This will remove the potential extra word of padding needed to give nsStyleStruct a nonzero size.

Comment 1

[[e-mail]]

Attachment: v1

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Looks good at first glance; I'll look more after (at least some of) the holidays.  This'll cause roc a bit of merge pain, but I think the best solution there is for him to land first, since this patch is probably a bit easier to redo in the necessary parts.

Also might be good to see if roc/boris agree that the idea is reasonable, but I think they well.

Also, probably better to update adding-style-props.html to have the current syntax, which would skip the line you changed and the one before it and change the one after to begin with GetStyleUIReset()->.  Though that doc really needs a lot more changes...

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds reasonable to me. it's probably not hard for me to merge, I have my MathML patch in a Mercurial tree so merging is fairly easy.

Comment 4

[Boris Zbarsky [:bzbarsky]]

While you're here, want to remove the (now unused) nsIFrame::GetStyleData and nsIFrame::GetStyleDataExternal methods and update the docs accordingly?

The rest of this looks good to me.

Comment 5

[[e-mail]]

Attachment: v2

incorporate adding-style-props.html fix and remove nsIFrame::GetStyleData{External}.

two questions:
1) GetStyleDataExternal is still used at http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsIFrame.h&rev=3.389&mark=673#673. i removed the entire #else block, and the #ifdef wrapping the block above, but i'm not sure if that's the right thing to do... maybe we at least want to keep the #ifdef?
2) should i rev the nsIFrame IID?

i don't mind landing after roc, so either way - this patch didn't take long to roll. ;)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, hmm.  We need to keep GetStyleDataExternal, then.  I somehow missed that caller.  :(

Comment 7

[[e-mail]]

Attachment: v3

alrighty then. third time's the charm :)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 294205 [details] [diff] [review]
v3

-    (*aRuleData->mPostResolveCallback)((nsStyleStruct*)res, aRuleData);
+    (*aRuleData->mPostResolveCallback)((void*)res, aRuleData);
 
Could you make that cast a const_cast ?

-      (ruleData.mPostResolveCallback)((nsStyleStruct*)aFont, &ruleData);
+      (ruleData.mPostResolveCallback)((void*)aFont, &ruleData);

same here

+  NS_HIDDEN_(const void*) 
     GetTableBorderData(nsStyleContext* aContext);

+  NS_HIDDEN_(const void*) 
     GetUserInterfaceData(nsStyleContext* aContext);

Could you unwrap these now that they fit in less than 80 characters?


r+sr=dbaron.  Sorry for the delay.

Comment 9

[[e-mail]]

Comment on attachment 294205 [details] [diff] [review]
v3

thanks! will fix those up on checkin.

requesting approval for a low-risk cleanup patch.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 294205 [details] [diff] [review]
v3

a=beltzner for 1.9

Comment 11

[[e-mail]]

checked in, with nits fixed. (i removed the void* cast on the first mPostResolveCallback call, rather than use const_cast, since |res| is already a non-const pointer.)

Comment 12

[[e-mail]]

er, i mean second call, and |aFont|.