Add more oilpan collections support

Source/heap/Handle.h

Index: Source/heap/Handle.h
diff --git a/Source/heap/Handle.h b/Source/heap/Handle.h
index 5e281866df16118b6aed095ab1e155c81cc18f05..5afd9e9d47ebe2f407440157105b1b0920dae7d6 100644
--- a/Source/heap/Handle.h
+++ b/Source/heap/Handle.h
@@ -38,12 +38,25 @@
 namespace WebCore {
 
 template<typename T> class Member;
+template<typename T> struct OffHeapCollectionTraceTrait;
 
 class PersistentNode {
 public:
-    explicit PersistentNode(TraceCallback trace) : m_trace(trace) { }
+    explicit PersistentNode(TraceCallback trace)
+        : m_trace(trace)
+#ifndef NDEBUG

[Mads Ager (chromium), 2014/01/15 08:48:17]

Thanks for adding verification; that should make finding bugs with corrupted
persistent chains easier! :)

We should be able to do that without the #ifndef NDEBUG code by just using the
TraceCallback member. Set it to 0 on destruction and check that it is non-zero?
You can add a protected method called isAlive to do that check to make it clear
what it means.

+        , m_alive(true)
+#endif
+    {
+    }
 
-    virtual ~PersistentNode() { }
+    virtual ~PersistentNode()
+    {
+#ifndef NDEBUG
+        ASSERT(m_alive);
+        m_alive = false;
+#endif
+    }
 
     // Ideally the trace method should be virtual and automatically dispatch
     // to the most specific implementation. However having a virtual method
@@ -60,6 +73,10 @@ public:
 protected:
     TraceCallback m_trace;
 
+#ifndef NDEBUG
+    bool m_alive;
+#endif
+
 private:
     PersistentNode* m_next;
     PersistentNode* m_prev;
@@ -75,6 +92,9 @@ public:
     ~PersistentBase()
     {
 #ifndef NDEBUG
+        ASSERT(m_alive);
+        ASSERT(m_next->m_alive);
+        ASSERT(m_prev->m_alive);
         m_threadState->checkThread();
 #endif
         m_next->m_prev = m_prev;
@@ -135,7 +155,11 @@ public:
     void trace(Visitor*) { }
 
 private:
-    virtual ~PersistentAnchor() { }
+    virtual ~PersistentAnchor()
+    {
+        ASSERT(m_next == this);
+        ASSERT(m_prev == this);
+    }
     PersistentAnchor() : PersistentNode(TraceMethodDelegate<PersistentAnchor, &PersistentAnchor::trace>::trampoline)
     {
         m_next = this;
@@ -321,9 +345,9 @@ public:
 
     void swap(Member<T>& other) { std::swap(m_raw, other.m_raw); }
 
-protected:
     T* raw() const { return m_raw; }
 
+protected:

[haraken, 2014/01/15 04:48:35]

Don't we want private here?

[zerny-chromium, 2014/01/15 08:05:27]

WeakMember needs access to m_raw.

btw, most of the friends decls will go with Mads's latest CL.

[Mads Ager (chromium), 2014/01/15 08:48:17]

Drive-by answer. :-)

We don't need private here. Since we are using conservative scanning there is no
longer anything dangerous is getting hold of the raw pointer. Leaving it private
makes it simpler to write the comparison operators as well. I have a patch out
to rename this to get(), make it public and use it in the comparisons.

     T* m_raw;
 
     template<typename U> friend class Member;
@@ -417,12 +441,60 @@ private:
 // Comparison operators between (Weak)Members and Persistents
 template<typename T, typename U> inline bool operator==(const Member<T>& a, const Member<U>& b) { return a.m_raw == b.m_raw; }
 template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Member<U>& b) { return a.m_raw != b.m_raw; }
-template<typename T, typename U> inline bool operator==(const Member<T>& a, const Persistent<U>& b) { return a.m_raw == b.m_raw; }
-template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Persistent<U>& b) { return a.m_raw != b.m_raw; }
-template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Member<U>& b) { return a.m_raw == b.m_raw; }
-template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Member<U>& b) { return a.m_raw != b.m_raw; }
-template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Persistent<U>& b) { return a.m_raw == b.m_raw; }
-template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Persistent<U>& b) { return a.m_raw != b.m_raw; }
+template<typename T, typename U> inline bool operator==(const Member<T>& a, const Persistent<U>& b) { return a.m_raw == b.raw(); }

[Mads Ager (chromium), 2014/01/15 08:48:17]

This is a complete mess. Let us just use a.get() == b.get() consistently for all
of these. I'll do that with my patch for the ones that are already in the
system.

+template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Persistent<U>& b) { return a.m_raw != b.raw(); }
+template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Member<U>& b) { return a.raw() == b.m_raw; }
+template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Member<U>& b) { return a.raw() != b.m_raw; }
+template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Persistent<U>& b) { return a.raw() == b.raw(); }
+template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Persistent<U>& b) { return a.raw() != b.raw(); }
+
+template<typename Collection, ThreadAffinity Affinity = MainThreadOnly> class CollectionPersistent;
+
+// Used to inject correctly typed operator[] into CollectionPersistent when we are wrapping Vector.
+template<typename T> class IndexingBehavior { };
+
+template<typename T, size_t inlineCapacity>
+class IndexingBehavior<CollectionPersistent<Vector<T, inlineCapacity, WTF::DefaultAllocator> > > {
+    typedef CollectionPersistent<Vector<T, inlineCapacity, WTF::DefaultAllocator> > CollectionPersistentType;
+public:
+    T& operator[] (size_t i) { return (**static_cast<CollectionPersistentType*>(this))[i]; }
+    const T& operator[] (size_t i) const { return (**static_cast<const CollectionPersistentType*>(this))[i]; }
+};
+
+template<typename Collection, ThreadAffinity Affinity>
+class CollectionPersistent
+    : public PersistentBase<Affinity, CollectionPersistent<Collection, Affinity> >
+    , public IndexingBehavior<CollectionPersistent<Collection, Affinity> > {
+public:
+    typedef Collection CollectionType;
+    typedef typename Collection::iterator iterator;
+    typedef typename Collection::const_iterator const_iterator;
+
+    CollectionPersistent() { }
+    explicit CollectionPersistent(size_t size) : m_collection(Collection(size)) { }
+    explicit CollectionPersistent(const Collection& collection) : m_collection(collection) { }
+    CollectionPersistent& operator=(const Collection& collection) { m_collection = collection; return *this; }
+    Collection* operator->() { return &m_collection; }
+    const Collection* operator->() const { return &m_collection; }
+    Collection& operator*() { return m_collection; }
+    const Collection& operator*() const { return m_collection; }
+
+    void trace(Visitor* visitor)
+    {
+        OffHeapCollectionTraceTrait<Collection>::mark(visitor, m_collection);                                                                     }

[haraken, 2014/01/15 04:48:35]

Write } in the next line.

[Erik Corry, 2014/01/15 09:27:32]

Done.

+
+#if defined(TRACE_GC_MARKING) && TRACE_GC_MARKING
+    virtual const char* name() OVERRIDE
+    {
+        ASSERT(this == static_cast<PersistentNode*>(this));
+        const char* n = FieldAnnotationBase::fromAddress(this);
+        return n ? n : "CollectionPersistent";
+    }
+#endif
+
+private:
+    Collection m_collection;
+};
 
 } // namespace WebCore
 
@@ -449,13 +521,13 @@ template<typename T> struct HashTraits<WebCore::Member<T> > : SimpleClassHashTra
     // in the marking Visitor.
     typedef T* PeekInType;
     typedef T* PassInType;
-    typedef T* IteratorGetType;
-    typedef T* IteratorConstGetType;
+    typedef WebCore::Member<T>* IteratorGetType;
+    typedef const WebCore::Member<T>* IteratorConstGetType;
     typedef T* IteratorReferenceType;
     typedef T* IteratorConstReferenceType;
     static IteratorConstGetType getToConstGetConversion(const WebCore::Member<T>* x) { return x->raw(); }
-    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x; }
-    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x; }
+    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x->raw(); }
+    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x->raw(); }
     // FIXME: Similarly, there is no need for a distinction between PeekOutType
     // and PassOutType without reference counting.
     typedef T* PeekOutType;
@@ -477,13 +549,13 @@ template<typename T> struct HashTraits<WebCore::WeakMember<T> > : SimpleClassHas
     // in the marking Visitor.
     typedef T* PeekInType;
     typedef T* PassInType;
-    typedef T* IteratorGetType;
-    typedef T* IteratorConstGetType;
+    typedef WebCore::WeakMember<T>* IteratorGetType;
+    typedef const WebCore::WeakMember<T>* IteratorConstGetType;
     typedef T* IteratorReferenceType;
     typedef T* IteratorConstReferenceType;
     static IteratorConstGetType getToConstGetConversion(const WebCore::WeakMember<T>* x) { return x->raw(); }
-    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x; }
-    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x; }
+    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x->raw(); }
+    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x->raw(); }
     // FIXME: Similarly, there is no need for a distinction between PeekOutType
     // and PassOutType without reference counting.
     typedef T* PeekOutType;