Oilpan:Allow custom handling of classes that contain weak pointers that are embedded in collections.

Source/platform/heap/HeapTest.cpp

Index: Source/platform/heap/HeapTest.cpp
diff --git a/Source/platform/heap/HeapTest.cpp b/Source/platform/heap/HeapTest.cpp
index 5c93ea68e5a3d0207641dd5d8846a9288692f6cd..b71ac33ee762787df8788919fb734b502f854a8c 100644
--- a/Source/platform/heap/HeapTest.cpp
+++ b/Source/platform/heap/HeapTest.cpp
@@ -44,6 +44,8 @@
 
 namespace WebCore {
 
+class IntWrapper;
+
 class ThreadMarker {
 public:
     ThreadMarker() : m_creatingThread(reinterpret_cast<ThreadState*>(0)), m_num(0) { }
@@ -75,6 +77,49 @@ struct ThreadMarkerHash {
     static const bool safeToCompareToEmptyOrDeleted = false;
 };
 
+typedef std::pair<Member<IntWrapper>, WeakMember<IntWrapper> > StrongWeakPair;
+
+struct PairWithWeakHandling : public StrongWeakPair {
+    ALLOW_ONLY_INLINE_ALLOCATION();
+
+public:
+    // Regular constructor.
+    PairWithWeakHandling(IntWrapper* one, IntWrapper* two)
+        : StrongWeakPair(one, two)
+    {
+        ASSERT(one); // We use null first field to indicate empty slots in the hash table.
+    }
+
+    // The HashTable (via the HashTrait) calls this constructor with a
+    // placement new to mark slots in the hash table as being deleted. We will
+    // never call trace or the destructor on these slots. We mark ourselves deleted
+    // with a pointer to -1 in the first field.
+    PairWithWeakHandling(WTF::HashTableDeletedValueType)
+        : StrongWeakPair(reinterpret_cast<IntWrapper*>(-1), nullptr)
+    {
+    }
+
+    // Used by the HashTable (via the HashTrait) to skip deleted slots in the
+    // table. Recognizes objects that were 'constructed' using the above
+    // constructor.
+    bool isHashTableDeletedValue() const { return first == reinterpret_cast<IntWrapper*>(-1); }
+
+    // Since we don't allocate independent objects of this type, we don't need
+    // a regular trace method. Instead, we use a traceInCollection method.
+    void traceInCollection(Visitor* visitor, ShouldWeakPointersBeMarkedStrongly strongify)
+    {
+        visitor->trace(first);
+        visitor->traceInCollection(second, strongify);
+    }
+    // The traceInCollection may not trace the weak members, so it is vital
+    // that shouldRemoveFromCollection checks them so the entry can be removed
+    // from the collection (otherwise it would contain a dangling pointer).
+    bool shouldRemoveFromCollection(Visitor* visitor)
+    {
+        return !visitor->isAlive(second);
+    }
+};
+
 }
 
 namespace WTF {
@@ -91,6 +136,29 @@ template<> struct HashTraits<WebCore::ThreadMarker> : GenericHashTraits<WebCore:
     static bool isDeletedValue(const WebCore::ThreadMarker& slot) { return slot.isHashTableDeletedValue(); }
 };
 
+// The hash algorithm for our custom pair class is just the standard double
+// hash for pairs. Note that this means you can't mutate either of the parts of
+// the pair while they are in the hash table, as that would change their hash
+// code and thus their preferred placement in the table.
+template<> struct DefaultHash<WebCore::PairWithWeakHandling> {
+    typedef PairHash<WebCore::Member<WebCore::IntWrapper>, WebCore::WeakMember<WebCore::IntWrapper> > Hash;
+};
+
+// Custom traits for the pair. These are weakness handling traits, which means
+// PairWithWeakHandling must implement the traceInCollection and
+// shouldRemoveFromCollection methods. In addition, these traits are concerned
+// with the two magic values for the object, that represent empty and deleted
+// slots in the hash table. The SimpleClassHashTraits allow empty slots in the
+// table to be initialzed with memset to zero, and we use -1 in the first part
+// of the pair to represent deleted slots.
+template<> struct HashTraits<WebCore::PairWithWeakHandling> : WebCore::WeakHandlingHashTraits<WebCore::PairWithWeakHandling> {
+    static const bool needsDestruction = false;
+    static const bool hasIsEmptyValueFunction = true;
+    static bool isEmptyValue(const WebCore::PairWithWeakHandling& value) { return !value.first; }
+    static void constructDeletedValue(WebCore::PairWithWeakHandling& slot) { new (NotNull, &slot) WebCore::PairWithWeakHandling(HashTableDeletedValue); }
+    static bool isDeletedValue(const WebCore::PairWithWeakHandling& value) { return value.isHashTableDeletedValue(); }
+};
+
 }
 
 namespace WebCore {
@@ -3316,12 +3384,19 @@ TEST(HeapTest, CheckAndMarkPointer)
         EXPECT_TRUE(scope.allThreadsParked());
         Heap::makeConsistentForGC();
         for (size_t i = 0; i < objectAddresses.size(); i++) {
-            EXPECT_FALSE(Heap::checkAndMarkPointer(&visitor, objectAddresses[i]));
-            EXPECT_FALSE(Heap::checkAndMarkPointer(&visitor, endAddresses[i]));
+            // We would like to assert that checkAndMarkPointer returned false
+            // here because the pointers no longer point into a valid object
+            // (it's been freed by the GCs. But checkAndMarkPointer will return
+            // true for any pointer that points into a heap page, regardless of
+            // whether it points at a valid object (this ensures the
+            // correctness of the page-based on-heap address caches), so we
+            // can't make that assert.
+            Heap::checkAndMarkPointer(&visitor, objectAddresses[i]);
+            Heap::checkAndMarkPointer(&visitor, endAddresses[i]);
         }
         EXPECT_EQ(0ul, visitor.count());
-        EXPECT_FALSE(Heap::checkAndMarkPointer(&visitor, largeObjectAddress));
-        EXPECT_FALSE(Heap::checkAndMarkPointer(&visitor, largeObjectEndAddress));
+        Heap::checkAndMarkPointer(&visitor, largeObjectAddress);
+        Heap::checkAndMarkPointer(&visitor, largeObjectEndAddress);
         EXPECT_EQ(0ul, visitor.count());
     }
     // This round of GC is important to make sure that the object start
@@ -3978,4 +4053,58 @@ TEST(HeapTest, NeedsAdjustAndMark)
     EXPECT_FALSE(NeedsAdjustAndMark<const UseMixin>::value);
 }
 
+TEST(HeapTest, TypesWithCustomWeaknessHandling)
+{
+    HeapHashSet<PairWithWeakHandling> set;
+    HeapHashSet<PairWithWeakHandling>* set2 = new HeapHashSet<PairWithWeakHandling>();
+    Persistent<HeapHashSet<PairWithWeakHandling> > set3(new HeapHashSet<PairWithWeakHandling>());
+    Persistent<IntWrapper> livingInt(IntWrapper::create(42));
+    set.add(PairWithWeakHandling(IntWrapper::create(0), IntWrapper::create(1)));
+    set2->add(PairWithWeakHandling(IntWrapper::create(2), IntWrapper::create(3)));
+    set3->add(PairWithWeakHandling(IntWrapper::create(4), IntWrapper::create(5)));
+    Heap::collectGarbage(ThreadState::HeapPointersOnStack);
+    typedef HeapHashSet<PairWithWeakHandling>::iterator Iterator;
+    // The first set is on-stack, so its backing store must be referenced from
+    // the stack. That makes the weak references strong.
+    Iterator i = set.begin();
+    EXPECT_EQ(0, i->first->value());
+    EXPECT_EQ(1, i->second->value());
+    // The second set is pointed to from the stack, so it's referenced, but the
+    // weak processing may have taken place.
+    if (set2->size()) {
+        Iterator i2 = set2->begin();
+        EXPECT_EQ(2, i2->first->value());
+        EXPECT_EQ(3, i2->second->value());
+    }
+    // The second set is pointed to from a persistent, so it's referenced, but
+    // the weak processing may have taken place.
+    if (set3->size()) {
+        Iterator i3 = set3->begin();
+        EXPECT_EQ(4, i3->first->value());
+        EXPECT_EQ(5, i3->second->value());
+    }
+    Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
+    // No looking at the set and set2 now, they are busted by our lying about the heap pointers on the stack!
+    if (set3->size()) {
+        Iterator i3 = set3->begin();
+        EXPECT_EQ(4, i3->first->value());
+        EXPECT_EQ(5, i3->second->value());
+    }
+    set3->clear();
+    set3->add(PairWithWeakHandling(IntWrapper::create(103), livingInt));
+    set3->add(PairWithWeakHandling(livingInt, IntWrapper::create(103))); // This one gets zapped at GC time because nothing holds the 103 alive.
+    set3->add(PairWithWeakHandling(IntWrapper::create(103), IntWrapper::create(103))); // This one gets zapped too.
+    set3->add(PairWithWeakHandling(livingInt, livingInt));
+    set3->add(PairWithWeakHandling(livingInt, livingInt)); // This one is identical to the previous and doesn't add anything.
+    EXPECT_EQ(4u, set3->size());
+    Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
+    EXPECT_EQ(2u, set3->size());
+    Iterator i3 = set3->begin();
+    EXPECT_TRUE(i3->first->value() == 103 || i3->first == livingInt);
+    EXPECT_EQ(livingInt, i3->second);
+    ++i3;
+    EXPECT_TRUE(i3->first->value() == 103 || i3->first == livingInt);
+    EXPECT_EQ(livingInt, i3->second);
+}
+
 } // WebCore namespace