Remove all but one use of WeakPtrFactory::DetachFromThread.

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
 // object vanishing and e.g. tasks posted to it being silently dropped.
 // Reference-counting such an object would complicate the ownership graph and
 // make it harder to reason about the object's lifetime.
 
 // EXAMPLE:
 //
 //  class Controller {
 //   public:
 //    void SpawnWorker() { Worker::StartNew(weak_factory_.GetWeakPtr()); }
 //    void WorkComplete(const Result& result) { ... }
 //   private:
+//    // Member variables should appear before the WeakPtrFactory.

[awong, 2013/05/15 01:19:15]

Explain why.

[Wez, 2013/05/15 22:51:50]

Done.

 //    WeakPtrFactory<Controller> weak_factory_;
 //  };
 //
 //  class Worker {
 //   public:
 //    static void StartNew(const WeakPtr<Controller>& controller) {
 //      Worker* worker = new Worker(controller);
 //      // Kick off asynchronous processing...
 //    }
 //   private:
 //    Worker(const WeakPtr<Controller>& controller)
 //        : controller_(controller) {}
 //    void DidCompleteAsynchronousProcessing(const Result& result) {
 //      if (controller_)
 //        controller_->WorkComplete(result);
 //    }
 //    WeakPtr<Controller> controller_;
 //  };
 //
 // With this implementation a caller may use SpawnWorker() to dispatch multiple
 // Workers and subsequently delete the Controller, without waiting for all
 // Workers to have completed.
 
 // ------------------------- IMPORTANT: Thread-safety -------------------------
 
 // Weak pointers must always be dereferenced and invalidated on the same thread
-// otherwise checking the pointer would be racey.  WeakPtrFactory enforces this
-// by binding itself to the current thread when a WeakPtr is first created
-// and un-binding only when those pointers are invalidated.  WeakPtrs may still
-// be handed off to other threads, however, so long as they are only actually
-// dereferenced on the originating thread. This includes posting tasks to the
-// thread using base::Bind() to invoke a method on the object via the WeakPtr.
+// otherwise checking the pointer would be racey.
 
-// Calling SupportsWeakPtr::DetachFromThread() can work around the limitations
-// above and cancel the thread binding of the object and all WeakPtrs pointing
-// to it, but it's not recommended and unsafe.  See crbug.com/232143.
+// To check this, a WeakPtrFactory and valid WeakPtrs it has issued become bound
+// to the calling thread, either when one of the pointers is dereferenced, or
+// when they are invalidated.  When a factory's pointers are invalidated, the
+// factory itself returns to being un-bound, and it and the object it dispenses
+// pointers for may be moved to a new thread.

[awong, 2013/05/15 01:19:15]

Would it be useful to list a reduction of the following snippet in the bug:

1. When a[n object containing a] WeakPtrFactory is created, it is un-bound.
2. Dispensing a WeakPtr from WeakPtrFactory does not cause it to become bound.
3. When a WeakPtr is dereferenced, it and the creating WeakPtrFactory become
bound.
4. When a WeakPtrFactory is invalidated, issued WeakPtrs become bound, and the
WeakPtrFactory itself becomes un-bound.

Or would it be too much info?

[Wez, 2013/05/15 22:51:50]

I've added that in.  It feels to me a little too implementation-detail oriented
- I'm thinking of a cleaner model for expressing this.

+
+// This allows a WeakPtr to an object to be created on one thread, but used to
+// post tasks to it on a second thread, so long as the WeakPtr is eventually
+// invalidated (e.g. by the object being deleted) on the second thread.
+
+// It also allows a WeakPtr to be created on one thread, and passed to a second,
+// for instance to use to post tasks back to the object on the first thread.
 
 #ifndef BASE_MEMORY_WEAK_PTR_H_
 #define BASE_MEMORY_WEAK_PTR_H_
 
 #include "base/basictypes.h"
 #include "base/base_export.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/template_util.h"
 #include "base/threading/thread_checker.h"
 
 namespace base {
 
 template <typename T> class SupportsWeakPtr;
 template <typename T> class WeakPtr;
 
 namespace internal {
 // These classes are part of the WeakPtr implementation.
 // DO NOT USE THESE CLASSES DIRECTLY YOURSELF.
 
 class BASE_EXPORT WeakReference {
  public:
-  // While Flag is bound to a specific thread, it may be deleted from another
+  // Although Flag is bound to a specific thread, it may be deleted from another
   // via base::WeakPtr::~WeakPtr().
   class Flag : public RefCountedThreadSafe<Flag> {
    public:
     Flag();
 
     void Invalidate();
     bool IsValid() const;
 
-    void DetachFromThread() { thread_checker_.DetachFromThread(); }
+    // Remove this when crbug.com/234964 is addressed.
+    void DetachFromThreadHack() { thread_checker_.DetachFromThread(); }
 
    private:
     friend class base::RefCountedThreadSafe<Flag>;
 
     ~Flag();
 
     ThreadChecker thread_checker_;
     bool is_valid_;
   };
 
@@ skipping 13 matching lines @@
   ~WeakReferenceOwner();
 
   WeakReference GetRef() const;
 
   bool HasRefs() const {
     return flag_.get() && !flag_->HasOneRef();
   }
 
   void Invalidate();
 
-  // Indicates that this object will be used on another thread from now on.
-  // Do not use this in new code. See crbug.com/232143.
-  void DetachFromThread() {
-    if (flag_) flag_->DetachFromThread();
+  // Remove this when crbug.com/234964 is addressed.
+  void DetachFromThreadHack() {
+    if (flag_) flag_->DetachFromThreadHack();
   }
 
  private:
   mutable scoped_refptr<WeakReference::Flag> flag_;
 };
 
 // This class simplifies the implementation of WeakPtr's type conversion
 // constructor by avoiding the need for a public accessor for ref_.  A
 // WeakPtr<T> cannot access the private members of WeakPtr<U>, so this
 // base class gives us a way to access ref_ in a protected fashion.
@@ skipping 125 matching lines @@
     DCHECK(ptr_);
     weak_reference_owner_.Invalidate();
   }
 
   // Call this method to determine if any weak pointers exist.
   bool HasWeakPtrs() const {
     DCHECK(ptr_);
     return weak_reference_owner_.HasRefs();
   }
 
-  // Indicates that this object will be used on another thread from now on.
-  // Do not use this in new code. See crbug.com/232143.
-  void DetachFromThread() {
-    DCHECK(ptr_);
-    weak_reference_owner_.DetachFromThread();
-  }
-
  private:
   internal::WeakReferenceOwner weak_reference_owner_;
   T* ptr_;
   DISALLOW_IMPLICIT_CONSTRUCTORS(WeakPtrFactory);
 };
 
 // A class may extend from SupportsWeakPtr to let others take weak pointers to
 // it. This avoids the class itself implementing boilerplate to dispense weak
 // pointers.  However, since SupportsWeakPtr's destructor won't invalidate
 // weak pointers to the class until after the derived class' members have been
 // destroyed, its use can lead to subtle use-after-destroy issues.
 template <class T>
 class SupportsWeakPtr : public internal::SupportsWeakPtrBase {
  public:
   SupportsWeakPtr() {}
 
   WeakPtr<T> AsWeakPtr() {
     return WeakPtr<T>(weak_reference_owner_.GetRef(), static_cast<T*>(this));
   }
 
-  // Indicates that this object will be used on another thread from now on.
-  // Do not use this in new code. See crbug.com/232143.
-  void DetachFromThread() {
-    weak_reference_owner_.DetachFromThread();
+  // Removes the binding, if any, from this object to a particular thread.
+  // This is used in WebGraphicsContext3DInProcessCommandBufferImpl to work-
+  // around access to cmmand buffer objects by more than one thread.
+  // Remove this when crbug.com/234964 is addressed.
+  void DetachFromThreadHack() {
+    weak_reference_owner_.DetachFromThreadHack();
   }
 
  protected:
   ~SupportsWeakPtr() {}
 
  private:
   internal::WeakReferenceOwner weak_reference_owner_;
   DISALLOW_COPY_AND_ASSIGN(SupportsWeakPtr);
 };
 
@@ skipping 16 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_