From 72c8d2b708d442d16700229fe22918d187534430 Mon Sep 17 00:00:00 2001 From: nisse Date: Fri, 15 Apr 2016 03:49:07 -0700 Subject: [PATCH] Rename BEGIN_PROXY_MAP --> BEGIN_SIGNALLING_PROXY_MAP. And BEGIN_WORKER_PROXY_MAP --> BEGIN_PROXY_MAP. This rename was suggested by Tommi, with the idea that a proxy invoking methods on the worker thread should be the common case. It's a followup to https://codereview.webrtc.org/1861633002/ This cl also adds unittests for proxy calls to the worker thread. BUG=webrtc:5426 Review URL: https://codereview.webrtc.org/1871833002 Cr-Commit-Position: refs/heads/master@{#12374} --- webrtc/api/datachannel.h | 4 +- webrtc/api/dtmfsender.h | 4 +- webrtc/api/mediastreamproxy.h | 4 +- webrtc/api/mediastreamtrackproxy.h | 8 +- webrtc/api/peerconnectionfactoryproxy.h | 4 +- webrtc/api/peerconnectionproxy.h | 4 +- webrtc/api/proxy.h | 34 +++-- webrtc/api/proxy_unittest.cc | 165 ++++++++++++++++++------ webrtc/api/rtpreceiverinterface.h | 4 +- webrtc/api/rtpsenderinterface.h | 4 +- webrtc/api/videosourceproxy.h | 4 +- 11 files changed, 166 insertions(+), 73 deletions(-) diff --git a/webrtc/api/datachannel.h b/webrtc/api/datachannel.h index b8830be300..3fb400b7ec 100644 --- a/webrtc/api/datachannel.h +++ b/webrtc/api/datachannel.h @@ -260,7 +260,7 @@ class DataChannel : public DataChannelInterface, }; // Define proxy for DataChannelInterface. -BEGIN_PROXY_MAP(DataChannel) +BEGIN_SIGNALING_PROXY_MAP(DataChannel) PROXY_METHOD1(void, RegisterObserver, DataChannelObserver*) PROXY_METHOD0(void, UnregisterObserver) PROXY_CONSTMETHOD0(std::string, label) @@ -275,7 +275,7 @@ BEGIN_PROXY_MAP(DataChannel) PROXY_CONSTMETHOD0(uint64_t, buffered_amount) PROXY_METHOD0(void, Close) PROXY_METHOD1(bool, Send, const DataBuffer&) -END_PROXY() +END_SIGNALING_PROXY() } // namespace webrtc diff --git a/webrtc/api/dtmfsender.h b/webrtc/api/dtmfsender.h index ae8aa44541..2523d6863c 100644 --- a/webrtc/api/dtmfsender.h +++ b/webrtc/api/dtmfsender.h @@ -103,7 +103,7 @@ class DtmfSender }; // Define proxy for DtmfSenderInterface. -BEGIN_PROXY_MAP(DtmfSender) +BEGIN_SIGNALING_PROXY_MAP(DtmfSender) PROXY_METHOD1(void, RegisterObserver, DtmfSenderObserverInterface*) PROXY_METHOD0(void, UnregisterObserver) PROXY_METHOD0(bool, CanInsertDtmf) @@ -112,7 +112,7 @@ BEGIN_PROXY_MAP(DtmfSender) PROXY_CONSTMETHOD0(std::string, tones) PROXY_CONSTMETHOD0(int, duration) PROXY_CONSTMETHOD0(int, inter_tone_gap) -END_PROXY() +END_SIGNALING_PROXY() // Get DTMF code from the DTMF event character. bool GetDtmfCode(char tone, int* code); diff --git a/webrtc/api/mediastreamproxy.h b/webrtc/api/mediastreamproxy.h index 645b28a3f7..06f8eb3b2c 100644 --- a/webrtc/api/mediastreamproxy.h +++ b/webrtc/api/mediastreamproxy.h @@ -16,7 +16,7 @@ namespace webrtc { -BEGIN_PROXY_MAP(MediaStream) +BEGIN_SIGNALING_PROXY_MAP(MediaStream) PROXY_CONSTMETHOD0(std::string, label) PROXY_METHOD0(AudioTrackVector, GetAudioTracks) PROXY_METHOD0(VideoTrackVector, GetVideoTracks) @@ -30,7 +30,7 @@ BEGIN_PROXY_MAP(MediaStream) PROXY_METHOD1(bool, RemoveTrack, VideoTrackInterface*) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) -END_PROXY() +END_SIGNALING_PROXY() } // namespace webrtc diff --git a/webrtc/api/mediastreamtrackproxy.h b/webrtc/api/mediastreamtrackproxy.h index 6ca6dbbe80..12fdc36b36 100644 --- a/webrtc/api/mediastreamtrackproxy.h +++ b/webrtc/api/mediastreamtrackproxy.h @@ -19,7 +19,7 @@ namespace webrtc { -BEGIN_PROXY_MAP(AudioTrack) +BEGIN_SIGNALING_PROXY_MAP(AudioTrack) PROXY_CONSTMETHOD0(std::string, kind) PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(TrackState, state) @@ -33,9 +33,9 @@ BEGIN_PROXY_MAP(AudioTrack) PROXY_METHOD1(bool, set_enabled, bool) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) -END_PROXY() +END_SIGNALING_PROXY() -BEGIN_WORKER_PROXY_MAP(VideoTrack) +BEGIN_PROXY_MAP(VideoTrack) PROXY_CONSTMETHOD0(std::string, kind) PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(TrackState, state) @@ -52,7 +52,7 @@ BEGIN_WORKER_PROXY_MAP(VideoTrack) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) -END_WORKER_PROXY() +END_PROXY() } // namespace webrtc diff --git a/webrtc/api/peerconnectionfactoryproxy.h b/webrtc/api/peerconnectionfactoryproxy.h index a25e93b4f5..ef47cdb869 100644 --- a/webrtc/api/peerconnectionfactoryproxy.h +++ b/webrtc/api/peerconnectionfactoryproxy.h @@ -20,7 +20,7 @@ namespace webrtc { -BEGIN_PROXY_MAP(PeerConnectionFactory) +BEGIN_SIGNALING_PROXY_MAP(PeerConnectionFactory) PROXY_METHOD1(void, SetOptions, const Options&) // Can't use PROXY_METHOD5 because scoped_ptr must be moved. // TODO(tommi,hbos): Use of templates to support scoped_ptr? @@ -93,7 +93,7 @@ BEGIN_PROXY_MAP(PeerConnectionFactory) return c_->CreatePeerConnection(a1, std::move(ptr_a3), std::move(ptr_a4), a5); } - END_PROXY() + END_SIGNALING_PROXY() } // namespace webrtc diff --git a/webrtc/api/peerconnectionproxy.h b/webrtc/api/peerconnectionproxy.h index 1183e61910..d35d5bacde 100644 --- a/webrtc/api/peerconnectionproxy.h +++ b/webrtc/api/peerconnectionproxy.h @@ -17,7 +17,7 @@ namespace webrtc { // Define proxy for PeerConnectionInterface. -BEGIN_PROXY_MAP(PeerConnection) +BEGIN_SIGNALING_PROXY_MAP(PeerConnection) PROXY_METHOD0(rtc::scoped_refptr, local_streams) PROXY_METHOD0(rtc::scoped_refptr, @@ -75,7 +75,7 @@ BEGIN_PROXY_MAP(PeerConnection) PROXY_METHOD0(IceConnectionState, ice_connection_state) PROXY_METHOD0(IceGatheringState, ice_gathering_state) PROXY_METHOD0(void, Close) -END_PROXY() +END_SIGNALING_PROXY() } // namespace webrtc diff --git a/webrtc/api/proxy.h b/webrtc/api/proxy.h index b5a8071bcf..663e6c8873 100644 --- a/webrtc/api/proxy.h +++ b/webrtc/api/proxy.h @@ -18,7 +18,7 @@ // public: // std::string FooA() = 0; // std::string FooB(bool arg1) const = 0; -// std::string FooC(bool arg1)= 0; +// std::string FooC(bool arg1) = 0; // }; // // Note that return types can not be a const reference. @@ -30,10 +30,19 @@ // BEGIN_PROXY_MAP(Test) // PROXY_METHOD0(std::string, FooA) // PROXY_CONSTMETHOD1(std::string, FooB, arg1) -// PROXY_METHOD1(std::string, FooC, arg1) +// PROXY_WORKER_METHOD1(std::string, FooC, arg1) // END_PROXY() // -// The proxy can be created using TestProxy::Create(Thread*, TestInterface*). +// where the first two methods are invoked on the signaling thread, +// and the third is invoked on the worker thread. +// +// The proxy can be created using +// +// TestProxy::Create(Thread* signaling_thread, Thread* worker_thread, +// TestInterface*). +// +// The variant defined with BEGIN_SIGNALING_PROXY_MAP is unaware of +// the worker thread, and invokes all methods on the signaling thread. #ifndef WEBRTC_API_PROXY_H_ #define WEBRTC_API_PROXY_H_ @@ -295,26 +304,25 @@ class MethodCall5 : public rtc::Message, T5 a5_; }; -// TODO(nisse): Rename this to {BEGIN|END}_SIGNALLING_PROXY_MAP, and -// the below to {BEGIN|END}_PROXY_MAP. Also rename the class to -// c##SignallingProxy. -#define BEGIN_PROXY_MAP(c) \ +#define BEGIN_SIGNALING_PROXY_MAP(c) \ class c##Proxy : public c##Interface { \ protected: \ typedef c##Interface C; \ c##Proxy(rtc::Thread* signaling_thread, C* c) \ : signaling_thread_(signaling_thread), c_(c) {} \ ~c##Proxy() { \ - MethodCall0 call(this, &c##Proxy::Release_s); \ + MethodCall0 call( \ + this, &c##Proxy::Release_s); \ call.Marshal(signaling_thread_); \ } \ \ public: \ static rtc::scoped_refptr Create(rtc::Thread* signaling_thread, C* c) { \ - return new rtc::RefCountedObject(signaling_thread, c); \ + return new rtc::RefCountedObject( \ + signaling_thread, c); \ } -#define BEGIN_WORKER_PROXY_MAP(c) \ +#define BEGIN_PROXY_MAP(c) \ class c##Proxy : public c##Interface { \ protected: \ typedef c##Interface C; \ @@ -397,16 +405,16 @@ class MethodCall5 : public rtc::Message, return call.Marshal(worker_thread_); \ } -#define END_PROXY() \ +#define END_SIGNALING_PROXY() \ private:\ void Release_s() {\ c_ = NULL;\ }\ mutable rtc::Thread* signaling_thread_;\ rtc::scoped_refptr c_;\ - };\ + }; -#define END_WORKER_PROXY() \ +#define END_PROXY() \ private: \ void Release_s() { \ c_ = NULL; \ diff --git a/webrtc/api/proxy_unittest.cc b/webrtc/api/proxy_unittest.cc index 2578e9b4b1..557c85bf11 100644 --- a/webrtc/api/proxy_unittest.cc +++ b/webrtc/api/proxy_unittest.cc @@ -40,16 +40,6 @@ class FakeInterface : public rtc::RefCountInterface { ~FakeInterface() {} }; -// Proxy for the test interface. -BEGIN_PROXY_MAP(Fake) - PROXY_METHOD0(void, VoidMethod0) - PROXY_METHOD0(std::string, Method0) - PROXY_CONSTMETHOD0(std::string, ConstMethod0) - PROXY_METHOD1(std::string, Method1, std::string) - PROXY_CONSTMETHOD1(std::string, ConstMethod1, std::string) - PROXY_METHOD2(std::string, Method2, std::string, std::string) -END_PROXY() - // Implementation of the test interface. class Fake : public FakeInterface { public: @@ -71,60 +61,156 @@ class Fake : public FakeInterface { ~Fake() {} }; -class ProxyTest: public testing::Test { +// Proxies for the test interface. +BEGIN_PROXY_MAP(Fake) + PROXY_METHOD0(void, VoidMethod0) + PROXY_METHOD0(std::string, Method0) + PROXY_CONSTMETHOD0(std::string, ConstMethod0) + PROXY_WORKER_METHOD1(std::string, Method1, std::string) + PROXY_CONSTMETHOD1(std::string, ConstMethod1, std::string) + PROXY_WORKER_METHOD2(std::string, Method2, std::string, std::string) +END_PROXY() + +// Preprocessor hack to get a proxy class a name different than FakeProxy. +#define FakeProxy FakeSignalingProxy +BEGIN_SIGNALING_PROXY_MAP(Fake) + PROXY_METHOD0(void, VoidMethod0) + PROXY_METHOD0(std::string, Method0) + PROXY_CONSTMETHOD0(std::string, ConstMethod0) + PROXY_METHOD1(std::string, Method1, std::string) + PROXY_CONSTMETHOD1(std::string, ConstMethod1, std::string) + PROXY_METHOD2(std::string, Method2, std::string, std::string) +END_SIGNALING_PROXY() +#undef FakeProxy + +class SignalingProxyTest : public testing::Test { public: - // Checks that the functions is called on the |signaling_thread_|. - void CheckThread() { - EXPECT_EQ(rtc::Thread::Current(), signaling_thread_.get()); - } + // Checks that the functions are called on the right thread. + void CheckSignalingThread() { EXPECT_TRUE(signaling_thread_->IsCurrent()); } protected: - virtual void SetUp() { + void SetUp() override { signaling_thread_.reset(new rtc::Thread()); ASSERT_TRUE(signaling_thread_->Start()); fake_ = Fake::Create(); - fake_proxy_ = FakeProxy::Create(signaling_thread_.get(), fake_.get()); + fake_signaling_proxy_ = + FakeSignalingProxy::Create(signaling_thread_.get(), fake_.get()); } protected: rtc::scoped_ptr signaling_thread_; - rtc::scoped_refptr fake_proxy_; + rtc::scoped_refptr fake_signaling_proxy_; rtc::scoped_refptr fake_; }; +TEST_F(SignalingProxyTest, VoidMethod0) { + EXPECT_CALL(*fake_, VoidMethod0()) + .Times(Exactly(1)) + .WillOnce( + InvokeWithoutArgs(this, &SignalingProxyTest::CheckSignalingThread)); + fake_signaling_proxy_->VoidMethod0(); +} + +TEST_F(SignalingProxyTest, Method0) { + EXPECT_CALL(*fake_, Method0()) + .Times(Exactly(1)) + .WillOnce(DoAll( + InvokeWithoutArgs(this, &SignalingProxyTest::CheckSignalingThread), + Return("Method0"))); + EXPECT_EQ("Method0", fake_signaling_proxy_->Method0()); +} + +TEST_F(SignalingProxyTest, ConstMethod0) { + EXPECT_CALL(*fake_, ConstMethod0()) + .Times(Exactly(1)) + .WillOnce(DoAll( + InvokeWithoutArgs(this, &SignalingProxyTest::CheckSignalingThread), + Return("ConstMethod0"))); + EXPECT_EQ("ConstMethod0", fake_signaling_proxy_->ConstMethod0()); +} + +TEST_F(SignalingProxyTest, Method1) { + const std::string arg1 = "arg1"; + EXPECT_CALL(*fake_, Method1(arg1)) + .Times(Exactly(1)) + .WillOnce(DoAll( + InvokeWithoutArgs(this, &SignalingProxyTest::CheckSignalingThread), + Return("Method1"))); + EXPECT_EQ("Method1", fake_signaling_proxy_->Method1(arg1)); +} + +TEST_F(SignalingProxyTest, ConstMethod1) { + const std::string arg1 = "arg1"; + EXPECT_CALL(*fake_, ConstMethod1(arg1)) + .Times(Exactly(1)) + .WillOnce(DoAll( + InvokeWithoutArgs(this, &SignalingProxyTest::CheckSignalingThread), + Return("ConstMethod1"))); + EXPECT_EQ("ConstMethod1", fake_signaling_proxy_->ConstMethod1(arg1)); +} + +TEST_F(SignalingProxyTest, Method2) { + const std::string arg1 = "arg1"; + const std::string arg2 = "arg2"; + EXPECT_CALL(*fake_, Method2(arg1, arg2)) + .Times(Exactly(1)) + .WillOnce(DoAll( + InvokeWithoutArgs(this, &SignalingProxyTest::CheckSignalingThread), + Return("Method2"))); + EXPECT_EQ("Method2", fake_signaling_proxy_->Method2(arg1, arg2)); +} + +class ProxyTest : public SignalingProxyTest { + public: + // Checks that the functions are called on the right thread. + void CheckWorkerThread() { EXPECT_TRUE(worker_thread_->IsCurrent()); } + + protected: + void SetUp() override { + SignalingProxyTest::SetUp(); + worker_thread_.reset(new rtc::Thread()); + ASSERT_TRUE(worker_thread_->Start()); + fake_proxy_ = FakeProxy::Create(signaling_thread_.get(), + worker_thread_.get(), fake_.get()); + } + + protected: + rtc::scoped_ptr worker_thread_; + rtc::scoped_refptr fake_proxy_; +}; + TEST_F(ProxyTest, VoidMethod0) { EXPECT_CALL(*fake_, VoidMethod0()) - .Times(Exactly(1)) - .WillOnce(InvokeWithoutArgs(this, &ProxyTest::CheckThread)); + .Times(Exactly(1)) + .WillOnce(InvokeWithoutArgs(this, &ProxyTest::CheckSignalingThread)); fake_proxy_->VoidMethod0(); } TEST_F(ProxyTest, Method0) { EXPECT_CALL(*fake_, Method0()) - .Times(Exactly(1)) - .WillOnce( - DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckThread), - Return("Method0"))); + .Times(Exactly(1)) + .WillOnce( + DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckSignalingThread), + Return("Method0"))); EXPECT_EQ("Method0", fake_proxy_->Method0()); } TEST_F(ProxyTest, ConstMethod0) { EXPECT_CALL(*fake_, ConstMethod0()) - .Times(Exactly(1)) - .WillOnce( - DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckThread), - Return("ConstMethod0"))); + .Times(Exactly(1)) + .WillOnce( + DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckSignalingThread), + Return("ConstMethod0"))); EXPECT_EQ("ConstMethod0", fake_proxy_->ConstMethod0()); } -TEST_F(ProxyTest, Method1) { +TEST_F(ProxyTest, WorkerMethod1) { const std::string arg1 = "arg1"; EXPECT_CALL(*fake_, Method1(arg1)) - .Times(Exactly(1)) - .WillOnce( - DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckThread), + .Times(Exactly(1)) + .WillOnce(DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckWorkerThread), Return("Method1"))); EXPECT_EQ("Method1", fake_proxy_->Method1(arg1)); } @@ -132,20 +218,19 @@ TEST_F(ProxyTest, Method1) { TEST_F(ProxyTest, ConstMethod1) { const std::string arg1 = "arg1"; EXPECT_CALL(*fake_, ConstMethod1(arg1)) - .Times(Exactly(1)) - .WillOnce( - DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckThread), - Return("ConstMethod1"))); + .Times(Exactly(1)) + .WillOnce( + DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckSignalingThread), + Return("ConstMethod1"))); EXPECT_EQ("ConstMethod1", fake_proxy_->ConstMethod1(arg1)); } -TEST_F(ProxyTest, Method2) { +TEST_F(ProxyTest, WorkerMethod2) { const std::string arg1 = "arg1"; const std::string arg2 = "arg2"; EXPECT_CALL(*fake_, Method2(arg1, arg2)) - .Times(Exactly(1)) - .WillOnce( - DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckThread), + .Times(Exactly(1)) + .WillOnce(DoAll(InvokeWithoutArgs(this, &ProxyTest::CheckWorkerThread), Return("Method2"))); EXPECT_EQ("Method2", fake_proxy_->Method2(arg1, arg2)); } diff --git a/webrtc/api/rtpreceiverinterface.h b/webrtc/api/rtpreceiverinterface.h index 5c7790db60..c2a579b37c 100644 --- a/webrtc/api/rtpreceiverinterface.h +++ b/webrtc/api/rtpreceiverinterface.h @@ -38,11 +38,11 @@ class RtpReceiverInterface : public rtc::RefCountInterface { }; // Define proxy for RtpReceiverInterface. -BEGIN_PROXY_MAP(RtpReceiver) +BEGIN_SIGNALING_PROXY_MAP(RtpReceiver) PROXY_CONSTMETHOD0(rtc::scoped_refptr, track) PROXY_CONSTMETHOD0(std::string, id) PROXY_METHOD0(void, Stop) -END_PROXY() +END_SIGNALING_PROXY() } // namespace webrtc diff --git a/webrtc/api/rtpsenderinterface.h b/webrtc/api/rtpsenderinterface.h index 776d01ace6..2291bb4e21 100644 --- a/webrtc/api/rtpsenderinterface.h +++ b/webrtc/api/rtpsenderinterface.h @@ -60,7 +60,7 @@ class RtpSenderInterface : public rtc::RefCountInterface { }; // Define proxy for RtpSenderInterface. -BEGIN_PROXY_MAP(RtpSender) +BEGIN_SIGNALING_PROXY_MAP(RtpSender) PROXY_METHOD1(bool, SetTrack, MediaStreamTrackInterface*) PROXY_CONSTMETHOD0(rtc::scoped_refptr, track) PROXY_METHOD1(void, SetSsrc, uint32_t) @@ -72,7 +72,7 @@ PROXY_CONSTMETHOD0(std::string, stream_id) PROXY_METHOD0(void, Stop) PROXY_CONSTMETHOD0(RtpParameters, GetParameters); PROXY_METHOD1(bool, SetParameters, const RtpParameters&) -END_PROXY() +END_SIGNALING_PROXY() } // namespace webrtc diff --git a/webrtc/api/videosourceproxy.h b/webrtc/api/videosourceproxy.h index 0a34967f13..6d4dfcb805 100644 --- a/webrtc/api/videosourceproxy.h +++ b/webrtc/api/videosourceproxy.h @@ -20,7 +20,7 @@ namespace webrtc { // implementation is // destroyed on the signaling thread and marshals all method calls to the // signaling thread. -BEGIN_WORKER_PROXY_MAP(VideoTrackSource) +BEGIN_PROXY_MAP(VideoTrackSource) PROXY_CONSTMETHOD0(SourceState, state) PROXY_CONSTMETHOD0(bool, remote) PROXY_METHOD0(void, Stop) @@ -37,7 +37,7 @@ BEGIN_WORKER_PROXY_MAP(VideoTrackSource) rtc::VideoSinkInterface*) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) -END_WORKER_PROXY() +END_PROXY() } // namespace webrtc