Diff
Added: trunk/LayoutTests/fast/dom/getElementsByClassName/conflict-tag-name-expected.txt (0 => 173648)
--- trunk/LayoutTests/fast/dom/getElementsByClassName/conflict-tag-name-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/getElementsByClassName/conflict-tag-name-expected.txt 2014-09-16 04:09:58 UTC (rev 173648)
@@ -0,0 +1,13 @@
+Checks that getElementsByClassName('foo') and getElementsByTagName('foo') do not conflict
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS withFooTag.length is 1
+PASS withFooTag[0].id is "a"
+PASS withFooClass.length is 1
+PASS withFooClass[0].id is "b"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/getElementsByClassName/conflict-tag-name.html (0 => 173648)
--- trunk/LayoutTests/fast/dom/getElementsByClassName/conflict-tag-name.html (rev 0)
+++ trunk/LayoutTests/fast/dom/getElementsByClassName/conflict-tag-name.html 2014-09-16 04:09:58 UTC (rev 173648)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<script src=""
+
+<foo id="a"></foo>
+<div id="b" class="foo"></div>
+
+<script>
+description("Checks that getElementsByClassName('foo') and getElementsByTagName('foo') do not conflict");
+
+var withFooTag = document.getElementsByTagName("foo");
+shouldBe("withFooTag.length", "1");
+shouldBeEqualToString("withFooTag[0].id", "a");
+
+var withFooClass = document.getElementsByClassName("foo");
+shouldBe("withFooClass.length", "1");
+shouldBeEqualToString("withFooClass[0].id", "b");
+</script>
Modified: trunk/Source/WebCore/ChangeLog (173647 => 173648)
--- trunk/Source/WebCore/ChangeLog 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/ChangeLog 2014-09-16 04:09:58 UTC (rev 173648)
@@ -1,5 +1,71 @@
2014-09-15 Chris Dumez <cdu...@apple.com>
+ Use an AtomicString as key for caching ClassNodeList objects
+ https://bugs.webkit.org/show_bug.cgi?id=136830
+
+ Reviewed by Benjamin Poulain.
+
+ Use an AtomicString as key for caching ClassNodeList objects instead of
+ a String. ClassNodeList is the only type using a String instead of an
+ AtomicString as key in the cache HashTable. This brings some
+ complexity.
+
+ I believe this was done to avoid unnecessarily atomizing the String,
+ for performance reasons. However, at the moment, the String gets
+ atomized anyway when constructing the ClassNodeList object. This is
+ because the ClassNodeList::m_classNames member is of SpaceSplitString
+ type and the SpaceSplitString constructor takes an AtomicString in
+ argument.
+
+ Using an AtomicString to cache ClassNodeLists simplifies the code quite
+ a bit and decreases the size of NodeListsNodeData as well.
+
+ Test: fast/dom/getElementsByClassName/conflict-tag-name.html
+
+ * WebCore.order:
+ Remove symbol corresponding to addCacheWithName() as it was removed.
+
+ * dom/ClassNodeList.cpp:
+ (WebCore::ClassNodeList::~ClassNodeList):
+ Update the constructor to take an AtomicString in argument instead of
+ a String, for clarity. The String gets atomized when initializing
+ m_classNames anyway.
+
+ (WebCore::ClassNodeList::ClassNodeList):
+ Call removeCacheWithAtomicName() instead of removeCacheWithName() now
+ that m_originalClassNames is an AtomicString.
+
+ * dom/ClassNodeList.h:
+ Use AtomicString instead of String type for classNames, in both the
+ constructor argument and the m_originalClassNames data member.
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::getElementsByClassName):
+ Call addCacheWithAtomicName() instead of addCacheWithName() now that
+ addCacheWithName() has been removed.
+
+ * dom/Node.cpp:
+ (WebCore::NodeListsNodeData::invalidateCaches):
+ Stop invalidating m_nameCaches as this HashMap no longer exists.
+
+ * dom/NodeRareData.h:
+ (WebCore::NodeListsNodeData::NodeListCacheMapEntryHash::hash):
+ (WebCore::NodeListsNodeData::NodeListCacheMapEntryHash::equal):
+ (WebCore::NodeListsNodeData::isEmpty):
+ (WebCore::NodeListsNodeData::adoptDocument):
+ (WebCore::NodeListsNodeData::namedNodeListKey):
+ (WebCore::NodeListsNodeData::deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList):
+ (WebCore::NodeListsNodeData::addCacheWithName): Deleted.
+ (WebCore::NodeListsNodeData::removeCacheWithName): Deleted.
+ - Drop addCacheWithName() / removeCacheWithName() now that no NodeList
+ uses a String as HashMap key.
+ - Drop m_nameCaches now that ClassNodeLists are cached in
+ m_atomicNameCaches instead.
+ - Remove StringType template parameter and hardcode AtomicString
+ instead.
+
+2014-09-15 Chris Dumez <cdu...@apple.com>
+
Return early in SelectorChecker::checkOne() if selector.isAttributeSelector() is true
https://bugs.webkit.org/show_bug.cgi?id=136838
Modified: trunk/Source/WebCore/WebCore.order (173647 => 173648)
--- trunk/Source/WebCore/WebCore.order 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/WebCore.order 2014-09-16 04:09:58 UTC (rev 173648)
@@ -6140,7 +6140,6 @@
__ZN3JSC21getStaticPropertySlotIN7WebCore27JSDOMCoreExceptionPrototypeENS_8JSObjectEEEbPNS_9ExecStateEPKNS_9HashTableEPT_NS_12PropertyNameERNS_12PropertySlotE
__ZN7WebCore48jsElementPrototypeFunctionGetElementsByClassNameEPN3JSC9ExecStateE
__ZN7WebCore4Node22getElementsByClassNameERKN3WTF6StringE
-__ZN7WebCore17NodeListsNodeData16addCacheWithNameINS_13ClassNodeListEEEN3WTF10PassRefPtrIT_EEPNS_4NodeENS_14CollectionTypeERKNS3_6StringE
__ZN7WebCore13ClassNodeListC1EN3WTF10PassRefPtrINS_4NodeEEERKNS1_6StringE
__ZN7WebCore13ClassNodeListC2EN3WTF10PassRefPtrINS_4NodeEEERKNS1_6StringE
__ZN7WebCore20firstMatchingElementINS_13ClassNodeListEEEPNS_7ElementEPKT_PNS_13ContainerNodeE
Modified: trunk/Source/WebCore/dom/ClassNodeList.cpp (173647 => 173648)
--- trunk/Source/WebCore/dom/ClassNodeList.cpp 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/dom/ClassNodeList.cpp 2014-09-16 04:09:58 UTC (rev 173648)
@@ -35,16 +35,14 @@
namespace WebCore {
-ClassNodeList::ClassNodeList(ContainerNode& rootNode, const String& classNames)
- : CachedLiveNodeList(rootNode, InvalidateOnClassAttrChange)
- , m_classNames(classNames, document().inQuirksMode())
- , m_originalClassNames(classNames)
+PassRef<ClassNodeList> ClassNodeList::create(ContainerNode& rootNode, const AtomicString& classNames)
{
+ return adoptRef(*new ClassNodeList(rootNode, classNames));
}
ClassNodeList::~ClassNodeList()
{
- ownerNode().nodeLists()->removeCacheWithName(this, m_originalClassNames);
+ ownerNode().nodeLists()->removeCacheWithAtomicName(this, m_originalClassNames);
}
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/ClassNodeList.h (173647 => 173648)
--- trunk/Source/WebCore/dom/ClassNodeList.h 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/dom/ClassNodeList.h 2014-09-16 04:09:58 UTC (rev 173648)
@@ -39,10 +39,7 @@
class ClassNodeList final : public CachedLiveNodeList<ClassNodeList> {
public:
- static PassRef<ClassNodeList> create(ContainerNode& rootNode, const String& classNames)
- {
- return adoptRef(*new ClassNodeList(rootNode, classNames));
- }
+ static PassRef<ClassNodeList> create(ContainerNode&, const AtomicString& classNames);
virtual ~ClassNodeList();
@@ -50,12 +47,19 @@
virtual bool isRootedAtDocument() const override { return false; }
private:
- ClassNodeList(ContainerNode& rootNode, const String& classNames);
+ ClassNodeList(ContainerNode& rootNode, const AtomicString& classNames);
SpaceSplitString m_classNames;
- String m_originalClassNames;
+ AtomicString m_originalClassNames;
};
+inline ClassNodeList::ClassNodeList(ContainerNode& rootNode, const AtomicString& classNames)
+ : CachedLiveNodeList(rootNode, InvalidateOnClassAttrChange)
+ , m_classNames(classNames, document().inQuirksMode())
+ , m_originalClassNames(classNames)
+{
+}
+
inline bool ClassNodeList::nodeMatches(Element* element) const
{
if (!element->hasClass())
Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (173647 => 173648)
--- trunk/Source/WebCore/dom/ContainerNode.cpp 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2014-09-16 04:09:58 UTC (rev 173648)
@@ -1054,9 +1054,9 @@
return ensureRareData().ensureNodeLists().addCacheWithAtomicName<NameNodeList>(*this, elementName);
}
-PassRefPtr<NodeList> ContainerNode::getElementsByClassName(const String& classNames)
+PassRefPtr<NodeList> ContainerNode::getElementsByClassName(const AtomicString& classNames)
{
- return ensureRareData().ensureNodeLists().addCacheWithName<ClassNodeList>(*this, classNames);
+ return ensureRareData().ensureNodeLists().addCacheWithAtomicName<ClassNodeList>(*this, classNames);
}
PassRefPtr<RadioNodeList> ContainerNode::radioNodeList(const AtomicString& name)
Modified: trunk/Source/WebCore/dom/ContainerNode.h (173647 => 173648)
--- trunk/Source/WebCore/dom/ContainerNode.h 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/dom/ContainerNode.h 2014-09-16 04:09:58 UTC (rev 173648)
@@ -137,7 +137,7 @@
PassRefPtr<NodeList> getElementsByTagName(const AtomicString&);
PassRefPtr<NodeList> getElementsByTagNameNS(const AtomicString& namespaceURI, const AtomicString& localName);
PassRefPtr<NodeList> getElementsByName(const String& elementName);
- PassRefPtr<NodeList> getElementsByClassName(const String& classNames);
+ PassRefPtr<NodeList> getElementsByClassName(const AtomicString& classNames);
PassRefPtr<RadioNodeList> radioNodeList(const AtomicString&);
protected:
Modified: trunk/Source/WebCore/dom/Node.cpp (173647 => 173648)
--- trunk/Source/WebCore/dom/Node.cpp 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/dom/Node.cpp 2014-09-16 04:09:58 UTC (rev 173648)
@@ -1695,9 +1695,6 @@
for (auto& atomicName : m_atomicNameCaches)
atomicName.value->invalidateCacheForAttribute(attrName);
- for (auto& name : m_nameCaches)
- name.value->invalidateCacheForAttribute(attrName);
-
for (auto& collection : m_cachedCollections)
collection.value->invalidateCache(attrName);
Modified: trunk/Source/WebCore/dom/NodeRareData.h (173647 => 173648)
--- trunk/Source/WebCore/dom/NodeRareData.h 2014-09-16 03:50:13 UTC (rev 173647)
+++ trunk/Source/WebCore/dom/NodeRareData.h 2014-09-16 04:09:58 UTC (rev 173648)
@@ -35,7 +35,6 @@
#include "TagNodeList.h"
#include <wtf/HashSet.h>
#include <wtf/text/AtomicString.h>
-#include <wtf/text/StringHash.h>
#if ENABLE(VIDEO_TRACK)
#include "TextTrack.h"
@@ -106,19 +105,17 @@
m_emptyChildNodeList = nullptr;
}
- template <typename StringType>
struct NodeListCacheMapEntryHash {
- static unsigned hash(const std::pair<unsigned char, StringType>& entry)
+ static unsigned hash(const std::pair<unsigned char, AtomicString>& entry)
{
- return DefaultHash<StringType>::Hash::hash(entry.second) + entry.first;
+ return DefaultHash<AtomicString>::Hash::hash(entry.second) + entry.first;
}
- static bool equal(const std::pair<unsigned char, StringType>& a, const std::pair<unsigned char, StringType>& b) { return a.first == b.first && DefaultHash<StringType>::Hash::equal(a.second, b.second); }
- static const bool safeToCompareToEmptyOrDeleted = DefaultHash<StringType>::Hash::safeToCompareToEmptyOrDeleted;
+ static bool equal(const std::pair<unsigned char, AtomicString>& a, const std::pair<unsigned char, AtomicString>& b) { return a.first == b.first && DefaultHash<AtomicString>::Hash::equal(a.second, b.second); }
+ static const bool safeToCompareToEmptyOrDeleted = DefaultHash<AtomicString>::Hash::safeToCompareToEmptyOrDeleted;
};
- typedef HashMap<std::pair<unsigned char, AtomicString>, LiveNodeList*, NodeListCacheMapEntryHash<AtomicString>> NodeListAtomicNameCacheMap;
- typedef HashMap<std::pair<unsigned char, String>, LiveNodeList*, NodeListCacheMapEntryHash<String>> NodeListNameCacheMap;
- typedef HashMap<std::pair<unsigned char, AtomicString>, HTMLCollection*, NodeListCacheMapEntryHash<AtomicString>> CollectionCacheMap;
+ typedef HashMap<std::pair<unsigned char, AtomicString>, LiveNodeList*, NodeListCacheMapEntryHash> NodeListAtomicNameCacheMap;
+ typedef HashMap<std::pair<unsigned char, AtomicString>, HTMLCollection*, NodeListCacheMapEntryHash> CollectionCacheMap;
typedef HashMap<QualifiedName, TagNodeList*> TagNodeListCacheNS;
template<typename T, typename ContainerType>
@@ -133,18 +130,6 @@
return list;
}
- template<typename T>
- ALWAYS_INLINE PassRef<T> addCacheWithName(ContainerNode& node, const String& name)
- {
- NodeListNameCacheMap::AddResult result = m_nameCaches.fastAdd(namedNodeListKey<T>(name), nullptr);
- if (!result.isNewEntry)
- return static_cast<T&>(*result.iterator->value);
-
- auto list = T::create(node, name);
- result.iterator->value = &list.get();
- return list;
- }
-
ALWAYS_INLINE PassRef<TagNodeList> addCacheWithQualifiedName(ContainerNode& node, const AtomicString& namespaceURI, const AtomicString& localName)
{
QualifiedName name(nullAtom, localName, namespaceURI);
@@ -196,15 +181,6 @@
m_atomicNameCaches.remove(namedNodeListKey<NodeListType>(name));
}
- template <class NodeListType>
- void removeCacheWithName(NodeListType* list, const String& name)
- {
- ASSERT(list == m_nameCaches.get(namedNodeListKey<NodeListType>(name)));
- if (deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(list->ownerNode()))
- return;
- m_nameCaches.remove(namedNodeListKey<NodeListType>(name));
- }
-
void removeCacheWithQualifiedName(LiveNodeList* list, const AtomicString& namespaceURI, const AtomicString& localName)
{
QualifiedName name(nullAtom, localName, namespaceURI);
@@ -225,7 +201,7 @@
void invalidateCaches(const QualifiedName* attrName = 0);
bool isEmpty() const
{
- return m_atomicNameCaches.isEmpty() && m_nameCaches.isEmpty() && m_cachedCollections.isEmpty() && m_tagNodeListCacheNS.isEmpty();
+ return m_atomicNameCaches.isEmpty() && m_cachedCollections.isEmpty() && m_tagNodeListCacheNS.isEmpty();
}
void adoptTreeScope()
@@ -244,9 +220,6 @@
for (auto& cache : m_atomicNameCaches.values())
cache->invalidateCache(*oldDocument);
- for (auto& cache : m_nameCaches.values())
- cache->invalidateCache(*oldDocument);
-
for (auto& list : m_tagNodeListCacheNS.values()) {
ASSERT(!list->isRootedAtDocument());
list->invalidateCache(*oldDocument);
@@ -263,9 +236,9 @@
}
template <class NodeListType>
- std::pair<unsigned char, String> namedNodeListKey(const String& name)
+ std::pair<unsigned char, AtomicString> namedNodeListKey(const AtomicString& name)
{
- return std::pair<unsigned char, String>(NodeListTypeIdentifier<NodeListType>::value(), name);
+ return std::pair<unsigned char, AtomicString>(NodeListTypeIdentifier<NodeListType>::value(), name);
}
bool deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(Node&);
@@ -275,7 +248,6 @@
EmptyNodeList* m_emptyChildNodeList;
NodeListAtomicNameCacheMap m_atomicNameCaches;
- NodeListNameCacheMap m_nameCaches;
TagNodeListCacheNS m_tagNodeListCacheNS;
CollectionCacheMap m_cachedCollections;
};
@@ -336,7 +308,7 @@
inline bool NodeListsNodeData::deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(Node& ownerNode)
{
ASSERT(ownerNode.nodeLists() == this);
- if ((m_childNodeList ? 1 : 0) + (m_emptyChildNodeList ? 1 : 0) + m_atomicNameCaches.size() + m_nameCaches.size()
+ if ((m_childNodeList ? 1 : 0) + (m_emptyChildNodeList ? 1 : 0) + m_atomicNameCaches.size()
+ m_tagNodeListCacheNS.size() + m_cachedCollections.size() != 1)
return false;
ownerNode.clearNodeLists();
