[webkit-reviews] review requested: [Bug 21818] Should be able to use AtomicString as the key for a HashMap and HashSet : [Attachment 24604] Patch, changelog

[bugzilla-daemon at webkit.org]

Simon Fraser <simon.fraser at apple.com> has asked  for review:
Bug 21818: Should be able to use AtomicString as the key for a HashMap and
HashSet
https://bugs.webkit.org/show_bug.cgi?id=21818

Attachment 24604: Patch, changelog
https://bugs.webkit.org/attachment.cgi?id=24604&action=edit

------- Additional Comments from Simon Fraser <simon.fraser at apple.com>
Final patch. This uses an AtomicString with a NULL impl as the
constructDeletedValue, which seems to work OK.

I tested it with this code:

diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index f43eda1..c1984e6 100644
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -279,6 +279,53 @@ private:
     RefPtr<Document> m_document;
 };
 
+
+#include <wtf/HashMap.h>
+#include "AtomicString.h"
+#include "AtomicStringHash.h"
+
+void testAtomicStringHashMap()
+{
+    HashMap<AtomicString, bool> testMap;
+    
+    testMap.add("test1", true);
+    testMap.add("test2", false);
+    
+    assert(testMap.contains("test1") && testMap.get("test1"));
+    assert(testMap.contains("test2") && !testMap.get("test2"));
+    assert(!testMap.contains("test3"));
+
+    testMap.remove("test2");
+    assert(!testMap.contains("test2"));
+    
+    AtomicString test1("test1");
+    assert(testMap.contains(test1) && testMap.get("test1"));
+
+    testMap.remove("test1");
+    assert(!testMap.contains(test1));
+
+    testMap.clear();
+    
+    {
+	 AtomicString one("foobar_one");
+	 testMap.add(one, true);
+
+	 AtomicString two("foobar_two");
+	 testMap.add(two, true);
+    }
+    
+    {
+	 AtomicString one("foobar_one");
+	 AtomicString two("foobar_two");
+	 AtomicString three("foobar_three");
+	 assert(testMap.contains(one));
+	 assert(testMap.contains(two));
+	 assert(!testMap.contains(three));
+    }
+}
+
+
+
 Document::Document(Frame* frame, bool isXHTML)
     : ContainerNode(0)
     , m_domtree_version(0)
@@ -320,6 +367,9 @@ Document::Document(Frame* frame, bool isXHTML)
     , m_inLowBandwidthDisplay(false)
 #endif
 {
+
+testAtomicStringHashMap();
+
     m_document.resetSkippingRef(this);
 
     m_printing = false;