Status

()

defect
P4
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

After bug 636039 I was planning to remove the nsCSS* structs, though I was thinking of making bigger changes to nsCSSExpandedDataBlock (maybe or maybe not along the lines of bug 553456).

However, I realized how *trivial* it is to remove them right now (thanks largely to bug 576044) -- so we should do that, since it lets us remove code we don't need and simplifies the process of adding new CSS properties.

Patch queue coming up in a bit -- patch 1 is the "stop using them" patch, and the rest of the patches are removing things we no longer need as a result.
Attachment #522302 - Attachment description: remove nsCSSStruct.h/cpp → patch 4: remove nsCSSStruct.h/cpp
Attachment #522303 - Attachment description: remove the datastruct_ and member_ fields of nsCSSPropList → patch 5: remove the datastruct_ and member_ fields of nsCSSPropList
Comment on attachment 522299 [details] [diff] [review]
patch 1: switch from nsCSS* to an array of nsCSSValue

r=me
Attachment #522299 - Flags: review?(bzbarsky) → review+
Comment on attachment 522300 [details] [diff] [review]
patch 2: remove nsCSSExpandedDataBlock::kOffsetTable

r=me
Attachment #522300 - Flags: review?(bzbarsky) → review+
Comment on attachment 522301 [details] [diff] [review]
patch 3: move nsCSSCornerSizes to nsCSSValue.h/cpp

r=me
Attachment #522301 - Flags: review?(bzbarsky) → review+
Comment on attachment 522302 [details] [diff] [review]
patch 4: remove nsCSSStruct.h/cpp

r=me
Attachment #522302 - Flags: review?(bzbarsky) → review+
Comment on attachment 522303 [details] [diff] [review]
patch 5: remove the datastruct_ and member_ fields of nsCSSPropList

r=me (though I semi-skimmed most of the list there).
Attachment #522303 - Flags: review?(bzbarsky) → review+
It's nice to see all this go away (less boilerplate for new CSS properties, yay)

It occurs to me that a lot of the cost of having two nsCSSExpandedDataBlock objects embedded in nsCSSParser (and therefore a lot of the motivation for bug 553456) is the cost of running the nsCSSValue constructor and destructor for every value slot.  What do you think of doing just-in-time initialization on value slots?  This would be quite easy now we have an array instead of a bunch of structs -- replace the array with a char[] storage block, and PropertyAt() uses placement new to initialize the appropriate slot if it hasn't already been done (as determined by the bitfields).  I can try to code that up later today if you like the idea.
Well, I'd like to move towards getting rid of nsCSSExpandedDataBlock entirely -- so I think such a patch would be temporary.  In other words, it seems reasonable if it's really simple.
You need to log in before you can comment on or make changes to this bug.