Move attributes-related methods from ElementData to AttributeCollection

Source/core/dom/ElementData.h

Index: Source/core/dom/ElementData.h
diff --git a/Source/core/dom/ElementData.h b/Source/core/dom/ElementData.h
index 99aa0123d074c0ec6182030440abc15403adbd73..857004b3def4ec16f8d50535b29999bf4c8de145 100644
--- a/Source/core/dom/ElementData.h
+++ b/Source/core/dom/ElementData.h
@@ -53,12 +53,28 @@ public:
         , m_size(size)
     { }
 
+    const Attribute& operator[](unsigned index) const { return at(index); }
+    const Attribute& at(unsigned index) const
+    {
+        RELEASE_ASSERT(index < m_size);
+        return m_array[index];
+    }
+
+    const Attribute* get(const QualifiedName&) const;
+    const Attribute* get(const AtomicString& name, bool shouldIgnoreCase) const;
+    size_t find(const QualifiedName&, bool shouldIgnoreCase = false) const;
+    size_t find(const AtomicString& name, bool shouldIgnoreCase) const;
+    size_t find(Attr*) const;

[esprehn, 2014/06/26 22:03:58]

We're going to have to be really careful here, originally it was called
findByName because get() does a linear search and get() methods are usually
assumed to be cheap while find() implies more work. We can see how this shakes
out though.

[Inactive, 2014/06/26 22:16:40]

To be fair, they were called getAttributeItem() / getAttributeItemIndex() before
I renamed them to find*ByName() a couple of week ago :)

However, I totally agree. I did not think about this but the name should
definitely reflect that the implementation is actually doing work (linear search
in this case). I'll propose a better naming.

+
     const_iterator begin() const { return m_array; }
     const_iterator end() const { return m_array + m_size; }
 
     unsigned size() const { return m_size; }
+    bool isEmpty() const { return !m_size; }
 
 private:
+    size_t findSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
+
     const Attribute* m_array;
     unsigned m_size;
 };
@@ -89,18 +105,8 @@ public:
 
     const StylePropertySet* presentationAttributeStyle() const;
 
-    // This is not a trivial getter and its return value should be cached for performance.
-    size_t attributeCount() const;
-    bool hasAttributes() const { return !!attributeCount(); }
-
     AttributeCollection attributes() const;
 
-    const Attribute& attributeAt(unsigned index) const;
-    const Attribute* findAttributeByName(const QualifiedName&) const;
-    size_t findAttributeIndexByName(const QualifiedName&, bool shouldIgnoreCase = false) const;
-    size_t findAttributeIndexByName(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
-    size_t findAttrNodeIndex(Attr*) const;
-
     bool hasID() const { return !m_idForStyleResolution.isNull(); }
     bool hasClass() const { return !m_classNames.isNull(); }
 
@@ -137,10 +143,6 @@ private:
     void destroy();
 #endif
 
-    const Attribute* attributeBase() const;
-    const Attribute* findAttributeByName(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
-    size_t findAttributeIndexByNameSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
-
     PassRefPtrWillBeRawPtr<UniqueElementData> makeUniqueCopy() const;
 };
 
@@ -173,6 +175,8 @@ public:
         return location;
     }
 
+    AttributeCollection attributes() const;
+
     Attribute m_attributeArray[0];
 };
 
@@ -195,6 +199,8 @@ public:
     void appendAttribute(const QualifiedName&, const AtomicString&);
     void removeAttributeAt(size_t index);
 
+    AttributeCollection attributes() const;
+
     Attribute& attributeAt(unsigned index);
     Attribute* findAttributeByName(const QualifiedName&);
 
@@ -221,13 +227,6 @@ inline void ElementData::deref()
 }
 #endif
 
-inline size_t ElementData::attributeCount() const
-{
-    if (isUnique())
-        return static_cast<const UniqueElementData*>(this)->m_attributeVector.size();
-    return m_arraySize;
-}
-
 inline const StylePropertySet* ElementData::presentationAttributeStyle() const
 {
     if (!m_isUnique)
@@ -235,27 +234,17 @@ inline const StylePropertySet* ElementData::presentationAttributeStyle() const
     return static_cast<const UniqueElementData*>(this)->m_presentationAttributeStyle.get();
 }
 
-inline const Attribute* ElementData::findAttributeByName(const AtomicString& name, bool shouldIgnoreAttributeCase) const
+inline const Attribute* AttributeCollection::get(const AtomicString& name, bool shouldIgnoreCase) const
 {
-    size_t index = findAttributeIndexByName(name, shouldIgnoreAttributeCase);
-    if (index != kNotFound)
-        return &attributeAt(index);
-    return 0;
-}
-
-inline const Attribute* ElementData::attributeBase() const
-{
-    if (m_isUnique)
-        return static_cast<const UniqueElementData*>(this)->m_attributeVector.begin();
-    return static_cast<const ShareableElementData*>(this)->m_attributeArray;
+    size_t index = find(name, shouldIgnoreCase);
+    return index != kNotFound ? &at(index) : 0;
 }
 
-inline size_t ElementData::findAttributeIndexByName(const QualifiedName& name, bool shouldIgnoreCase) const
+inline size_t AttributeCollection::find(const QualifiedName& name, bool shouldIgnoreCase) const
 {
-    AttributeCollection attributes = this->attributes();
-    AttributeCollection::const_iterator end = attributes.end();
+    const_iterator end = this->end();
     unsigned index = 0;
-    for (AttributeCollection::const_iterator it = attributes.begin(); it != end; ++it, ++index) {
+    for (const_iterator it = begin(); it != end; ++it, ++index) {
         if (it->name().matchesPossiblyIgnoringCase(name, shouldIgnoreCase))
             return index;
     }
@@ -264,15 +253,14 @@ inline size_t ElementData::findAttributeIndexByName(const QualifiedName& name, b
 
 // We use a boolean parameter instead of calling shouldIgnoreAttributeCase so that the caller
 // can tune the behavior (hasAttribute is case sensitive whereas getAttribute is not).
-inline size_t ElementData::findAttributeIndexByName(const AtomicString& name, bool shouldIgnoreAttributeCase) const
+inline size_t AttributeCollection::find(const AtomicString& name, bool shouldIgnoreCase) const
 {
-    bool doSlowCheck = shouldIgnoreAttributeCase;
+    bool doSlowCheck = shouldIgnoreCase;
 
     // Optimize for the case where the attribute exists and its name exactly matches.
-    AttributeCollection attributes = this->attributes();
-    AttributeCollection::const_iterator end = attributes.end();
+    const_iterator end = this->end();
     unsigned index = 0;
-    for (AttributeCollection::const_iterator it = attributes.begin(); it != end; ++it, ++index) {
+    for (const_iterator it = begin(); it != end; ++it, ++index) {
         // FIXME: Why check the prefix? Namespaces should be all that matter.
         // Most attributes (all of HTML and CSS) have no namespace.
         if (!it->name().hasPrefix()) {
@@ -284,37 +272,37 @@ inline size_t ElementData::findAttributeIndexByName(const AtomicString& name, bo
     }
 
     if (doSlowCheck)
-        return findAttributeIndexByNameSlowCase(name, shouldIgnoreAttributeCase);
+        return findSlowCase(name, shouldIgnoreCase);
     return kNotFound;
 }
 
 inline AttributeCollection ElementData::attributes() const
 {
-    if (isUnique()) {
-        const Vector<Attribute, 4>& attributeVector = static_cast<const UniqueElementData*>(this)->m_attributeVector;
-        return AttributeCollection(attributeVector.data(), attributeVector.size());
-    }
-    return AttributeCollection(static_cast<const ShareableElementData*>(this)->m_attributeArray, m_arraySize);
+    if (isUnique())
+        return static_cast<const UniqueElementData*>(this)->attributes();
+    return static_cast<const ShareableElementData*>(this)->attributes();
 }
 
-inline const Attribute* ElementData::findAttributeByName(const QualifiedName& name) const
+inline AttributeCollection ShareableElementData::attributes() const
 {
-    AttributeCollection attributes = this->attributes();
-    AttributeCollection::const_iterator end = attributes.end();
-    for (AttributeCollection::const_iterator it = attributes.begin(); it != end; ++it) {
+    return AttributeCollection(m_attributeArray, m_arraySize);
+}
+
+inline AttributeCollection UniqueElementData::attributes() const
+{
+    return AttributeCollection(m_attributeVector.data(), m_attributeVector.size());
+}
+
+inline const Attribute* AttributeCollection::get(const QualifiedName& name) const
+{
+    const_iterator end = this->end();
+    for (const_iterator it = begin(); it != end; ++it) {
         if (it->name().matches(name))
             return it;
     }
     return 0;
 }
 
-inline const Attribute& ElementData::attributeAt(unsigned index) const
-{
-    RELEASE_ASSERT(index < attributeCount());
-    ASSERT(attributeBase() + index);
-    return *(attributeBase() + index);
-}
-
 inline void UniqueElementData::appendAttribute(const QualifiedName& attributeName, const AtomicString& value)
 {
     m_attributeVector.append(Attribute(attributeName, value));