Make Callback::Reset() return a copy to support use-cases where Run() ends up modifying |*this|. Callers can use

cb.Reset().Run(args...);
to avoid reentrancy-like bugs.

Replace the special-purpose versions of ResetAndRunCB in the media/ codebase
with this more-general facility.

Review URL: http://codereview.chromium.org/9717021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128772 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 56 insertions, 10 deletions

diff --git a/base/base.gypi b/base/base.gypi
index bca1c09..5bd9885 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -60,6 +60,7 @@
           'build_time.cc',
           'build_time.h',
           'callback.h',
+          'callback_helpers.h',
           'callback_internal.cc',
           'callback_internal.h',
           'cancelable_callback.h',
diff --git a/base/callback_helpers.h b/base/callback_helpers.h
new file mode 100644
index 0000000..52cb71b
--- /dev/null
+++ b/base/callback_helpers.h
@@ -0,0 +1,30 @@
+// 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.
+
+// This defines helpful methods for dealing with Callbacks.  Because Callbacks
+// are implemented using templates, with a class per callback signature, adding
+// methods to Callback<> itself is unattractive (lots of extra code gets
+// generated).  Instead, consider adding methods here.
+//
+// ResetAndReturn(&cb) is like cb.Reset() but allows executing a callback (via a
+// copy) after the original callback is Reset().  This can be handy if Run()
+// reads/writes the variable holding the Callback.
+
+#ifndef BASE_CALLBACK_HELPERS_H_
+#define BASE_CALLBACK_HELPERS_H_
+
+#include "base/callback.h"
+
+namespace base {
+
+template <typename Sig>
+base::Callback<Sig> ResetAndReturn(base::Callback<Sig>* cb) {
+  base::Callback<Sig> ret(*cb);
+  cb->Reset();
+  return ret;
+}
+
+}  // namespace base
+
+#endif  // BASE_CALLBACK_HELPERS_H_
diff --git a/base/callback_unittest.cc b/base/callback_unittest.cc
index 1c3db04..f0f3915 100644
--- a/base/callback_unittest.cc
+++ b/base/callback_unittest.cc
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "base/callback.h"
+#include "base/callback_helpers.h"
 #include "base/callback_internal.h"
 #include "base/memory/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -11,21 +13,12 @@ namespace base {
 
 namespace {
 
-class HelperObject {
- public:
-  HelperObject() : next_number_(0) { }
-  int GetNextNumber() { return ++next_number_; }
-  void GetNextNumberArg(int* number) { *number = GetNextNumber(); }
-
- private:
-  int next_number_;
-};
-
 struct FakeInvoker {
   typedef void(RunType)(internal::BindStateBase*);
   static void Run(internal::BindStateBase*) {
   }
 };
+
 }  // namespace
 
 namespace internal {
@@ -130,5 +123,27 @@ TEST_F(CallbackTest, Reset) {
   EXPECT_TRUE(callback_a_.Equals(null_callback_));
 }
 
+struct TestForReentrancy {
+  TestForReentrancy()
+      : cb_already_run(false),
+        cb(Bind(&TestForReentrancy::AssertCBIsNull, Unretained(this))) {
+  }
+  void AssertCBIsNull() {
+    ASSERT_TRUE(cb.is_null());
+    cb_already_run = true;
+  }
+  bool cb_already_run;
+  Closure cb;
+};
+
+TEST_F(CallbackTest, ResetAndReturn) {
+  TestForReentrancy tfr;
+  ASSERT_FALSE(tfr.cb.is_null());
+  ASSERT_FALSE(tfr.cb_already_run);
+  ResetAndReturn(&tfr.cb).Run();
+  ASSERT_TRUE(tfr.cb.is_null());
+  ASSERT_TRUE(tfr.cb_already_run);
+}
+
 }  // namespace
 }  // namespace base