HTMLPropertiesCollection should share more code with HTMLCollection
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2012 22:03:01 +0000 (22:03 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2012 22:03:01 +0000 (22:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90842

Reviewed by Anders Carlsson.

Got rid of HTMLPropertiesCollection::m_cache, and added m_itemRefElements, m_propertyNames, m_propertyCache,
m_hasPropertyNameCache, and m_hasItemRefElements to HTMLPropertiesCollection itself. These are caches specific
to HTMLPropertiesCollection. Note that hasNameCache has been renamed to m_hasPropertyNameCache and itemRefElementPosition
has been replaced by cachedElementsArrayOffset() in HTMLCollectionCacheBase (also used in HTMLFormCollection).

Also deleted all methods on m_cache except updatePropertyCache since caches can be accessed directly from
HTMLPropertiesCollection.

* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::invalidateCacheIfNeeded):
(WebCore::HTMLCollection::invalidateCache):
* html/HTMLCollection.h:
(HTMLCollection):
* html/HTMLPropertiesCollection.cpp:
(WebCore::HTMLPropertiesCollection::HTMLPropertiesCollection):
(WebCore):
(WebCore::HTMLPropertiesCollection::updateRefElements):
(WebCore::HTMLPropertiesCollection::itemAfter):
(WebCore::HTMLPropertiesCollection::calcLength):
(WebCore::HTMLPropertiesCollection::cacheFirstItem):
(WebCore::HTMLPropertiesCollection::item):
(WebCore::HTMLPropertiesCollection::findProperties):
(WebCore::HTMLPropertiesCollection::updateNameCache):
(WebCore::HTMLPropertiesCollection::names):
(WebCore::HTMLPropertiesCollection::namedItem):
(WebCore::HTMLPropertiesCollection::hasNamedItem):
* html/HTMLPropertiesCollection.h:
(HTMLPropertiesCollection):
(WebCore::HTMLPropertiesCollection::clearCache):
(WebCore::HTMLPropertiesCollection::updatePropertyCache):

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLCollection.cpp
Source/WebCore/html/HTMLCollection.h
Source/WebCore/html/HTMLPropertiesCollection.cpp
Source/WebCore/html/HTMLPropertiesCollection.h

index 81f7b47..b912ceb 100644 (file)
@@ -1,3 +1,41 @@
+2012-07-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        HTMLPropertiesCollection should share more code with HTMLCollection
+        https://bugs.webkit.org/show_bug.cgi?id=90842
+
+        Reviewed by Anders Carlsson.
+
+        Got rid of HTMLPropertiesCollection::m_cache, and added m_itemRefElements, m_propertyNames, m_propertyCache,
+        m_hasPropertyNameCache, and m_hasItemRefElements to HTMLPropertiesCollection itself. These are caches specific
+        to HTMLPropertiesCollection. Note that hasNameCache has been renamed to m_hasPropertyNameCache and itemRefElementPosition
+        has been replaced by cachedElementsArrayOffset() in HTMLCollectionCacheBase (also used in HTMLFormCollection).
+
+        Also deleted all methods on m_cache except updatePropertyCache since caches can be accessed directly from
+        HTMLPropertiesCollection.
+
+        * html/HTMLCollection.cpp:
+        (WebCore::HTMLCollection::invalidateCacheIfNeeded):
+        (WebCore::HTMLCollection::invalidateCache):
+        * html/HTMLCollection.h:
+        (HTMLCollection):
+        * html/HTMLPropertiesCollection.cpp:
+        (WebCore::HTMLPropertiesCollection::HTMLPropertiesCollection):
+        (WebCore):
+        (WebCore::HTMLPropertiesCollection::updateRefElements):
+        (WebCore::HTMLPropertiesCollection::itemAfter):
+        (WebCore::HTMLPropertiesCollection::calcLength):
+        (WebCore::HTMLPropertiesCollection::cacheFirstItem):
+        (WebCore::HTMLPropertiesCollection::item):
+        (WebCore::HTMLPropertiesCollection::findProperties):
+        (WebCore::HTMLPropertiesCollection::updateNameCache):
+        (WebCore::HTMLPropertiesCollection::names):
+        (WebCore::HTMLPropertiesCollection::namedItem):
+        (WebCore::HTMLPropertiesCollection::hasNamedItem):
+        * html/HTMLPropertiesCollection.h:
+        (HTMLPropertiesCollection):
+        (WebCore::HTMLPropertiesCollection::clearCache):
+        (WebCore::HTMLPropertiesCollection::updatePropertyCache):
+
 2012-07-10  Ojan Vafai  <ojan@chromium.org>
 
         Add support for min-height:auto and min-width:auto
index bf7aaef..1f1c9c5 100644 (file)
 #include "HTMLOptionElement.h"
 #include "NodeList.h"
 
+#if ENABLE(MICRODATA)
+#include "HTMLPropertiesCollection.h"
+#endif
+
 #include <utility>
 
 namespace WebCore {
@@ -101,11 +105,16 @@ void HTMLCollection::invalidateCacheIfNeeded() const
     if (cacheTreeVersion() == docversion)
         return;
 
-    clearCache(docversion);
+    invalidateCache();
 }
 
-void HTMLCollection::invalidateCache()
+void HTMLCollection::invalidateCache() const
 {
+#if ENABLE(MICRODATA)
+    // FIXME: There should be more generic mechanism to clear caches in subclasses.
+    if (type() == ItemProperties)
+        static_cast<const HTMLPropertiesCollection*>(this)->clearCache();
+#endif
     clearCache(static_cast<HTMLDocument*>(m_base->document())->domTreeVersion());
 }
 
index ed980e1..bb7c745 100644 (file)
@@ -136,8 +136,9 @@ public:
 
     Node* base() const { return m_base.get(); }
 
-    void invalidateCache();
+    void invalidateCache() const;
     void invalidateCacheIfNeeded() const;
+
 protected:
     HTMLCollection(Node* base, CollectionType);
 
index 90dc97c..87394c2 100644 (file)
@@ -52,36 +52,27 @@ PassRefPtr<HTMLPropertiesCollection> HTMLPropertiesCollection::create(Node* item
 
 HTMLPropertiesCollection::HTMLPropertiesCollection(Node* itemNode)
     : HTMLCollection(itemNode, ItemProperties)
+    , m_hasPropertyNameCache(false)
+    , m_hasItemRefElements(false)
 {
-    m_cache.clear();
 }
 
 HTMLPropertiesCollection::~HTMLPropertiesCollection()
 {
 }
 
-void HTMLPropertiesCollection::invalidateCacheIfNeeded() const
-{
-    uint64_t docversion = base()->document()->domTreeVersion();
-
-    if (m_cache.version == docversion)
-        return;
-
-    m_cache.clear();
-    m_cache.version = docversion;
-}
-
 void HTMLPropertiesCollection::updateRefElements() const
 {
-    if (m_cache.hasItemRefElements)
+    if (m_hasItemRefElements)
         return;
 
-    Vector<Element*> itemRefElements;
     HTMLElement* baseElement = toHTMLElement(base());
 
+    m_itemRefElements.clear();
+
     if (!baseElement->fastHasAttribute(itemrefAttr)) {
-        itemRefElements.append(baseElement);
-        m_cache.setItemRefElements(itemRefElements);
+        m_itemRefElements.append(baseElement);
+        m_hasItemRefElements = true;
         return;
     }
 
@@ -95,7 +86,7 @@ void HTMLPropertiesCollection::updateRefElements() const
         HTMLElement* element = toHTMLElement(current);
 
         if (element == baseElement) {
-            itemRefElements.append(element);
+            m_itemRefElements.append(element);
             continue;
         }
 
@@ -103,11 +94,10 @@ void HTMLPropertiesCollection::updateRefElements() const
         if (!processedItemRef->tokens().contains(id) && itemRef->tokens().contains(id)) {
             processedItemRef->setValue(id);
             if (!element->isDescendantOf(baseElement))
-                itemRefElements.append(element);
+                m_itemRefElements.append(element);
         }
     }
-
-    m_cache.setItemRefElements(itemRefElements);
+    m_hasItemRefElements = true;
 }
 
 static Node* nextNodeWithProperty(Node* base, Node* node)
@@ -120,7 +110,7 @@ static Node* nextNodeWithProperty(Node* base, Node* node)
             ? node->traverseNextNode(base) : node->traverseNextSibling(base);
 }
 
-Element* HTMLPropertiesCollection::itemAfter(Element* base, Element* previous) const
+Element* HTMLPropertiesCollection::itemAfter(Element* base, Node* previous) const
 {
     Node* current;
     current = previous ? nextNodeWithProperty(base, previous) : base;
@@ -139,45 +129,27 @@ Element* HTMLPropertiesCollection::itemAfter(Element* base, Element* previous) c
 
 unsigned HTMLPropertiesCollection::calcLength() const
 {
+    if (!toHTMLElement(base())->fastHasAttribute(itemscopeAttr))
+        return 0;
+
     unsigned length = 0;
     updateRefElements();
 
-    const Vector<Element*>& itemRefElements = m_cache.getItemRefElements();
-    for (unsigned i = 0; i < itemRefElements.size(); ++i) {
-        for (Element* element = itemAfter(itemRefElements[i], 0); element; element = itemAfter(itemRefElements[i], element))
+    for (unsigned i = 0; i < m_itemRefElements.size(); ++i) {
+        for (Element* element = itemAfter(m_itemRefElements[i], 0); element; element = itemAfter(m_itemRefElements[i], element))
             ++length;
     }
 
     return length;
 }
 
-unsigned HTMLPropertiesCollection::length() const
-{
-    if (!toHTMLElement(base())->fastHasAttribute(itemscopeAttr))
-        return 0;
-
-    invalidateCacheIfNeeded();
-
-    if (!m_cache.hasLength)
-        m_cache.updateLength(calcLength());
-
-    return m_cache.length;
-}
-
-Element* HTMLPropertiesCollection::firstProperty() const
+void HTMLPropertiesCollection::cacheFirstItem() const
 {
-    Element* element = 0;
-    m_cache.resetPosition();
-    const Vector<Element*>& itemRefElements = m_cache.getItemRefElements();
-    for (unsigned i = 0; i < itemRefElements.size(); ++i) {
-        element = itemAfter(itemRefElements[i], 0);
-        if (element) {
-            m_cache.itemRefElementPosition = i;
-            break;
-        }
+    for (unsigned i = 0; i < m_itemRefElements.size(); ++i) {
+        if (Element* element = itemAfter(m_itemRefElements[i], 0))
+            return setItemCache(element, 0, i);
     }
-
-    return element;
+    setItemCache(0, 0, 0);
 }
 
 Node* HTMLPropertiesCollection::item(unsigned index) const
@@ -186,43 +158,41 @@ Node* HTMLPropertiesCollection::item(unsigned index) const
         return 0;
 
     invalidateCacheIfNeeded();
-    if (m_cache.current && m_cache.position == index)
-        return m_cache.current;
+    if (isItemCacheValid() && cachedItemOffset() == index)
+        return cachedItem();
 
-    if (m_cache.hasLength && m_cache.length <= index)
+    if (isLengthCacheValid() && cachedLength() <= index)
         return 0;
 
     updateRefElements();
-    if (!m_cache.current || m_cache.position > index) {
-        m_cache.current = firstProperty();
-        if (!m_cache.current)
-            return 0;
+    if (!isItemCacheValid() || cachedItemOffset() > index) {
+        cacheFirstItem();
+        ASSERT(isItemCacheValid());
+        if (!cachedItem() || cachedItemOffset() == index)
+            return cachedItem();
     }
 
-    unsigned currentPosition = m_cache.position;
-    Element* element = m_cache.current;
-    unsigned itemRefElementPos = m_cache.itemRefElementPosition;
-    const Vector<Element*>& itemRefElements = m_cache.getItemRefElements();
-
-    bool found = (m_cache.position == index);
+    unsigned currentPosition = cachedItemOffset();
+    Node* element = cachedItem();
+    ASSERT(currentPosition != index);
 
-    for (unsigned i = itemRefElementPos; i < itemRefElements.size() && !found; ++i) {
+    for (unsigned i = cachedElementsArrayOffset(); i < m_itemRefElements.size(); ++i) {
         while (currentPosition < index) {
-            element = itemAfter(itemRefElements[i], element);
+            element = itemAfter(m_itemRefElements[i], element);
             if (!element)
                 break;
             currentPosition++;
 
             if (currentPosition == index) {
-                found = true;
-                itemRefElementPos = i;
-                break;
+                setItemCache(element, currentPosition, i);
+                return cachedItem();
             }
         }
     }
 
-    m_cache.updateCurrentItem(element, index, itemRefElementPos);
-    return m_cache.current;
+    setLengthCache(currentPosition);
+
+    return 0;
 }
 
 void HTMLPropertiesCollection::findProperties(Element* base) const
@@ -230,23 +200,22 @@ void HTMLPropertiesCollection::findProperties(Element* base) const
     for (Element* element = itemAfter(base, 0); element; element = itemAfter(base, element)) {
         DOMSettableTokenList* itemProperty = element->itemProp();
         for (unsigned i = 0; i < itemProperty->length(); ++i)
-            m_cache.updatePropertyCache(element, itemProperty->item(i));
+            updatePropertyCache(element, itemProperty->item(i));
     }
 }
 
 void HTMLPropertiesCollection::updateNameCache() const
 {
     invalidateCacheIfNeeded();
-    if (m_cache.hasNameCache)
+    if (m_hasPropertyNameCache)
         return;
 
     updateRefElements();
 
-    const Vector<Element*>& itemRefElements = m_cache.getItemRefElements();
-    for (unsigned i = 0; i < itemRefElements.size(); ++i)
-        findProperties(itemRefElements[i]);
+    for (unsigned i = 0; i < m_itemRefElements.size(); ++i)
+        findProperties(m_itemRefElements[i]);
 
-    m_cache.hasNameCache = true;
+    m_hasPropertyNameCache = true;
 }
 
 PassRefPtr<DOMStringList> HTMLPropertiesCollection::names() const
@@ -256,7 +225,7 @@ PassRefPtr<DOMStringList> HTMLPropertiesCollection::names() const
 
     updateNameCache();
 
-    return m_cache.propertyNames;
+    return m_propertyNames;
 }
 
 PassRefPtr<NodeList> HTMLPropertiesCollection::namedItem(const String& name) const
@@ -268,7 +237,7 @@ PassRefPtr<NodeList> HTMLPropertiesCollection::namedItem(const String& name) con
 
     updateNameCache();
 
-    Vector<Element*>* propertyResults = m_cache.propertyCache.get(AtomicString(name).impl());
+    Vector<Element*>* propertyResults = m_propertyCache.get(AtomicString(name).impl());
     for (unsigned i = 0; propertyResults && i < propertyResults->size(); ++i)
         namedItems.append(propertyResults->at(i));
 
@@ -283,7 +252,7 @@ bool HTMLPropertiesCollection::hasNamedItem(const AtomicString& name) const
 
     updateNameCache();
 
-    if (Vector<Element*>* propertyCache = m_cache.propertyCache.get(name.impl())) {
+    if (Vector<Element*>* propertyCache = m_propertyCache.get(name.impl())) {
         if (!propertyCache->isEmpty())
             return true;
     }
index 261eca3..7826df4 100644 (file)
@@ -45,8 +45,6 @@ public:
     static PassRefPtr<HTMLPropertiesCollection> create(Node*);
     virtual ~HTMLPropertiesCollection();
 
-    unsigned length() const OVERRIDE;
-
     virtual Node* item(unsigned) const OVERRIDE;
 
     PassRefPtr<DOMStringList> names() const;
@@ -54,96 +52,49 @@ public:
     virtual PassRefPtr<NodeList> namedItem(const String&) const OVERRIDE;
     virtual bool hasNamedItem(const AtomicString&) const OVERRIDE;
 
+    void clearCache() const
+    {
+        m_itemRefElements.clear();
+        m_propertyNames.clear();
+        m_propertyCache.clear();
+        m_hasPropertyNameCache = false;
+        m_hasItemRefElements = false;
+    }
+
 private:
     HTMLPropertiesCollection(Node*);
 
-    unsigned calcLength() const;
+    virtual unsigned calcLength() const OVERRIDE;
     void findProperties(Element* base) const;
 
     Node* findRefElements(Node* previous) const;
 
-    Element* firstProperty() const;
-    Element* itemAfter(Element* base, Element* previous) const;
+    void cacheFirstItem() const;
+    Element* itemAfter(Element* base, Node* previous) const;
 
     void updateNameCache() const;
     void updateRefElements() const;
 
-    void invalidateCacheIfNeeded() const;
-
-    mutable struct {
-        uint64_t version;
-        Element* current;
-        unsigned position;
-        unsigned length;
-        bool hasLength;
-        bool hasNameCache;
-        NodeCacheMap propertyCache;
-        Vector<Element*> itemRefElements;
-        RefPtr<DOMStringList> propertyNames;
-        unsigned itemRefElementPosition;
-        bool hasItemRefElements;
-
-        void clear()
-        {
-            version = 0;
-            current = 0;
-            position = 0;
-            length = 0;
-            hasLength = false;
-            hasNameCache = false;
-            propertyCache.clear();
-            itemRefElements.clear();
-            propertyNames.clear();
-            itemRefElementPosition = 0;
-            hasItemRefElements = false;
-        }
-
-        void setItemRefElements(const Vector<Element*>& elements)
-        {
-            itemRefElements = elements;
-            hasItemRefElements = true;
-        }
-
-        const Vector<Element*>& getItemRefElements()
-        {
-            return itemRefElements;
-        }
-
-        void updateLength(unsigned len)
-        {
-            length = len;
-            hasLength = true;
-        }
-
-        void updatePropertyCache(Element* element, const AtomicString& propertyName)
-        {
-            if (!propertyNames)
-                propertyNames = DOMStringList::create();
-
-            if (!propertyNames->contains(propertyName))
-                propertyNames->append(propertyName);
-
-            Vector<Element*>* propertyResults = propertyCache.get(propertyName.impl());
-            if (!propertyResults || !propertyResults->contains(element))
-                append(propertyCache, propertyName, element);
-        }
-
-        void updateCurrentItem(Element* element, unsigned pos, unsigned itemRefElementPos)
-        {
-            current = element;
-            position = pos;
-            itemRefElementPosition = itemRefElementPos;
-        }
-
-        void resetPosition()
-        {
-            current = 0;
-            position = 0;
-            itemRefElementPosition = 0;
-        }
-
-    } m_cache;
+    void updatePropertyCache(Element* element, const AtomicString& propertyName) const
+    {
+        if (!m_propertyNames)
+            m_propertyNames = DOMStringList::create();
+
+        if (!m_propertyNames->contains(propertyName))
+            m_propertyNames->append(propertyName);
+
+        Vector<Element*>* propertyResults = m_propertyCache.get(propertyName.impl());
+        if (!propertyResults || !propertyResults->contains(element))
+            append(m_propertyCache, propertyName, element);
+    }
+
+    mutable Vector<Element*> m_itemRefElements;
+    mutable RefPtr<DOMStringList> m_propertyNames;
+    mutable NodeCacheMap m_propertyCache;
 
+    // FIXME: Move these variables to DynamicNodeListCacheBase for better bit packing.
+    mutable bool m_hasPropertyNameCache : 1;
+    mutable bool m_hasItemRefElements : 1;
 };
 
 } // namespace WebCore