diff options
author | sail@chromium.org <sail@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-14 21:48:54 +0000 |
---|---|---|
committer | sail@chromium.org <sail@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-14 21:48:54 +0000 |
commit | 115288648041898b6d50f7d2e17391e2d1972a76 (patch) | |
tree | eee67d927a7ebb40e87848924a8f26c5c5cb99ca /base | |
parent | e91ac22d99252095839ca7c82c092972bd56445e (diff) | |
download | chromium_src-115288648041898b6d50f7d2e17391e2d1972a76.zip chromium_src-115288648041898b6d50f7d2e17391e2d1972a76.tar.gz chromium_src-115288648041898b6d50f7d2e17391e2d1972a76.tar.bz2 |
Revert 114494 - 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
TBR=ajwong@chromium.org
Review URL: http://codereview.chromium.org/8914022
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114495 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base')
-rw-r--r-- | base/bind.h | 171 | ||||
-rw-r--r-- | base/bind.h.pump | 17 | ||||
-rw-r--r-- | base/bind_unittest.cc | 27 | ||||
-rw-r--r-- | base/callback.h | 198 | ||||
-rw-r--r-- | base/callback.h.pump | 58 | ||||
-rw-r--r-- | base/callback_internal.cc | 12 | ||||
-rw-r--r-- | base/callback_internal.h | 30 | ||||
-rw-r--r-- | base/callback_unittest.cc | 36 | ||||
-rw-r--r-- | base/cancelable_callback.h | 3 | ||||
-rw-r--r-- | base/debug/trace_event.cc | 3 | ||||
-rw-r--r-- | base/debug/trace_event.h | 5 | ||||
-rw-r--r-- | base/debug/trace_event_unittest.cc | 4 | ||||
-rw-r--r-- | base/test/trace_event_analyzer_unittest.cc | 7 |
13 files changed, 265 insertions, 306 deletions
diff --git a/base/bind.h b/base/bind.h index 22a3b4b..aa2cc6b 100644 --- a/base/bind.h +++ b/base/bind.h @@ -44,12 +44,11 @@ namespace base { template <typename Functor> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + internal::BindState< typename internal::FunctorTraits<Functor>::RunnableType, typename internal::FunctorTraits<Functor>::RunType, - void()> - ::UnboundRunType> + void()> > Bind(Functor functor) { // Typedefs for how to store and run the functor. typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType; @@ -61,20 +60,18 @@ Bind(Functor functor) { typedef internal::FunctionTraits<typename RunnableType::RunType> BoundFunctorTraits; - typedef internal::BindState<RunnableType, RunType, void()> BindState; - - return Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor))); + return internal::MakeBindStateHolder( + new internal::BindState<RunnableType, RunType, void()>( + internal::MakeRunnable(functor))); } template <typename Functor, typename P1> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + internal::BindState< typename internal::FunctorTraits<Functor>::RunnableType, typename internal::FunctorTraits<Functor>::RunType, - void(typename internal::CallbackParamTraits<P1>::StorageType)> - ::UnboundRunType> + void(typename internal::CallbackParamTraits<P1>::StorageType)> > Bind(Functor functor, const P1& p1) { // Typedefs for how to store and run the functor. typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType; @@ -106,22 +103,20 @@ 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 Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), p1)); + return internal::MakeBindStateHolder( + new internal::BindState<RunnableType, RunType, + void(typename internal::CallbackParamTraits<P1>::StorageType)>( + internal::MakeRunnable(functor), p1)); } template <typename Functor, typename P1, typename P2> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + internal::BindState< typename internal::FunctorTraits<Functor>::RunnableType, typename internal::FunctorTraits<Functor>::RunType, void(typename internal::CallbackParamTraits<P1>::StorageType, - typename internal::CallbackParamTraits<P2>::StorageType)> - ::UnboundRunType> + typename internal::CallbackParamTraits<P2>::StorageType)> > 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; @@ -156,24 +151,22 @@ 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 Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), p1, p2)); + return internal::MakeBindStateHolder( + new internal::BindState<RunnableType, RunType, + void(typename internal::CallbackParamTraits<P1>::StorageType, + typename internal::CallbackParamTraits<P2>::StorageType)>( + internal::MakeRunnable(functor), p1, p2)); } template <typename Functor, typename P1, typename P2, typename P3> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + 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)> - ::UnboundRunType> + typename internal::CallbackParamTraits<P3>::StorageType)> > 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; @@ -211,26 +204,24 @@ 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 Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), p1, p2, p3)); + 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)); } template <typename Functor, typename P1, typename P2, typename P3, typename P4> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + 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)> - ::UnboundRunType> + typename internal::CallbackParamTraits<P4>::StorageType)> > 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; @@ -271,29 +262,27 @@ 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 Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4)); + 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)); } template <typename Functor, typename P1, typename P2, typename P3, typename P4, typename P5> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + 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)> - ::UnboundRunType> + typename internal::CallbackParamTraits<P5>::StorageType)> > 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. @@ -338,22 +327,21 @@ 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 Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4, p5)); + 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)); } template <typename Functor, typename P1, typename P2, typename P3, typename P4, typename P5, typename P6> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + internal::BindState< typename internal::FunctorTraits<Functor>::RunnableType, typename internal::FunctorTraits<Functor>::RunType, void(typename internal::CallbackParamTraits<P1>::StorageType, @@ -361,8 +349,7 @@ base::Callback< typename internal::CallbackParamTraits<P3>::StorageType, typename internal::CallbackParamTraits<P4>::StorageType, typename internal::CallbackParamTraits<P5>::StorageType, - typename internal::CallbackParamTraits<P6>::StorageType)> - ::UnboundRunType> + typename internal::CallbackParamTraits<P6>::StorageType)> > 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. @@ -410,23 +397,22 @@ 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 Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), p1, p2, p3, p4, p5, p6)); + 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)); } template <typename Functor, typename P1, typename P2, typename P3, typename P4, typename P5, typename P6, typename P7> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + internal::BindState< typename internal::FunctorTraits<Functor>::RunnableType, typename internal::FunctorTraits<Functor>::RunType, void(typename internal::CallbackParamTraits<P1>::StorageType, @@ -435,8 +421,7 @@ base::Callback< typename internal::CallbackParamTraits<P4>::StorageType, typename internal::CallbackParamTraits<P5>::StorageType, typename internal::CallbackParamTraits<P6>::StorageType, - typename internal::CallbackParamTraits<P7>::StorageType)> - ::UnboundRunType> + typename internal::CallbackParamTraits<P7>::StorageType)> > 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. @@ -487,19 +472,17 @@ 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); - 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)); + + 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)); } } // namespace base diff --git a/base/bind.h.pump b/base/bind.h.pump index 494d716..9d4c5ee 100644 --- a/base/bind.h.pump +++ b/base/bind.h.pump @@ -71,12 +71,11 @@ $range ARG 1..ARITY template <typename Functor[[]] $if ARITY > 0 [[, ]] $for ARG , [[typename P$(ARG)]]> -base::Callback< - typename internal::BindState< +internal::BindStateHolder< + internal::BindState< typename internal::FunctorTraits<Functor>::RunnableType, typename internal::FunctorTraits<Functor>::RunType, - void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])> - ::UnboundRunType> + void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])> > Bind(Functor functor $if ARITY > 0 [[, ]] $for ARG , [[const P$(ARG)& p$(ARG)]]) { // Typedefs for how to store and run the functor. @@ -126,13 +125,11 @@ $if ARG == 1 [[ ]] $$ $for ARG - typedef internal::BindState<RunnableType, RunType, [[]] -void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])> [[]] -BindState; - - return Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor)[[]] + return internal::MakeBindStateHolder( + new internal::BindState<RunnableType, RunType, [[]] +void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])>( + internal::MakeRunnable(functor)[[]] $if ARITY > 0 [[, ]] $for ARG , [[p$(ARG)]])); } diff --git a/base/bind_unittest.cc b/base/bind_unittest.cc index 372523b..654a277 100644 --- a/base/bind_unittest.cc +++ b/base/bind_unittest.cc @@ -199,18 +199,10 @@ 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() { @@ -292,25 +284,6 @@ 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 abaa438..4bf474d 100644 --- a/base/callback.h +++ b/base/callback.h @@ -129,28 +129,29 @@ // 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 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. +// 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. // -// 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. +// 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. // -// To BindState<> objects are created inside the Bind() functions. -// These functions, along with a set of internal templates, are responsible for +// To create BindStateHolder<> objects, we use the Bind() functions. +// These functions, along with a set of internal templates, are reponsible for // // - Unwrapping the function signature into return type, and parameters // - Determining the number of parameters that are bound -// - Creating the BindState storing the bound parameters +// - Creating the storage for the bound parameters // - Performing compile-time asserts to avoid error-prone behavior -// - 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. +// - 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. // // The Bind functions do the above using type-inference, and template // specializations. @@ -238,30 +239,27 @@ 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) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -287,20 +285,22 @@ class Callback<R(A1)> : public internal::CallbackBase { public: typedef R(RunType)(A1); - Callback() : CallbackBase(NULL) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -327,20 +327,22 @@ class Callback<R(A1, A2)> : public internal::CallbackBase { public: typedef R(RunType)(A1, A2); - Callback() : CallbackBase(NULL) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -370,20 +372,22 @@ class Callback<R(A1, A2, A3)> : public internal::CallbackBase { public: typedef R(RunType)(A1, A2, A3); - Callback() : CallbackBase(NULL) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -416,20 +420,22 @@ class Callback<R(A1, A2, A3, A4)> : public internal::CallbackBase { public: typedef R(RunType)(A1, A2, A3, A4); - Callback() : CallbackBase(NULL) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -466,20 +472,22 @@ class Callback<R(A1, A2, A3, A4, A5)> : public internal::CallbackBase { public: typedef R(RunType)(A1, A2, A3, A4, A5); - Callback() : CallbackBase(NULL) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -519,20 +527,22 @@ 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) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } @@ -575,20 +585,22 @@ 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) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } diff --git a/base/callback.h.pump b/base/callback.h.pump index 2216a3c..91a41e0 100644 --- a/base/callback.h.pump +++ b/base/callback.h.pump @@ -134,28 +134,29 @@ $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 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. +// 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. // -// 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. +// 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. // -// To BindState<> objects are created inside the Bind() functions. -// These functions, along with a set of internal templates, are responsible for +// To create BindStateHolder<> objects, we use the Bind() functions. +// These functions, along with a set of internal templates, are reponsible for // // - Unwrapping the function signature into return type, and parameters // - Determining the number of parameters that are bound -// - Creating the BindState storing the bound parameters +// - Creating the storage for the bound parameters // - Performing compile-time asserts to avoid error-prone behavior -// - 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. +// - 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. // // The Bind functions do the above using type-inference, and template // specializations. @@ -243,11 +244,6 @@ 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 [[ @@ -264,20 +260,22 @@ class Callback<R($for ARG , [[A$(ARG)]])> : public internal::CallbackBase { public: typedef R(RunType)($for ARG , [[A$(ARG)]]); - Callback() : CallbackBase(NULL) { } + Callback() : CallbackBase(NULL, 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 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 + 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 // so the compiler will typecheck that the passed in Run() method has // the correct type. - PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, RunType, BoundArgsType> - ::InvokerType::Run; + PolymorphicInvoke invoke_func = &T::InvokerType::Run; polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); } diff --git a/base/callback_internal.cc b/base/callback_internal.cc index a483293..582fbdc 100644 --- a/base/callback_internal.cc +++ b/base/callback_internal.cc @@ -4,8 +4,6 @@ #include "base/callback_internal.h" -#include "base/logging.h" - namespace base { namespace internal { @@ -23,10 +21,12 @@ bool CallbackBase::Equals(const CallbackBase& other) const { polymorphic_invoke_ == other.polymorphic_invoke_; } -CallbackBase::CallbackBase(BindStateBase* bind_state) - : bind_state_(bind_state), - polymorphic_invoke_(NULL) { - DCHECK(!bind_state_ || bind_state_->HasOneRef()); +CallbackBase::CallbackBase(InvokeFuncStorage polymorphic_invoke, + scoped_refptr<BindStateBase>* bind_state) + : polymorphic_invoke_(polymorphic_invoke) { + if (bind_state) { + bind_state_.swap(*bind_state); + } } CallbackBase::~CallbackBase() { diff --git a/base/callback_internal.h b/base/callback_internal.h index 4bb8aa9..81c87c0 100644 --- a/base/callback_internal.h +++ b/base/callback_internal.h @@ -29,6 +29,29 @@ 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 { @@ -49,11 +72,8 @@ class BASE_EXPORT CallbackBase { // Returns true if this callback equals |other|. |other| may be null. bool Equals(const CallbackBase& other) const; - // 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); + CallbackBase(InvokeFuncStorage polymorphic_invoke, + scoped_refptr<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 e42b933..8527e70 100644 --- a/base/callback_unittest.cc +++ b/base/callback_unittest.cc @@ -9,7 +9,6 @@ #include "testing/gtest/include/gtest/gtest.h" namespace base { - namespace { class HelperObject { @@ -27,40 +26,18 @@ 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. 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 { +// comparators and emptiness APIs. +class FakeBindState1 : public internal::BindStateBase { public: typedef FakeInvoker InvokerType; }; -template <> -struct BindState<void(void), void(void), - void(FakeInvoker, FakeInvoker)> - : public BindStateBase { +class FakeBindState2 : public internal::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; @@ -83,8 +60,8 @@ TEST(CallbackOld, ReturnValue) { class CallbackTest : public ::testing::Test { public: CallbackTest() - : callback_a_(new FakeBindState1()), - callback_b_(new FakeBindState2()) { + : callback_a_(MakeBindStateHolder(new FakeBindState1())), + callback_b_(MakeBindStateHolder(new FakeBindState2())) { } virtual ~CallbackTest() { @@ -128,7 +105,8 @@ TEST_F(CallbackTest, Equals) { EXPECT_FALSE(callback_b_.Equals(callback_a_)); // We should compare based on instance, not type. - Callback<void(void)> callback_c(new FakeBindState1()); + Callback<void(void)> callback_c( + MakeBindStateHolder(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 4de7d12..b432f61 100644 --- a/base/cancelable_callback.h +++ b/base/cancelable_callback.h @@ -173,7 +173,8 @@ class CancelableCallback<void(A1)> { } private: - void Forward(A1 a1) const { + void Forward( + typename internal::CallbackParamTraits<A1>::ForwardType a1) const { callback_.Run(a1); } diff --git a/base/debug/trace_event.cc b/base/debug/trace_event.cc index 61d3daf..5be1557 100644 --- a/base/debug/trace_event.cc +++ b/base/debug/trace_event.cc @@ -296,8 +296,7 @@ TraceResultBuffer::TraceResultBuffer() : append_comma_(false) { TraceResultBuffer::~TraceResultBuffer() { } -void TraceResultBuffer::SetOutputCallback( - const OutputCallback& json_chunk_callback) { +void TraceResultBuffer::SetOutputCallback(OutputCallback json_chunk_callback) { output_callback_ = json_chunk_callback; } diff --git a/base/debug/trace_event.h b/base/debug/trace_event.h index 621d6f06..ab56dfc 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(const OutputCallback& json_chunk_callback); + void SetOutputCallback(OutputCallback json_chunk_callback); // Start JSON output. This resets all internal state, so you can reuse // the TraceResultBuffer by calling Start. @@ -818,8 +818,7 @@ 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(const scoped_refptr<RefCountedString>&)> - OutputCallback; + typedef base::Callback<void(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 913a9d3..0ff986a 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( - const scoped_refptr<TraceLog::RefCountedString>& events_str); + 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( - const scoped_refptr<TraceLog::RefCountedString>& events_str) { + 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 1c9fafe..96f8045 100644 --- a/base/test/trace_event_analyzer_unittest.cc +++ b/base/test/trace_event_analyzer_unittest.cc @@ -15,8 +15,7 @@ class TraceEventAnalyzerTest : public testing::Test { public: void ManualSetUp(); void OnTraceDataCollected( - const scoped_refptr<base::debug::TraceLog::RefCountedString>& - json_events_str); + scoped_refptr<base::debug::TraceLog::RefCountedString> json_events_str); void BeginTracing(); void EndTracing(); @@ -36,8 +35,7 @@ void TraceEventAnalyzerTest::ManualSetUp() { } void TraceEventAnalyzerTest::OnTraceDataCollected( - const scoped_refptr<base::debug::TraceLog::RefCountedString>& - json_events_str) { + scoped_refptr<base::debug::TraceLog::RefCountedString> json_events_str) { buffer_.AddFragment(json_events_str->data); } @@ -611,3 +609,4 @@ TEST_F(TraceEventAnalyzerTest, RateStats) { } // namespace trace_analyzer + |