Rename/re-comment tests and added new death tests for WeakPtr

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 help in cases where you have many objects referring back to a
 // shared object and you wish for the lifetime of the shared object to not be
 // bound to the lifetime of the referrers.  In other words, this is useful when
 // reference counting is not a good fit.
 //
-// Thread-safety notes:
-// When you get a WeakPtr (from a WeakPtrFactory or SupportsWeakPtr),
-// the WeakPtr becomes bound to the current thread. You may only
-// dereference the WeakPtr on that thread. However, it is safe to
-// destroy the WeakPtr object on another thread.
-// Since a WeakPtr object may be destroyed on a background thread,
-// querying WeakPtrFactory's HasWeakPtrs() method can be racy.
-//
-//
 // A common alternative to weak pointers is to have the shared object hold a
 // list of all referrers, and then when the shared object is destroyed, it
 // calls a method on the referrers to tell them to drop their references.  This
 // approach also requires the referrers to tell the shared object when they get
 // destroyed so that the shared object can remove the referrer from its list of
 // referrers.  Such a solution works, but it is a bit complex.
 //
 // EXAMPLE:
 //
 //  class Controller : public SupportsWeakPtr<Controller> {
@@ skipping 18 matching lines @@
 //    WeakPtr<Controller> controller_;
 //  };
 //
 // Given the above classes, a consumer may allocate a Controller object, call
 // SpawnWorker several times, and then destroy the Controller object before all
 // of the workers have completed.  Because the Worker class only holds a weak
 // pointer to the Controller, we don't have to worry about the Worker
 // dereferencing the Controller back pointer after the Controller has been
 // destroyed.
 //
+// ------------------------ Thread-safety notes ------------------------
+// When you get a WeakPtr (from a WeakPtrFactory or SupportsWeakPtr), the
+// WeakPtr becomes bound to the current thread. You may only dereference the
+// WeakPtr on that thread. However, it is safe to destroy the WeakPtr object
+// on another thread. Since a WeakPtr object may be destroyed on a background
+// thread, querying WeakPtrFactory's HasWeakPtrs() method can be racy.
+//
+// On the other hand, the object that supports WeakPtr (extends SupportsWeakPtr)
+// will be bound to the first thread that creates a WeakPtr pointing to it,
+// and can not be deleted from other threads unless all WeakPtrs are deleted.

[Ryan Sleevi, 2012/07/10 22:41:54]

nit: s/unless/until/

[kaiwang, 2012/07/11 01:17:05]

Done.

+// Calling SupportsWeakPtr::DetachFromThread() can work around this and cancel
+// the thread binding, but it's not recommended and unsafe.

[Ryan Sleevi, 2012/07/10 22:41:54]

I almost wonder if we shouldn't remove DetachFromThread. I'm curious if it's
being used, and if the consumers are correct.

However, for sake of simplicitly, I think providing examples of "how to use it
correctly" may be... well.. too complex.

[kaiwang, 2012/07/11 01:17:05]

Searched the code and seems this function is used but not in many places.

+//
+// WeakPtrs are usually copied by other threads. Note the copied WeakPtr is

[Ryan Sleevi, 2012/07/10 22:41:54]

nit: s/are usually/may be/.
nit: drop the "Note"

Write the comments assuming no other code exists - talking about how it
should/can be used, not how it commonly is used.


The following may be better
"WeakPtrs may be copy-constructed, assigned, or destructed on threads other than
the thread they are bound to. However, any copies will remain bound to the
thread that the WeakPtr was originally created on, and thus may only be
dereferenced on that thread."

[kaiwang, 2012/07/11 01:17:05]

Done. Good suggestion :)

+// still bound to original thread, not the thread doing the copy. So the second
+// thread can delete the copied WeakPtr but can not dereference it.
 
 #ifndef BASE_MEMORY_WEAK_PTR_H_
 #define BASE_MEMORY_WEAK_PTR_H_
 #pragma once
 
 #include "base/basictypes.h"
 #include "base/base_export.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/template_util.h"
@@ skipping 151 matching lines @@
     ref_ = internal::WeakReference();
     ptr_ = NULL;
   }
 
  private:
   friend class internal::SupportsWeakPtrBase;
   friend class SupportsWeakPtr<T>;
   friend class WeakPtrFactory<T>;
 
   WeakPtr(const internal::WeakReference& ref, T* ptr)
-      : WeakPtrBase(ref), ptr_(ptr) {
+      : WeakPtrBase(ref),
+        ptr_(ptr) {
   }
 
   // This pointer is only valid when ref_.is_valid() is true.  Otherwise, its
   // value is undefined (as opposed to NULL).
   T* ptr_;
 };
 
 // A class may extend from SupportsWeakPtr to expose weak pointers to itself.
 // This is useful in cases where you want others to be able to get a weak
 // pointer to your class.  It also has the property that you don't need to
@@ skipping 83 matching lines @@
 
  private:
   internal::WeakReferenceOwner weak_reference_owner_;
   T* ptr_;
   DISALLOW_IMPLICIT_CONSTRUCTORS(WeakPtrFactory);
 };
 
 }  // namespace base
 
 #endif  // BASE_MEMORY_WEAK_PTR_H_