Modified: trunk/Source/WebCore/ChangeLog (122266 => 122267)
--- trunk/Source/WebCore/ChangeLog 2012-07-10 22:01:32 UTC (rev 122266)
+++ trunk/Source/WebCore/ChangeLog 2012-07-10 22:03:01 UTC (rev 122267)
@@ -1,3 +1,41 @@
+2012-07-10 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Add support for min-height:auto and min-width:auto
Modified: trunk/Source/WebCore/html/HTMLCollection.cpp (122266 => 122267)
--- trunk/Source/WebCore/html/HTMLCollection.cpp 2012-07-10 22:01:32 UTC (rev 122266)
+++ trunk/Source/WebCore/html/HTMLCollection.cpp 2012-07-10 22:03:01 UTC (rev 122267)
@@ -30,6 +30,10 @@
#include "HTMLOptionElement.h"
#include "NodeList.h"
+#if ENABLE(MICRODATA)
+#include "HTMLPropertiesCollection.h"
+#endif
+
#include <utility>
namespace WebCore {
@@ -101,11 +105,16 @@
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());
}
Modified: trunk/Source/WebCore/html/HTMLCollection.h (122266 => 122267)
--- trunk/Source/WebCore/html/HTMLCollection.h 2012-07-10 22:01:32 UTC (rev 122266)
+++ trunk/Source/WebCore/html/HTMLCollection.h 2012-07-10 22:03:01 UTC (rev 122267)
@@ -136,8 +136,9 @@
Node* base() const { return m_base.get(); }
- void invalidateCache();
+ void invalidateCache() const;
void invalidateCacheIfNeeded() const;
+
protected:
HTMLCollection(Node* base, CollectionType);
Modified: trunk/Source/WebCore/html/HTMLPropertiesCollection.cpp (122266 => 122267)
--- trunk/Source/WebCore/html/HTMLPropertiesCollection.cpp 2012-07-10 22:01:32 UTC (rev 122266)
+++ trunk/Source/WebCore/html/HTMLPropertiesCollection.cpp 2012-07-10 22:03:01 UTC (rev 122267)
@@ -52,36 +52,27 @@
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 @@
HTMLElement* element = toHTMLElement(current);
if (element == baseElement) {
- itemRefElements.append(element);
+ m_itemRefElements.append(element);
continue;
}
@@ -103,11 +94,10 @@
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 @@
? 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 @@
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
+void HTMLPropertiesCollection::cacheFirstItem() 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
-{
- 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 @@
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();
+ unsigned currentPosition = cachedItemOffset();
+ Node* element = cachedItem();
+ ASSERT(currentPosition != index);
- bool found = (m_cache.position == 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 @@
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 @@
updateNameCache();
- return m_cache.propertyNames;
+ return m_propertyNames;
}
PassRefPtr<NodeList> HTMLPropertiesCollection::namedItem(const String& name) const
@@ -268,7 +237,7 @@
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 @@
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;
}
Modified: trunk/Source/WebCore/html/HTMLPropertiesCollection.h (122266 => 122267)
--- trunk/Source/WebCore/html/HTMLPropertiesCollection.h 2012-07-10 22:01:32 UTC (rev 122266)
+++ trunk/Source/WebCore/html/HTMLPropertiesCollection.h 2012-07-10 22:03:01 UTC (rev 122267)
@@ -45,8 +45,6 @@
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 @@
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;
+ void updatePropertyCache(Element* element, const AtomicString& propertyName) const
+ {
+ if (!m_propertyNames)
+ m_propertyNames = DOMStringList::create();
- 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;
+ if (!m_propertyNames->contains(propertyName))
+ m_propertyNames->append(propertyName);
- void clear()
- {
- version = 0;
- current = 0;
- position = 0;
- length = 0;
- hasLength = false;
- hasNameCache = false;
- propertyCache.clear();
- itemRefElements.clear();
- propertyNames.clear();
- itemRefElementPosition = 0;
- hasItemRefElements = false;
- }
+ Vector<Element*>* propertyResults = m_propertyCache.get(propertyName.impl());
+ if (!propertyResults || !propertyResults->contains(element))
+ append(m_propertyCache, propertyName, element);
+ }
- void setItemRefElements(const Vector<Element*>& elements)
- {
- itemRefElements = elements;
- hasItemRefElements = true;
- }
+ mutable Vector<Element*> m_itemRefElements;
+ mutable RefPtr<DOMStringList> m_propertyNames;
+ mutable NodeCacheMap m_propertyCache;
- 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;
-
+ // FIXME: Move these variables to DynamicNodeListCacheBase for better bit packing.
+ mutable bool m_hasPropertyNameCache : 1;
+ mutable bool m_hasItemRefElements : 1;
};
} // namespace WebCore
