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 318 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) {
+TEST(WeakPtrTest, MoveOwnershipOfUnreferencedObject) {
   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();
+
+    // 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);

[Wez, 2013/08/01 21:09:10]

nit: This doesn't feel like it belongs in this test, since it's testing a
different property (that a WeakPtr "owned" on thread A can be deleted on any
other thread) - perhaps create a small test separately for that?

[no sievers, 2013/08/02 22:24:09]

Done.
NonOwnerThreadCanDeleteWeakPtr already covers the other case you mention.

 }
 
+TEST(WeakPtrTest, MoveOwnershipAfterInvalidate) {
+  BackgroundThread background;
+  background.Start();
+
+  Arrow arrow;
+  {
+    Target* target = new Target;

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

I could add some plumbing in Background to avoid instantiating SupportsWeakPtr
here which makes it a bit confusing in the test, but it's all a bit constructed
around Target/Arrow (Arrow also points to Target specifically).

[Wez, 2013/08/01 21:09:10]

You can create a Target, wrap it with a WeakPtrFactory<Target> and pull a
WeakPtr from that to assign to the Arrow.

[Wez, 2013/08/01 21:09:10]

nit: scoped_ptr<Target> target = new Target, and then
DeleteTarget(target.release()) later on.

[no sievers, 2013/08/02 22:24:09]

Done.

+    WeakPtrFactory<Target> factory(target);
+
+    // Bind to main thread.
+    arrow.target = factory.GetWeakPtr();
+    EXPECT_EQ(target, arrow.target.get());
+
+    factory.InvalidateWeakPtrs();
+    EXPECT_EQ(NULL, arrow.target.get());
+
+    arrow.target = factory.GetWeakPtr();
+    // Re-bind to background thread.
+    EXPECT_EQ(target, background.DeRef(&arrow));
+
+    // And the background thread can now delete the target.
+    background.DeleteTarget(target);
+
+    // Make sure we can delete the factory on the main thread.

[Wez, 2013/08/01 21:09:10]

This code's not deleting the factory, it's dropping the WeakPtr to the Target,
right?

+    arrow.target.reset();

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

This wouldn't be needed if I made the test the opposite and invalidated on the
background thread.

[Wez, 2013/08/01 21:09:10]

Why is it needed at all? It's just clearing the WeakPtr, which can be done on
any thread.

[no sievers, 2013/08/01 21:15:12]

Because the factory's flag is now bound to the background thread and deleting
the factory invalidates the flag. That fails a check unless there is no
outstanding pointers to it.

+  }
+}
+
 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();
 
   Target target;
@@ skipping 194 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