Index: base/memory/weak_ptr.h |
diff --git a/base/memory/weak_ptr.h b/base/memory/weak_ptr.h |
index 3544439dd3cafc9d8cc849a2cb4181cf70d2e947..bfa17685a374cc7e4f6a13b4ca175d8b3ab3f42f 100644 |
--- a/base/memory/weak_ptr.h |
+++ b/base/memory/weak_ptr.h |
@@ -95,17 +95,46 @@ class BASE_EXPORT WeakReference { |
class BASE_EXPORT Flag : public RefCountedThreadSafe<Flag> { |
public: |
Flag(); |
- |
- void Invalidate(); |
- bool IsValid() const; |
+ static Flag* nullFlag(); |
Nico
2017/06/28 14:54:10
google (and chromium) style spells functions NullF
hans
2017/06/28 20:37:35
Done.
|
+ |
+ void Invalidate() { |
+#if DCHECK_IS_ON() |
+ if (this == nullFlag()) |
+ return; |
Nico
2017/06/28 14:54:09
This looks kind of confusing – do you not want to
hans
2017/06/28 20:37:35
I've tried to make this clearer with a comment. Th
|
+ // The flag being invalidated with a single ref implies that there are no |
+ // weak pointers in existence. Allow deletion on other thread in this |
+ // case. |
+ DCHECK(sequence_checker_.CalledOnValidSequence() || HasOneRef()) |
+ << "WeakPtrs must be invalidated on the same sequenced thread."; |
+#endif |
+ is_valid_ = 0; |
+ } |
+ uintptr_t IsValid() const { |
Nico
2017/06/28 14:54:10
Add comment that this returns either all 0s or all
hans
2017/06/28 20:37:35
Done.
|
+#if DCHECK_IS_ON() |
+ if (this == nullFlag()) { |
+ DCHECK(!is_valid_); |
+ return 0; |
+ } |
+ DCHECK(sequence_checker_.CalledOnValidSequence()) |
+ << "WeakPtrs must be checked on the same sequenced thread."; |
+#endif |
+ return is_valid_; |
+ } |
private: |
friend class base::RefCountedThreadSafe<Flag>; |
+ enum NullFlagTag { kNullFlagTag }; |
+ Flag(NullFlagTag); |
+ |
~Flag(); |
+ uintptr_t is_valid_; |
+#if DCHECK_IS_ON() |
+ // Even if SequenceChecker is an empty class in non-dcheck builds, it still |
+ // takes up space in the class. |
Nico
2017/06/28 14:36:56
This bit seems unrelated to what's in the CL descr
hans
2017/06/28 20:37:35
It's related in that it keeps the size from growin
Nico
2017/06/28 20:57:03
Ah ok, sounds good then
|
SequenceChecker sequence_checker_; |
- bool is_valid_; |
+#endif |
}; |
WeakReference(); |
@@ -117,9 +146,11 @@ class BASE_EXPORT WeakReference { |
WeakReference& operator=(WeakReference&& other) = default; |
WeakReference& operator=(const WeakReference& other) = default; |
- bool is_valid() const; |
+ uintptr_t is_valid() const { return flag_->IsValid(); } |
private: |
+ // Note: To avoid null-checks, flag_ always points to either Flag::nullFlag() |
+ // or some other object. |
scoped_refptr<const Flag> flag_; |
}; |
@@ -130,9 +161,7 @@ class BASE_EXPORT WeakReferenceOwner { |
WeakReference GetRef() const; |
- bool HasRefs() const { |
- return flag_.get() && !flag_->HasOneRef(); |
- } |
+ bool HasRefs() const { return flag_->IsValid() && !flag_->HasOneRef(); } |
void Invalidate(); |
@@ -158,6 +187,10 @@ class BASE_EXPORT WeakPtrBase { |
explicit WeakPtrBase(const WeakReference& ref); |
WeakReference ref_; |
+ |
+ // This pointer is only valid when ref_.is_valid() is true. Otherwise, its |
+ // value is undefined (as opposed to nullptr). |
+ uintptr_t ptr_; |
Nico
2017/06/28 14:54:10
Moving this to the base class also feels kind of u
hans
2017/06/28 20:37:35
Done. Splitting this out.
|
}; |
// This class provides a common implementation of common functions that would |
@@ -185,7 +218,8 @@ class SupportsWeakPtrBase { |
static WeakPtr<Derived> AsWeakPtrImpl( |
Derived* t, const SupportsWeakPtr<Base>&) { |
WeakPtr<Base> ptr = t->Base::AsWeakPtr(); |
- return WeakPtr<Derived>(ptr.ref_, static_cast<Derived*>(ptr.ptr_)); |
+ return WeakPtr<Derived>( |
+ ptr.ref_, static_cast<Derived*>(reinterpret_cast<Base*>(ptr.ptr_))); |
} |
}; |
@@ -209,20 +243,24 @@ template <typename T> class WeakPtrFactory; |
template <typename T> |
class WeakPtr : public internal::WeakPtrBase { |
public: |
- WeakPtr() : ptr_(nullptr) {} |
+ WeakPtr() {} |
- WeakPtr(std::nullptr_t) : ptr_(nullptr) {} |
+ WeakPtr(std::nullptr_t) {} |
// Allow conversion from U to T provided U "is a" T. Note that this |
// is separate from the (implicit) copy and move constructors. |
template <typename U> |
- WeakPtr(const WeakPtr<U>& other) : WeakPtrBase(other), ptr_(other.ptr_) { |
+ WeakPtr(const WeakPtr<U>& other) : WeakPtrBase(other) { |
+ static_assert(std::is_base_of<T, U>::value, |
+ "other must be a subclass of T"); |
} |
template <typename U> |
- WeakPtr(WeakPtr<U>&& other) |
- : WeakPtrBase(std::move(other)), ptr_(other.ptr_) {} |
+ WeakPtr(WeakPtr<U>&& other) : WeakPtrBase(std::move(other)) { |
+ static_assert(std::is_base_of<T, U>::value, |
+ "other must be a subclass of T"); |
+ } |
- T* get() const { return ref_.is_valid() ? ptr_ : nullptr; } |
+ T* get() const { return reinterpret_cast<T*>(ref_.is_valid() & ptr_); } |
Nico
2017/06/28 14:54:09
Since this is so unusual, I'd probably add `// Int
hans
2017/06/28 20:37:35
Done.
|
T& operator*() const { |
DCHECK(get() != nullptr); |
@@ -235,7 +273,7 @@ class WeakPtr : public internal::WeakPtrBase { |
void reset() { |
ref_ = internal::WeakReference(); |
- ptr_ = nullptr; |
+ ptr_ = 0; |
} |
// Allow conditionals to test validity, e.g. if (weak_ptr) {...}; |
@@ -247,14 +285,9 @@ class WeakPtr : public internal::WeakPtrBase { |
friend class SupportsWeakPtr<T>; |
friend class WeakPtrFactory<T>; |
- WeakPtr(const internal::WeakReference& ref, T* ptr) |
- : WeakPtrBase(ref), |
- ptr_(ptr) { |
+ WeakPtr(const internal::WeakReference& ref, T* ptr) : WeakPtrBase(ref) { |
+ ptr_ = reinterpret_cast<uintptr_t>(ptr); |
} |
- |
- // This pointer is only valid when ref_.is_valid() is true. Otherwise, its |
- // value is undefined (as opposed to nullptr). |
- T* ptr_; |
}; |
// Allow callers to compare WeakPtrs against nullptr to test validity. |
@@ -275,22 +308,33 @@ bool operator==(std::nullptr_t, const WeakPtr<T>& weak_ptr) { |
return weak_ptr == nullptr; |
} |
+namespace internal { |
+class BASE_EXPORT WeakPtrFactoryBase { |
+ protected: |
+ WeakPtrFactoryBase(uintptr_t ptr); |
+ ~WeakPtrFactoryBase(); |
+ internal::WeakReferenceOwner weak_reference_owner_; |
+ uintptr_t ptr_; |
+}; |
+} // namespace internal |
+ |
// A class may be composed of a WeakPtrFactory and thereby |
// control how it exposes weak pointers to itself. This is helpful if you only |
// need weak pointers within the implementation of a class. This class is also |
// useful when working with primitive types. For example, you could have a |
// WeakPtrFactory<bool> that is used to pass around a weak reference to a bool. |
template <class T> |
-class WeakPtrFactory { |
+class WeakPtrFactory : public internal::WeakPtrFactoryBase { |
public: |
- explicit WeakPtrFactory(T* ptr) : ptr_(ptr) { |
- } |
+ explicit WeakPtrFactory(T* ptr) |
+ : WeakPtrFactoryBase(reinterpret_cast<uintptr_t>(ptr)) {} |
- ~WeakPtrFactory() { ptr_ = nullptr; } |
+ ~WeakPtrFactory() {} |
WeakPtr<T> GetWeakPtr() { |
DCHECK(ptr_); |
- return WeakPtr<T>(weak_reference_owner_.GetRef(), ptr_); |
+ return WeakPtr<T>(weak_reference_owner_.GetRef(), |
+ reinterpret_cast<T*>(ptr_)); |
} |
// Call this method to invalidate all existing weak pointers. |
@@ -306,8 +350,6 @@ class WeakPtrFactory { |
} |
private: |
- internal::WeakReferenceOwner weak_reference_owner_; |
- T* ptr_; |
DISALLOW_IMPLICIT_CONSTRUCTORS(WeakPtrFactory); |
}; |