Improve GetWeakPtr() to better protect against unsafe usage patterns.

base/memory/weak_ptr.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Weak pointers are pointers to an object that do not affect its lifetime,
 // and which may be invalidated (i.e. reset to NULL) by the object, or its
 // owner, at any time, most commonly when the object is about to be deleted.
 
 // Weak pointers are useful when an object needs to be accessed safely by one
 // or more objects other than its owner, and those callers can cope with the
@@ skipping 254 matching lines @@
 template <class T>
 class WeakPtrFactory {
  public:
   explicit WeakPtrFactory(T* ptr) : ptr_(ptr) {
   }
 
   ~WeakPtrFactory() {
     ptr_ = NULL;
   }
 
+  // Returns a new WeakPtr. This is not thread-safe - in general callers should

[dcheng, 2016/02/19 20:45:00]

"in general" is only true for code that wants a weak pointer to (presumably)
bind on multiple threads, right? This recommendation feels a bit too strong, and
it doesn't seem to be consistent with how it's typically used today.

[Wez, 2016/03/01 23:20:28]

I've reworded the comment to be more explicit about concurrency, but without
making it even more verbose it is non-trivial to see why it would _ever_ be safe
to call GetWeakPtr() directly on the factory from another thread, since that
means the caller is making sure the WeakPtrFactory is still live at the time, in
which case it's questionable why they need the WeakPtr...

I'm thinking that replacing the wall-of-words under THREAD SAFETY with some
simple SAFE and UNSAFE threading examples, with a brief comment here, would be
better - WDYT?

+  // take and store a WeakPtr to pass to code that needs to post tasks rather
+  // than call GetWeakPtr() from different threads.
   WeakPtr<T> GetWeakPtr() {
     DCHECK(ptr_);
     return WeakPtr<T>(weak_reference_owner_.GetRef(), ptr_);
   }
 
   // Call this method to invalidate all existing weak pointers.
   void InvalidateWeakPtrs() {
     DCHECK(ptr_);
     weak_reference_owner_.Invalidate();
   }
@@ skipping 51 matching lines @@
 //   base::WeakPtr<Derived> ptr = derived.AsWeakPtr();  // Fails.
 
 template <typename Derived>
 WeakPtr<Derived> AsWeakPtr(Derived* t) {
   return internal::SupportsWeakPtrBase::StaticAsWeakPtr<Derived>(t);
 }
 
 }  // namespace base
 
 #endif  // BASE_MEMORY_WEAK_PTR_H_