Commit 687a70a8 authored by keishi's avatar keishi Committed by Commit bot

Revert of Allow CrossThreadPersistent in collections (patchset #7 id:120001 of...

Revert of Allow CrossThreadPersistent in collections (patchset #7 id:120001 of https://codereview.chromium.org/2007283002/ )

Reason for revert:
blink_perf.bindings regression on android.webview
crbug.com/615832

Original issue's description:
> Allow CrossThreadPersistent in collections
>
> BUG=591606
>
> Committed: https://crrev.com/5adc4454691bffcbfdc7cd60866c145a4e67de0d
> Cr-Commit-Position: refs/heads/master@{#396410}

TBR=oilpan-reviews@chromium.org,haraken@chromium.org,sigbjornf@opera.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=591606

Review-Url: https://codereview.chromium.org/2028433003
Cr-Commit-Position: refs/heads/master@{#396772}
parent 6abb2c8a
......@@ -6,7 +6,6 @@
#define HeapAllocator_h
#include "platform/heap/Heap.h"
#include "platform/heap/Persistent.h"
#include "platform/heap/TraceTraits.h"
#include "wtf/Allocator.h"
#include "wtf/Assertions.h"
......@@ -423,16 +422,6 @@ template <typename T> struct VectorTraits<blink::UntracedMember<T>> : VectorTrai
static const bool canMoveWithMemcpy = true;
};
template <typename T, blink::WeaknessPersistentConfiguration weaknessConfiguration, blink::CrossThreadnessPersistentConfiguration crossThreadnessConfiguration>
struct VectorTraits<blink::PersistentBase<T, weaknessConfiguration, crossThreadnessConfiguration>>
: VectorTraitsBase<blink::PersistentBase<T, weaknessConfiguration, crossThreadnessConfiguration>> {
STATIC_ONLY(VectorTraits);
static const bool needsDestruction = true;
static const bool canInitializeWithMemset = true;
static const bool canClearUnusedSlotsWithMemset = false;
static const bool canMoveWithMemcpy = true;
};
template <typename T> struct VectorTraits<blink::HeapVector<T, 0>> : VectorTraitsBase<blink::HeapVector<T, 0>> {
STATIC_ONLY(VectorTraits);
static const bool needsDestruction = false;
......@@ -465,35 +454,53 @@ template <typename T, size_t inlineCapacity> struct VectorTraits<blink::HeapDequ
static const bool canMoveWithMemcpy = VectorTraits<T>::canMoveWithMemcpy;
};
template<typename T, typename H> struct HandleHashTraits : SimpleClassHashTraits<H> {
STATIC_ONLY(HandleHashTraits);
// TODO: The distinction between PeekInType and PassInType is there for
template<typename T> struct HashTraits<blink::Member<T>> : SimpleClassHashTraits<blink::Member<T>> {
STATIC_ONLY(HashTraits);
// FIXME: The distinction between PeekInType and PassInType is there for
// the sake of the reference counting handles. When they are gone the two
// types can be merged into PassInType.
// TODO: Implement proper const'ness for iterator types. Requires support
// FIXME: Implement proper const'ness for iterator types. Requires support
// in the marking Visitor.
using PeekInType = T*;
using PassInType = T*;
using IteratorGetType = H*;
using IteratorConstGetType = const H*;
using IteratorReferenceType = H&;
using IteratorConstReferenceType = const H&;
using IteratorGetType = blink::Member<T>*;
using IteratorConstGetType = const blink::Member<T>*;
using IteratorReferenceType = blink::Member<T>&;
using IteratorConstReferenceType = const blink::Member<T>&;
static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return *x; }
static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return *x; }
using PeekOutType = T*;
template<typename U>
static void store(const U& value, H& storage) { storage = value; }
static void store(const U& value, blink::Member<T>& storage) { storage = value; }
static PeekOutType peek(const H& value) { return value; }
static PeekOutType peek(const blink::Member<T>& value) { return value; }
};
template<typename T> struct HashTraits<blink::Member<T>> : HandleHashTraits<T, blink::Member<T>> { };
template<typename T> struct HashTraits<blink::WeakMember<T>> : HandleHashTraits<T, blink::WeakMember<T>> {
template<typename T> struct HashTraits<blink::WeakMember<T>> : SimpleClassHashTraits<blink::WeakMember<T>> {
STATIC_ONLY(HashTraits);
static const bool needsDestruction = false;
// FIXME: The distinction between PeekInType and PassInType is there for
// the sake of the reference counting handles. When they are gone the two
// types can be merged into PassInType.
// FIXME: Implement proper const'ness for iterator types. Requires support
// in the marking Visitor.
using PeekInType = T*;
using PassInType = T*;
using IteratorGetType = blink::WeakMember<T>*;
using IteratorConstGetType = const blink::WeakMember<T>*;
using IteratorReferenceType = blink::WeakMember<T>&;
using IteratorConstReferenceType = const blink::WeakMember<T>&;
static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return *x; }
static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return *x; }
using PeekOutType = T*;
template<typename U>
static void store(const U& value, blink::WeakMember<T>& storage) { storage = value; }
static PeekOutType peek(const blink::WeakMember<T>& value) { return value; }
template<typename VisitorDispatcher>
static bool traceInCollection(VisitorDispatcher visitor, blink::WeakMember<T>& weakMember, ShouldWeakPointersBeMarkedStrongly strongify)
......@@ -506,9 +513,27 @@ template<typename T> struct HashTraits<blink::WeakMember<T>> : HandleHashTraits<
}
};
template<typename T> struct HashTraits<blink::UntracedMember<T>> : HandleHashTraits<T, blink::UntracedMember<T>> {
template<typename T> struct HashTraits<blink::UntracedMember<T>> : SimpleClassHashTraits<blink::UntracedMember<T>> {
STATIC_ONLY(HashTraits);
static const bool needsDestruction = false;
// FIXME: The distinction between PeekInType and PassInType is there for
// the sake of the reference counting handles. When they are gone the two
// types can be merged into PassInType.
// FIXME: Implement proper const'ness for iterator types.
using PeekInType = T*;
using PassInType = T*;
using IteratorGetType = blink::UntracedMember<T>*;
using IteratorConstGetType = const blink::UntracedMember<T>*;
using IteratorReferenceType = blink::UntracedMember<T>&;
using IteratorConstReferenceType = const blink::UntracedMember<T>&;
static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return *x; }
static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return *x; }
using PeekOutType = T*;
template<typename U>
static void store(const U& value, blink::UntracedMember<T>& storage) { storage = value; }
static PeekOutType peek(const blink::UntracedMember<T>& value) { return value; }
};
template<typename T, size_t inlineCapacity>
......@@ -526,10 +551,6 @@ struct IsGarbageCollectedType<ListHashSetNode<T, blink::HeapListHashSetAllocator
static const bool value = true;
};
template<typename T> struct HashTraits<blink::Persistent<T>> : HandleHashTraits<T, blink::Persistent<T>> { };
template<typename T> struct HashTraits<blink::CrossThreadPersistent<T>> : HandleHashTraits<T, blink::CrossThreadPersistent<T>> { };
} // namespace WTF
#endif
......@@ -2897,244 +2897,6 @@ TEST(HeapTest, HeapCollectionTypes)
EXPECT_EQ(1u, dequeUW2->size());
}
TEST(HeapTest, PersistentVector)
{
IntWrapper::s_destructorCalls = 0;
typedef Vector<Persistent<IntWrapper>> PersistentVector;
Persistent<IntWrapper> one(IntWrapper::create(1));
Persistent<IntWrapper> two(IntWrapper::create(2));
Persistent<IntWrapper> three(IntWrapper::create(3));
Persistent<IntWrapper> four(IntWrapper::create(4));
Persistent<IntWrapper> five(IntWrapper::create(5));
Persistent<IntWrapper> six(IntWrapper::create(6));
{
PersistentVector vector;
vector.append(one);
vector.append(two);
conservativelyCollectGarbage();
EXPECT_TRUE(vector.contains(one));
EXPECT_TRUE(vector.contains(two));
vector.append(three);
vector.append(four);
conservativelyCollectGarbage();
EXPECT_TRUE(vector.contains(one));
EXPECT_TRUE(vector.contains(two));
EXPECT_TRUE(vector.contains(three));
EXPECT_TRUE(vector.contains(four));
vector.shrink(1);
conservativelyCollectGarbage();
EXPECT_TRUE(vector.contains(one));
EXPECT_FALSE(vector.contains(two));
EXPECT_FALSE(vector.contains(three));
EXPECT_FALSE(vector.contains(four));
}
{
PersistentVector vector1;
PersistentVector vector2;
vector1.append(one);
vector2.append(two);
vector1.swap(vector2);
conservativelyCollectGarbage();
EXPECT_TRUE(vector1.contains(two));
EXPECT_TRUE(vector2.contains(one));
}
{
PersistentVector vector1;
PersistentVector vector2;
vector1.append(one);
vector1.append(two);
vector2.append(three);
vector2.append(four);
vector2.append(five);
vector2.append(six);
vector1.swap(vector2);
conservativelyCollectGarbage();
EXPECT_TRUE(vector1.contains(three));
EXPECT_TRUE(vector1.contains(four));
EXPECT_TRUE(vector1.contains(five));
EXPECT_TRUE(vector1.contains(six));
EXPECT_TRUE(vector2.contains(one));
EXPECT_TRUE(vector2.contains(two));
}
}
TEST(HeapTest, CrossThreadPersistentVector)
{
IntWrapper::s_destructorCalls = 0;
typedef Vector<CrossThreadPersistent<IntWrapper>> CrossThreadPersistentVector;
CrossThreadPersistent<IntWrapper> one(IntWrapper::create(1));
CrossThreadPersistent<IntWrapper> two(IntWrapper::create(2));
CrossThreadPersistent<IntWrapper> three(IntWrapper::create(3));
CrossThreadPersistent<IntWrapper> four(IntWrapper::create(4));
CrossThreadPersistent<IntWrapper> five(IntWrapper::create(5));
CrossThreadPersistent<IntWrapper> six(IntWrapper::create(6));
{
CrossThreadPersistentVector vector;
vector.append(one);
vector.append(two);
conservativelyCollectGarbage();
EXPECT_TRUE(vector.contains(one));
EXPECT_TRUE(vector.contains(two));
vector.append(three);
vector.append(four);
conservativelyCollectGarbage();
EXPECT_TRUE(vector.contains(one));
EXPECT_TRUE(vector.contains(two));
EXPECT_TRUE(vector.contains(three));
EXPECT_TRUE(vector.contains(four));
vector.shrink(1);
conservativelyCollectGarbage();
EXPECT_TRUE(vector.contains(one));
EXPECT_FALSE(vector.contains(two));
EXPECT_FALSE(vector.contains(three));
EXPECT_FALSE(vector.contains(four));
}
{
CrossThreadPersistentVector vector1;
CrossThreadPersistentVector vector2;
vector1.append(one);
vector2.append(two);
vector1.swap(vector2);
conservativelyCollectGarbage();
EXPECT_TRUE(vector1.contains(two));
EXPECT_TRUE(vector2.contains(one));
}
{
CrossThreadPersistentVector vector1;
CrossThreadPersistentVector vector2;
vector1.append(one);
vector1.append(two);
vector2.append(three);
vector2.append(four);
vector2.append(five);
vector2.append(six);
vector1.swap(vector2);
conservativelyCollectGarbage();
EXPECT_TRUE(vector1.contains(three));
EXPECT_TRUE(vector1.contains(four));
EXPECT_TRUE(vector1.contains(five));
EXPECT_TRUE(vector1.contains(six));
EXPECT_TRUE(vector2.contains(one));
EXPECT_TRUE(vector2.contains(two));
}
}
TEST(HeapTest, PersistentSet)
{
IntWrapper::s_destructorCalls = 0;
typedef HashSet<Persistent<IntWrapper>> PersistentSet;
IntWrapper* oneRaw = IntWrapper::create(1);
Persistent<IntWrapper> one(oneRaw);
Persistent<IntWrapper> one2(oneRaw);
Persistent<IntWrapper> two(IntWrapper::create(2));
Persistent<IntWrapper> three(IntWrapper::create(3));
Persistent<IntWrapper> four(IntWrapper::create(4));
Persistent<IntWrapper> five(IntWrapper::create(5));
Persistent<IntWrapper> six(IntWrapper::create(6));
{
PersistentSet set;
set.add(one);
set.add(two);
conservativelyCollectGarbage();
EXPECT_TRUE(set.contains(one));
EXPECT_TRUE(set.contains(one2));
EXPECT_TRUE(set.contains(two));
set.add(three);
set.add(four);
conservativelyCollectGarbage();
EXPECT_TRUE(set.contains(one));
EXPECT_TRUE(set.contains(two));
EXPECT_TRUE(set.contains(three));
EXPECT_TRUE(set.contains(four));
set.clear();
conservativelyCollectGarbage();
EXPECT_FALSE(set.contains(one));
EXPECT_FALSE(set.contains(two));
EXPECT_FALSE(set.contains(three));
EXPECT_FALSE(set.contains(four));
}
{
PersistentSet set1;
PersistentSet set2;
set1.add(one);
set2.add(two);
set1.swap(set2);
conservativelyCollectGarbage();
EXPECT_TRUE(set1.contains(two));
EXPECT_TRUE(set2.contains(one));
EXPECT_TRUE(set2.contains(one2));
}
}
TEST(HeapTest, CrossThreadPersistentSet)
{
IntWrapper::s_destructorCalls = 0;
typedef HashSet<CrossThreadPersistent<IntWrapper>> CrossThreadPersistentSet;
IntWrapper* oneRaw = IntWrapper::create(1);
CrossThreadPersistent<IntWrapper> one(oneRaw);
CrossThreadPersistent<IntWrapper> one2(oneRaw);
CrossThreadPersistent<IntWrapper> two(IntWrapper::create(2));
CrossThreadPersistent<IntWrapper> three(IntWrapper::create(3));
CrossThreadPersistent<IntWrapper> four(IntWrapper::create(4));
CrossThreadPersistent<IntWrapper> five(IntWrapper::create(5));
CrossThreadPersistent<IntWrapper> six(IntWrapper::create(6));
{
CrossThreadPersistentSet set;
set.add(one);
set.add(two);
conservativelyCollectGarbage();
EXPECT_TRUE(set.contains(one));
EXPECT_TRUE(set.contains(one2));
EXPECT_TRUE(set.contains(two));
set.add(three);
set.add(four);
conservativelyCollectGarbage();
EXPECT_TRUE(set.contains(one));
EXPECT_TRUE(set.contains(two));
EXPECT_TRUE(set.contains(three));
EXPECT_TRUE(set.contains(four));
set.clear();
conservativelyCollectGarbage();
EXPECT_FALSE(set.contains(one));
EXPECT_FALSE(set.contains(two));
EXPECT_FALSE(set.contains(three));
EXPECT_FALSE(set.contains(four));
}
{
CrossThreadPersistentSet set1;
CrossThreadPersistentSet set2;
set1.add(one);
set2.add(two);
set1.swap(set2);
conservativelyCollectGarbage();
EXPECT_TRUE(set1.contains(two));
EXPECT_TRUE(set2.contains(one));
EXPECT_TRUE(set2.contains(one2));
}
}
class NonTrivialObject final {
DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
public:
......
......@@ -77,20 +77,12 @@ public:
checkPointer();
}
PersistentBase(WTF::HashTableDeletedValueType) : m_raw(reinterpret_cast<T*>(-1))
{
initialize();
checkPointer();
}
~PersistentBase()
{
uninitialize();
m_raw = nullptr;
}
bool isHashTableDeletedValue() const { return m_raw == reinterpret_cast<T*>(-1); }
template<typename VisitorDispatcher>
void trace(VisitorDispatcher visitor)
{
......@@ -192,7 +184,7 @@ private:
void initialize()
{
ASSERT(!m_persistentNode);
if (!m_raw || isHashTableDeletedValue())
if (!m_raw)
return;
TraceCallback traceCallback = TraceMethodDelegate<PersistentBase<T, weaknessConfiguration, crossThreadnessConfiguration>, &PersistentBase<T, weaknessConfiguration, crossThreadnessConfiguration>::trace>::trampoline;
......@@ -229,7 +221,7 @@ private:
void checkPointer()
{
#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
if (!m_raw || isHashTableDeletedValue())
if (!m_raw)
return;
// ThreadHeap::isHeapObjectAlive(m_raw) checks that m_raw is a traceable
......@@ -271,7 +263,6 @@ public:
Persistent(const Persistent<U>& other) : Parent(other) { }
template<typename U>
Persistent(const Member<U>& other) : Parent(other) { }
Persistent(WTF::HashTableDeletedValueType x) : Parent(x) { }
template<typename U>
Persistent& operator=(U* other)
......@@ -381,7 +372,6 @@ public:
CrossThreadPersistent(const CrossThreadPersistent<U>& other) : Parent(other) { }
template<typename U>
CrossThreadPersistent(const Member<U>& other) : Parent(other) { }
CrossThreadPersistent(WTF::HashTableDeletedValueType x) : Parent(x) { }
T* atomicGet() { return Parent::atomicGet(); }
......@@ -692,28 +682,6 @@ template<typename T, typename U> inline bool operator!=(const Persistent<T>& a,
namespace WTF {
template <typename T>
struct PersistentHash : MemberHash<T> {
STATIC_ONLY(PersistentHash);
};
template <typename T>
struct CrossThreadPersistentHash : MemberHash<T> {
STATIC_ONLY(CrossThreadPersistentHash);
};
template <typename T>
struct DefaultHash<blink::Persistent<T>> {
STATIC_ONLY(DefaultHash);
using Hash = PersistentHash<T>;
};
template <typename T>
struct DefaultHash<blink::CrossThreadPersistent<T>> {
STATIC_ONLY(DefaultHash);
using Hash = CrossThreadPersistentHash<T>;
};
template<typename T>
struct ParamStorageTraits<blink::WeakPersistentThisPointer<T>> {
STATIC_ONLY(ParamStorageTraits);
......
......@@ -124,8 +124,7 @@ void PersistentRegion::tracePersistentNodes(Visitor* visitor, ShouldTraceCallbac
bool CrossThreadPersistentRegion::shouldTracePersistentNode(Visitor* visitor, PersistentNode* node)
{
CrossThreadPersistent<DummyGCBase>* persistent = reinterpret_cast<CrossThreadPersistent<DummyGCBase>*>(node->self());
DCHECK(persistent);
DCHECK(!persistent->isHashTableDeletedValue());
ASSERT(persistent);
Address rawObject = reinterpret_cast<Address>(persistent->get());
if (!rawObject)
return false;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment