From b274547ebdefd0db470e21cfe113457cabd99dff Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Tue, 25 Aug 2015 17:56:22 +0200 Subject: [PATCH] rtc::Bind: Capture scoped_refptr reference arguments by value R=tommi@webrtc.org Review URL: https://codereview.webrtc.org/1308563004 . Cr-Commit-Position: refs/heads/master@{#9780} --- webrtc/base/bind.h | 96 ++++++++++++++------------ webrtc/base/bind.h.pump | 16 ++++- webrtc/base/bind_unittest.cc | 128 ++++++++++++++++++++++++----------- 3 files changed, 158 insertions(+), 82 deletions(-) diff --git a/webrtc/base/bind.h b/webrtc/base/bind.h index 2d8114052d..923fda21b5 100644 --- a/webrtc/base/bind.h +++ b/webrtc/base/bind.h @@ -128,6 +128,18 @@ struct PointerType { T*>::type type; }; +// RemoveScopedPtrRef will capture scoped_refptr by-value instead of +// by-reference. +template struct RemoveScopedPtrRef { typedef T type; }; +template +struct RemoveScopedPtrRef&> { + typedef scoped_refptr type; +}; +template +struct RemoveScopedPtrRef&> { + typedef scoped_refptr type; +}; + } // namespace detail template @@ -208,7 +220,7 @@ class MethodFunctor1 { private: MethodT method_; typename detail::PointerType::type object_; - P1 p1_; + typename detail::RemoveScopedPtrRef::type p1_; }; template ::type p1_; }; @@ -291,8 +303,8 @@ class MethodFunctor2 { private: MethodT method_; typename detail::PointerType::type object_; - P1 p1_; - P2 p2_; + typename detail::RemoveScopedPtrRef::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; }; template ::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; }; @@ -389,9 +401,9 @@ class MethodFunctor3 { private: MethodT method_; typename detail::PointerType::type object_; - P1 p1_; - P2 p2_; - P3 p3_; + typename detail::RemoveScopedPtrRef::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; }; template ::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; }; @@ -502,10 +514,10 @@ class MethodFunctor4 { private: MethodT method_; typename detail::PointerType::type object_; - P1 p1_; - P2 p2_; - P3 p3_; - P4 p4_; + typename detail::RemoveScopedPtrRef::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; + typename detail::RemoveScopedPtrRef::type p4_; }; template ::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; + typename detail::RemoveScopedPtrRef::type p4_; }; @@ -630,11 +642,11 @@ class MethodFunctor5 { private: MethodT method_; typename detail::PointerType::type object_; - P1 p1_; - P2 p2_; - P3 p3_; - P4 p4_; - P5 p5_; + typename detail::RemoveScopedPtrRef::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; + typename detail::RemoveScopedPtrRef::type p4_; + typename detail::RemoveScopedPtrRef::type p5_; }; template ::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; + typename detail::RemoveScopedPtrRef::type p4_; + typename detail::RemoveScopedPtrRef::type p5_; }; @@ -773,12 +785,12 @@ class MethodFunctor6 { private: MethodT method_; typename detail::PointerType::type object_; - P1 p1_; - P2 p2_; - P3 p3_; - P4 p4_; - P5 p5_; - P6 p6_; + typename detail::RemoveScopedPtrRef::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; + typename detail::RemoveScopedPtrRef::type p4_; + typename detail::RemoveScopedPtrRef::type p5_; + typename detail::RemoveScopedPtrRef::type p6_; }; template ::type p1_; + typename detail::RemoveScopedPtrRef::type p2_; + typename detail::RemoveScopedPtrRef::type p3_; + typename detail::RemoveScopedPtrRef::type p4_; + typename detail::RemoveScopedPtrRef::type p5_; + typename detail::RemoveScopedPtrRef::type p6_; }; diff --git a/webrtc/base/bind.h.pump b/webrtc/base/bind.h.pump index 9a4bc664c3..e1cea61fc2 100644 --- a/webrtc/base/bind.h.pump +++ b/webrtc/base/bind.h.pump @@ -124,6 +124,18 @@ struct PointerType { T*>::type type; }; +// RemoveScopedPtrRef will capture scoped_refptr by-value instead of +// by-reference. +template struct RemoveScopedPtrRef { typedef T type; }; +template +struct RemoveScopedPtrRef&> { + typedef scoped_refptr type; +}; +template +struct RemoveScopedPtrRef&> { + typedef scoped_refptr type; +}; + } // namespace detail $var n = 6 @@ -145,7 +157,7 @@ class MethodFunctor$i { MethodT method_; typename detail::PointerType::type object_;$for j [[ - P$j p$(j)_;]] + typename detail::RemoveScopedPtrRef::type p$(j)_;]] }; @@ -162,7 +174,7 @@ Functor$i(const FunctorT& functor$for j [[, P$j p$j]]) private: FunctorT functor_;$for j [[ - P$j p$(j)_;]] + typename detail::RemoveScopedPtrRef::type p$(j)_;]] }; diff --git a/webrtc/base/bind_unittest.cc b/webrtc/base/bind_unittest.cc index 7a621dce2c..d38729d9df 100644 --- a/webrtc/base/bind_unittest.cc +++ b/webrtc/base/bind_unittest.cc @@ -17,6 +17,8 @@ namespace rtc { namespace { +struct LifeTimeCheck; + struct MethodBindTester { void NullaryVoid() { ++call_count; } int NullaryInt() { ++call_count; return 1; } @@ -25,6 +27,10 @@ struct MethodBindTester { template T Identity(T value) { ++call_count; return value; } int UnaryByRef(int& value) const { ++call_count; return ++value; } // NOLINT int Multiply(int a, int b) const { ++call_count; return a * b; } + void RefArgument(const scoped_refptr& object) { + EXPECT_TRUE(object.get() != nullptr); + } + mutable int call_count; }; @@ -42,19 +48,12 @@ struct F { void Release(); }; -class LifeTimeCheck : public RefCountInterface { - public: - LifeTimeCheck(bool* has_died) : has_died_(has_died), is_ok_to_die_(false) {} - ~LifeTimeCheck() { - EXPECT_TRUE(is_ok_to_die_); - *has_died_ = true; - } - void PrepareToDie() { is_ok_to_die_ = true; } +struct LifeTimeCheck { + LifeTimeCheck() : ref_count_(0) {} + void AddRef() { ++ref_count_; } + void Release() { --ref_count_; } void NullaryVoid() {} - - private: - bool* const has_died_; - bool is_ok_to_die_; + int ref_count_; }; int Return42() { return 42; } @@ -65,6 +64,27 @@ int Multiply(int a, int b) { return a * b; } // Try to catch any problem with scoped_refptr type deduction in rtc::Bind at // compile time. +static_assert(is_same&>::type, + scoped_refptr>::value, + "const scoped_refptr& should be captured by value"); + +static_assert(is_same&>::type, + scoped_refptr>::value, + "const scoped_refptr& should be captured by value"); + +static_assert( + is_same::type, const int&>::value, + "const int& should be captured as const int&"); + +static_assert( + is_same::type, const F&>::value, + "const F& should be captured as const F&"); + +static_assert( + is_same::type, F&>::value, + "F& should be captured as F&"); + #define EXPECT_IS_CAPTURED_AS_PTR(T) \ static_assert(is_same::type, T*>::value, \ "PointerType") @@ -124,46 +144,78 @@ TEST(BindTest, BindToFunction) { // Test Bind where method object implements RefCountInterface and is passed as a // pointer. TEST(BindTest, CapturePointerAsScopedRefPtr) { - bool object_has_died = false; - scoped_refptr object = - new RefCountedObject(&object_has_died); + LifeTimeCheck object; + EXPECT_EQ(object.ref_count_, 0); + scoped_refptr scoped_object(&object); + EXPECT_EQ(object.ref_count_, 1); { - auto functor = Bind(&LifeTimeCheck::PrepareToDie, object.get()); - object = nullptr; - EXPECT_FALSE(object_has_died); - // Run prepare to die via functor. - functor(); + auto functor = Bind(&LifeTimeCheck::NullaryVoid, &object); + EXPECT_EQ(object.ref_count_, 2); + scoped_object = nullptr; + EXPECT_EQ(object.ref_count_, 1); } - EXPECT_TRUE(object_has_died); + EXPECT_EQ(object.ref_count_, 0); } // Test Bind where method object implements RefCountInterface and is passed as a // scoped_refptr<>. TEST(BindTest, CaptureScopedRefPtrAsScopedRefPtr) { - bool object_has_died = false; - scoped_refptr object = - new RefCountedObject(&object_has_died); + LifeTimeCheck object; + EXPECT_EQ(object.ref_count_, 0); + scoped_refptr scoped_object(&object); + EXPECT_EQ(object.ref_count_, 1); { - auto functor = Bind(&LifeTimeCheck::PrepareToDie, object); - object = nullptr; - EXPECT_FALSE(object_has_died); - // Run prepare to die via functor. - functor(); + auto functor = Bind(&LifeTimeCheck::NullaryVoid, scoped_object); + EXPECT_EQ(object.ref_count_, 2); + scoped_object = nullptr; + EXPECT_EQ(object.ref_count_, 1); } - EXPECT_TRUE(object_has_died); + EXPECT_EQ(object.ref_count_, 0); } // Test Bind where method object is captured as scoped_refptr<> and the functor // dies while there are references left. TEST(BindTest, FunctorReleasesObjectOnDestruction) { - bool object_has_died = false; - scoped_refptr object = - new RefCountedObject(&object_has_died); - Bind(&LifeTimeCheck::NullaryVoid, object.get())(); - EXPECT_FALSE(object_has_died); - object->PrepareToDie(); - object = nullptr; - EXPECT_TRUE(object_has_died); + LifeTimeCheck object; + EXPECT_EQ(object.ref_count_, 0); + scoped_refptr scoped_object(&object); + EXPECT_EQ(object.ref_count_, 1); + Bind(&LifeTimeCheck::NullaryVoid, &object)(); + EXPECT_EQ(object.ref_count_, 1); + scoped_object = nullptr; + EXPECT_EQ(object.ref_count_, 0); +} + +// Test Bind with scoped_refptr<> argument. +TEST(BindTest, ScopedRefPointerArgument) { + LifeTimeCheck object; + EXPECT_EQ(object.ref_count_, 0); + scoped_refptr scoped_object(&object); + EXPECT_EQ(object.ref_count_, 1); + { + MethodBindTester bind_tester; + auto functor = + Bind(&MethodBindTester::RefArgument, &bind_tester, scoped_object); + EXPECT_EQ(object.ref_count_, 2); + } + EXPECT_EQ(object.ref_count_, 1); + scoped_object = nullptr; + EXPECT_EQ(object.ref_count_, 0); +} + +namespace { + +const int* Ref(const int& a) { return &a; } + +} // anonymous namespace + +// Test Bind with non-scoped_refptr<> reference argument. +TEST(BindTest, RefArgument) { + const int x = 42; + EXPECT_TRUE(Ref(x) == &x); + // Bind() should not make a copy of |x|, i.e. the pointers should be the same. + auto functor = Bind(&Ref, x); + EXPECT_TRUE(functor() == &x); } } // namespace rtc