Bug 75635 – Add a compile-time assertion for the size of CSSValue

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Tony Chang]

From CSSValue.h:

protected:
    // The bits in this section are only used by specific subclasses but kept here
    // to maximize struct packing.

    // CSSPrimitiveValue bits:
    unsigned char m_primitiveUnitType : 7; // CSSPrimitiveValue::UnitTypes
    mutable bool m_hasCachedCSSText : 1;
    bool m_isQuirkValue : 1;

    // CSSInitialValue bits:
    bool m_isImplicitInitialValue : 1;

    // CSSValueList bits:
    bool m_isSpaceSeparatedValueList : 1;

private:
    unsigned char m_classType : ClassTypeBits; // ClassType


ClassTypeBits is 5 bits.  Swapping between bools and unsigned chars is bad for MSVC.

[Ryosuke Niwa]

It turned out that Visual Studio can pack them nicely as is because the only other POD type used here is unsigned char, which has the same size as bool inside bitfields on Visual Studio.

[Ryosuke Niwa]

It appears that http://trac.webkit.org/changeset/101119/trunk/Source/WebCore/css/CSSValue.cpp added a compile-time assertion to make sure CSSValue is less than 4 bytes but this isn't tight enough on some platforms where it can fit in 2 bytes.

[Luke Macpherson]

Don't forget that CSSValue extends RefCounted<CSSValue>, so best case is 6 bytes for CSSValue, and on some platforms it is 8 bytes due to alignment.

[Ryosuke Niwa]

Created attachment 121355 [details]
Tightens the assertion

[Tony Chang]

Comment on attachment 121355 [details]
Tightens the assertion

View in context: https://bugs.webkit.org/attachment.cgi?id=121355&action=review

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:26946
> - 				07A6D1EB1491137700051D0C /* MediaFragmentURIParser.cpp in Sources */,
> +				07A6D1EB1491137700051D0C /* MediaFragmentURIParser.cpp in Sources */,

Not sure if you meant to include this in the change, but it seems harmless (I would probably commit it in a separate change w/o review).

[Ryosuke Niwa]

Landed in http://trac.webkit.org/changeset/104236.