Remove obsolete WeakPtr hack.

base/memory/weak_ptr_unittest.cc

 // 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.
 
 #include "base/memory/weak_ptr.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/debug/leak_annotations.h"
@@ skipping 26 matching lines @@
     *result = new T;
   }
 };
 
 struct Base {
   std::string member;
 };
 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;
 };
 
 // Helper class to create and destroy weak pointer copies
 // and delete objects on a background thread.
 class BackgroundThread : public Thread {
  public:
   BackgroundThread() : Thread("owner_thread") {}
@@ skipping 57 matching lines @@
   Target* DeRef(const Arrow* arrow) {
     WaitableEvent completion(true, false);
     Target* result = NULL;
     message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&BackgroundThread::DoDeRef, arrow, &result, &completion));
     completion.Wait();
     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,
                                      WaitableEvent* completion) {
     *arrow = new Arrow;
     **arrow = *other;
     completion->Signal();
   }
 
   static void DoCreateArrowFromTarget(Arrow** arrow,
@@ skipping 32 matching lines @@
     // Assignment operator.
     WeakPtr<TargetBase> c;
     c = object->target;
     completion->Signal();
   }
 
   static void DoDeleteArrow(Arrow* object, WaitableEvent* completion) {
     delete object;
     completion->Signal();
   }
+
+  static void DoInvalidateArrowsForTarget(Target* target, WaitableEvent*
+                                          completion) {
+    target->InvalidateWeakPtrs();
+    completion->Signal();
+  }
 };
 
 }  // namespace
 
 TEST(WeakPtrFactoryTest, Basic) {
   int data;
   WeakPtrFactory<int> factory(&data);
   WeakPtr<int> ptr = factory.GetWeakPtr();
   EXPECT_EQ(&data, ptr.get());
 }
@@ skipping 142 matching lines @@
 
     // The new WeakPtr is owned by background thread.
     EXPECT_EQ(target, background.DeRef(&arrow));
   }
 
   // Target can only be deleted on background thread.
   background.DeleteTarget(target);
   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();
-
-  // 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.
-  {
-    WeakPtr<Target> weak_ptr = target.AsWeakPtr();
-  }
-  target.DetachFromThreadHack();
-}
-
 TEST(WeakPtrTest, MoveOwnershipExplicitly) {
   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();
+    // 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());
 
-    // Main thread can now delete the target.
+    // And the main thread can now delete the target.
   }
 
   // WeakPtr can be deleted on non-owner thread.
+  background.DeleteArrow(arrow);
+}
+
+TEST(WeakPtrTest, MoveOwnershipExplicitlyWithInvalidate) {
+  BackgroundThread background;
+  background.Start();
+
+  Arrow* arrow;
+  {
+    Target target;
+    // Background thread creates WeakPtr.
+    background.CreateArrowFromTarget(&arrow, &target);
+
+    // Bind to background thread.
+    EXPECT_EQ(&target, background.DeRef(arrow));
+
+    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());
+
+    // And the main thread can now delete the target.
+  }
+
+  // WeakPtr can be deleted on non-owner thread.
   background.DeleteArrow(arrow);

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

IIRC this aspect of WeakPtr semantics is tested elsewhere, so there's no real
need for it here.

 }
 
 TEST(WeakPtrTest, MainThreadRefOutlivesBackgroundThreadRef) {
   // Originating thread has a WeakPtr that outlives others.
   // - Main thread creates a WeakPtr
   // - Background thread creates a WeakPtr copy from the one in main thread
   // - Destruct the WeakPtr on background thread
   // - Destruct the WeakPtr on main thread
   BackgroundThread background;
   background.Start();
@@ skipping 196 matching lines @@
   background.Start();
   background.DeleteTarget(target.release());
 
   // Main thread attempts to dereference the target, violating thread binding.
   ASSERT_DEATH(arrow.target.get(), "");
 }
 
 #endif
 
 }  // namespace base