Remove BindStateHolder and have Bind() return a Callback<> object directly.

This removes some complexity and also fixes a bug where if you call Bind() with the result of Bind(), the resulting Callback would only be valid during the first call.  Ouch.

BUG=none
TEST=new unittests


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

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

13 files changed, 306 insertions, 265 deletions

diff --git a/base/bind.h b/base/bind.h
index aa2cc6b..22a3b4b 100644
--- a/base/bind.h
+++ b/base/bind.h
@@ -44,11 +44,12 @@
 namespace base {
 
 template <typename Functor>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
-        void()> >
+        void()>
+            ::UnboundRunType>
 Bind(Functor functor) {
   // Typedefs for how to store and run the functor.
   typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
@@ -60,18 +61,20 @@ Bind(Functor functor) {
   typedef internal::FunctionTraits<typename RunnableType::RunType>
       BoundFunctorTraits;
 
+  typedef internal::BindState<RunnableType, RunType, void()> BindState;
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType, void()>(
-          internal::MakeRunnable(functor)));
+
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor)));
 }
 
 template <typename Functor, typename P1>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
-        void(typename internal::CallbackParamTraits<P1>::StorageType)> >
+        void(typename internal::CallbackParamTraits<P1>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1) {
   // Typedefs for how to store and run the functor.
   typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
@@ -103,20 +106,22 @@ Bind(Functor functor, const P1& p1) {
   COMPILE_ASSERT(!internal::HasIsMethodTag<RunnableType>::value ||
                      !is_array<P1>::value,
                  first_bound_argument_to_method_cannot_be_array);
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType)> BindState;
+
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType)>(
-          internal::MakeRunnable(functor), p1));
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1));
 }
 
 template <typename Functor, typename P1, typename P2>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
         void(typename internal::CallbackParamTraits<P1>::StorageType,
-            typename internal::CallbackParamTraits<P2>::StorageType)> >
+            typename internal::CallbackParamTraits<P2>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1, const P2& p2) {
   // Typedefs for how to store and run the functor.
   typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
@@ -151,22 +156,24 @@ Bind(Functor functor, const P1& p1, const P2& p2) {
                  first_bound_argument_to_method_cannot_be_array);
   COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P2>::value,
                  p2_is_refcounted_type_and_needs_scoped_refptr);
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType,
+      typename internal::CallbackParamTraits<P2>::StorageType)> BindState;
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType,
-          typename internal::CallbackParamTraits<P2>::StorageType)>(
-          internal::MakeRunnable(functor), p1, p2));
+
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1, p2));
 }
 
 template <typename Functor, typename P1, typename P2, typename P3>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
         void(typename internal::CallbackParamTraits<P1>::StorageType,
             typename internal::CallbackParamTraits<P2>::StorageType,
-            typename internal::CallbackParamTraits<P3>::StorageType)> >
+            typename internal::CallbackParamTraits<P3>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3) {
   // Typedefs for how to store and run the functor.
   typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
@@ -204,24 +211,26 @@ Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3) {
                  p2_is_refcounted_type_and_needs_scoped_refptr);
   COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P3>::value,
                  p3_is_refcounted_type_and_needs_scoped_refptr);
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType,
+      typename internal::CallbackParamTraits<P2>::StorageType,
+      typename internal::CallbackParamTraits<P3>::StorageType)> BindState;
+
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType,
-          typename internal::CallbackParamTraits<P2>::StorageType,
-          typename internal::CallbackParamTraits<P3>::StorageType)>(
-          internal::MakeRunnable(functor), p1, p2, p3));
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1, p2, p3));
 }
 
 template <typename Functor, typename P1, typename P2, typename P3, typename P4>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
         void(typename internal::CallbackParamTraits<P1>::StorageType,
             typename internal::CallbackParamTraits<P2>::StorageType,
             typename internal::CallbackParamTraits<P3>::StorageType,
-            typename internal::CallbackParamTraits<P4>::StorageType)> >
+            typename internal::CallbackParamTraits<P4>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4) {
   // Typedefs for how to store and run the functor.
   typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
@@ -262,27 +271,29 @@ Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4) {
                  p3_is_refcounted_type_and_needs_scoped_refptr);
   COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P4>::value,
                  p4_is_refcounted_type_and_needs_scoped_refptr);
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType,
+      typename internal::CallbackParamTraits<P2>::StorageType,
+      typename internal::CallbackParamTraits<P3>::StorageType,
+      typename internal::CallbackParamTraits<P4>::StorageType)> BindState;
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType,
-          typename internal::CallbackParamTraits<P2>::StorageType,
-          typename internal::CallbackParamTraits<P3>::StorageType,
-          typename internal::CallbackParamTraits<P4>::StorageType)>(
-          internal::MakeRunnable(functor), p1, p2, p3, p4));
+
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4));
 }
 
 template <typename Functor, typename P1, typename P2, typename P3, typename P4,
     typename P5>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
         void(typename internal::CallbackParamTraits<P1>::StorageType,
             typename internal::CallbackParamTraits<P2>::StorageType,
             typename internal::CallbackParamTraits<P3>::StorageType,
             typename internal::CallbackParamTraits<P4>::StorageType,
-            typename internal::CallbackParamTraits<P5>::StorageType)> >
+            typename internal::CallbackParamTraits<P5>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4,
     const P5& p5) {
   // Typedefs for how to store and run the functor.
@@ -327,21 +338,22 @@ Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4,
                  p4_is_refcounted_type_and_needs_scoped_refptr);
   COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P5>::value,
                  p5_is_refcounted_type_and_needs_scoped_refptr);
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType,
+      typename internal::CallbackParamTraits<P2>::StorageType,
+      typename internal::CallbackParamTraits<P3>::StorageType,
+      typename internal::CallbackParamTraits<P4>::StorageType,
+      typename internal::CallbackParamTraits<P5>::StorageType)> BindState;
+
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType,
-          typename internal::CallbackParamTraits<P2>::StorageType,
-          typename internal::CallbackParamTraits<P3>::StorageType,
-          typename internal::CallbackParamTraits<P4>::StorageType,
-          typename internal::CallbackParamTraits<P5>::StorageType)>(
-          internal::MakeRunnable(functor), p1, p2, p3, p4, p5));
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4, p5));
 }
 
 template <typename Functor, typename P1, typename P2, typename P3, typename P4,
     typename P5, typename P6>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
         void(typename internal::CallbackParamTraits<P1>::StorageType,
@@ -349,7 +361,8 @@ internal::BindStateHolder<
             typename internal::CallbackParamTraits<P3>::StorageType,
             typename internal::CallbackParamTraits<P4>::StorageType,
             typename internal::CallbackParamTraits<P5>::StorageType,
-            typename internal::CallbackParamTraits<P6>::StorageType)> >
+            typename internal::CallbackParamTraits<P6>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4,
     const P5& p5, const P6& p6) {
   // Typedefs for how to store and run the functor.
@@ -397,22 +410,23 @@ Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4,
                  p5_is_refcounted_type_and_needs_scoped_refptr);
   COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P6>::value,
                  p6_is_refcounted_type_and_needs_scoped_refptr);
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType,
+      typename internal::CallbackParamTraits<P2>::StorageType,
+      typename internal::CallbackParamTraits<P3>::StorageType,
+      typename internal::CallbackParamTraits<P4>::StorageType,
+      typename internal::CallbackParamTraits<P5>::StorageType,
+      typename internal::CallbackParamTraits<P6>::StorageType)> BindState;
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType,
-          typename internal::CallbackParamTraits<P2>::StorageType,
-          typename internal::CallbackParamTraits<P3>::StorageType,
-          typename internal::CallbackParamTraits<P4>::StorageType,
-          typename internal::CallbackParamTraits<P5>::StorageType,
-          typename internal::CallbackParamTraits<P6>::StorageType)>(
-          internal::MakeRunnable(functor), p1, p2, p3, p4, p5, p6));
+
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4, p5, p6));
 }
 
 template <typename Functor, typename P1, typename P2, typename P3, typename P4,
     typename P5, typename P6, typename P7>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
         void(typename internal::CallbackParamTraits<P1>::StorageType,
@@ -421,7 +435,8 @@ internal::BindStateHolder<
             typename internal::CallbackParamTraits<P4>::StorageType,
             typename internal::CallbackParamTraits<P5>::StorageType,
             typename internal::CallbackParamTraits<P6>::StorageType,
-            typename internal::CallbackParamTraits<P7>::StorageType)> >
+            typename internal::CallbackParamTraits<P7>::StorageType)>
+            ::UnboundRunType>
 Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4,
     const P5& p5, const P6& p6, const P7& p7) {
   // Typedefs for how to store and run the functor.
@@ -472,17 +487,19 @@ Bind(Functor functor, const P1& p1, const P2& p2, const P3& p3, const P4& p4,
                  p6_is_refcounted_type_and_needs_scoped_refptr);
   COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P7>::value,
                  p7_is_refcounted_type_and_needs_scoped_refptr);
-
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType,
-          void(typename internal::CallbackParamTraits<P1>::StorageType,
-          typename internal::CallbackParamTraits<P2>::StorageType,
-          typename internal::CallbackParamTraits<P3>::StorageType,
-          typename internal::CallbackParamTraits<P4>::StorageType,
-          typename internal::CallbackParamTraits<P5>::StorageType,
-          typename internal::CallbackParamTraits<P6>::StorageType,
-          typename internal::CallbackParamTraits<P7>::StorageType)>(
-          internal::MakeRunnable(functor), p1, p2, p3, p4, p5, p6, p7));
+  typedef internal::BindState<RunnableType, RunType,
+      void(typename internal::CallbackParamTraits<P1>::StorageType,
+      typename internal::CallbackParamTraits<P2>::StorageType,
+      typename internal::CallbackParamTraits<P3>::StorageType,
+      typename internal::CallbackParamTraits<P4>::StorageType,
+      typename internal::CallbackParamTraits<P5>::StorageType,
+      typename internal::CallbackParamTraits<P6>::StorageType,
+      typename internal::CallbackParamTraits<P7>::StorageType)> BindState;
+
+
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4, p5, p6,
+          p7));
 }
 
 }  // namespace base
diff --git a/base/bind.h.pump b/base/bind.h.pump
index 9d4c5ee..494d716 100644
--- a/base/bind.h.pump
+++ b/base/bind.h.pump
@@ -71,11 +71,12 @@ $range ARG 1..ARITY
 
 template <typename Functor[[]]
 $if ARITY > 0 [[, ]] $for ARG , [[typename P$(ARG)]]>
-internal::BindStateHolder<
-    internal::BindState<
+base::Callback<
+    typename internal::BindState<
         typename internal::FunctorTraits<Functor>::RunnableType,
         typename internal::FunctorTraits<Functor>::RunType,
-        void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])> >
+        void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])>
+            ::UnboundRunType>
 Bind(Functor functor
 $if ARITY > 0 [[, ]] $for ARG , [[const P$(ARG)& p$(ARG)]]) {
   // Typedefs for how to store and run the functor.
@@ -125,11 +126,13 @@ $if ARG == 1 [[
 
 ]]  $$ $for ARG
 
+  typedef internal::BindState<RunnableType, RunType, [[]]
+void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])> [[]]
+BindState;
 
-  return internal::MakeBindStateHolder(
-      new internal::BindState<RunnableType, RunType, [[]]
-void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])>(
-          internal::MakeRunnable(functor)[[]]
+
+  return Callback<typename BindState::UnboundRunType>(
+      new BindState(internal::MakeRunnable(functor)[[]]
 $if ARITY > 0 [[, ]] $for ARG , [[p$(ARG)]]));
 }
 
diff --git a/base/bind_unittest.cc b/base/bind_unittest.cc
index 654a277..372523b 100644
--- a/base/bind_unittest.cc
+++ b/base/bind_unittest.cc
@@ -199,10 +199,18 @@ void RefArgSet(int &n) {
   n = 2;
 }
 
+void PtrArgSet(int *n) {
+  *n = 2;
+}
+
 int FunctionWithWeakFirstParam(WeakPtr<NoRef> o, int n) {
   return n;
 }
 
+void TakesACallback(const Closure& callback) {
+  callback.Run();
+}
+
 class BindTest : public ::testing::Test {
  public:
   BindTest() {
@@ -284,6 +292,25 @@ TEST_F(BindTest, CurryingTest) {
   EXPECT_EQ(63, c0.Run());
 }
 
+// Test that currying the rvalue result of another Bind() works correctly.
+//   - rvalue should be usable as argument to Bind().
+//   - multiple runs of resulting Callback remain valid.
+TEST_F(BindTest, CurryingRvalueResultOfBind) {
+  int n = 0;
+  Closure cb = base::Bind(&TakesACallback, base::Bind(&PtrArgSet, &n));
+
+  // If we implement Bind() such that the return value has auto_ptr-like
+  // semantics, the second call here will fail because ownership of
+  // the internal BindState<> would have been transfered to a *temporary*
+  // constructon of a Callback object on the first call.
+  cb.Run();
+  EXPECT_EQ(2, n);
+
+  n = 0;
+  cb.Run();
+  EXPECT_EQ(2, n);
+}
+
 // Function type support.
 //   - Normal function.
 //   - Normal function bound with non-refcounted first argument.
diff --git a/base/callback.h b/base/callback.h
index 4bf474d..abaa438 100644
--- a/base/callback.h
+++ b/base/callback.h
@@ -129,29 +129,28 @@
 // The Callback classes represent a generic function pointer. Internally,
 // it stores a refcounted piece of state that represents the target function
 // and all its bound parameters.  Each Callback specialization has a templated
-// constructor that takes an BindStateHolder<> object.  In the context of
-// the constructor, the static type of this BindStateHolder<> object
-// uniquely identifies the function it is representing, all its bound
-// parameters, and a DoInvoke() that is capable of invoking the target.
+// constructor that takes an BindState<>*.  In the context of the constructor,
+// the static type of this BindState<> pointer uniquely identifies the
+// function it is representing, all its bound parameters, and a Run() method
+// that is capable of invoking the target.
 //
-// Callback's constructor is takes the BindStateHolder<> that has the
-// full static type and erases the target function type, and the bound
-// parameters.  It does this by storing a pointer to the specific DoInvoke()
-// function, and upcasting the state of BindStateHolder<> to a
-// BindStateBase. This is safe as long as this BindStateBase pointer
-// is only used with the stored DoInvoke() pointer.
+// Callback's constructor takes the BindState<>* that has the full static type
+// and erases the target function type as well as the types of the bound
+// parameters.  It does this by storing a pointer to the specific Run()
+// function, and upcasting the state of BindState<>* to a
+// BindStateBase*. This is safe as long as this BindStateBase pointer
+// is only used with the stored Run() pointer.
 //
-// To create BindStateHolder<> objects, we use the Bind() functions.
-// These functions, along with a set of internal templates, are reponsible for
+// To BindState<> objects are created inside the Bind() functions.
+// These functions, along with a set of internal templates, are responsible for
 //
 //  - Unwrapping the function signature into return type, and parameters
 //  - Determining the number of parameters that are bound
-//  - Creating the storage for the bound parameters
+//  - Creating the BindState storing the bound parameters
 //  - Performing compile-time asserts to avoid error-prone behavior
-//  - Returning an BindStateHolder<> with an DoInvoke() that has an arity
-//    matching the number of unbound parameters, and knows the correct
-//    refcounting semantics for the target object if we are binding a class
-//    method.
+//  - Returning an Callback<> with an arity matching the number of unbound
+//    parameters and that knows the correct refcounting semantics for the
+//    target object if we are binding a method.
 //
 // The Bind functions do the above using type-inference, and template
 // specializations.
@@ -239,27 +238,30 @@ namespace base {
 template <typename Sig>
 class Callback;
 
+namespace internal {
+template <typename Runnable, typename RunType, typename BoundArgsType>
+struct BindState;
+}  // namespace internal
+
 template <typename R>
 class Callback<R(void)> : public internal::CallbackBase {
  public:
   typedef R(RunType)();
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -285,22 +287,20 @@ class Callback<R(A1)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -327,22 +327,20 @@ class Callback<R(A1, A2)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -372,22 +370,20 @@ class Callback<R(A1, A2, A3)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -420,22 +416,20 @@ class Callback<R(A1, A2, A3, A4)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -472,22 +466,20 @@ class Callback<R(A1, A2, A3, A4, A5)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4, A5);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -527,22 +519,20 @@ class Callback<R(A1, A2, A3, A4, A5, A6)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4, A5, A6);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -585,22 +575,20 @@ class Callback<R(A1, A2, A3, A4, A5, A6, A7)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4, A5, A6, A7);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
diff --git a/base/callback.h.pump b/base/callback.h.pump
index 91a41e0..2216a3c 100644
--- a/base/callback.h.pump
+++ b/base/callback.h.pump
@@ -134,29 +134,28 @@ $var MAX_ARITY = 7
 // The Callback classes represent a generic function pointer. Internally,
 // it stores a refcounted piece of state that represents the target function
 // and all its bound parameters.  Each Callback specialization has a templated
-// constructor that takes an BindStateHolder<> object.  In the context of
-// the constructor, the static type of this BindStateHolder<> object
-// uniquely identifies the function it is representing, all its bound
-// parameters, and a DoInvoke() that is capable of invoking the target.
+// constructor that takes an BindState<>*.  In the context of the constructor,
+// the static type of this BindState<> pointer uniquely identifies the
+// function it is representing, all its bound parameters, and a Run() method
+// that is capable of invoking the target.
 //
-// Callback's constructor is takes the BindStateHolder<> that has the
-// full static type and erases the target function type, and the bound
-// parameters.  It does this by storing a pointer to the specific DoInvoke()
-// function, and upcasting the state of BindStateHolder<> to a
-// BindStateBase. This is safe as long as this BindStateBase pointer
-// is only used with the stored DoInvoke() pointer.
+// Callback's constructor takes the BindState<>* that has the full static type
+// and erases the target function type as well as the types of the bound
+// parameters.  It does this by storing a pointer to the specific Run()
+// function, and upcasting the state of BindState<>* to a
+// BindStateBase*. This is safe as long as this BindStateBase pointer
+// is only used with the stored Run() pointer.
 //
-// To create BindStateHolder<> objects, we use the Bind() functions.
-// These functions, along with a set of internal templates, are reponsible for
+// To BindState<> objects are created inside the Bind() functions.
+// These functions, along with a set of internal templates, are responsible for
 //
 //  - Unwrapping the function signature into return type, and parameters
 //  - Determining the number of parameters that are bound
-//  - Creating the storage for the bound parameters
+//  - Creating the BindState storing the bound parameters
 //  - Performing compile-time asserts to avoid error-prone behavior
-//  - Returning an BindStateHolder<> with an DoInvoke() that has an arity
-//    matching the number of unbound parameters, and knows the correct
-//    refcounting semantics for the target object if we are binding a class
-//    method.
+//  - Returning an Callback<> with an arity matching the number of unbound
+//    parameters and that knows the correct refcounting semantics for the
+//    target object if we are binding a method.
 //
 // The Bind functions do the above using type-inference, and template
 // specializations.
@@ -244,6 +243,11 @@ namespace base {
 template <typename Sig>
 class Callback;
 
+namespace internal {
+template <typename Runnable, typename RunType, typename BoundArgsType>
+struct BindState;
+}  // namespace internal
+
 
 $range ARITY 0..MAX_ARITY
 $for ARITY [[
@@ -260,22 +264,20 @@ class Callback<R($for ARG , [[A$(ARG)]])> : public internal::CallbackBase {
  public:
   typedef R(RunType)($for ARG , [[A$(ARG)]]);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
diff --git a/base/callback_internal.cc b/base/callback_internal.cc
index 582fbdc..a483293 100644
--- a/base/callback_internal.cc
+++ b/base/callback_internal.cc
@@ -4,6 +4,8 @@
 
 #include "base/callback_internal.h"
 
+#include "base/logging.h"
+
 namespace base {
 namespace internal {
 
@@ -21,12 +23,10 @@ bool CallbackBase::Equals(const CallbackBase& other) const {
          polymorphic_invoke_ == other.polymorphic_invoke_;
 }
 
-CallbackBase::CallbackBase(InvokeFuncStorage polymorphic_invoke,
-                           scoped_refptr<BindStateBase>* bind_state)
-    : polymorphic_invoke_(polymorphic_invoke) {
-  if (bind_state) {
-    bind_state_.swap(*bind_state);
-  }
+CallbackBase::CallbackBase(BindStateBase* bind_state)
+    : bind_state_(bind_state),
+      polymorphic_invoke_(NULL) {
+  DCHECK(!bind_state_ || bind_state_->HasOneRef());
 }
 
 CallbackBase::~CallbackBase() {
diff --git a/base/callback_internal.h b/base/callback_internal.h
index 81c87c0..4bb8aa9 100644
--- a/base/callback_internal.h
+++ b/base/callback_internal.h
@@ -29,29 +29,6 @@ class BindStateBase : public RefCountedThreadSafe<BindStateBase> {
   virtual ~BindStateBase() {}
 };
 
-// This structure exists purely to pass the returned |bind_state_| from
-// Bind() to Callback while avoiding an extra AddRef/Release() pair.
-//
-// To do this, the constructor of Callback<> must take a const-ref.  The
-// reference must be to a const object otherwise the compiler will emit a
-// warning about taking a reference to a temporary.
-//
-// Unfortunately, this means that the internal |bind_state_| field must
-// be made mutable.
-template <typename T>
-struct BindStateHolder {
-  explicit BindStateHolder(T* bind_state)
-      : bind_state_(bind_state) {
-  }
-
-  mutable scoped_refptr<BindStateBase> bind_state_;
-};
-
-template <typename T>
-BindStateHolder<T> MakeBindStateHolder(T* o) {
-  return BindStateHolder<T>(o);
-}
-
 // Holds the Callback methods that don't require specialization to reduce
 // template bloat.
 class BASE_EXPORT CallbackBase {
@@ -72,8 +49,11 @@ class BASE_EXPORT CallbackBase {
   // Returns true if this callback equals |other|. |other| may be null.
   bool Equals(const CallbackBase& other) const;
 
-  CallbackBase(InvokeFuncStorage polymorphic_invoke,
-               scoped_refptr<BindStateBase>* bind_state);
+  // Allow initializing of |bind_state_| via the constructor to avoid default
+  // initialization of the scoped_refptr.  We do not also initialize
+  // |polymorphic_invoke_| here because doing a normal assignment in the
+  // derived Callback templates makes for much nicer compiler errors.
+  explicit CallbackBase(BindStateBase* bind_state);
 
   // Force the destructor to be instantiated inside this translation unit so
   // that our subclasses will not get inlined versions.  Avoids more template
diff --git a/base/callback_unittest.cc b/base/callback_unittest.cc
index 8527e70..e42b933 100644
--- a/base/callback_unittest.cc
+++ b/base/callback_unittest.cc
@@ -9,6 +9,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
+
 namespace {
 
 class HelperObject {
@@ -26,18 +27,40 @@ struct FakeInvoker {
   static void Run(internal::BindStateBase*) {
   }
 };
+}  // namespace
+
+namespace internal {
+template <typename Runnable, typename RunType, typename BoundArgsType>
+struct BindState;
 
 // White-box testpoints to inject into a Callback<> object for checking
-// comparators and emptiness APIs.
-class FakeBindState1 : public internal::BindStateBase {
+// comparators and emptiness APIs.  Use a BindState that is specialized
+// based on a type we declared in the anonymous namespace above to remove any
+// chance of colliding with another instantiation and breaking the
+// one-definition-rule.
+template <>
+struct BindState<void(void), void(void), void(FakeInvoker)>
+    : public BindStateBase {
  public:
   typedef FakeInvoker InvokerType;
 };
 
-class FakeBindState2 : public internal::BindStateBase {
+template <>
+struct BindState<void(void), void(void),
+                           void(FakeInvoker, FakeInvoker)>
+    : public BindStateBase {
  public:
   typedef FakeInvoker InvokerType;
 };
+}  // namespace internal
+
+namespace {
+
+typedef internal::BindState<void(void), void(void), void(FakeInvoker)>
+    FakeBindState1;
+typedef internal::BindState<void(void), void(void),
+                            void(FakeInvoker, FakeInvoker)>
+   FakeBindState2;
 
 TEST(CallbackOld, OneArg) {
   HelperObject obj;
@@ -60,8 +83,8 @@ TEST(CallbackOld, ReturnValue) {
 class CallbackTest : public ::testing::Test {
  public:
   CallbackTest()
-      : callback_a_(MakeBindStateHolder(new FakeBindState1())),
-        callback_b_(MakeBindStateHolder(new FakeBindState2())) {
+      : callback_a_(new FakeBindState1()),
+        callback_b_(new FakeBindState2()) {
   }
 
   virtual ~CallbackTest() {
@@ -105,8 +128,7 @@ TEST_F(CallbackTest, Equals) {
   EXPECT_FALSE(callback_b_.Equals(callback_a_));
 
   // We should compare based on instance, not type.
-  Callback<void(void)> callback_c(
-      MakeBindStateHolder(new FakeBindState1()));
+  Callback<void(void)> callback_c(new FakeBindState1());
   Callback<void(void)> callback_a2 = callback_a_;
   EXPECT_TRUE(callback_a_.Equals(callback_a2));
   EXPECT_FALSE(callback_a_.Equals(callback_c));
diff --git a/base/cancelable_callback.h b/base/cancelable_callback.h
index b432f61..4de7d12 100644
--- a/base/cancelable_callback.h
+++ b/base/cancelable_callback.h
@@ -173,8 +173,7 @@ class CancelableCallback<void(A1)> {
   }
 
  private:
-  void Forward(
-      typename internal::CallbackParamTraits<A1>::ForwardType a1) const {
+  void Forward(A1 a1) const {
     callback_.Run(a1);
   }
 
diff --git a/base/debug/trace_event.cc b/base/debug/trace_event.cc
index 5be1557..61d3daf 100644
--- a/base/debug/trace_event.cc
+++ b/base/debug/trace_event.cc
@@ -296,7 +296,8 @@ TraceResultBuffer::TraceResultBuffer() : append_comma_(false) {
 TraceResultBuffer::~TraceResultBuffer() {
 }
 
-void TraceResultBuffer::SetOutputCallback(OutputCallback json_chunk_callback) {
+void TraceResultBuffer::SetOutputCallback(
+    const OutputCallback& json_chunk_callback) {
   output_callback_ = json_chunk_callback;
 }
 
diff --git a/base/debug/trace_event.h b/base/debug/trace_event.h
index ab56dfc..621d6f06 100644
--- a/base/debug/trace_event.h
+++ b/base/debug/trace_event.h
@@ -751,7 +751,7 @@ class BASE_EXPORT TraceResultBuffer {
   // JSON output and during AddFragment and Finish with following JSON output
   // chunks. The callback target must live past the last calls to
   // TraceResultBuffer::Start/AddFragment/Finish.
-  void SetOutputCallback(OutputCallback json_chunk_callback);
+  void SetOutputCallback(const OutputCallback& json_chunk_callback);
 
   // Start JSON output. This resets all internal state, so you can reuse
   // the TraceResultBuffer by calling Start.
@@ -818,7 +818,8 @@ class BASE_EXPORT TraceLog {
   // undefined. Use TraceResultBuffer to convert one or more trace strings to
   // JSON.
   typedef RefCountedData<std::string> RefCountedString;
-  typedef base::Callback<void(scoped_refptr<RefCountedString>)> OutputCallback;
+  typedef base::Callback<void(const scoped_refptr<RefCountedString>&)>
+      OutputCallback;
   void SetOutputCallback(const OutputCallback& cb);
 
   // The trace buffer does not flush dynamically, so when it fills up,
diff --git a/base/debug/trace_event_unittest.cc b/base/debug/trace_event_unittest.cc
index 0ff986a..913a9d3 100644
--- a/base/debug/trace_event_unittest.cc
+++ b/base/debug/trace_event_unittest.cc
@@ -42,7 +42,7 @@ class TraceEventTestFixture : public testing::Test {
   // up multiple times when testing AtExit. Use ManualTestSetUp for this.
   void ManualTestSetUp();
   void OnTraceDataCollected(
-      scoped_refptr<TraceLog::RefCountedString> events_str);
+      const scoped_refptr<TraceLog::RefCountedString>& events_str);
   DictionaryValue* FindMatchingTraceEntry(const JsonKeyValue* key_values);
   DictionaryValue* FindNamePhase(const char* name, const char* phase);
   DictionaryValue* FindNamePhaseKeyValue(const char* name,
@@ -89,7 +89,7 @@ void TraceEventTestFixture::ManualTestSetUp() {
 }
 
 void TraceEventTestFixture::OnTraceDataCollected(
-    scoped_refptr<TraceLog::RefCountedString> events_str) {
+    const scoped_refptr<TraceLog::RefCountedString>& events_str) {
   AutoLock lock(lock_);
   json_output_.json_output.clear();
   trace_buffer_.Start();
diff --git a/base/test/trace_event_analyzer_unittest.cc b/base/test/trace_event_analyzer_unittest.cc
index 96f8045..1c9fafe 100644
--- a/base/test/trace_event_analyzer_unittest.cc
+++ b/base/test/trace_event_analyzer_unittest.cc
@@ -15,7 +15,8 @@ class TraceEventAnalyzerTest : public testing::Test {
  public:
   void ManualSetUp();
   void OnTraceDataCollected(
-      scoped_refptr<base::debug::TraceLog::RefCountedString> json_events_str);
+      const scoped_refptr<base::debug::TraceLog::RefCountedString>&
+          json_events_str);
   void BeginTracing();
   void EndTracing();
 
@@ -35,7 +36,8 @@ void TraceEventAnalyzerTest::ManualSetUp() {
 }
 
 void TraceEventAnalyzerTest::OnTraceDataCollected(
-    scoped_refptr<base::debug::TraceLog::RefCountedString> json_events_str) {
+    const scoped_refptr<base::debug::TraceLog::RefCountedString>&
+        json_events_str) {
   buffer_.AddFragment(json_events_str->data);
 }
 
@@ -609,4 +611,3 @@ TEST_F(TraceEventAnalyzerTest, RateStats) {
 
 
 }  // namespace trace_analyzer
-