Remove all but one use of WeakPtrFactory::DetachFromThread.

base/memory/weak_ptr.h

Index: base/memory/weak_ptr.h
diff --git a/base/memory/weak_ptr.h b/base/memory/weak_ptr.h
index 19def01eb70f1c1a8ece45b7096e8475c74ad342..ee6435e52c249587788f53b79a2333db44a2998a 100644
--- a/base/memory/weak_ptr.h
+++ b/base/memory/weak_ptr.h
@@ -19,6 +19,7 @@
 //    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_;
 //  };
 //
@@ -45,16 +46,20 @@
 // ------------------------- 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_
@@ -77,7 +82,7 @@ namespace internal {
 
 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:
@@ -86,7 +91,8 @@ class BASE_EXPORT WeakReference {
     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>;
@@ -120,10 +126,9 @@ class BASE_EXPORT WeakReferenceOwner {
 
   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:
@@ -269,13 +274,6 @@ class WeakPtrFactory {
     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_;
@@ -296,10 +294,12 @@ class SupportsWeakPtr : public internal::SupportsWeakPtrBase {
     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: