From c4a3173db0359b6b246ac07b5bd3bdbcc51680b0 Mon Sep 17 00:00:00 2001 From: nisse Date: Tue, 16 May 2017 05:51:29 -0700 Subject: [PATCH] Delete unused features of AsyncInvoke. Also eliminates its dependency on callback.h. BUG=None Review-Url: https://codereview.webrtc.org/2871403003 Cr-Commit-Position: refs/heads/master@{#18163} --- webrtc/base/asyncinvoker-inl.h | 88 ------------------ webrtc/base/asyncinvoker.cc | 35 ------- webrtc/base/asyncinvoker.h | 37 -------- webrtc/base/thread_unittest.cc | 164 --------------------------------- 4 files changed, 324 deletions(-) diff --git a/webrtc/base/asyncinvoker-inl.h b/webrtc/base/asyncinvoker-inl.h index 93e7671a95..5f7cd4959a 100644 --- a/webrtc/base/asyncinvoker-inl.h +++ b/webrtc/base/asyncinvoker-inl.h @@ -13,7 +13,6 @@ #include "webrtc/base/atomicops.h" #include "webrtc/base/bind.h" -#include "webrtc/base/callback.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/messagehandler.h" #include "webrtc/base/sigslot.h" @@ -52,93 +51,6 @@ class FireAndForgetAsyncClosure : public AsyncClosure { FunctorT functor_; }; -// Base class for closures that may trigger a callback for the calling thread. -// Listens for the "destroyed" signals from the calling thread and the invoker, -// and cancels the callback to the calling thread if either is destroyed. -class NotifyingAsyncClosureBase : public AsyncClosure, - public sigslot::has_slots<> { - public: - ~NotifyingAsyncClosureBase() override; - - protected: - NotifyingAsyncClosureBase(AsyncInvoker* invoker, - const Location& callback_posted_from, - Thread* calling_thread); - void TriggerCallback(); - void SetCallback(const Callback0& callback) { - CritScope cs(&crit_); - callback_ = callback; - } - bool CallbackCanceled() const { - CritScope cs(&crit_); - return calling_thread_ == nullptr; - } - - private: - Location callback_posted_from_; - CriticalSection crit_; - Callback0 callback_ GUARDED_BY(crit_); - Thread* calling_thread_ GUARDED_BY(crit_); - - void CancelCallback(); -}; - -// Closures that have a non-void return value and require a callback. -template -class NotifyingAsyncClosure : public NotifyingAsyncClosureBase { - public: - NotifyingAsyncClosure(AsyncInvoker* invoker, - const Location& callback_posted_from, - Thread* calling_thread, - const FunctorT& functor, - void (HostT::*callback)(ReturnT), - HostT* callback_host) - : NotifyingAsyncClosureBase(invoker, - callback_posted_from, - calling_thread), - functor_(functor), - callback_(callback), - callback_host_(callback_host) {} - virtual void Execute() { - ReturnT result = functor_(); - if (!CallbackCanceled()) { - SetCallback(Callback0(Bind(callback_, callback_host_, result))); - TriggerCallback(); - } - } - - private: - FunctorT functor_; - void (HostT::*callback_)(ReturnT); - HostT* callback_host_; -}; - -// Closures that have a void return value and require a callback. -template -class NotifyingAsyncClosure - : public NotifyingAsyncClosureBase { - public: - NotifyingAsyncClosure(AsyncInvoker* invoker, - const Location& callback_posted_from, - Thread* calling_thread, - const FunctorT& functor, - void (HostT::*callback)(), - HostT* callback_host) - : NotifyingAsyncClosureBase(invoker, - callback_posted_from, - calling_thread), - functor_(functor) { - SetCallback(Callback0(Bind(callback, callback_host))); - } - virtual void Execute() { - functor_(); - TriggerCallback(); - } - - private: - FunctorT functor_; -}; - } // namespace rtc #endif // WEBRTC_BASE_ASYNCINVOKER_INL_H_ diff --git a/webrtc/base/asyncinvoker.cc b/webrtc/base/asyncinvoker.cc index cabd8b506d..bfd13172f7 100644 --- a/webrtc/base/asyncinvoker.cc +++ b/webrtc/base/asyncinvoker.cc @@ -116,39 +116,4 @@ AsyncClosure::~AsyncClosure() { invoker_->invocation_complete_.Set(); } -NotifyingAsyncClosureBase::NotifyingAsyncClosureBase( - AsyncInvoker* invoker, - const Location& callback_posted_from, - Thread* calling_thread) - : AsyncClosure(invoker), - callback_posted_from_(callback_posted_from), - calling_thread_(calling_thread) { - calling_thread->SignalQueueDestroyed.connect( - this, &NotifyingAsyncClosureBase::CancelCallback); - // Note: We don't need to listen for the invoker being destroyed, because it - // will wait for this closure to be destroyed (and pending_invocations_ to go - // to 0) before its destructor completes. -} - -NotifyingAsyncClosureBase::~NotifyingAsyncClosureBase() { - disconnect_all(); -} - -void NotifyingAsyncClosureBase::TriggerCallback() { - CritScope cs(&crit_); - if (!CallbackCanceled() && !callback_.empty()) { - invoker_->AsyncInvoke(callback_posted_from_, calling_thread_, - callback_); - } -} - -void NotifyingAsyncClosureBase::CancelCallback() { - // If the callback is triggering when this is called, block the - // destructor of the dying object here by waiting until the callback - // is done triggering. - CritScope cs(&crit_); - // calling_thread_ == nullptr means do not trigger the callback. - calling_thread_ = nullptr; -} - } // namespace rtc diff --git a/webrtc/base/asyncinvoker.h b/webrtc/base/asyncinvoker.h index 27f6a9fec0..5414867418 100644 --- a/webrtc/base/asyncinvoker.h +++ b/webrtc/base/asyncinvoker.h @@ -100,43 +100,6 @@ class AsyncInvoker : public MessageHandler { DoInvokeDelayed(posted_from, thread, std::move(closure), delay_ms, id); } - // Call |functor| asynchronously on |thread|, calling |callback| when done. - // Uses a separate Location for |callback_posted_from| so that the functor - // invoke and the callback invoke can be differentiated. - template - void AsyncInvoke(const Location& posted_from, - const Location& callback_posted_from, - Thread* thread, - const FunctorT& functor, - void (HostT::*callback)(ReturnT), - HostT* callback_host, - uint32_t id = 0) { - std::unique_ptr closure( - new NotifyingAsyncClosure( - this, callback_posted_from, Thread::Current(), functor, callback, - callback_host)); - DoInvoke(posted_from, thread, std::move(closure), id); - } - - // Call |functor| asynchronously on |thread|, calling |callback| when done. - // Uses a separate Location for |callback_posted_from| so that the functor - // invoke and the callback invoke can be differentiated. - // Overloaded for void return. - template - void AsyncInvoke(const Location& posted_from, - const Location& callback_posted_from, - Thread* thread, - const FunctorT& functor, - void (HostT::*callback)(), - HostT* callback_host, - uint32_t id = 0) { - std::unique_ptr closure( - new NotifyingAsyncClosure( - this, callback_posted_from, Thread::Current(), functor, callback, - callback_host)); - DoInvoke(posted_from, thread, std::move(closure), id); - } - // Synchronously execute on |thread| all outstanding calls we own // that are pending on |thread|, and wait for calls to complete // before returning. Optionally filter by message id. diff --git a/webrtc/base/thread_unittest.cc b/webrtc/base/thread_unittest.cc index 35ea60468c..c143120fbc 100644 --- a/webrtc/base/thread_unittest.cc +++ b/webrtc/base/thread_unittest.cc @@ -421,13 +421,6 @@ class AsyncInvokeTest : public testing::Test { EXPECT_EQ(expected_thread_, Thread::Current()); int_value_ = value; } - void AsyncInvokeIntCallback(AsyncInvoker* invoker, Thread* thread) { - expected_thread_ = thread; - invoker->AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, thread, FunctorC(), - &AsyncInvokeTest::IntCallback, - static_cast(this)); - invoke_started_.Set(); - } void SetExpectedThreadForIntCallback(Thread* thread) { expected_thread_ = thread; } @@ -436,11 +429,9 @@ class AsyncInvokeTest : public testing::Test { enum { kWaitTimeout = 1000 }; AsyncInvokeTest() : int_value_(0), - invoke_started_(true, false), expected_thread_(nullptr) {} int int_value_; - Event invoke_started_; Thread* expected_thread_; }; @@ -455,71 +446,6 @@ TEST_F(AsyncInvokeTest, FireAndForget) { EXPECT_TRUE_WAIT(called.get(), kWaitTimeout); } -TEST_F(AsyncInvokeTest, WithCallback) { - AsyncInvoker invoker; - // Create and start the thread. - Thread thread; - thread.Start(); - // Try calling functor. - SetExpectedThreadForIntCallback(Thread::Current()); - invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, &thread, FunctorA(), - &AsyncInvokeTest::IntCallback, - static_cast(this)); - EXPECT_EQ_WAIT(42, int_value_, kWaitTimeout); -} - -TEST_F(AsyncInvokeTest, CancelInvoker) { - // Create and start the thread. - Thread thread; - thread.Start(); - // Try destroying invoker during call. - { - AsyncInvoker invoker; - invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, &thread, FunctorC(), - &AsyncInvokeTest::IntCallback, - static_cast(this)); - } - // With invoker gone, callback should be cancelled. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - -TEST_F(AsyncInvokeTest, CancelCallingThread) { - AsyncInvoker invoker; - { // Create and start the thread. - Thread thread; - thread.Start(); - // Try calling functor. - thread.Invoke( - RTC_FROM_HERE, - Bind(&AsyncInvokeTest::AsyncInvokeIntCallback, - static_cast(this), &invoker, Thread::Current())); - // Wait for the call to begin. - ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout)); - } - // Calling thread is gone. Return message shouldn't happen. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - -TEST_F(AsyncInvokeTest, KillInvokerBeforeExecute) { - Thread thread; - thread.Start(); - { - AsyncInvoker invoker; - // Try calling functor. - thread.Invoke( - RTC_FROM_HERE, - Bind(&AsyncInvokeTest::AsyncInvokeIntCallback, - static_cast(this), &invoker, Thread::Current())); - // Wait for the call to begin. - ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout)); - } - // Invoker is destroyed. Function should not execute. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - TEST_F(AsyncInvokeTest, KillInvokerDuringExecute) { // Use these events to get in a state where the functor is in the middle of // executing, and then to wait for it to finish, ensuring the "EXPECT_FALSE" @@ -599,13 +525,6 @@ class GuardedAsyncInvokeTest : public testing::Test { EXPECT_EQ(expected_thread_, Thread::Current()); int_value_ = value; } - void AsyncInvokeIntCallback(GuardedAsyncInvoker* invoker, Thread* thread) { - expected_thread_ = thread; - invoker->AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorC(), - &GuardedAsyncInvokeTest::IntCallback, - static_cast(this)); - invoke_started_.Set(); - } void SetExpectedThreadForIntCallback(Thread* thread) { expected_thread_ = thread; } @@ -614,11 +533,9 @@ class GuardedAsyncInvokeTest : public testing::Test { const static int kWaitTimeout = 1000; GuardedAsyncInvokeTest() : int_value_(0), - invoke_started_(true, false), expected_thread_(nullptr) {} int int_value_; - Event invoke_started_; Thread* expected_thread_; }; @@ -648,26 +565,6 @@ TEST_F(GuardedAsyncInvokeTest, KillThreadFireAndForget) { EXPECT_FALSE(called.get()); } -// Test that we can call AsyncInvoke with callback after the thread died. -TEST_F(GuardedAsyncInvokeTest, KillThreadWithCallback) { - // Create and start the thread. - std::unique_ptr thread(new Thread()); - thread->Start(); - std::unique_ptr invoker; - // Create the invoker on |thread|. - thread->Invoke(RTC_FROM_HERE, CreateInvoker(&invoker)); - // Kill |thread|. - thread = nullptr; - // Try calling functor. - EXPECT_FALSE( - invoker->AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorC(), - &GuardedAsyncInvokeTest::IntCallback, - static_cast(this))); - // With thread gone, callback should be cancelled. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - // The remaining tests check that GuardedAsyncInvoker behaves as AsyncInvoker // when Thread is still alive. TEST_F(GuardedAsyncInvokeTest, FireAndForget) { @@ -678,67 +575,6 @@ TEST_F(GuardedAsyncInvokeTest, FireAndForget) { EXPECT_TRUE_WAIT(called.get(), kWaitTimeout); } -TEST_F(GuardedAsyncInvokeTest, WithCallback) { - GuardedAsyncInvoker invoker; - // Try calling functor. - SetExpectedThreadForIntCallback(Thread::Current()); - EXPECT_TRUE(invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorA(), - &GuardedAsyncInvokeTest::IntCallback, - static_cast(this))); - EXPECT_EQ_WAIT(42, int_value_, kWaitTimeout); -} - -TEST_F(GuardedAsyncInvokeTest, CancelInvoker) { - // Try destroying invoker during call. - { - GuardedAsyncInvoker invoker; - EXPECT_TRUE( - invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorC(), - &GuardedAsyncInvokeTest::IntCallback, - static_cast(this))); - } - // With invoker gone, callback should be cancelled. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - -TEST_F(GuardedAsyncInvokeTest, CancelCallingThread) { - GuardedAsyncInvoker invoker; - // Try destroying calling thread during call. - { - Thread thread; - thread.Start(); - // Try calling functor. - thread.Invoke(RTC_FROM_HERE, - Bind(&GuardedAsyncInvokeTest::AsyncInvokeIntCallback, - static_cast(this), - &invoker, Thread::Current())); - // Wait for the call to begin. - ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout)); - } - // Calling thread is gone. Return message shouldn't happen. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - -TEST_F(GuardedAsyncInvokeTest, KillInvokerBeforeExecute) { - Thread thread; - thread.Start(); - { - GuardedAsyncInvoker invoker; - // Try calling functor. - thread.Invoke(RTC_FROM_HERE, - Bind(&GuardedAsyncInvokeTest::AsyncInvokeIntCallback, - static_cast(this), - &invoker, Thread::Current())); - // Wait for the call to begin. - ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout)); - } - // Invoker is destroyed. Function should not execute. - Thread::Current()->ProcessMessages(kWaitTimeout); - EXPECT_EQ(0, int_value_); -} - TEST_F(GuardedAsyncInvokeTest, Flush) { GuardedAsyncInvoker invoker; AtomicBool flag1;