Check if Member owner thread matches pointer thread and current thread

third_party/WebKit/Source/platform/heap/Member.h

Index: third_party/WebKit/Source/platform/heap/Member.h
diff --git a/third_party/WebKit/Source/platform/heap/Member.h b/third_party/WebKit/Source/platform/heap/Member.h
index f342ba79f7e4751234d23411a47e357d0d3ddcb4..d370d5f371939ef00be9251fd2198e8404942d51 100644
--- a/third_party/WebKit/Source/platform/heap/Member.h
+++ b/third_party/WebKit/Source/platform/heap/Member.h
@@ -13,54 +13,67 @@ namespace blink {
 
 template<typename T> class Persistent;
 
+enum TracenessMemberConfiguration {

[haraken, 2016/08/16 13:27:13]

Shall we use an enum class?

enum class TracedMemberConfiguration {
  Traced,
  Untraced,
};

[keishi, 2016/08/17 11:18:29]

Done.

+    TracedMemberConfiguration,
+    UntracedMemberConfiguration
+};
+
 // Members are used in classes to contain strong pointers to other oilpan heap
 // allocated objects.
 // All Member fields of a class must be traced in the class' trace method.
 // During the mark phase of the GC all live objects are marked as live and
 // all Member fields of a live object will be traced marked as live as well.

[haraken, 2016/08/16 13:27:13]

Remove this comment.

[keishi, 2016/08/17 11:18:28]

Done.

-template<typename T>
-class Member {
+template<typename T, TracenessMemberConfiguration tracenessConfiguration = TracedMemberConfiguration>
+class MemberBase {
     DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
 public:
-    Member() : m_raw(nullptr)
+    MemberBase() : m_raw(nullptr)
     {
+        saveCreationThreadHeap();
     }
 
-    Member(std::nullptr_t) : m_raw(nullptr)
+    MemberBase(std::nullptr_t) : m_raw(nullptr)
     {
+        saveCreationThreadHeap();
     }
 
-    Member(T* raw) : m_raw(raw)
+    MemberBase(T* raw) : m_raw(raw)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
-    explicit Member(T& raw) : m_raw(&raw)
+    explicit MemberBase(T& raw) : m_raw(&raw)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
-    Member(WTF::HashTableDeletedValueType) : m_raw(reinterpret_cast<T*>(-1))
+    MemberBase(WTF::HashTableDeletedValueType) : m_raw(reinterpret_cast<T*>(-1))
     {
+        saveCreationThreadHeap();
     }
 
     bool isHashTableDeletedValue() const { return m_raw == reinterpret_cast<T*>(-1); }
 
-    Member(const Member& other) : m_raw(other)
+    MemberBase(const MemberBase& other) : m_raw(other)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
     template<typename U>
-    Member(const Persistent<U>& other)
+    MemberBase(const Persistent<U>& other)
     {
+        saveCreationThreadHeap();
         m_raw = other;
         checkPointer();
     }
 
     template<typename U>
-    Member(const Member<U>& other) : m_raw(other)
+    MemberBase(const MemberBase<U>& other) : m_raw(other)
     {
+        saveCreationThreadHeap();
         checkPointer();
     }
 
@@ -79,7 +92,7 @@ public:
     T& operator*() const { return *m_raw; }
 
     template<typename U>
-    Member& operator=(const Persistent<U>& other)
+    MemberBase& operator=(const Persistent<U>& other)
     {
         m_raw = other;
         checkPointer();
@@ -87,7 +100,7 @@ public:
     }
 
     template<typename U>
-    Member& operator=(const Member<U>& other)
+    MemberBase& operator=(const MemberBase<U>& other)
     {
         m_raw = other;
         checkPointer();
@@ -95,20 +108,20 @@ public:
     }
 
     template<typename U>
-    Member& operator=(U* other)
+    MemberBase& operator=(U* other)
     {
         m_raw = other;
         checkPointer();
         return *this;
     }
 
-    Member& operator=(std::nullptr_t)
+    MemberBase& operator=(std::nullptr_t)
     {
         m_raw = nullptr;
         return *this;
     }
 
-    void swap(Member<T>& other)
+    void swap(MemberBase<T>& other)
     {
         std::swap(m_raw, other.m_raw);
         checkPointer();
@@ -122,7 +135,7 @@ public:
 protected:
     void checkPointer()
     {
-#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
+#if DCHECK_IS_ON()
         if (!m_raw)
             return;
         // HashTable can store a special value (which is not aligned to the
@@ -132,6 +145,17 @@ protected:
         if (reinterpret_cast<intptr_t>(m_raw) % allocationGranularity)
             return;
 
+        ThreadState* current = ThreadState::current();
+        if (tracenessConfiguration != UntracedMemberConfiguration && current) {

[keishi, 2016/08/16 13:05:49]

Ignoring UntracedMember because in LayoutTests, GraphicsLayer::TakeDebugInfo
gets called on the compositor thread, which calls LayoutObject::debugNode which
creates a UntracedMember<Node>

[haraken, 2016/08/16 13:27:13]

'tracenessConfiguration != UntracedMemberConfiguration' makes sense, but do we
need to check 'current'?

If ThreadState::current() is nullptr, Member/WeakMember should not be used
whereas UntracedMember may be allowed.

[keishi, 2016/08/17 11:18:29]

Done. 

+            if (m_creationThreadHeap) {

[haraken, 2016/08/16 13:27:13]

How is it possible that m_creationThreadHeap is nullptr when we reach here?

[keishi, 2016/08/17 11:18:28]

I think heap collections use memset to initialize the Member and don't call the
constructor.

[haraken, 2016/08/17 11:40:41]

OK, let's add a comment about it.

[keishi, 2016/08/17 12:23:20]

Done.

+                DCHECK_EQ(&ThreadState::fromObject(m_raw)->heap(), m_creationThreadHeap);
+                DCHECK_EQ(&current->heap(), m_creationThreadHeap);
+            } else {
+                DCHECK(&ThreadState::fromObject(m_raw)->heap() == &current->heap());

[haraken, 2016/08/16 13:27:13]

Use DCHECK_EQ.

[keishi, 2016/08/17 11:18:29]

Done.

+            }
+        }
+
+#if defined(ADDRESS_SANITIZER)
         // TODO(haraken): What we really want to check here is that the pointer
         // is a traceable object. In other words, the pointer is either of:
         //
@@ -145,15 +169,90 @@ protected:
         if (IsFullyDefined<T>::value && !IsGarbageCollectedMixin<T>::value)
             ASSERT(HeapObjectHeader::fromPayload(m_raw)->checkHeader());
 #endif
+#endif
+    }
+
+    void saveCreationThreadHeap()

[keishi, 2016/08/16 13:05:50]

|this| may be on the stack, and if it is we can't tell it apart so I record
m_creationThreadHeap.

+    {
+#if DCHECK_IS_ON()
+        if (ThreadState::current() && tracenessConfiguration != UntracedMemberConfiguration)

[haraken, 2016/08/16 13:27:13]

Do we really need to check ThreadState::current()?

As far as I understand, if ThreadState::current() is nullptr, Member/WeakMember
should not be created. UntracedMember may be allowed though.

[keishi, 2016/08/17 11:18:28]

Done.

+            m_creationThreadHeap = &ThreadState::current()->heap();
+        else
+            m_creationThreadHeap = nullptr;
+#endif
     }
 
     T* m_raw;
+#if DCHECK_IS_ON()
+    const ThreadHeap* m_creationThreadHeap;
+#endif
 
     template<bool x, WTF::WeakHandlingFlag y, WTF::ShouldWeakPointersBeMarkedStrongly z, typename U, typename V> friend struct CollectionBackingTraceTrait;
     friend class Visitor;
 
 };
 
+// Members are used in classes to contain strong pointers to other oilpan heap
+// allocated objects.
+// All Member fields of a class must be traced in the class' trace method.
+// During the mark phase of the GC all live objects are marked as live and
+// all Member fields of a live object will be traced marked as live as well.
+template<typename T>
+class Member : public MemberBase<T, TracedMemberConfiguration> {
+    DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
+    typedef MemberBase<T, TracedMemberConfiguration> Parent;
+public:
+    Member() : Parent() { }
+    Member(std::nullptr_t) : Parent(nullptr) { }
+    Member(T* raw) : Parent(raw) { }
+    Member(T& raw) : Parent(raw) { }
+    Member(WTF::HashTableDeletedValueType x) : Parent(x) { }
+
+    Member(const Member& other) : Parent(other) { }
+    template<typename U>
+    Member(const Member<U>& other) : Parent(other) { }
+    template<typename U>
+    Member(const Persistent<U>& other) : Parent(other) { }
+
+    template<typename U>
+    Member& operator=(const Persistent<U>& other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    template<typename U>
+    Member& operator=(const Member<U>& other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    template<typename U>
+    Member& operator=(const WeakMember<U>& other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    template<typename U>
+    Member& operator=(U* other)
+    {
+        Parent::operator=(other);
+        return *this;
+    }
+
+    Member& operator=(std::nullptr_t)
+    {
+        Parent::operator=(nullptr);
+        return *this;
+    }
+
+protected:
+    template<bool x, WTF::WeakHandlingFlag y, WTF::ShouldWeakPointersBeMarkedStrongly z, typename U, typename V> friend struct CollectionBackingTraceTrait;
+    friend class Visitor;
+};
+
 // WeakMember is similar to Member in that it is used to point to other oilpan
 // heap allocated objects.
 // However instead of creating a strong pointer to the object, the WeakMember creates
@@ -161,21 +260,22 @@ protected:
 // to a heap allocated object are weak the object will be garbage collected. At the
 // time of GC the weak pointers will automatically be set to null.
 template<typename T>
-class WeakMember : public Member<T> {
+class WeakMember : public MemberBase<T, TracedMemberConfiguration> {
+    typedef MemberBase<T, TracedMemberConfiguration> Parent;
 public:
-    WeakMember() : Member<T>() { }
+    WeakMember() : Parent() { }
 
-    WeakMember(std::nullptr_t) : Member<T>(nullptr) { }
+    WeakMember(std::nullptr_t) : Parent(nullptr) { }
 
-    WeakMember(T* raw) : Member<T>(raw) { }
+    WeakMember(T* raw) : Parent(raw) { }
 
-    WeakMember(WTF::HashTableDeletedValueType x) : Member<T>(x) { }
+    WeakMember(WTF::HashTableDeletedValueType x) : Parent(x) { }
 
     template<typename U>
-    WeakMember(const Persistent<U>& other) : Member<T>(other) { }
+    WeakMember(const Persistent<U>& other) : Parent(other) { }
 
     template<typename U>
-    WeakMember(const Member<U>& other) : Member<T>(other) { }
+    WeakMember(const Member<U>& other) : Parent(other) { }
 
     template<typename U>
     WeakMember& operator=(const Persistent<U>& other)
@@ -222,21 +322,22 @@ private:
 // course, it must be guaranteed that the pointing on-heap object is kept alive
 // while the raw pointer is pointing to the object.
 template<typename T>
-class UntracedMember final : public Member<T> {
+class UntracedMember final : public MemberBase<T, UntracedMemberConfiguration> {
+    typedef MemberBase<T, UntracedMemberConfiguration> Parent;
 public:
-    UntracedMember() : Member<T>() { }
+    UntracedMember() : Parent() { }
 
-    UntracedMember(std::nullptr_t) : Member<T>(nullptr) { }
+    UntracedMember(std::nullptr_t) : Parent(nullptr) { }
 
-    UntracedMember(T* raw) : Member<T>(raw) { }
+    UntracedMember(T* raw) : Parent(raw) { }
 
     template<typename U>
-    UntracedMember(const Persistent<U>& other) : Member<T>(other) { }
+    UntracedMember(const Persistent<U>& other) : Parent(other) { }
 
     template<typename U>
-    UntracedMember(const Member<U>& other) : Member<T>(other) { }
+    UntracedMember(const Member<U>& other) : Parent(other) { }
 
-    UntracedMember(WTF::HashTableDeletedValueType x) : Member<T>(x) { }
+    UntracedMember(WTF::HashTableDeletedValueType x) : Parent(x) { }
 
     template<typename U>
     UntracedMember& operator=(const Persistent<U>& other)