Make Callback.Reset safe against deleting itself.

base/callback_unittest.cc

Index: base/callback_unittest.cc
diff --git a/base/callback_unittest.cc b/base/callback_unittest.cc
index f0f3915a2cb8ce2e5c84e7c9ce13bd56a3839d78..b5cfbf0a9bf59883661038c379357b1dae764a61 100644
--- a/base/callback_unittest.cc
+++ b/base/callback_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/callback_internal.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -145,5 +146,36 @@ TEST_F(CallbackTest, ResetAndReturn) {
   ASSERT_TRUE(tfr.cb_already_run);
 }
 
+class CallbackOwner : public base::RefCounted<CallbackOwner> {
+ public:
+  CallbackOwner(bool *deleted) {

[awong, 2012/06/08 17:33:15]

* should associate with bool to stay consistent with the rest of the callback
code.

[mattm, 2012/06/09 02:10:56]

Done.

+    callback_ = Bind(&CallbackOwner::Unused, this);
+    deleted_ = deleted;
+  }
+  void Reset() {
+    callback_.Reset();
+    // We are deleted here if no-one else had a ref to us.
+  }
+
+ private:
+  friend class base::RefCounted<CallbackOwner>;
+  virtual ~CallbackOwner() {
+    *deleted_ = true;
+  }
+  void Unused() {
+    ASSERT_TRUE(false);

[awong, 2012/06/08 17:33:15]

How about:

   FAIL() << "Should never be called.";

instead?

[mattm, 2012/06/09 02:10:56]

Done.

+  }
+
+  Callback<void(void)> callback_;

[awong, 2012/06/08 17:33:15]

Use Closure instead of Callback<void(void)>

[mattm, 2012/06/09 02:10:56]

Done.

+  bool* deleted_;
+};
+
+TEST_F(CallbackTest, ResetRefCountedOwnerOfCallback) {

[awong, 2012/06/08 17:33:15]

The name of the test doesn't quite express the high level intent.  How about

CallbackHasLastRefOnContainingObject

?

[mattm, 2012/06/09 02:10:56]

Done.

+  bool deleted = false;
+  CallbackOwner* owner = new CallbackOwner(&deleted);
+  owner->Reset();
+  ASSERT_TRUE(deleted);
+}
+
 }  // namespace
 }  // namespace base