Remove obsolete WeakPtr hack.

base/memory/weak_ptr_unittest.cc

Index: base/memory/weak_ptr_unittest.cc
diff --git a/base/memory/weak_ptr_unittest.cc b/base/memory/weak_ptr_unittest.cc
index be3b6dbf6d1e6104435bc8a29e1cbda42621e946..d9d09991b97468a8995f460bbec68f7a40b9b1e4 100644
--- a/base/memory/weak_ptr_unittest.cc
+++ b/base/memory/weak_ptr_unittest.cc
@@ -44,7 +44,10 @@ struct Base {
 struct Derived : public Base {};
 
 struct TargetBase {};
-struct Target : public TargetBase, public SupportsWeakPtr<Target> {};
+struct Target : public TargetBase, public SupportsWeakPtr<Target> {
+ public:
+  void InvalidateWeakPtrs() { SupportsWeakPtr::InvalidateWeakPtrs(); }
+};
 struct DerivedTarget : public Target {};
 struct Arrow {
   WeakPtr<Target> target;
@@ -122,6 +125,16 @@ class BackgroundThread : public Thread {
     return result;
   }
 
+  void InvalidateArrowsForTarget(Target* target) {
+    WaitableEvent completion(true, false);
+    message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(&BackgroundThread::DoInvalidateArrowsForTarget,
+                   target,
+                   &completion));
+    completion.Wait();
+  }
+
  protected:
   static void DoCreateArrowFromArrow(Arrow** arrow,
                                      const Arrow* other,
@@ -174,6 +187,12 @@ class BackgroundThread : public Thread {
     delete object;
     completion->Signal();
   }
+
+  static void DoInvalidateArrowsForTarget(Target* target, WaitableEvent*
+                                          completion) {
+    target->InvalidateWeakPtrs();
+    completion->Signal();
+  }
 };
 
 }  // namespace
@@ -336,39 +355,61 @@ TEST(WeakPtrTest, MoveOwnershipImplicitly) {
   background.DeleteArrow(arrow);
 }
 
-TEST(WeakPtrTest, MoveOwnershipExplicitlyObjectNotReferenced) {
-  // Case 1: The target is not bound to any thread yet. So calling
-  // DetachFromThread() is a no-op.
-  Target target;
-  target.DetachFromThreadHack();
+TEST(WeakPtrTest, MoveOwnershipExplicitly) {
+  BackgroundThread background;
+  background.Start();
 
-  // Case 2: The target is bound to main thread but no WeakPtr is pointing to
-  // it. In this case, it will be re-bound to any thread trying to get a
-  // WeakPtr pointing to it. So detach function call is again no-op.
+  Arrow* arrow;
   {
-    WeakPtr<Target> weak_ptr = target.AsWeakPtr();
+    Target target;
+    // Background thread creates WeakPtr.
+    background.CreateArrowFromTarget(&arrow, &target);
+
+    // Bind to background thread.
+    EXPECT_EQ(&target, background.DeRef(arrow));
+
+    // Release the only WeakPtr.
+    arrow->target.reset();

[no sievers, 2013/08/01 01:12:04]

Ok, to follow up on the original discussion:

This test is legal because releasing the WeakPtr here does not invalidate the
flag. And we only deref on the bound thread (line 369 and 378 after rebinding.)

[Wez, 2013/08/01 18:30:31]

Right, so this test is really MoveOwnershipOfUnreferencedObject.

[no sievers, 2013/08/01 20:57:58]

Done.

+
+    // Now we should be able to create a new reference from this thread.
+    arrow->target = target.AsWeakPtr();
+
+    // Re-bind to main thread.
+    EXPECT_EQ(&target, arrow->target.get());
+
+    // And the main thread can now delete the target.
   }
-  target.DetachFromThreadHack();
+
+  // WeakPtr can be deleted on non-owner thread.
+  background.DeleteArrow(arrow);
 }
 
-TEST(WeakPtrTest, MoveOwnershipExplicitly) {
+TEST(WeakPtrTest, MoveOwnershipExplicitlyWithInvalidate) {
   BackgroundThread background;
   background.Start();
 
   Arrow* arrow;
   {
     Target target;
-    // Background thread creates WeakPtr(and implicitly owns the object).
+    // Background thread creates WeakPtr.
     background.CreateArrowFromTarget(&arrow, &target);
+
+    // Bind to background thread.
     EXPECT_EQ(&target, background.DeRef(arrow));
 
-    // Detach from background thread.
-    target.DetachFromThreadHack();
+    background.InvalidateArrowsForTarget(&target);

[Wez, 2013/08/01 18:30:31]

If you can rewrite this specific test to use a WeakPtrFactory<> and WeakPtr
directly, rather than the Target and Arrow combination, it'll avoid
SupportsWeakPtr needing an InvalidateWeakPtrs, and the extra boilerplate in
Target.  WDYT?

+    EXPECT_EQ(NULL, background.DeRef(arrow));

[no sievers, 2013/08/01 01:12:04]

This is the same test as above, but it invalidates all WeakPtrs on the
background thread before rebinding.

+
+    // Release the only WeakPtr.
+    arrow->target.reset();

[Wez, 2013/08/01 18:30:31]

Don't do this here - the test is specifically for the case of other objects
continuing to hold WeakPtrs that used to refer to the object, but which are now
safely NULL thanks you the invalidation.

[no sievers, 2013/08/01 20:57:58]

Yea, that's making sense now. Done.

+
+    // Now we should be able to create a new reference from this thread.
+    arrow->target = target.AsWeakPtr();
 
     // Re-bind to main thread.
     EXPECT_EQ(&target, arrow->target.get());
 
-    // Main thread can now delete the target.
+    // And the main thread can now delete the target.
   }
 
   // WeakPtr can be deleted on non-owner thread.