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

Source/heap/HeapTest.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 914 matching lines @@
             o->m_data->willFinalize();
             o->m_data = nullptr;
             o->m_didCallWillFinalize = true;
         }
     }
 
     WeakMember<T> m_data;
     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> {
 public:
     static PassRefPtrWillBeRawPtr<PointsBack> create()
     {
         return adoptRefWillBeNoop(new PointsBack());
     }
 
     ~PointsBack()
@@ skipping 1679 matching lines @@
     {
         Observable* foo = Observable::create(Bar::create());
         // |o| observes |foo|.
         o = FinalizationObserver<Observable>::create(foo);
     }
     // FinalizationObserver 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_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)
 {
     Persistent<Bar> barPersistent = Bar::create();
     Persistent<Foo> fooPersistent = Foo::create(barPersistent);
     EXPECT_TRUE(barPersistent != fooPersistent);
     barPersistent = fooPersistent;
     EXPECT_TRUE(barPersistent == fooPersistent);
 }
@@ skipping 362 matching lines @@
     // swept away and zapped later in the same sweeping phase.
     EXPECT_EQ(42, wrapper->value());
 
     wrapper.clear();
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(10, IntWrapper::s_destructorCalls);
     EXPECT_EQ(512, OneKiloByteObject::s_destructorCalls);
 }
 
 } // WebCore namespace