Changeset 236284 in webkit


Ignore:
Timestamp:
Sep 20, 2018 2:33:31 PM (6 years ago)
Author:
achristensen@apple.com
Message:

Unreviewed, rolling out r235976.

Broke ARM

Reverted changeset:

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

Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r236101 r236284  
     12018-09-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Unreviewed, rolling out r235976.
     4
     5        Broke ARM
     6
     7        Reverted changeset:
     8
     9        "Use a Variant instead of a union in CSSSelector"
     10        https://bugs.webkit.org/show_bug.cgi?id=188559
     11        https://trac.webkit.org/changeset/235976
     12
    1132018-09-17  Yusuke Suzuki  <utatane.tea@gmail.com>
    214

  • trunk/Source/WTF/wtf/Variant.h

    r235976 r236284  
    14361436{};
    14371437
    1438 #pragma pack(push, 1)
    14391438template<typename ... _Types>
    14401439class Variant:
    … …  
    17231722    }
    17241723};
    1725 #pragma pack(pop)
    17261724
    17271725template<>

  • trunk/Source/WebCore/ChangeLog

    r236278 r236284  
     12018-09-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Unreviewed, rolling out r235976.
     4
     5        Broke ARM
     6
     7        Reverted changeset:
     8
     9        "Use a Variant instead of a union in CSSSelector"
     10        https://bugs.webkit.org/show_bug.cgi?id=188559
     11        https://trac.webkit.org/changeset/235976
     12
    1132018-09-20  Oriol Brufau  <obrufau@igalia.com>
    214

  • trunk/Source/WebCore/css/CSSSelector.cpp

    r235976 r236284  
    4141using namespace HTMLNames;
    4242
     43struct SameSizeAsCSSSelector {
     44    unsigned flags;
     45    void* unionPointer;
     46};
     47
    4348static_assert(CSSSelector::RelationType::Subselector == 0, "Subselector must be 0 for consumeCombinator.");
    44 static_assert(sizeof(CSSSelector) == sizeof(void*) + sizeof(unsigned), "CSSSelector should remain small.");
     49static_assert(sizeof(CSSSelector) == sizeof(SameSizeAsCSSSelector), "CSSSelector should remain small.");
    4550
    4651CSSSelector::CSSSelector(const QualifiedName& tagQName, bool tagIsForNamespaceRule)
    4752    : m_relation(DescendantSpace)
    4853    , m_match(Tag)
     54    , m_pseudoType(0)
    4955    , m_isLastInSelectorList(false)
    5056    , m_isLastInTagHistory(true)
    51     , m_pseudoType(0)
     57    , m_hasRareData(false)
     58    , m_hasNameWithCase(false)
     59    , m_isForPage(false)
    5260    , m_tagIsForNamespaceRule(tagIsForNamespaceRule)
    53 #if !ASSERT_DISABLED
    54     , m_isForPage(false)
    55 #endif
     61    , m_caseInsensitiveAttributeValueMatching(false)
    5662#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
    5763    , m_destructorHasBeenCalled(false)
    … …  
    6167    const AtomicString tagLocalNameASCIILowercase = tagLocalName.convertToASCIILowercase();
    6268
    63     if (tagLocalName == tagLocalNameASCIILowercase)
    64         m_data = tagQName;
    65     else
    66         m_data = adoptRef(new NameWithCase(tagQName, tagLocalNameASCIILowercase));
     69    if (tagLocalName == tagLocalNameASCIILowercase) {
     70        m_data.m_tagQName = tagQName.impl();
     71        m_data.m_tagQName->ref();
     72    } else {
     73        m_data.m_nameWithCase = adoptRef(new NameWithCase(tagQName, tagLocalNameASCIILowercase)).leakRef();
     74        m_hasNameWithCase = true;
     75    }
    6776}
    6877
    … …  
    7079{
    7180    ASSERT(match() != Tag);
    72     ASSERT(!WTF::holds_alternative<RefPtr<NameWithCase>>(m_data));
    73     if (WTF::holds_alternative<RefPtr<RareData>>(m_data))
     81    ASSERT(!m_hasNameWithCase);
     82    if (m_hasRareData)
    7483        return;
    7584    // Move the value to the rare data stucture.
    76     AtomicString value = WTF::get<AtomicString>(m_data);
    77     m_data = adoptRef(new RareData(WTFMove(value)));
     85    AtomicString value { adoptRef(m_data.m_value) };
     86    m_data.m_rareData = &RareData::create(WTFMove(value)).leakRef();
     87    m_hasRareData = true;
    7888}
    7989
    … …  
    740750{
    741751    createRareData();
    742     WTF::get<RefPtr<RareData>>(m_data)->m_attribute = value;
    743     WTF::get<RefPtr<RareData>>(m_data)->m_attributeCanonicalLocalName = convertToLowercase ? value.localName().convertToASCIILowercase() : value.localName();
    744     WTF::get<RefPtr<RareData>>(m_data)->m_caseInsensitiveAttributeValueMatching = matchType == CaseInsensitive;
     752    m_data.m_rareData->m_attribute = value;
     753    m_data.m_rareData->m_attributeCanonicalLocalName = convertToLowercase ? value.localName().convertToASCIILowercase() : value.localName();
     754    m_caseInsensitiveAttributeValueMatching = matchType == CaseInsensitive;
    745755}
    746756   
    … …  
    748758{
    749759    createRareData();
    750     WTF::get<RefPtr<RareData>>(m_data)->m_argument = value;
     760    m_data.m_rareData->m_argument = value;
    751761}
    752762
    … …  
    754764{
    755765    createRareData();
    756     WTF::get<RefPtr<RareData>>(m_data)->m_langArgumentList = WTFMove(argumentList);
     766    m_data.m_rareData->m_langArgumentList = WTFMove(argumentList);
    757767}
    758768
    … …  
    760770{
    761771    createRareData();
    762     WTF::get<RefPtr<RareData>>(m_data)->m_selectorList = WTFMove(selectorList);
     772    m_data.m_rareData->m_selectorList = WTFMove(selectorList);
    763773}
    764774
    … …  
    766776{
    767777    createRareData();
    768     WTF::get<RefPtr<RareData>>(m_data)->m_a = a;
    769     WTF::get<RefPtr<RareData>>(m_data)->m_b = b;
     778    m_data.m_rareData->m_a = a;
     779    m_data.m_rareData->m_b = b;
    770780}
    771781
    772782bool CSSSelector::matchNth(int count) const
    773783{
    774     return WTF::get<RefPtr<RareData>>(m_data)->matchNth(count);
     784    ASSERT(m_hasRareData);
     785    return m_data.m_rareData->matchNth(count);
    775786}
    776787
    777788int CSSSelector::nthA() const
    778789{
    779     return WTF::get<RefPtr<RareData>>(m_data)->m_a;
     790    ASSERT(m_hasRareData);
     791    return m_data.m_rareData->m_a;
    780792}
    781793
    782794int CSSSelector::nthB() const
    783795{
    784     return WTF::get<RefPtr<RareData>>(m_data)->m_b;
     796    ASSERT(m_hasRareData);
     797    return m_data.m_rareData->m_b;
    785798}
    786799
    … …  
    788801    : m_matchingValue(value)
    789802    , m_serializingValue(value)
     803    , m_a(0)
     804    , m_b(0)
     805    , m_attribute(anyQName())
     806    , m_argument(nullAtom())
    790807{
    791808}

  • trunk/Source/WebCore/css/CSSSelector.h

    r235976 r236284  
    2424#include "QualifiedName.h"
    2525#include "RenderStyleConstants.h"
    26 #include <wtf/Variant.h>
    2726
    2827namespace WebCore {
    … …  
    236235        const QualifiedName& attribute() const;
    237236        const AtomicString& attributeCanonicalLocalName() const;
    238         const AtomicString& argument() const { return WTF::holds_alternative<RefPtr<RareData>>(m_data) ? WTF::get<RefPtr<RareData>>(m_data)->m_argument : nullAtom(); }
     237        const AtomicString& argument() const { return m_hasRareData ? m_data.m_rareData->m_argument : nullAtom(); }
    239238        bool attributeValueMatchingIsCaseInsensitive() const;
    240         const Vector<AtomicString>* langArgumentList() const { return WTF::holds_alternative<RefPtr<RareData>>(m_data) ? WTF::get<RefPtr<RareData>>(m_data)->m_langArgumentList.get() : nullptr; }
    241         const CSSSelectorList* selectorList() const { return WTF::holds_alternative<RefPtr<RareData>>(m_data) ? WTF::get<RefPtr<RareData>>(m_data)->m_selectorList.get() : nullptr; }
     239        const Vector<AtomicString>* langArgumentList() const { return m_hasRareData ? m_data.m_rareData->m_langArgumentList.get() : nullptr; }
     240        const CSSSelectorList* selectorList() const { return m_hasRareData ? m_data.m_rareData->m_selectorList.get() : nullptr; }
    242241
    243242        void setValue(const AtomicString&, bool matchLowerCase = false);
    … …  
    316315        void setNotLastInTagHistory() { m_isLastInTagHistory = false; }
    317316
    318 #if !ASSERT_DISABLED
    319317        bool isForPage() const { return m_isForPage; }
    320318        void setForPage() { m_isForPage = true; }
    321 #endif
    322319
    323320    private:
    324         struct RareData;
    325         struct NameWithCase;
    326         Variant<AtomicString, QualifiedName, RefPtr<RareData>, RefPtr<NameWithCase>> m_data;
    327 
    328         unsigned char m_relation               : 3; // enum RelationType.
    329         mutable unsigned char m_match          : 4; // enum Match.
    330         unsigned char m_isLastInSelectorList   : 1;
    331         unsigned char m_isLastInTagHistory     : 1;
    332         mutable unsigned char m_pseudoType     : 7; // enum PseudoClassType.
    333         unsigned char m_tagIsForNamespaceRule  : 1;
    334 #if !ASSERT_DISABLED
    335         unsigned char m_isForPage              : 1;
    336 #endif
     321        unsigned m_relation              : 4; // enum RelationType.
     322        mutable unsigned m_match         : 4; // enum Match.
     323        mutable unsigned m_pseudoType    : 8; // PseudoType.
     324        unsigned m_isLastInSelectorList  : 1;
     325        unsigned m_isLastInTagHistory    : 1;
     326        unsigned m_hasRareData           : 1;
     327        unsigned m_hasNameWithCase       : 1;
     328        unsigned m_isForPage             : 1;
     329        unsigned m_tagIsForNamespaceRule : 1;
     330        unsigned m_caseInsensitiveAttributeValueMatching : 1;
    337331#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
    338         unsigned char m_destructorHasBeenCalled : 1;
     332        unsigned m_destructorHasBeenCalled : 1;
    339333#endif
    340334
    … …  
    345339
    346340        struct RareData : public RefCounted<RareData> {
    347             RareData(AtomicString&& value);
     341            static Ref<RareData> create(AtomicString&& value) { return adoptRef(*new RareData(WTFMove(value))); }
    348342            ~RareData();
    349343
    … …  
    356350            AtomicString m_serializingValue;
    357351           
    358             int m_a { 0 }; // Used for :nth-*
    359             int m_b { 0 }; // Used for :nth-*
    360             QualifiedName m_attribute { anyQName() }; // used for attribute selector
     352            int m_a; // Used for :nth-*
     353            int m_b; // Used for :nth-*
     354            QualifiedName m_attribute; // used for attribute selector
    361355            AtomicString m_attributeCanonicalLocalName;
    362             AtomicString m_argument { nullAtom() }; // Used for :contains and :nth-*
     356            AtomicString m_argument; // Used for :contains and :nth-*
    363357            std::unique_ptr<Vector<AtomicString>> m_langArgumentList; // Used for :lang arguments.
    364358            std::unique_ptr<CSSSelectorList> m_selectorList; // Used for :matches() and :not().
    365             bool m_caseInsensitiveAttributeValueMatching { false };
     359       
     360        private:
     361            RareData(AtomicString&& value);
    366362        };
    367363        void createRareData();
    … …  
    378374            const AtomicString m_lowercaseLocalName;
    379375        };
     376
     377        union DataUnion {
     378            DataUnion() : m_value(0) { }
     379            AtomicStringImpl* m_value;
     380            QualifiedName::QualifiedNameImpl* m_tagQName;
     381            RareData* m_rareData;
     382            NameWithCase* m_nameWithCase;
     383        } m_data;
    380384    };
    381385
    … …  
    383387{
    384388    ASSERT(isAttributeSelector());
    385     ASSERT(WTF::holds_alternative<RefPtr<RareData>>(m_data));
    386     return WTF::get<RefPtr<RareData>>(m_data)->m_attribute;
     389    ASSERT(m_hasRareData);
     390    return m_data.m_rareData->m_attribute;
    387391}
    388392
    … …  
    390394{
    391395    ASSERT(isAttributeSelector());
    392     ASSERT(WTF::holds_alternative<RefPtr<RareData>>(m_data));
    393     return WTF::get<RefPtr<RareData>>(m_data)->m_attributeCanonicalLocalName;
     396    ASSERT(m_hasRareData);
     397    return m_data.m_rareData->m_attributeCanonicalLocalName;
    394398}
    395399
    … …  
    453457    ASSERT(match() != Tag);
    454458    AtomicString matchingValue = matchLowerCase ? value.convertToASCIILowercase() : value;
    455     if (!WTF::holds_alternative<RefPtr<RareData>>(m_data) && matchingValue != value)
     459    if (!m_hasRareData && matchingValue != value)
    456460        createRareData();
    457461   
    458     if (!WTF::holds_alternative<RefPtr<RareData>>(m_data)) {
    459         m_data = value;
     462    // Need to do ref counting manually for the union.
     463    if (!m_hasRareData) {
     464        if (m_data.m_value)
     465            m_data.m_value->deref();
     466        m_data.m_value = value.impl();
     467        m_data.m_value->ref();
    460468        return;
    461469    }
    462470
    463     WTF::get<RefPtr<RareData>>(m_data)->m_matchingValue = WTFMove(matchingValue);
    464     WTF::get<RefPtr<RareData>>(m_data)->m_serializingValue = value;
     471    m_data.m_rareData->m_matchingValue = WTFMove(matchingValue);
     472    m_data.m_rareData->m_serializingValue = value;
    465473}
    466474
    … …  
    468476    : m_relation(DescendantSpace)
    469477    , m_match(Unknown)
     478    , m_pseudoType(0)
    470479    , m_isLastInSelectorList(false)
    471480    , m_isLastInTagHistory(true)
    472     , m_pseudoType(0)
     481    , m_hasRareData(false)
     482    , m_hasNameWithCase(false)
     483    , m_isForPage(false)
    473484    , m_tagIsForNamespaceRule(false)
    474 #if !ASSERT_DISABLED
    475     , m_isForPage(false)
    476 #endif
     485    , m_caseInsensitiveAttributeValueMatching(false)
    477486#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
    478487    , m_destructorHasBeenCalled(false)
    … …  
    482491
    483492inline CSSSelector::CSSSelector(const CSSSelector& o)
    484     : m_data(o.m_data)
    485     , m_relation(o.m_relation)
     493    : m_relation(o.m_relation)
    486494    , m_match(o.m_match)
     495    , m_pseudoType(o.m_pseudoType)
    487496    , m_isLastInSelectorList(o.m_isLastInSelectorList)
    488497    , m_isLastInTagHistory(o.m_isLastInTagHistory)
    489     , m_pseudoType(o.m_pseudoType)
     498    , m_hasRareData(o.m_hasRareData)
     499    , m_hasNameWithCase(o.m_hasNameWithCase)
     500    , m_isForPage(o.m_isForPage)
    490501    , m_tagIsForNamespaceRule(o.m_tagIsForNamespaceRule)
    491 #if !ASSERT_DISABLED
    492     , m_isForPage(o.m_isForPage)
    493 #endif
     502    , m_caseInsensitiveAttributeValueMatching(o.m_caseInsensitiveAttributeValueMatching)
    494503#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
    495504    , m_destructorHasBeenCalled(false)
    496505#endif
    497506{
     507    if (o.m_hasRareData) {
     508        m_data.m_rareData = o.m_data.m_rareData;
     509        m_data.m_rareData->ref();
     510    } else if (o.m_hasNameWithCase) {
     511        m_data.m_nameWithCase = o.m_data.m_nameWithCase;
     512        m_data.m_nameWithCase->ref();
     513    } if (o.match() == Tag) {
     514        m_data.m_tagQName = o.m_data.m_tagQName;
     515        m_data.m_tagQName->ref();
     516    } else if (o.m_data.m_value) {
     517        m_data.m_value = o.m_data.m_value;
     518        m_data.m_value->ref();
     519    }
    498520}
    499521
    … …  
    504526    m_destructorHasBeenCalled = true;
    505527#endif
     528    if (m_hasRareData) {
     529        m_data.m_rareData->deref();
     530        m_data.m_rareData = nullptr;
     531        m_hasRareData = false;
     532    } else if (m_hasNameWithCase) {
     533        m_data.m_nameWithCase->deref();
     534        m_data.m_nameWithCase = nullptr;
     535        m_hasNameWithCase = false;
     536    } else if (match() == Tag) {
     537        m_data.m_tagQName->deref();
     538        m_data.m_tagQName = nullptr;
     539        m_match = Unknown;
     540    } else if (m_data.m_value) {
     541        m_data.m_value->deref();
     542        m_data.m_value = nullptr;
     543    }
    506544}
    507545
    … …  
    509547{
    510548    ASSERT(match() == Tag);
    511     if (WTF::holds_alternative<RefPtr<NameWithCase>>(m_data))
    512         return WTF::get<RefPtr<NameWithCase>>(m_data)->m_originalName;
    513     return WTF::get<QualifiedName>(m_data);
     549    if (m_hasNameWithCase)
     550        return m_data.m_nameWithCase->m_originalName;
     551    return *reinterpret_cast<const QualifiedName*>(&m_data.m_tagQName);
    514552}
    515553
    516554inline const AtomicString& CSSSelector::tagLowercaseLocalName() const
    517555{
    518     if (WTF::holds_alternative<RefPtr<NameWithCase>>(m_data))
    519         return WTF::get<RefPtr<NameWithCase>>(m_data)->m_lowercaseLocalName;
    520     return WTF::get<QualifiedName>(m_data).localName();
     556    if (m_hasNameWithCase)
     557        return m_data.m_nameWithCase->m_lowercaseLocalName;
     558    return m_data.m_tagQName->m_localName;
    521559}
    522560
    … …  
    524562{
    525563    ASSERT(match() != Tag);
    526     if (WTF::holds_alternative<RefPtr<RareData>>(m_data))
    527         return WTF::get<RefPtr<RareData>>(m_data)->m_matchingValue;
    528 
    529     return WTF::get<AtomicString>(m_data);
     564    if (m_hasRareData)
     565        return m_data.m_rareData->m_matchingValue;
     566
     567    // AtomicString is really just an AtomicStringImpl* so the cast below is safe.
     568    return *reinterpret_cast<const AtomicString*>(&m_data.m_value);
    530569}
    531570
    … …  
    533572{
    534573    ASSERT(match() != Tag);
    535     if (WTF::holds_alternative<RefPtr<RareData>>(m_data))
    536         return WTF::get<RefPtr<RareData>>(m_data)->m_serializingValue;
     574    if (m_hasRareData)
     575        return m_data.m_rareData->m_serializingValue;
    537576   
    538     return WTF::get<AtomicString>(m_data);
     577    // AtomicString is really just an AtomicStringImpl* so the cast below is safe.
     578    return *reinterpret_cast<const AtomicString*>(&m_data.m_value);
    539579}
    540580   
    541581inline bool CSSSelector::attributeValueMatchingIsCaseInsensitive() const
    542582{
    543     return WTF::holds_alternative<RefPtr<RareData>>(m_data) && WTF::get<RefPtr<RareData>>(m_data)->m_caseInsensitiveAttributeValueMatching;
     583    return m_caseInsensitiveAttributeValueMatching;
    544584}
    545585

  • trunk/Source/WebCore/css/parser/CSSParserImpl.cpp

    r235976 r236284  
    295295    }
    296296
    297 #if !ASSERT_DISABLED
    298297    selector->setForPage();
    299 #endif
    300298    return CSSSelectorList { Vector<std::unique_ptr<CSSParserSelector>>::from(WTFMove(selector)) };
    301299}

  • trunk/Source/WebCore/css/parser/CSSParserSelector.h

    r235976 r236284  
    6060    void setMatch(CSSSelector::Match value) { m_selector->setMatch(value); }
    6161    void setRelation(CSSSelector::RelationType value) { m_selector->setRelation(value); }
    62 #if !ASSERT_DISABLED
    6362    void setForPage() { m_selector->setForPage(); }
    64 #endif
    6563
    6664    CSSSelector::Match match() const { return m_selector->match(); }
Note: See TracChangeset for help on using the changeset viewer.