From 52c0fec34c2030d05874b051f3661f95457012d7 Mon Sep 17 00:00:00 2001 From: "hta@webrtc.org" Date: Tue, 17 Apr 2012 12:39:04 +0000 Subject: [PATCH] Added UDP socket factory function to UdpTransportImpl constructor This is a refactoring in preparation for creating small unit tests for the udp_transport module. BUG= TEST=unittest Review URL: https://webrtc-codereview.appspot.com/482004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2041 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../source/udp_transport_impl.cc | 63 ++++++++++--------- .../udp_transport/source/udp_transport_impl.h | 14 ++++- .../source/udp_transport_unittest.cc | 22 ++++++- 3 files changed, 65 insertions(+), 34 deletions(-) diff --git a/src/modules/udp_transport/source/udp_transport_impl.cc b/src/modules/udp_transport/source/udp_transport_impl.cc index 28ab8926e2..16aa575ffe 100644 --- a/src/modules/udp_transport/source/udp_transport_impl.cc +++ b/src/modules/udp_transport/source/udp_transport_impl.cc @@ -69,7 +69,8 @@ namespace webrtc { UdpTransport* UdpTransport::Create(const WebRtc_Word32 id, WebRtc_UWord8& numSocketThreads) { - return new UdpTransportImpl(id, numSocketThreads); + return new UdpTransportImpl(id, numSocketThreads, + &UdpSocketWrapper::CreateSocket); } void UdpTransport::Destroy(UdpTransport* module) @@ -81,8 +82,10 @@ void UdpTransport::Destroy(UdpTransport* module) } UdpTransportImpl::UdpTransportImpl(const WebRtc_Word32 id, - WebRtc_UWord8& numSocketThreads) + WebRtc_UWord8& numSocketThreads, + SocketMaker* maker) : _id(id), + _socket_creator(maker), _crit(CriticalSectionWrapper::CreateCriticalSection()), _critFilter(CriticalSectionWrapper::CreateCriticalSection()), _critPacketCallback(CriticalSectionWrapper::CreateCriticalSection()), @@ -349,13 +352,13 @@ WebRtc_Word32 UdpTransportImpl::InitializeReceiveSockets( _tos=0; _pcp=0; - _ptrRtpSocket = UdpSocketWrapper::CreateSocket(_id, _mgr, this, - IncomingRTPCallback, - IpV6Enabled()); + _ptrRtpSocket = _socket_creator(_id, _mgr, this, + IncomingRTPCallback, + IpV6Enabled(), false); - _ptrRtcpSocket = UdpSocketWrapper::CreateSocket(_id, _mgr, this, - IncomingRTCPCallback, - IpV6Enabled()); + _ptrRtcpSocket = _socket_creator(_id, _mgr, this, + IncomingRTCPCallback, + IpV6Enabled(), false); ErrorCode retVal = BindLocalRTPSocket(); if(retVal != kNoSocketError) @@ -829,13 +832,13 @@ WebRtc_Word32 UdpTransportImpl::SetToS(WebRtc_Word32 DSCP, bool useSetSockOpt) { CloseSendSockets(); _ptrSendRtpSocket = - UdpSocketWrapper::CreateSocket(_id, _mgr, NULL, - NULL, IpV6Enabled(), - true); + _socket_creator(_id, _mgr, NULL, + NULL, IpV6Enabled(), + true); _ptrSendRtcpSocket = - UdpSocketWrapper::CreateSocket(_id, _mgr, NULL, - NULL, IpV6Enabled(), - true); + _socket_creator(_id, _mgr, NULL, + NULL, IpV6Enabled(), + true); rtpSock=_ptrSendRtpSocket; rtcpSock=_ptrSendRtcpSocket; ErrorCode retVal = BindRTPSendSocket(); @@ -864,12 +867,12 @@ WebRtc_Word32 UdpTransportImpl::SetToS(WebRtc_Word32 DSCP, bool useSetSockOpt) } } CloseReceiveSockets(); - _ptrRtpSocket = UdpSocketWrapper::CreateSocket( - _id, _mgr, this, IncomingRTPCallback, - IpV6Enabled(), true); - _ptrRtcpSocket = UdpSocketWrapper::CreateSocket( - _id, _mgr, this, IncomingRTCPCallback, - IpV6Enabled(),true); + _ptrRtpSocket = _socket_creator(_id, _mgr, this, + IncomingRTPCallback, + IpV6Enabled(), true); + _ptrRtcpSocket = _socket_creator(_id, _mgr, this, + IncomingRTCPCallback, + IpV6Enabled(),true); rtpSock=_ptrRtpSocket; rtcpSock=_ptrRtcpSocket; ErrorCode retVal = BindLocalRTPSocket(); @@ -1527,10 +1530,10 @@ WebRtc_Word32 UdpTransportImpl::InitializeSourcePorts(WebRtc_UWord16 rtpPort, _tos=0; _pcp=0; - _ptrSendRtpSocket = UdpSocketWrapper::CreateSocket(_id, _mgr, NULL, NULL, - IpV6Enabled()); - _ptrSendRtcpSocket = UdpSocketWrapper::CreateSocket(_id, _mgr, NULL, NULL, - IpV6Enabled()); + _ptrSendRtpSocket = _socket_creator(_id, _mgr, NULL, NULL, + IpV6Enabled(), false); + _ptrSendRtcpSocket = _socket_creator(_id, _mgr, NULL, NULL, + IpV6Enabled(), false); ErrorCode retVal = BindRTPSendSocket(); if(retVal != kNoSocketError) @@ -1980,9 +1983,9 @@ int UdpTransportImpl::SendPacket(int /*channel*/, const void* data, int length) "Creating RTP socket since no receive or source socket is\ configured"); - _ptrRtpSocket = UdpSocketWrapper::CreateSocket(_id, _mgr, this, - IncomingRTPCallback, - IpV6Enabled()); + _ptrRtpSocket = _socket_creator(_id, _mgr, this, + IncomingRTPCallback, + IpV6Enabled(), false); // Don't bind to a specific IP address. if(! IpV6Enabled()) @@ -2046,9 +2049,9 @@ int UdpTransportImpl::SendRTCPPacket(int /*channel*/, const void* data, "Creating RTCP socket since no receive or source socket is\ configured"); - _ptrRtcpSocket = UdpSocketWrapper::CreateSocket(_id, _mgr, this, - IncomingRTCPCallback, - IpV6Enabled()); + _ptrRtcpSocket = _socket_creator(_id, _mgr, this, + IncomingRTCPCallback, + IpV6Enabled(), false); // Don't bind to a specific IP address. if(! IpV6Enabled()) diff --git a/src/modules/udp_transport/source/udp_transport_impl.h b/src/modules/udp_transport/source/udp_transport_impl.h index c5a73f9479..827cb15b79 100644 --- a/src/modules/udp_transport/source/udp_transport_impl.h +++ b/src/modules/udp_transport/source/udp_transport_impl.h @@ -22,8 +22,17 @@ class UdpSocketManager; class UdpTransportImpl : public UdpTransport { public: - // Factory method. Constructor disabled. - UdpTransportImpl(const WebRtc_Word32 id, WebRtc_UWord8& numSocketThreads); + // A function that returns a wrapped UDP socket or equivalent. + typedef UdpSocketWrapper* (SocketMaker)(const WebRtc_Word32 id, + UdpSocketManager* mgr, + CallbackObj obj, + IncomingSocketCallback cb, + bool ipV6Enable, + bool disableGQOS); + + // Constructor, only called by UdpTransport::Create and tests. + UdpTransportImpl(const WebRtc_Word32 id, WebRtc_UWord8& numSocketThreads, + SocketMaker* maker); virtual ~UdpTransportImpl(); // Module functions @@ -174,6 +183,7 @@ private: WebRtc_UWord16& sourcePort); WebRtc_Word32 _id; + SocketMaker* _socket_creator; // Protects the sockets from being re-configured while receiving packets. CriticalSectionWrapper* _crit; CriticalSectionWrapper* _critFilter; diff --git a/src/modules/udp_transport/source/udp_transport_unittest.cc b/src/modules/udp_transport/source/udp_transport_unittest.cc index b8944355c9..594e74f392 100644 --- a/src/modules/udp_transport/source/udp_transport_unittest.cc +++ b/src/modules/udp_transport/source/udp_transport_unittest.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. * * Use of this source code is governed by a BSD-style license * that can be found in the LICENSE file in the root of the source @@ -13,5 +13,23 @@ */ #include "udp_transport.h" #include "gtest/gtest.h" +// We include the implementation header file to get at the dependency-injecting +// constructor. +#include "udp_transport_impl.h" -TEST(UDPTransportTest, EmptyTestToGetCodeCoverage) {} +TEST(UDPTransportTest, CreateTransport) { + WebRtc_Word32 id = 0; + WebRtc_UWord8 threads = 0; + webrtc::UdpTransport* transport = webrtc::UdpTransport::Create(id, threads); + webrtc::UdpTransport::Destroy(transport); +} + +// This test verifies that the mock_socket is not called from the constructor. +TEST(UDPTransportTest, ConstructorDoesNotCreateSocket) { + WebRtc_Word32 id = 0; + WebRtc_UWord8 threads = 0; + webrtc::UdpTransportImpl::SocketMaker* null_maker = NULL; + webrtc::UdpTransport* transport = new webrtc::UdpTransportImpl(id, threads, + null_maker); + webrtc::UdpTransport::Destroy(transport); +}