From 884a7284bd11e46840006e92ea8b5fe52abdde27 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 17 Feb 2017 15:57:05 -0800 Subject: [PATCH] Revert of Use std::unique_ptr instead of rtc::scoped_refptr in AsyncInvoker. (patchset #2 id:20001 of https://codereview.webrtc.org/2689233003/ ) Reason for revert: The change to messagequeue.h isn't backwards compatible. Will reland after making it backwards compatible. Original issue's description: > Use std::unique_ptr instead of rtc::scoped_refptr in AsyncInvoker. > > The AsyncClosures only ever have one thing referencing them, so they > should be using std::unique_ptr to manage ownership. Maybe this code was > written before std::unique_ptr was available. > > BUG=None > > Review-Url: https://codereview.webrtc.org/2689233003 > Cr-Commit-Position: refs/heads/master@{#16680} > Committed: https://chromium.googlesource.com/external/webrtc/+/a5a472927bb209a053b2648d1f0b006f4c8c30ac TBR=pthatcher@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=None Review-Url: https://codereview.webrtc.org/2703613006 Cr-Commit-Position: refs/heads/master@{#16683} --- webrtc/base/asyncinvoker-inl.h | 10 +++++++--- webrtc/base/asyncinvoker.cc | 19 +++++++++++-------- webrtc/base/asyncinvoker.h | 32 +++++++++++++++----------------- webrtc/base/messagequeue.h | 8 +++----- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/webrtc/base/asyncinvoker-inl.h b/webrtc/base/asyncinvoker-inl.h index c3691b5659..15fafa888b 100644 --- a/webrtc/base/asyncinvoker-inl.h +++ b/webrtc/base/asyncinvoker-inl.h @@ -15,6 +15,8 @@ #include "webrtc/base/callback.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/messagehandler.h" +#include "webrtc/base/refcount.h" +#include "webrtc/base/scoped_ref_ptr.h" #include "webrtc/base/sigslot.h" #include "webrtc/base/thread.h" @@ -23,13 +25,15 @@ namespace rtc { class AsyncInvoker; // Helper class for AsyncInvoker. Runs a task and triggers a callback -// on the calling thread if necessary. -class AsyncClosure { +// on the calling thread if necessary. Instances are ref-counted so their +// lifetime can be independent of AsyncInvoker. +class AsyncClosure : public RefCountInterface { public: - virtual ~AsyncClosure() {} // Runs the asynchronous task, and triggers a callback to the calling // thread if needed. Should be called from the target thread. virtual void Execute() = 0; + protected: + ~AsyncClosure() override {} }; // Simple closure that doesn't trigger a callback for the calling thread. diff --git a/webrtc/base/asyncinvoker.cc b/webrtc/base/asyncinvoker.cc index cc9cde59c0..83a873811e 100644 --- a/webrtc/base/asyncinvoker.cc +++ b/webrtc/base/asyncinvoker.cc @@ -26,11 +26,14 @@ AsyncInvoker::~AsyncInvoker() { void AsyncInvoker::OnMessage(Message* msg) { // Get the AsyncClosure shared ptr from this message's data. - ScopedMessageData* data = - static_cast*>(msg->pdata); + ScopedRefMessageData* data = + static_cast*>(msg->pdata); + scoped_refptr closure = data->data(); + delete msg->pdata; + msg->pdata = NULL; + // Execute the closure and trigger the return message if needed. - data->data().Execute(); - delete data; + closure->Execute(); } void AsyncInvoker::Flush(Thread* thread, uint32_t id /*= MQID_ANY*/) { @@ -53,19 +56,19 @@ void AsyncInvoker::Flush(Thread* thread, uint32_t id /*= MQID_ANY*/) { void AsyncInvoker::DoInvoke(const Location& posted_from, Thread* thread, - std::unique_ptr closure, + const scoped_refptr& closure, uint32_t id) { if (destroying_) { LOG(LS_WARNING) << "Tried to invoke while destroying the invoker."; return; } thread->Post(posted_from, this, id, - new ScopedMessageData(std::move(closure))); + new ScopedRefMessageData(closure)); } void AsyncInvoker::DoInvokeDelayed(const Location& posted_from, Thread* thread, - std::unique_ptr closure, + const scoped_refptr& closure, uint32_t delay_ms, uint32_t id) { if (destroying_) { @@ -73,7 +76,7 @@ void AsyncInvoker::DoInvokeDelayed(const Location& posted_from, return; } thread->PostDelayed(posted_from, delay_ms, this, id, - new ScopedMessageData(std::move(closure))); + new ScopedRefMessageData(closure)); } GuardedAsyncInvoker::GuardedAsyncInvoker() : thread_(Thread::Current()) { diff --git a/webrtc/base/asyncinvoker.h b/webrtc/base/asyncinvoker.h index 43fffeac10..d91e30601e 100644 --- a/webrtc/base/asyncinvoker.h +++ b/webrtc/base/asyncinvoker.h @@ -11,13 +11,11 @@ #ifndef WEBRTC_BASE_ASYNCINVOKER_H_ #define WEBRTC_BASE_ASYNCINVOKER_H_ -#include -#include - #include "webrtc/base/asyncinvoker-inl.h" #include "webrtc/base/bind.h" #include "webrtc/base/constructormagic.h" #include "webrtc/base/sigslot.h" +#include "webrtc/base/scopedptrcollection.h" #include "webrtc/base/thread.h" namespace rtc { @@ -81,9 +79,9 @@ class AsyncInvoker : public MessageHandler { Thread* thread, const FunctorT& functor, uint32_t id = 0) { - std::unique_ptr closure( - new FireAndForgetAsyncClosure(functor)); - DoInvoke(posted_from, thread, std::move(closure), id); + scoped_refptr closure( + new RefCountedObject >(functor)); + DoInvoke(posted_from, thread, closure, id); } // Call |functor| asynchronously on |thread| with |delay_ms|, with no callback @@ -94,9 +92,9 @@ class AsyncInvoker : public MessageHandler { const FunctorT& functor, uint32_t delay_ms, uint32_t id = 0) { - std::unique_ptr closure( - new FireAndForgetAsyncClosure(functor)); - DoInvokeDelayed(posted_from, thread, std::move(closure), delay_ms, id); + scoped_refptr closure( + new RefCountedObject >(functor)); + DoInvokeDelayed(posted_from, thread, closure, delay_ms, id); } // Call |functor| asynchronously on |thread|, calling |callback| when done. @@ -110,11 +108,11 @@ class AsyncInvoker : public MessageHandler { void (HostT::*callback)(ReturnT), HostT* callback_host, uint32_t id = 0) { - std::unique_ptr closure( - new NotifyingAsyncClosure( + scoped_refptr closure( + new RefCountedObject >( this, callback_posted_from, Thread::Current(), functor, callback, callback_host)); - DoInvoke(posted_from, thread, std::move(closure), id); + DoInvoke(posted_from, thread, closure, id); } // Call |functor| asynchronously on |thread|, calling |callback| when done. @@ -129,11 +127,11 @@ class AsyncInvoker : public MessageHandler { void (HostT::*callback)(), HostT* callback_host, uint32_t id = 0) { - std::unique_ptr closure( - new NotifyingAsyncClosure( + scoped_refptr closure( + new RefCountedObject >( this, callback_posted_from, Thread::Current(), functor, callback, callback_host)); - DoInvoke(posted_from, thread, std::move(closure), id); + DoInvoke(posted_from, thread, closure, id); } // Synchronously execute on |thread| all outstanding calls we own @@ -150,11 +148,11 @@ class AsyncInvoker : public MessageHandler { void OnMessage(Message* msg) override; void DoInvoke(const Location& posted_from, Thread* thread, - std::unique_ptr closure, + const scoped_refptr& closure, uint32_t id); void DoInvokeDelayed(const Location& posted_from, Thread* thread, - std::unique_ptr closure, + const scoped_refptr& closure, uint32_t delay_ms, uint32_t id); bool destroying_; diff --git a/webrtc/base/messagequeue.h b/webrtc/base/messagequeue.h index 3750eb7271..429a56a8c1 100644 --- a/webrtc/base/messagequeue.h +++ b/webrtc/base/messagequeue.h @@ -17,7 +17,6 @@ #include #include #include -#include #include #include "webrtc/base/basictypes.h" @@ -99,10 +98,9 @@ class TypedMessageData : public MessageData { template class ScopedMessageData : public MessageData { public: - explicit ScopedMessageData(std::unique_ptr data) - : data_(std::move(data)) {} - const T& data() const { return *data_; } - T& data() { return *data_; } + explicit ScopedMessageData(T* data) : data_(data) { } + const std::unique_ptr& data() const { return data_; } + std::unique_ptr& data() { return data_; } private: std::unique_ptr data_;