[webkit-reviews] review granted: [Bug 71835] Move CSSPrimitiveValue bitfields up into CSSValue. : [Attachment 114118] Patch without double ChangeLog..

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 71835: Move CSSPrimitiveValue bitfields up into CSSValue.
https://bugs.webkit.org/show_bug.cgi?id=71835

Attachment 114118: Patch without double ChangeLog..
https://bugs.webkit.org/attachment.cgi?id=114118&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=114118&action=review


We should figure out where to put the kind of comment or compile time assertion
that will prevent people from adding virtual functions back into CSSValue or
adding data members to CSSPrimitiveValue.

> Source/WebCore/css/CSSValue.h:136
>      CSSValue(ClassType classType)

I think this constructor should be explicit.

> Source/WebCore/css/CSSValue.h:137
> +	   : m_primitiveUnitType(0)

Might be nice to have some assertions that this is still 0 somewhere.

> Source/WebCore/css/CSSValue.h:197
> +    // These bits are only used by CSSPrimitiveValue but kept here
> +    // to maximize struct packing.

Great comment!

> Source/WebCore/css/CSSValue.h:198
> +    signed m_primitiveUnitType : 8; // CSSPrimitiveValue::UnitTypes

Why signed instead of unsigned?