Oilpan: Add a test to confirm that pre-finalization hook can be implmemented with HeapHashSet<WeakM…

Source/heap/HeapTest.cpp

Index: Source/heap/HeapTest.cpp
diff --git a/Source/heap/HeapTest.cpp b/Source/heap/HeapTest.cpp
index 55c02897339210f1bcf493a0f381655c4be1f686..8fc134bd05903d3dfb92321f127e3823e9c96436 100644
--- a/Source/heap/HeapTest.cpp
+++ b/Source/heap/HeapTest.cpp
@@ -932,6 +932,44 @@ private:
     bool m_didCallWillFinalize;
 };
 
+class FinalizationObserverWithHashMap {
+public:
+    typedef HeapHashMap<WeakMember<Observable>, OwnPtr<FinalizationObserverWithHashMap> > ObserverMap;
+
+    FinalizationObserverWithHashMap(Observable& target) : m_target(target) { }
+    ~FinalizationObserverWithHashMap()
+    {
+        m_target.willFinalize();
+        s_didCallWillFinalize = true;
+    }
+
+    static ObserverMap& observe(Observable& target)

[Mads Ager (chromium), 2014/03/19 06:38:23]

I would just pass in the pointer here instead of a reference. Changing from
pointer to reference and then back to pointer doesn't seem to add much?

[tkent, 2014/03/19 08:00:14]

We prefer references in Blink if values can't be null.

[Mads Ager (chromium), 2014/03/19 09:40:23]

Well, this object is dynamically allocated. If you are using new, there is no
guarantee that it cannot be null. In this particular case the object is in the
heap and therefore if allocation fail we will terminate instead of returning 0.
However, in general there is no guarantee that something that you dynamically
allocate cannot be null. I find references reasonable for stack allocated
objects and part objects. For dynamically allocated objects like these I find it
misleading and I don't think we should use it here. It makes the code harder to
read for no reason.

+    {
+        ObserverMap& map = observers();
+        ObserverMap::AddResult result = map.add(&target, nullptr);
+        if (result.isNewEntry)
+            result.storedValue->value = adoptPtr(new FinalizationObserverWithHashMap(target));
+        else
+            ASSERT(result.storedValue->value);
+        return map;
+    }
+
+    static bool s_didCallWillFinalize;
+
+private:
+    static ObserverMap& observers()
+    {
+        DEFINE_STATIC_LOCAL(Persistent<ObserverMap>, observerMap, ());
+        if (!observerMap)
+            observerMap = new ObserverMap();
+        return *observerMap;
+    }
+
+    Observable& m_target;
+};
+
+bool FinalizationObserverWithHashMap::s_didCallWillFinalize = false;
+
 class SuperClass;
 
 class PointsBack : public RefCountedWillBeGarbageCollectedFinalized<PointsBack> {
@@ -2631,6 +2669,18 @@ TEST(HeapTest, FinalizationObserver)
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(0u, Bar::s_live);
     EXPECT_TRUE(o->didCallWillFinalize());
+
+    FinalizationObserverWithHashMap::s_didCallWillFinalize = false;
+    Observable* foo = Observable::create(Bar::create());
+    FinalizationObserverWithHashMap::ObserverMap& map = FinalizationObserverWithHashMap::observe(*foo);
+    EXPECT_EQ(1u, map.size());
+    foo = nullptr;

[Mads Ager (chromium), 2014/03/19 06:38:23]

As the try bots say, this needs to just be 0 for the compilers that do not have
real nullptr support yet. :(

[tkent, 2014/03/19 08:00:14]

Done.

+    // FinalizationObserverWithHashMap doesn't have a strong reference to
+    // |foo|. So |foo| and its member will be collected.
+    Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
+    EXPECT_EQ(0u, Bar::s_live);
+    EXPECT_EQ(0u, map.size());
+    EXPECT_TRUE(FinalizationObserverWithHashMap::s_didCallWillFinalize);
 }
 
 TEST(HeapTest, Comparisons)