Title: [234859] trunk/Source
Revision
234859
Author
[email protected]
Date
2018-08-14 11:24:02 -0700 (Tue, 14 Aug 2018)

Log Message

Use a Variant instead of a union in CSSSelector
https://bugs.webkit.org/show_bug.cgi?id=188559

Reviewed by Antti Koivisto.

Source/WebCore:

No change in behavior. This just makes some of the existing problems more obvious and easy to fix.

I moved m_caseInsensitiveAttributeValueMatching to RareData because it's only used with RareData.
I only have m_isForPage when assertions are enabled because it's only used for an assertion.
The rest is pretty straightforward translating union syntax to Variant syntax.
I use RefPtr for now where I could use Ref because it's never null to make copying easier, but that's temporary.

* css/CSSSelector.cpp:
(WebCore::CSSSelector::CSSSelector):
(WebCore::CSSSelector::createRareData):
(WebCore::CSSSelector::setAttribute):
(WebCore::CSSSelector::setArgument):
(WebCore::CSSSelector::setLangArgumentList):
(WebCore::CSSSelector::setSelectorList):
(WebCore::CSSSelector::setNth):
(WebCore::CSSSelector::matchNth const):
(WebCore::CSSSelector::nthA const):
(WebCore::CSSSelector::nthB const):
(WebCore::CSSSelector::RareData::RareData):
* css/CSSSelector.h:
(WebCore::CSSSelector::argument const):
(WebCore::CSSSelector::langArgumentList const):
(WebCore::CSSSelector::selectorList const):
(WebCore::CSSSelector::attribute const):
(WebCore::CSSSelector::attributeCanonicalLocalName const):
(WebCore::CSSSelector::setValue):
(WebCore::CSSSelector::CSSSelector):
(WebCore::CSSSelector::~CSSSelector):
(WebCore::CSSSelector::tagQName const):
(WebCore::CSSSelector::tagLowercaseLocalName const):
(WebCore::CSSSelector::value const):
(WebCore::CSSSelector::serializingValue const):
(WebCore::CSSSelector::attributeValueMatchingIsCaseInsensitive const):
(WebCore::CSSSelector::RareData::create): Deleted.
* css/parser/CSSParserImpl.cpp:
(WebCore::CSSParserImpl::parsePageSelector):
* css/parser/CSSParserSelector.h:

Source/WTF:

* wtf/Variant.h:
Add packing macros to make it so Variant-containing structures don't always have 7 bytes of padding per Variant.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (234858 => 234859)


--- trunk/Source/WTF/ChangeLog	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WTF/ChangeLog	2018-08-14 18:24:02 UTC (rev 234859)
@@ -1,3 +1,13 @@
+2018-08-14  Alex Christensen  <[email protected]>
+
+        Use a Variant instead of a union in CSSSelector
+        https://bugs.webkit.org/show_bug.cgi?id=188559
+
+        Reviewed by Antti Koivisto.
+
+        * wtf/Variant.h:
+        Add packing macros to make it so Variant-containing structures don't always have 7 bytes of padding per Variant.
+
 2018-08-13  Don Olmstead  <[email protected]>
 
         Meaning of OptionSet::contains is unclear when used with OptionSet argument

Modified: trunk/Source/WTF/wtf/Variant.h (234858 => 234859)


--- trunk/Source/WTF/wtf/Variant.h	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WTF/wtf/Variant.h	2018-08-14 18:24:02 UTC (rev 234859)
@@ -1435,6 +1435,7 @@
 __noexcept_variant_swap_impl<__all_swappable<_Types...>::value,_Types...>
 {};
 
+#pragma pack(push, 1)
 template<typename ... _Types>
 class Variant:
         private __variant_base<
@@ -1721,6 +1722,7 @@
         }
     }
 };
+#pragma pack(pop)
 
 template<>
 class Variant<>{

Modified: trunk/Source/WebCore/ChangeLog (234858 => 234859)


--- trunk/Source/WebCore/ChangeLog	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WebCore/ChangeLog	2018-08-14 18:24:02 UTC (rev 234859)
@@ -1,3 +1,48 @@
+2018-08-14  Alex Christensen  <[email protected]>
+
+        Use a Variant instead of a union in CSSSelector
+        https://bugs.webkit.org/show_bug.cgi?id=188559
+
+        Reviewed by Antti Koivisto.
+
+        No change in behavior. This just makes some of the existing problems more obvious and easy to fix.
+
+        I moved m_caseInsensitiveAttributeValueMatching to RareData because it's only used with RareData.
+        I only have m_isForPage when assertions are enabled because it's only used for an assertion.
+        The rest is pretty straightforward translating union syntax to Variant syntax.
+        I use RefPtr for now where I could use Ref because it's never null to make copying easier, but that's temporary.
+
+        * css/CSSSelector.cpp:
+        (WebCore::CSSSelector::CSSSelector):
+        (WebCore::CSSSelector::createRareData):
+        (WebCore::CSSSelector::setAttribute):
+        (WebCore::CSSSelector::setArgument):
+        (WebCore::CSSSelector::setLangArgumentList):
+        (WebCore::CSSSelector::setSelectorList):
+        (WebCore::CSSSelector::setNth):
+        (WebCore::CSSSelector::matchNth const):
+        (WebCore::CSSSelector::nthA const):
+        (WebCore::CSSSelector::nthB const):
+        (WebCore::CSSSelector::RareData::RareData):
+        * css/CSSSelector.h:
+        (WebCore::CSSSelector::argument const):
+        (WebCore::CSSSelector::langArgumentList const):
+        (WebCore::CSSSelector::selectorList const):
+        (WebCore::CSSSelector::attribute const):
+        (WebCore::CSSSelector::attributeCanonicalLocalName const):
+        (WebCore::CSSSelector::setValue):
+        (WebCore::CSSSelector::CSSSelector):
+        (WebCore::CSSSelector::~CSSSelector):
+        (WebCore::CSSSelector::tagQName const):
+        (WebCore::CSSSelector::tagLowercaseLocalName const):
+        (WebCore::CSSSelector::value const):
+        (WebCore::CSSSelector::serializingValue const):
+        (WebCore::CSSSelector::attributeValueMatchingIsCaseInsensitive const):
+        (WebCore::CSSSelector::RareData::create): Deleted.
+        * css/parser/CSSParserImpl.cpp:
+        (WebCore::CSSParserImpl::parsePageSelector):
+        * css/parser/CSSParserSelector.h:
+
 2018-08-14  Yusuke Suzuki  <[email protected]>
 
         Unhandled Promise Rejection logging in workers should not emit ErrorEvent to host Worker object

Modified: trunk/Source/WebCore/css/CSSSelector.cpp (234858 => 234859)


--- trunk/Source/WebCore/css/CSSSelector.cpp	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WebCore/css/CSSSelector.cpp	2018-08-14 18:24:02 UTC (rev 234859)
@@ -40,13 +40,8 @@
 
 using namespace HTMLNames;
 
-struct SameSizeAsCSSSelector {
-    unsigned flags;
-    void* unionPointer;
-};
-
 static_assert(CSSSelector::RelationType::Subselector == 0, "Subselector must be 0 for consumeCombinator.");
-static_assert(sizeof(CSSSelector) == sizeof(SameSizeAsCSSSelector), "CSSSelector should remain small.");
+static_assert(sizeof(CSSSelector) == sizeof(void*) + sizeof(unsigned), "CSSSelector should remain small.");
 
 CSSSelector::CSSSelector(const QualifiedName& tagQName, bool tagIsForNamespaceRule)
     : m_relation(DescendantSpace)
@@ -54,11 +49,10 @@
     , m_pseudoType(0)
     , m_isLastInSelectorList(false)
     , m_isLastInTagHistory(true)
-    , m_hasRareData(false)
-    , m_hasNameWithCase(false)
+    , m_tagIsForNamespaceRule(tagIsForNamespaceRule)
+#if !ASSERT_DISABLED
     , m_isForPage(false)
-    , m_tagIsForNamespaceRule(tagIsForNamespaceRule)
-    , m_caseInsensitiveAttributeValueMatching(false)
+#endif
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
     , m_destructorHasBeenCalled(false)
 #endif
@@ -66,25 +60,21 @@
     const AtomicString& tagLocalName = tagQName.localName();
     const AtomicString tagLocalNameASCIILowercase = tagLocalName.convertToASCIILowercase();
 
-    if (tagLocalName == tagLocalNameASCIILowercase) {
-        m_data.m_tagQName = tagQName.impl();
-        m_data.m_tagQName->ref();
-    } else {
-        m_data.m_nameWithCase = adoptRef(new NameWithCase(tagQName, tagLocalNameASCIILowercase)).leakRef();
-        m_hasNameWithCase = true;
-    }
+    if (tagLocalName == tagLocalNameASCIILowercase)
+        m_data = tagQName;
+    else
+        m_data = adoptRef(new NameWithCase(tagQName, tagLocalNameASCIILowercase));
 }
 
 void CSSSelector::createRareData()
 {
     ASSERT(match() != Tag);
-    ASSERT(!m_hasNameWithCase);
-    if (m_hasRareData)
+    ASSERT(!WTF::holds_alternative<RefPtr<NameWithCase>>(m_data));
+    if (WTF::holds_alternative<RefPtr<RareData>>(m_data))
         return;
     // Move the value to the rare data stucture.
-    AtomicString value { adoptRef(m_data.m_value) };
-    m_data.m_rareData = &RareData::create(WTFMove(value)).leakRef();
-    m_hasRareData = true;
+    AtomicString value = WTF::get<AtomicString>(m_data);
+    m_data = adoptRef(new RareData(WTFMove(value)));
 }
 
 static unsigned simpleSelectorSpecificityInternal(const CSSSelector& simpleSelector, bool isComputingMaximumSpecificity);
@@ -753,61 +743,54 @@
 void CSSSelector::setAttribute(const QualifiedName& value, bool convertToLowercase, AttributeMatchType matchType)
 {
     createRareData();
-    m_data.m_rareData->m_attribute = value;
-    m_data.m_rareData->m_attributeCanonicalLocalName = convertToLowercase ? value.localName().convertToASCIILowercase() : value.localName();
-    m_caseInsensitiveAttributeValueMatching = matchType == CaseInsensitive;
+    WTF::get<RefPtr<RareData>>(m_data)->m_attribute = value;
+    WTF::get<RefPtr<RareData>>(m_data)->m_attributeCanonicalLocalName = convertToLowercase ? value.localName().convertToASCIILowercase() : value.localName();
+    WTF::get<RefPtr<RareData>>(m_data)->m_caseInsensitiveAttributeValueMatching = matchType == CaseInsensitive;
 }
     
 void CSSSelector::setArgument(const AtomicString& value)
 {
     createRareData();
-    m_data.m_rareData->m_argument = value;
+    WTF::get<RefPtr<RareData>>(m_data)->m_argument = value;
 }
 
 void CSSSelector::setLangArgumentList(std::unique_ptr<Vector<AtomicString>> argumentList)
 {
     createRareData();
-    m_data.m_rareData->m_langArgumentList = WTFMove(argumentList);
+    WTF::get<RefPtr<RareData>>(m_data)->m_langArgumentList = WTFMove(argumentList);
 }
 
 void CSSSelector::setSelectorList(std::unique_ptr<CSSSelectorList> selectorList)
 {
     createRareData();
-    m_data.m_rareData->m_selectorList = WTFMove(selectorList);
+    WTF::get<RefPtr<RareData>>(m_data)->m_selectorList = WTFMove(selectorList);
 }
 
 void CSSSelector::setNth(int a, int b)
 {
     createRareData();
-    m_data.m_rareData->m_a = a;
-    m_data.m_rareData->m_b = b;
+    WTF::get<RefPtr<RareData>>(m_data)->m_a = a;
+    WTF::get<RefPtr<RareData>>(m_data)->m_b = b;
 }
 
 bool CSSSelector::matchNth(int count) const
 {
-    ASSERT(m_hasRareData);
-    return m_data.m_rareData->matchNth(count);
+    return WTF::get<RefPtr<RareData>>(m_data)->matchNth(count);
 }
 
 int CSSSelector::nthA() const
 {
-    ASSERT(m_hasRareData);
-    return m_data.m_rareData->m_a;
+    return WTF::get<RefPtr<RareData>>(m_data)->m_a;
 }
 
 int CSSSelector::nthB() const
 {
-    ASSERT(m_hasRareData);
-    return m_data.m_rareData->m_b;
+    return WTF::get<RefPtr<RareData>>(m_data)->m_b;
 }
 
 CSSSelector::RareData::RareData(AtomicString&& value)
     : m_matchingValue(value)
     , m_serializingValue(value)
-    , m_a(0)
-    , m_b(0)
-    , m_attribute(anyQName())
-    , m_argument(nullAtom())
 {
 }
 

Modified: trunk/Source/WebCore/css/CSSSelector.h (234858 => 234859)


--- trunk/Source/WebCore/css/CSSSelector.h	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WebCore/css/CSSSelector.h	2018-08-14 18:24:02 UTC (rev 234859)
@@ -23,379 +23,378 @@
 
 #include "QualifiedName.h"
 #include "RenderStyleConstants.h"
+#include <wtf/Variant.h>
 
 namespace WebCore {
-    class CSSSelectorList;
 
-    enum class SelectorSpecificityIncrement {
-        ClassA = 0x10000,
-        ClassB = 0x100,
-        ClassC = 1
-    };
+class CSSSelectorList;
 
-    // this class represents a selector for a StyleRule
-    class CSSSelector {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        CSSSelector();
-        CSSSelector(const CSSSelector&);
-        explicit CSSSelector(const QualifiedName&, bool tagIsForNamespaceRule = false);
+enum class SelectorSpecificityIncrement {
+    ClassA = 0x10000,
+    ClassB = 0x100,
+    ClassC = 1
+};
 
-        ~CSSSelector();
+#pragma pack(push, 1)
+// this class represents a selector for a StyleRule
+class CSSSelector {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    CSSSelector();
+    CSSSelector(const CSSSelector&);
+    explicit CSSSelector(const QualifiedName&, bool tagIsForNamespaceRule = false);
 
-        /**
-         * Re-create selector text from selector's data
-         */
-        String selectorText(const String& = emptyString()) const;
+    ~CSSSelector();
 
-        // checks if the 2 selectors (including sub selectors) agree.
-        bool operator==(const CSSSelector&) const;
+    /**
+     * Re-create selector text from selector's data
+     */
+    String selectorText(const String& = emptyString()) const;
 
-        static const unsigned maxValueMask = 0xffffff;
-        static const unsigned idMask = 0xff0000;
-        static const unsigned classMask = 0xff00;
-        static const unsigned elementMask = 0xff;
+    // checks if the 2 selectors (including sub selectors) agree.
+    bool operator==(const CSSSelector&) const;
 
-        unsigned staticSpecificity(bool& ok) const;
-        unsigned specificityForPage() const;
-        unsigned simpleSelectorSpecificity() const;
-        static unsigned addSpecificities(unsigned, unsigned);
+    static const unsigned maxValueMask = 0xffffff;
+    static const unsigned idMask = 0xff0000;
+    static const unsigned classMask = 0xff00;
+    static const unsigned elementMask = 0xff;
 
-        /* how the attribute value has to match.... Default is Exact */
-        enum Match {
-            Unknown = 0,
-            Tag,
-            Id,
-            Class,
-            Exact,
-            Set,
-            List,
-            Hyphen,
-            PseudoClass,
-            PseudoElement,
-            Contain, // css3: E[foo*="bar"]
-            Begin, // css3: E[foo^="bar"]
-            End, // css3: E[foo$="bar"]
-            PagePseudoClass
-        };
+    unsigned staticSpecificity(bool& ok) const;
+    unsigned specificityForPage() const;
+    unsigned simpleSelectorSpecificity() const;
+    static unsigned addSpecificities(unsigned, unsigned);
 
-        enum RelationType {
-            Subselector,
-            DescendantSpace,
-            Child,
-            DirectAdjacent,
-            IndirectAdjacent,
-            ShadowDescendant
-        };
+    /* how the attribute value has to match.... Default is Exact */
+    enum Match {
+        Unknown = 0,
+        Tag,
+        Id,
+        Class,
+        Exact,
+        Set,
+        List,
+        Hyphen,
+        PseudoClass,
+        PseudoElement,
+        Contain, // css3: E[foo*="bar"]
+        Begin, // css3: E[foo^="bar"]
+        End, // css3: E[foo$="bar"]
+        PagePseudoClass
+    };
 
-        enum PseudoClassType {
-            PseudoClassUnknown = 0,
-            PseudoClassEmpty,
-            PseudoClassFirstChild,
-            PseudoClassFirstOfType,
-            PseudoClassLastChild,
-            PseudoClassLastOfType,
-            PseudoClassOnlyChild,
-            PseudoClassOnlyOfType,
-            PseudoClassNthChild,
-            PseudoClassNthOfType,
-            PseudoClassNthLastChild,
-            PseudoClassNthLastOfType,
-            PseudoClassLink,
-            PseudoClassVisited,
-            PseudoClassAny,
-            PseudoClassAnyLink,
-            PseudoClassAnyLinkDeprecated,
-            PseudoClassAutofill,
-            PseudoClassAutofillStrongPassword,
-            PseudoClassHover,
-            PseudoClassDrag,
-            PseudoClassFocus,
-            PseudoClassFocusWithin,
-            PseudoClassActive,
-            PseudoClassChecked,
-            PseudoClassEnabled,
-            PseudoClassFullPageMedia,
-            PseudoClassDefault,
-            PseudoClassDisabled,
-            PseudoClassMatches,
-            PseudoClassOptional,
-            PseudoClassPlaceholderShown,
-            PseudoClassRequired,
-            PseudoClassReadOnly,
-            PseudoClassReadWrite,
-            PseudoClassValid,
-            PseudoClassInvalid,
-            PseudoClassIndeterminate,
-            PseudoClassTarget,
-            PseudoClassLang,
-            PseudoClassNot,
-            PseudoClassRoot,
-            PseudoClassScope,
-            PseudoClassWindowInactive,
-            PseudoClassCornerPresent,
-            PseudoClassDecrement,
-            PseudoClassIncrement,
-            PseudoClassHorizontal,
-            PseudoClassVertical,
-            PseudoClassStart,
-            PseudoClassEnd,
-            PseudoClassDoubleButton,
-            PseudoClassSingleButton,
-            PseudoClassNoButton,
+    enum RelationType {
+        Subselector,
+        DescendantSpace,
+        Child,
+        DirectAdjacent,
+        IndirectAdjacent,
+        ShadowDescendant
+    };
+
+    enum PseudoClassType {
+        PseudoClassUnknown = 0,
+        PseudoClassEmpty,
+        PseudoClassFirstChild,
+        PseudoClassFirstOfType,
+        PseudoClassLastChild,
+        PseudoClassLastOfType,
+        PseudoClassOnlyChild,
+        PseudoClassOnlyOfType,
+        PseudoClassNthChild,
+        PseudoClassNthOfType,
+        PseudoClassNthLastChild,
+        PseudoClassNthLastOfType,
+        PseudoClassLink,
+        PseudoClassVisited,
+        PseudoClassAny,
+        PseudoClassAnyLink,
+        PseudoClassAnyLinkDeprecated,
+        PseudoClassAutofill,
+        PseudoClassAutofillStrongPassword,
+        PseudoClassHover,
+        PseudoClassDrag,
+        PseudoClassFocus,
+        PseudoClassFocusWithin,
+        PseudoClassActive,
+        PseudoClassChecked,
+        PseudoClassEnabled,
+        PseudoClassFullPageMedia,
+        PseudoClassDefault,
+        PseudoClassDisabled,
+        PseudoClassMatches,
+        PseudoClassOptional,
+        PseudoClassPlaceholderShown,
+        PseudoClassRequired,
+        PseudoClassReadOnly,
+        PseudoClassReadWrite,
+        PseudoClassValid,
+        PseudoClassInvalid,
+        PseudoClassIndeterminate,
+        PseudoClassTarget,
+        PseudoClassLang,
+        PseudoClassNot,
+        PseudoClassRoot,
+        PseudoClassScope,
+        PseudoClassWindowInactive,
+        PseudoClassCornerPresent,
+        PseudoClassDecrement,
+        PseudoClassIncrement,
+        PseudoClassHorizontal,
+        PseudoClassVertical,
+        PseudoClassStart,
+        PseudoClassEnd,
+        PseudoClassDoubleButton,
+        PseudoClassSingleButton,
+        PseudoClassNoButton,
 #if ENABLE(FULLSCREEN_API)
-            PseudoClassFullScreen,
-            PseudoClassFullScreenDocument,
-            PseudoClassFullScreenAncestor,
-            PseudoClassAnimatingFullScreenTransition,
-            PseudoClassFullScreenControlsHidden,
+        PseudoClassFullScreen,
+        PseudoClassFullScreenDocument,
+        PseudoClassFullScreenAncestor,
+        PseudoClassAnimatingFullScreenTransition,
+        PseudoClassFullScreenControlsHidden,
 #endif
-            PseudoClassInRange,
-            PseudoClassOutOfRange,
+        PseudoClassInRange,
+        PseudoClassOutOfRange,
 #if ENABLE(VIDEO_TRACK)
-            PseudoClassFuture,
-            PseudoClassPast,
+        PseudoClassFuture,
+        PseudoClassPast,
 #endif
 #if ENABLE(CSS_SELECTORS_LEVEL4)
-            PseudoClassDir,
-            PseudoClassRole,
+        PseudoClassDir,
+        PseudoClassRole,
 #endif
-            PseudoClassHost,
-            PseudoClassDefined,
-        };
+        PseudoClassHost,
+        PseudoClassDefined,
+    };
 
-        enum PseudoElementType {
-            PseudoElementUnknown = 0,
-            PseudoElementAfter,
-            PseudoElementBefore,
+    enum PseudoElementType {
+        PseudoElementUnknown = 0,
+        PseudoElementAfter,
+        PseudoElementBefore,
 #if ENABLE(VIDEO_TRACK)
-            PseudoElementCue,
+        PseudoElementCue,
 #endif
-            PseudoElementFirstLetter,
-            PseudoElementFirstLine,
-            PseudoElementMarker,
-            PseudoElementResizer,
-            PseudoElementScrollbar,
-            PseudoElementScrollbarButton,
-            PseudoElementScrollbarCorner,
-            PseudoElementScrollbarThumb,
-            PseudoElementScrollbarTrack,
-            PseudoElementScrollbarTrackPiece,
-            PseudoElementSelection,
-            PseudoElementSlotted,
-            PseudoElementUserAgentCustom,
-            PseudoElementWebKitCustom,
+        PseudoElementFirstLetter,
+        PseudoElementFirstLine,
+        PseudoElementMarker,
+        PseudoElementResizer,
+        PseudoElementScrollbar,
+        PseudoElementScrollbarButton,
+        PseudoElementScrollbarCorner,
+        PseudoElementScrollbarThumb,
+        PseudoElementScrollbarTrack,
+        PseudoElementScrollbarTrackPiece,
+        PseudoElementSelection,
+        PseudoElementSlotted,
+        PseudoElementUserAgentCustom,
+        PseudoElementWebKitCustom,
 
-            // WebKitCustom that appeared in an old prefixed form
-            // and need special handling.
-            PseudoElementWebKitCustomLegacyPrefixed,
-        };
+        // WebKitCustom that appeared in an old prefixed form
+        // and need special handling.
+        PseudoElementWebKitCustomLegacyPrefixed,
+    };
 
-        enum PagePseudoClassType {
-            PagePseudoClassFirst = 1,
-            PagePseudoClassLeft,
-            PagePseudoClassRight,
-        };
+    enum PagePseudoClassType {
+        PagePseudoClassFirst = 1,
+        PagePseudoClassLeft,
+        PagePseudoClassRight,
+    };
 
-        enum MarginBoxType {
-            TopLeftCornerMarginBox,
-            TopLeftMarginBox,
-            TopCenterMarginBox,
-            TopRightMarginBox,
-            TopRightCornerMarginBox,
-            BottomLeftCornerMarginBox,
-            BottomLeftMarginBox,
-            BottomCenterMarginBox,
-            BottomRightMarginBox,
-            BottomRightCornerMarginBox,
-            LeftTopMarginBox,
-            LeftMiddleMarginBox,
-            LeftBottomMarginBox,
-            RightTopMarginBox,
-            RightMiddleMarginBox,
-            RightBottomMarginBox,
-        };
+    enum MarginBoxType {
+        TopLeftCornerMarginBox,
+        TopLeftMarginBox,
+        TopCenterMarginBox,
+        TopRightMarginBox,
+        TopRightCornerMarginBox,
+        BottomLeftCornerMarginBox,
+        BottomLeftMarginBox,
+        BottomCenterMarginBox,
+        BottomRightMarginBox,
+        BottomRightCornerMarginBox,
+        LeftTopMarginBox,
+        LeftMiddleMarginBox,
+        LeftBottomMarginBox,
+        RightTopMarginBox,
+        RightMiddleMarginBox,
+        RightBottomMarginBox,
+    };
 
-        enum AttributeMatchType {
-            CaseSensitive,
-            CaseInsensitive,
-        };
+    enum AttributeMatchType {
+        CaseSensitive,
+        CaseInsensitive,
+    };
 
-        static PseudoElementType parsePseudoElementType(const String&);
-        static PseudoId pseudoId(PseudoElementType);
+    static PseudoElementType parsePseudoElementType(const String&);
+    static PseudoId pseudoId(PseudoElementType);
 
-        // Selectors are kept in an array by CSSSelectorList. The next component of the selector is
-        // the next item in the array.
-        const CSSSelector* tagHistory() const { return m_isLastInTagHistory ? 0 : const_cast<CSSSelector*>(this + 1); }
+    // Selectors are kept in an array by CSSSelectorList. The next component of the selector is
+    // the next item in the array.
+    const CSSSelector* tagHistory() const { return m_isLastInTagHistory ? 0 : const_cast<CSSSelector*>(this + 1); }
 
-        const QualifiedName& tagQName() const;
-        const AtomicString& tagLowercaseLocalName() const;
+    const QualifiedName& tagQName() const;
+    const AtomicString& tagLowercaseLocalName() const;
 
-        const AtomicString& value() const;
-        const AtomicString& serializingValue() const;
-        const QualifiedName& attribute() const;
-        const AtomicString& attributeCanonicalLocalName() const;
-        const AtomicString& argument() const { return m_hasRareData ? m_data.m_rareData->m_argument : nullAtom(); }
-        bool attributeValueMatchingIsCaseInsensitive() const;
-        const Vector<AtomicString>* langArgumentList() const { return m_hasRareData ? m_data.m_rareData->m_langArgumentList.get() : nullptr; }
-        const CSSSelectorList* selectorList() const { return m_hasRareData ? m_data.m_rareData->m_selectorList.get() : nullptr; }
+    const AtomicString& value() const;
+    const AtomicString& serializingValue() const;
+    const QualifiedName& attribute() const;
+    const AtomicString& attributeCanonicalLocalName() const;
+    const AtomicString& argument() const { return WTF::holds_alternative<RefPtr<RareData>>(m_data) ? WTF::get<RefPtr<RareData>>(m_data)->m_argument : nullAtom(); }
+    bool attributeValueMatchingIsCaseInsensitive() const;
+    const Vector<AtomicString>* langArgumentList() const { return WTF::holds_alternative<RefPtr<RareData>>(m_data) ? WTF::get<RefPtr<RareData>>(m_data)->m_langArgumentList.get() : nullptr; }
+    const CSSSelectorList* selectorList() const { return WTF::holds_alternative<RefPtr<RareData>>(m_data) ? WTF::get<RefPtr<RareData>>(m_data)->m_selectorList.get() : nullptr; }
 
-        void setValue(const AtomicString&, bool matchLowerCase = false);
-        
-        void setAttribute(const QualifiedName&, bool convertToLowercase, AttributeMatchType);
-        void setNth(int a, int b);
-        void setArgument(const AtomicString&);
-        void setLangArgumentList(std::unique_ptr<Vector<AtomicString>>);
-        void setSelectorList(std::unique_ptr<CSSSelectorList>);
+    void setValue(const AtomicString&, bool matchLowerCase = false);
+    
+    void setAttribute(const QualifiedName&, bool convertToLowercase, AttributeMatchType);
+    void setNth(int a, int b);
+    void setArgument(const AtomicString&);
+    void setLangArgumentList(std::unique_ptr<Vector<AtomicString>>);
+    void setSelectorList(std::unique_ptr<CSSSelectorList>);
 
-        bool matchNth(int count) const;
-        int nthA() const;
-        int nthB() const;
+    bool matchNth(int count) const;
+    int nthA() const;
+    int nthB() const;
 
-        bool hasDescendantRelation() const { return relation() == DescendantSpace; }
+    bool hasDescendantRelation() const { return relation() == DescendantSpace; }
 
-        bool hasDescendantOrChildRelation() const { return relation() == Child || hasDescendantRelation(); }
+    bool hasDescendantOrChildRelation() const { return relation() == Child || hasDescendantRelation(); }
 
-        PseudoClassType pseudoClassType() const
-        {
-            ASSERT(match() == PseudoClass);
-            return static_cast<PseudoClassType>(m_pseudoType);
-        }
-        void setPseudoClassType(PseudoClassType pseudoType)
-        {
-            m_pseudoType = pseudoType;
-            ASSERT(m_pseudoType == pseudoType);
-        }
+    PseudoClassType pseudoClassType() const
+    {
+        ASSERT(match() == PseudoClass);
+        return static_cast<PseudoClassType>(m_pseudoType);
+    }
+    void setPseudoClassType(PseudoClassType pseudoType)
+    {
+        m_pseudoType = pseudoType;
+        ASSERT(m_pseudoType == pseudoType);
+    }
 
-        PseudoElementType pseudoElementType() const
-        {
-            ASSERT(match() == PseudoElement);
-            return static_cast<PseudoElementType>(m_pseudoType);
-        }
-        void setPseudoElementType(PseudoElementType pseudoElementType)
-        {
-            m_pseudoType = pseudoElementType;
-            ASSERT(m_pseudoType == pseudoElementType);
-        }
+    PseudoElementType pseudoElementType() const
+    {
+        ASSERT(match() == PseudoElement);
+        return static_cast<PseudoElementType>(m_pseudoType);
+    }
+    void setPseudoElementType(PseudoElementType pseudoElementType)
+    {
+        m_pseudoType = pseudoElementType;
+        ASSERT(m_pseudoType == pseudoElementType);
+    }
 
-        PagePseudoClassType pagePseudoClassType() const
-        {
-            ASSERT(match() == PagePseudoClass);
-            return static_cast<PagePseudoClassType>(m_pseudoType);
-        }
-        void setPagePseudoType(PagePseudoClassType pagePseudoType)
-        {
-            m_pseudoType = pagePseudoType;
-            ASSERT(m_pseudoType == pagePseudoType);
-        }
+    PagePseudoClassType pagePseudoClassType() const
+    {
+        ASSERT(match() == PagePseudoClass);
+        return static_cast<PagePseudoClassType>(m_pseudoType);
+    }
+    void setPagePseudoType(PagePseudoClassType pagePseudoType)
+    {
+        m_pseudoType = pagePseudoType;
+        ASSERT(m_pseudoType == pagePseudoType);
+    }
 
-        bool matchesPseudoElement() const;
-        bool isUnknownPseudoElement() const;
-        bool isCustomPseudoElement() const;
-        bool isWebKitCustomPseudoElement() const;
-        bool isSiblingSelector() const;
-        bool isAttributeSelector() const;
+    bool matchesPseudoElement() const;
+    bool isUnknownPseudoElement() const;
+    bool isCustomPseudoElement() const;
+    bool isWebKitCustomPseudoElement() const;
+    bool isSiblingSelector() const;
+    bool isAttributeSelector() const;
 
-        RelationType relation() const { return static_cast<RelationType>(m_relation); }
-        void setRelation(RelationType relation)
-        {
-            m_relation = relation;
-            ASSERT(m_relation == relation);
-        }
+    RelationType relation() const { return static_cast<RelationType>(m_relation); }
+    void setRelation(RelationType relation)
+    {
+        m_relation = relation;
+        ASSERT(m_relation == relation);
+    }
 
-        Match match() const { return static_cast<Match>(m_match); }
-        void setMatch(Match match)
-        {
-            m_match = match;
-            ASSERT(m_match == match);
-        }
+    Match match() const { return static_cast<Match>(m_match); }
+    void setMatch(Match match)
+    {
+        m_match = match;
+        ASSERT(m_match == match);
+    }
 
-        bool isLastInSelectorList() const { return m_isLastInSelectorList; }
-        void setLastInSelectorList() { m_isLastInSelectorList = true; }
-        bool isLastInTagHistory() const { return m_isLastInTagHistory; }
-        void setNotLastInTagHistory() { m_isLastInTagHistory = false; }
+    bool isLastInSelectorList() const { return m_isLastInSelectorList; }
+    void setLastInSelectorList() { m_isLastInSelectorList = true; }
+    bool isLastInTagHistory() const { return m_isLastInTagHistory; }
+    void setNotLastInTagHistory() { m_isLastInTagHistory = false; }
 
-        bool isForPage() const { return m_isForPage; }
-        void setForPage() { m_isForPage = true; }
+#if !ASSERT_DISABLED
+    bool isForPage() const { return m_isForPage; }
+    void setForPage() { m_isForPage = true; }
+#endif
 
-    private:
-        unsigned m_relation              : 4; // enum RelationType.
-        mutable unsigned m_match         : 4; // enum Match.
-        mutable unsigned m_pseudoType    : 8; // PseudoType.
-        unsigned m_isLastInSelectorList  : 1;
-        unsigned m_isLastInTagHistory    : 1;
-        unsigned m_hasRareData           : 1;
-        unsigned m_hasNameWithCase       : 1;
-        unsigned m_isForPage             : 1;
-        unsigned m_tagIsForNamespaceRule : 1;
-        unsigned m_caseInsensitiveAttributeValueMatching : 1;
+private:
+    struct RareData;
+    struct NameWithCase;
+    Variant<AtomicString, QualifiedName, RefPtr<RareData>, RefPtr<NameWithCase>> m_data;
+
+    unsigned m_relation               : 3; // enum RelationType.
+    mutable unsigned m_match          : 4; // enum Match.
+    mutable unsigned m_pseudoType     : 7; // enum PseudoClassType.
+    unsigned m_isLastInSelectorList   : 1;
+    unsigned m_isLastInTagHistory     : 1;
+    unsigned m_tagIsForNamespaceRule  : 1;
+#if !ASSERT_DISABLED
+    unsigned m_isForPage              : 1;
+#endif
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
-        unsigned m_destructorHasBeenCalled : 1;
+    unsigned m_destructorHasBeenCalled : 1;
 #endif
 
-        unsigned simpleSelectorSpecificityForPage() const;
+    unsigned simpleSelectorSpecificityForPage() const;
 
-        // Hide.
-        CSSSelector& operator=(const CSSSelector&);
+    // Hide.
+    CSSSelector& operator=(const CSSSelector&);
 
-        struct RareData : public RefCounted<RareData> {
-            static Ref<RareData> create(AtomicString&& value) { return adoptRef(*new RareData(WTFMove(value))); }
-            ~RareData();
+    struct RareData : public RefCounted<RareData> {
+        RareData(AtomicString&& value);
+        ~RareData();
 
-            bool matchNth(int count);
+        bool matchNth(int count);
 
-            // For quirks mode, class and id are case-insensitive. In the case where uppercase
-            // letters are used in quirks mode, |m_matchingValue| holds the lowercase class/id
-            // and |m_serializingValue| holds the original string.
-            AtomicString m_matchingValue;
-            AtomicString m_serializingValue;
-            
-            int m_a; // Used for :nth-*
-            int m_b; // Used for :nth-*
-            QualifiedName m_attribute; // used for attribute selector
-            AtomicString m_attributeCanonicalLocalName;
-            AtomicString m_argument; // Used for :contains and :nth-*
-            std::unique_ptr<Vector<AtomicString>> m_langArgumentList; // Used for :lang arguments.
-            std::unique_ptr<CSSSelectorList> m_selectorList; // Used for :matches() and :not().
+        // For quirks mode, class and id are case-insensitive. In the case where uppercase
+        // letters are used in quirks mode, |m_matchingValue| holds the lowercase class/id
+        // and |m_serializingValue| holds the original string.
+        AtomicString m_matchingValue;
+        AtomicString m_serializingValue;
         
-        private:
-            RareData(AtomicString&& value);
-        };
-        void createRareData();
+        int m_a { 0 }; // Used for :nth-*
+        int m_b { 0 }; // Used for :nth-*
+        QualifiedName m_attribute { anyQName() }; // used for attribute selector
+        AtomicString m_attributeCanonicalLocalName;
+        AtomicString m_argument { nullAtom() }; // Used for :contains and :nth-*
+        std::unique_ptr<Vector<AtomicString>> m_langArgumentList; // Used for :lang arguments.
+        std::unique_ptr<CSSSelectorList> m_selectorList; // Used for :matches() and :not().
+        bool m_caseInsensitiveAttributeValueMatching { false };
+    };
+    void createRareData();
 
-        struct NameWithCase : public RefCounted<NameWithCase> {
-            NameWithCase(const QualifiedName& originalName, const AtomicString& lowercaseName)
-                : m_originalName(originalName)
-                , m_lowercaseLocalName(lowercaseName)
-            {
-                ASSERT(originalName.localName() != lowercaseName);
-            }
+    struct NameWithCase : public RefCounted<NameWithCase> {
+        NameWithCase(const QualifiedName& originalName, const AtomicString& lowercaseName)
+            : m_originalName(originalName)
+            , m_lowercaseLocalName(lowercaseName)
+        {
+            ASSERT(originalName.localName() != lowercaseName);
+        }
 
-            const QualifiedName m_originalName;
-            const AtomicString m_lowercaseLocalName;
-        };
-
-        union DataUnion {
-            DataUnion() : m_value(0) { }
-            AtomicStringImpl* m_value;
-            QualifiedName::QualifiedNameImpl* m_tagQName;
-            RareData* m_rareData;
-            NameWithCase* m_nameWithCase;
-        } m_data;
+        const QualifiedName m_originalName;
+        const AtomicString m_lowercaseLocalName;
     };
+};
+#pragma pack(pop)
 
 inline const QualifiedName& CSSSelector::attribute() const
 {
     ASSERT(isAttributeSelector());
-    ASSERT(m_hasRareData);
-    return m_data.m_rareData->m_attribute;
+    ASSERT(WTF::holds_alternative<RefPtr<RareData>>(m_data));
+    return WTF::get<RefPtr<RareData>>(m_data)->m_attribute;
 }
 
 inline const AtomicString& CSSSelector::attributeCanonicalLocalName() const
 {
     ASSERT(isAttributeSelector());
-    ASSERT(m_hasRareData);
-    return m_data.m_rareData->m_attributeCanonicalLocalName;
+    ASSERT(WTF::holds_alternative<RefPtr<RareData>>(m_data));
+    return WTF::get<RefPtr<RareData>>(m_data)->m_attributeCanonicalLocalName;
 }
 
 inline bool CSSSelector::matchesPseudoElement() const
@@ -458,20 +457,16 @@
 {
     ASSERT(match() != Tag);
     AtomicString matchingValue = matchLowerCase ? value.convertToASCIILowercase() : value;
-    if (!m_hasRareData && matchingValue != value)
+    if (!WTF::holds_alternative<RefPtr<RareData>>(m_data) && matchingValue != value)
         createRareData();
     
-    // Need to do ref counting manually for the union.
-    if (!m_hasRareData) {
-        if (m_data.m_value)
-            m_data.m_value->deref();
-        m_data.m_value = value.impl();
-        m_data.m_value->ref();
+    if (!WTF::holds_alternative<RefPtr<RareData>>(m_data)) {
+        m_data = value;
         return;
     }
 
-    m_data.m_rareData->m_matchingValue = WTFMove(matchingValue);
-    m_data.m_rareData->m_serializingValue = value;
+    WTF::get<RefPtr<RareData>>(m_data)->m_matchingValue = WTFMove(matchingValue);
+    WTF::get<RefPtr<RareData>>(m_data)->m_serializingValue = value;
 }
 
 inline CSSSelector::CSSSelector()
@@ -480,11 +475,10 @@
     , m_pseudoType(0)
     , m_isLastInSelectorList(false)
     , m_isLastInTagHistory(true)
-    , m_hasRareData(false)
-    , m_hasNameWithCase(false)
+    , m_tagIsForNamespaceRule(false)
+#if !ASSERT_DISABLED
     , m_isForPage(false)
-    , m_tagIsForNamespaceRule(false)
-    , m_caseInsensitiveAttributeValueMatching(false)
+#endif
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
     , m_destructorHasBeenCalled(false)
 #endif
@@ -492,33 +486,20 @@
 }
 
 inline CSSSelector::CSSSelector(const CSSSelector& o)
-    : m_relation(o.m_relation)
+    : m_data(o.m_data)
+    , m_relation(o.m_relation)
     , m_match(o.m_match)
     , m_pseudoType(o.m_pseudoType)
     , m_isLastInSelectorList(o.m_isLastInSelectorList)
     , m_isLastInTagHistory(o.m_isLastInTagHistory)
-    , m_hasRareData(o.m_hasRareData)
-    , m_hasNameWithCase(o.m_hasNameWithCase)
+    , m_tagIsForNamespaceRule(o.m_tagIsForNamespaceRule)
+#if !ASSERT_DISABLED
     , m_isForPage(o.m_isForPage)
-    , m_tagIsForNamespaceRule(o.m_tagIsForNamespaceRule)
-    , m_caseInsensitiveAttributeValueMatching(o.m_caseInsensitiveAttributeValueMatching)
+#endif
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
     , m_destructorHasBeenCalled(false)
 #endif
 {
-    if (o.m_hasRareData) {
-        m_data.m_rareData = o.m_data.m_rareData;
-        m_data.m_rareData->ref();
-    } else if (o.m_hasNameWithCase) {
-        m_data.m_nameWithCase = o.m_data.m_nameWithCase;
-        m_data.m_nameWithCase->ref();
-    } if (o.match() == Tag) {
-        m_data.m_tagQName = o.m_data.m_tagQName;
-        m_data.m_tagQName->ref();
-    } else if (o.m_data.m_value) {
-        m_data.m_value = o.m_data.m_value;
-        m_data.m_value->ref();
-    }
 }
 
 inline CSSSelector::~CSSSelector()
@@ -527,62 +508,44 @@
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
     m_destructorHasBeenCalled = true;
 #endif
-    if (m_hasRareData) {
-        m_data.m_rareData->deref();
-        m_data.m_rareData = nullptr;
-        m_hasRareData = false;
-    } else if (m_hasNameWithCase) {
-        m_data.m_nameWithCase->deref();
-        m_data.m_nameWithCase = nullptr;
-        m_hasNameWithCase = false;
-    } else if (match() == Tag) {
-        m_data.m_tagQName->deref();
-        m_data.m_tagQName = nullptr;
-        m_match = Unknown;
-    } else if (m_data.m_value) {
-        m_data.m_value->deref();
-        m_data.m_value = nullptr;
-    }
 }
 
 inline const QualifiedName& CSSSelector::tagQName() const
 {
     ASSERT(match() == Tag);
-    if (m_hasNameWithCase)
-        return m_data.m_nameWithCase->m_originalName;
-    return *reinterpret_cast<const QualifiedName*>(&m_data.m_tagQName);
+    if (WTF::holds_alternative<RefPtr<NameWithCase>>(m_data))
+        return WTF::get<RefPtr<NameWithCase>>(m_data)->m_originalName;
+    return WTF::get<QualifiedName>(m_data);
 }
 
 inline const AtomicString& CSSSelector::tagLowercaseLocalName() const
 {
-    if (m_hasNameWithCase)
-        return m_data.m_nameWithCase->m_lowercaseLocalName;
-    return m_data.m_tagQName->m_localName;
+    if (WTF::holds_alternative<RefPtr<NameWithCase>>(m_data))
+        return WTF::get<RefPtr<NameWithCase>>(m_data)->m_lowercaseLocalName;
+    return WTF::get<QualifiedName>(m_data).localName();
 }
 
 inline const AtomicString& CSSSelector::value() const
 {
     ASSERT(match() != Tag);
-    if (m_hasRareData)
-        return m_data.m_rareData->m_matchingValue;
+    if (WTF::holds_alternative<RefPtr<RareData>>(m_data))
+        return WTF::get<RefPtr<RareData>>(m_data)->m_matchingValue;
 
-    // AtomicString is really just an AtomicStringImpl* so the cast below is safe.
-    return *reinterpret_cast<const AtomicString*>(&m_data.m_value);
+    return WTF::get<AtomicString>(m_data);
 }
 
 inline const AtomicString& CSSSelector::serializingValue() const
 {
     ASSERT(match() != Tag);
-    if (m_hasRareData)
-        return m_data.m_rareData->m_serializingValue;
+    if (WTF::holds_alternative<RefPtr<RareData>>(m_data))
+        return WTF::get<RefPtr<RareData>>(m_data)->m_serializingValue;
     
-    // AtomicString is really just an AtomicStringImpl* so the cast below is safe.
-    return *reinterpret_cast<const AtomicString*>(&m_data.m_value);
+    return WTF::get<AtomicString>(m_data);
 }
     
 inline bool CSSSelector::attributeValueMatchingIsCaseInsensitive() const
 {
-    return m_caseInsensitiveAttributeValueMatching;
+    return WTF::holds_alternative<RefPtr<RareData>>(m_data) && WTF::get<RefPtr<RareData>>(m_data)->m_caseInsensitiveAttributeValueMatching;
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/css/parser/CSSParserImpl.cpp (234858 => 234859)


--- trunk/Source/WebCore/css/parser/CSSParserImpl.cpp	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WebCore/css/parser/CSSParserImpl.cpp	2018-08-14 18:24:02 UTC (rev 234859)
@@ -294,7 +294,9 @@
             selector->prependTagSelector(QualifiedName(nullAtom(), typeSelector, styleSheet->defaultNamespace()));
     }
 
+#if !ASSERT_DISABLED
     selector->setForPage();
+#endif
     return { Vector<std::unique_ptr<CSSParserSelector>>::from(WTFMove(selector)) };
 }
 

Modified: trunk/Source/WebCore/css/parser/CSSParserSelector.h (234858 => 234859)


--- trunk/Source/WebCore/css/parser/CSSParserSelector.h	2018-08-14 17:59:32 UTC (rev 234858)
+++ trunk/Source/WebCore/css/parser/CSSParserSelector.h	2018-08-14 18:24:02 UTC (rev 234859)
@@ -59,7 +59,9 @@
     void setNth(int a, int b) { m_selector->setNth(a, b); }
     void setMatch(CSSSelector::Match value) { m_selector->setMatch(value); }
     void setRelation(CSSSelector::RelationType value) { m_selector->setRelation(value); }
+#if !ASSERT_DISABLED
     void setForPage() { m_selector->setForPage(); }
+#endif
 
     CSSSelector::Match match() const { return m_selector->match(); }
     CSSSelector::PseudoElementType pseudoElementType() const { return m_selector->pseudoElementType(); }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to