Begin unraveling the mess that is QuotesData
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2013 23:07:14 +0000 (23:07 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2013 23:07:14 +0000 (23:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115765

Reviewed by Andreas Kling.

Change QuotesData to be an immutable object and fix other things that are broken.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::applyProperty):
QuotesData::addPair is gone. Instead, create the Vector up front and pass it to QuotesData.

* rendering/RenderQuote.cpp:
(WebCore::RenderQuote::originalText):
Update for renames.

* rendering/style/QuotesData.cpp:
(WebCore::QuotesData::create):
Remove the create overload that wasn't used. Add a new create overload that takes a Vector.

(WebCore::QuotesData::openQuote):
Rename this from getOpenQuote and clean it up.

(WebCore::QuotesData::closeQuote):
Rename this from getCloseQuote and clean it up.

(WebCore::operator==):
Replace the equals member function with a proper equality operator.

* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::diff):
Stop calling QuotesData::equals. Use the same idiom as used for other properties.

(WebCore::RenderStyle::setQuotes):
* rendering/style/StyleRareInheritedData.cpp:
Use operator==.

(WebCore::quotesDataEquivalent):
Add helper function.

(WebCore::StyleRareInheritedData::operator==):
Call quotesDataEquivalent.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@149700 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/rendering/RenderQuote.cpp
Source/WebCore/rendering/style/QuotesData.cpp
Source/WebCore/rendering/style/QuotesData.h
Source/WebCore/rendering/style/RenderStyle.cpp
Source/WebCore/rendering/style/StyleRareInheritedData.cpp

index b6b3bb1..c1f11f8 100644 (file)
@@ -1,3 +1,47 @@
+2013-05-07  Anders Carlsson  <andersca@apple.com>
+
+        Begin unraveling the mess that is QuotesData
+        https://bugs.webkit.org/show_bug.cgi?id=115765
+
+        Reviewed by Andreas Kling.
+
+        Change QuotesData to be an immutable object and fix other things that are broken.
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::applyProperty):
+        QuotesData::addPair is gone. Instead, create the Vector up front and pass it to QuotesData.
+
+        * rendering/RenderQuote.cpp:
+        (WebCore::RenderQuote::originalText):
+        Update for renames.
+
+        * rendering/style/QuotesData.cpp:
+        (WebCore::QuotesData::create):
+        Remove the create overload that wasn't used. Add a new create overload that takes a Vector.
+
+        (WebCore::QuotesData::openQuote):
+        Rename this from getOpenQuote and clean it up.
+
+        (WebCore::QuotesData::closeQuote):
+        Rename this from getCloseQuote and clean it up.
+
+        (WebCore::operator==):
+        Replace the equals member function with a proper equality operator.
+
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::diff):
+        Stop calling QuotesData::equals. Use the same idiom as used for other properties.
+
+        (WebCore::RenderStyle::setQuotes):
+        * rendering/style/StyleRareInheritedData.cpp:
+        Use operator==.
+
+        (WebCore::quotesDataEquivalent):
+        Add helper function.
+
+        (WebCore::StyleRareInheritedData::operator==):
+        Call quotesDataEquivalent.
+
 2013-05-06  Enrica Casucci  <enrica@apple.com>
 
         Support -webkit-system-font on OS X.
index 4ab70bf..708d332 100644 (file)
@@ -2356,7 +2356,7 @@ void StyleResolver::applyProperty(CSSPropertyID id, CSSValue* value)
         }
         if (value->isValueList()) {
             CSSValueList* list = static_cast<CSSValueList*>(value);
-            RefPtr<QuotesData> quotes = QuotesData::create();
+            Vector<std::pair<String, String> > quotes;
             for (size_t i = 0; i < list->length(); i += 2) {
                 CSSValue* first = list->itemWithoutBoundsCheck(i);
                 // item() returns null if out of bounds so this is safe.
@@ -2367,14 +2367,14 @@ void StyleResolver::applyProperty(CSSPropertyID id, CSSValue* value)
                 ASSERT_WITH_SECURITY_IMPLICATION(second->isPrimitiveValue());
                 String startQuote = static_cast<CSSPrimitiveValue*>(first)->getStringValue();
                 String endQuote = static_cast<CSSPrimitiveValue*>(second)->getStringValue();
-                quotes->addPair(std::make_pair(startQuote, endQuote));
+                quotes.append(std::make_pair(startQuote, endQuote));
             }
-            state.style()->setQuotes(quotes);
+            state.style()->setQuotes(QuotesData::create(quotes));
             return;
         }
         if (primitiveValue) {
             if (primitiveValue->getIdent() == CSSValueNone)
-                state.style()->setQuotes(QuotesData::create());
+                state.style()->setQuotes(QuotesData::create(Vector<std::pair<String, String> >()));
         }
         return;
     // Shorthand properties.
index 05290d0..3e45d6b 100644 (file)
@@ -240,9 +240,9 @@ PassRefPtr<StringImpl> RenderQuote::originalText() const
     case NO_CLOSE_QUOTE:
         return StringImpl::empty();
     case CLOSE_QUOTE:
-        return quotesData()->getCloseQuote(m_depth - 1).impl();
+        return quotesData()->closeQuote(m_depth - 1).impl();
     case OPEN_QUOTE:
-        return quotesData()->getOpenQuote(m_depth).impl();
+        return quotesData()->openQuote(m_depth).impl();
     }
     ASSERT_NOT_REACHED();
     return StringImpl::empty();
index 900f8ea..756da84 100644 (file)
 
 namespace WebCore {
 
-PassRefPtr<QuotesData> QuotesData::create(String open, String close)
+PassRefPtr<QuotesData> QuotesData::create(const String& open1, const String& close1, const String& open2, const String& close2)
 {
-    RefPtr<QuotesData> data = QuotesData::create();
-    data->addPair(std::make_pair(open, close));
-    return data;
-}
+    Vector<std::pair<String, String> > quotes;
+    quotes.reserveInitialCapacity(2);
+    quotes.uncheckedAppend(std::make_pair(open1, close1));
+    quotes.uncheckedAppend(std::make_pair(open2, close2));
 
-PassRefPtr<QuotesData> QuotesData::create(String open1, String close1, String open2, String close2)
-{
-    RefPtr<QuotesData> data = QuotesData::create();
-    data->addPair(std::make_pair(open1, close1));
-    data->addPair(std::make_pair(open2, close2));
-    return data;
+    return QuotesData::create(quotes);
 }
 
-void QuotesData::addPair(const std::pair<String, String>& quotePair)
+PassRefPtr<QuotesData> QuotesData::create(const Vector<std::pair<String, String> >& quotes)
 {
-    m_quotePairs.append(quotePair);
+    RefPtr<QuotesData> quotesData = adoptRef(new QuotesData);
+    quotesData->m_quotePairs = quotes;
+
+    return quotesData.release();
 }
 
-const String QuotesData::getOpenQuote(int index) const
+const String& QuotesData::openQuote(unsigned index) const
 {
-    ASSERT(index >= 0);
-    if (!m_quotePairs.size() || index < 0)
+    if (!m_quotePairs.isEmpty())
         return emptyString();
-    if ((size_t)index >= m_quotePairs.size())
+
+    if (index >= m_quotePairs.size())
         return m_quotePairs.last().first;
-    return m_quotePairs.at(index).first;
+
+    return m_quotePairs[index].first;
 }
 
-const String QuotesData::getCloseQuote(int index) const
+const String& QuotesData::closeQuote(unsigned index) const
 {
-    ASSERT(index >= -1);
-    if (!m_quotePairs.size() || index < 0)
+    if (m_quotePairs.isEmpty())
         return emptyString();
-    if ((size_t)index >= m_quotePairs.size())
+
+    if (index >= m_quotePairs.size())
         return m_quotePairs.last().second;
+
     return m_quotePairs.at(index).second;
 }
 
-bool QuotesData::equals(const QuotesData* a, const QuotesData* b)
+bool operator==(const QuotesData& a, const QuotesData& b)
 {
-    if (a == b)
-        return true;
-    if (!a || !b)
-        return false;
-    return a->m_quotePairs == b->m_quotePairs;
+    return a.m_quotePairs == b.m_quotePairs;
 }
 
 } // namespace WebCore
index 2fc8a69..82564ef 100644 (file)
@@ -31,16 +31,13 @@ namespace WebCore {
 
 class QuotesData : public RefCounted<QuotesData> {
 public:
-    static PassRefPtr<QuotesData> create() { return adoptRef(new QuotesData()); }
-    static PassRefPtr<QuotesData> create(const String open, const String close);
-    static PassRefPtr<QuotesData> create(const String open1, const String close1, const String open2, const String close2);
+    static PassRefPtr<QuotesData> create(const String& open1, const String& close1, const String& open2, const String& close2);
+    static PassRefPtr<QuotesData> create(const Vector<std::pair<String, String> >& quotes);
 
-    // FIXME: this should be an operator==.
-    static bool equals(const QuotesData*, const QuotesData*);
+    friend bool operator==(const QuotesData&, const QuotesData&);
 
-    void addPair(const std::pair<String, String>& quotePair);
-    const String getOpenQuote(int index) const;
-    const String getCloseQuote(int index) const;
+    const String& openQuote(unsigned index) const;
+    const String& closeQuote(unsigned index) const;
 
 private:
     QuotesData() { }
@@ -48,6 +45,11 @@ private:
     Vector<std::pair<String, String> > m_quotePairs;
 };
 
+inline bool operator!=(const QuotesData& a, const QuotesData& b)
+{
+    return !(a == b);
+}
+
 } // namespace WebCore
 
 #endif // QuotesData_h
index b775c3c..ddfff33 100644 (file)
@@ -613,7 +613,9 @@ StyleDifference RenderStyle::diff(const RenderStyle* other, unsigned& changedCon
         return StyleDifferenceLayout;
     }
 
-    if (!QuotesData::equals(rareInheritedData->quotes.get(), other->rareInheritedData->quotes.get()))
+    const QuotesData* quotesDataA = rareInheritedData->quotes.get();
+    const QuotesData* quotesDataB = other->rareInheritedData->quotes.get();
+    if (!(quotesDataA == quotesDataB || (quotesDataA && quotesDataB && *quotesDataA == *quotesDataB)))
         return StyleDifferenceLayout;
 
 #if ENABLE(SVG)
@@ -750,8 +752,9 @@ void RenderStyle::setCursorList(PassRefPtr<CursorList> other)
 
 void RenderStyle::setQuotes(PassRefPtr<QuotesData> q)
 {
-    if (QuotesData::equals(rareInheritedData->quotes.get(), q.get()))
+    if (rareInheritedData->quotes == q || (rareInheritedData->quotes && q && *rareInheritedData->quotes == *q))
         return;
+
     rareInheritedData.access()->quotes = q;
 }
 
index 7c03f3a..2e1d9d0 100644 (file)
@@ -217,6 +217,15 @@ static bool cursorDataEquivalent(const CursorList* c1, const CursorList* c2)
     return (*c1 == *c2);
 }
 
+static bool quotesDataEquivalent(const QuotesData* q1, const QuotesData* q2)
+{
+    if (q1 == q2)
+        return true;
+    if ((!q1 && q2) || (q1 && !q2))
+        return false;
+    return (*q1 == *q2);
+}
+
 bool StyleRareInheritedData::operator==(const StyleRareInheritedData& o) const
 {
     return textStrokeColor == o.textStrokeColor
@@ -267,7 +276,7 @@ bool StyleRareInheritedData::operator==(const StyleRareInheritedData& o) const
         && hyphenationString == o.hyphenationString
         && locale == o.locale
         && textEmphasisCustomMark == o.textEmphasisCustomMark
-        && QuotesData::equals(quotes.get(), o.quotes.get())
+        && quotesDataEquivalent(quotes.get(), o.quotes.get())
         && m_tabSize == o.m_tabSize
         && m_lineGrid == o.m_lineGrid
 #if ENABLE(CSS_IMAGE_ORIENTATION)