Use a StringImpl* instead of an AtomicString for NodeListAtomicNameCacheMap's key

Source/core/dom/NodeRareData.h

 /*
  * Copyright (C) 2008, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2008 David Smith <catfish.man@gmail.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 76 matching lines @@
     template <typename StringType>
     struct NodeListCacheMapEntryHash {
         static unsigned hash(const std::pair<unsigned char, StringType>& entry)
         {
             return DefaultHash<StringType>::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 == b; }
         static const bool safeToCompareToEmptyOrDeleted = DefaultHash<StringType>::Hash::safeToCompareToEmptyOrDeleted;
     };
 
-    typedef HashMap<std::pair<unsigned char, AtomicString>, LiveNodeListBase*, NodeListCacheMapEntryHash<AtomicString> > NodeListAtomicNameCacheMap;
+    struct NodeListAtomicCacheMapEntryHash {
+        static unsigned hash(const std::pair<unsigned char, StringImpl*>& entry)
+        {
+            return DefaultHash<StringImpl*>::Hash::hash(entry.second) + entry.first;
+        }
+        static bool equal(const std::pair<unsigned char, StringImpl*>& a, const std::pair<unsigned char, StringImpl*>& b) { return a == b; }
+        static const bool safeToCompareToEmptyOrDeleted = DefaultHash<StringImpl*>::Hash::safeToCompareToEmptyOrDeleted;
+    };
+
+    typedef HashMap<std::pair<unsigned char, StringImpl*>, LiveNodeListBase*, NodeListAtomicCacheMapEntryHash> NodeListAtomicNameCacheMap;
     typedef HashMap<std::pair<unsigned char, String>, LiveNodeListBase*, NodeListCacheMapEntryHash<String> > NodeListNameCacheMap;
     typedef HashMap<QualifiedName, TagCollection*> TagCollectionCacheNS;
 
     template<typename T>
     PassRefPtr<T> addCacheWithAtomicName(ContainerNode* node, CollectionType collectionType, const AtomicString& name)
     {
         NodeListAtomicNameCacheMap::AddResult result = m_atomicNameCaches.add(namedNodeListKey(collectionType, name), 0);
         if (!result.isNewEntry)
             return static_cast<T*>(result.iterator->value);
 
@@ skipping 113 matching lines @@
                 newDocument.registerNodeList(list);
             }
         }
     }
 
 private:
     NodeListsNodeData()
         : m_childNodeList(0)
     { }
 
-    std::pair<unsigned char, AtomicString> namedNodeListKey(CollectionType type, const AtomicString& name)
+    std::pair<unsigned char, StringImpl*> namedNodeListKey(CollectionType type, const AtomicString& name)
     {
-        return std::pair<unsigned char, AtomicString>(type, name);
+        return std::pair<unsigned char, StringImpl*>(type, name.impl());

[abarth-chromium, 2014/02/09 03:46:27]

Why is this memory safe?  What retains |name| ?

[Inactive, 2014/02/09 14:06:54]

Nothing retains name. This is safe because we are never doing anything with this
StringImpl* besides pointer comparison. It merely serves as a unique identifier
in the HashTable. It seems to be quite common in Blink to use atomic StringImpl*
for keys in Hash Tables without ref'ing them (see for e.g.
DocumentOrderedMap.h).

[Inactive, 2014/02/09 14:28:02]

Never mind my previous comment, it is inaccurate. The AtomicStrings are retained
by the NodeLists (e.g. HTMLNameCollection.m_name, TagCollection.m_localName,
...). And the node lists are in charge of removing themselves from the cache on
deletion. 

     }
 
     std::pair<unsigned char, String> namedNodeListKey(CollectionType type, const String& name)
     {
         return std::pair<unsigned char, String>(type, name);
     }
 
     bool deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(Node*);
 
     // Can be a ChildNodeList or an EmptyNodeList.
@@ skipping 71 matching lines @@
     ownerNode->clearNodeLists();
     return true;
 }
 
 // Ensure the 10 bits reserved for the m_connectedFrameCount cannot overflow
 COMPILE_ASSERT(Page::maxNumberOfFrames < 1024, Frame_limit_should_fit_in_rare_data_count);
 
 } // namespace WebCore
 
 #endif // NodeRareData_h