From 8cc47e926c34d48c8b4d61146d611dd548df7f19 Mon Sep 17 00:00:00 2001 From: "tkchin@webrtc.org" Date: Wed, 18 Mar 2015 23:38:04 +0000 Subject: [PATCH] Objective-C readability review. BUG= R=rsesek@chromium.org Review URL: https://webrtc-codereview.appspot.com/34679004 Cr-Commit-Position: refs/heads/master@{#8784} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8784 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../objc/AppRTCDemo/ARDAppClient+Internal.h | 1 + talk/examples/objc/AppRTCDemo/ARDAppClient.h | 7 +- talk/examples/objc/AppRTCDemo/ARDAppClient.m | 66 +++++++++++++------ .../objc/AppRTCDemo/ARDAppEngineClient.m | 12 ++-- .../objc/AppRTCDemo/tests/ARDAppClientTest.mm | 1 - 5 files changed, 59 insertions(+), 28 deletions(-) diff --git a/talk/examples/objc/AppRTCDemo/ARDAppClient+Internal.h b/talk/examples/objc/AppRTCDemo/ARDAppClient+Internal.h index 8bb6609c7f..4d49239e6b 100644 --- a/talk/examples/objc/AppRTCDemo/ARDAppClient+Internal.h +++ b/talk/examples/objc/AppRTCDemo/ARDAppClient+Internal.h @@ -38,6 +38,7 @@ @interface ARDAppClient () +// All properties should only be mutated from the main queue. @property(nonatomic, strong) id roomServerClient; @property(nonatomic, strong) id channel; @property(nonatomic, strong) id turnClient; diff --git a/talk/examples/objc/AppRTCDemo/ARDAppClient.h b/talk/examples/objc/AppRTCDemo/ARDAppClient.h index 94077f2b9a..b88b5702ef 100644 --- a/talk/examples/objc/AppRTCDemo/ARDAppClient.h +++ b/talk/examples/objc/AppRTCDemo/ARDAppClient.h @@ -39,6 +39,8 @@ typedef NS_ENUM(NSInteger, ARDAppClientState) { }; @class ARDAppClient; +// The delegate is informed of pertinent events and will be called on the +// main queue. @protocol ARDAppClientDelegate - (void)appClient:(ARDAppClient *)client @@ -58,12 +60,15 @@ typedef NS_ENUM(NSInteger, ARDAppClientState) { @end -// Handles connections to the AppRTC server for a given room. +// Handles connections to the AppRTC server for a given room. Methods on this +// class should only be called from the main queue. @interface ARDAppClient : NSObject @property(nonatomic, readonly) ARDAppClientState state; @property(nonatomic, weak) id delegate; +// Convenience constructor since all expected use cases will need a delegate +// in order to receive remote tracks. - (instancetype)initWithDelegate:(id)delegate; // Establishes a connection with the AppRTC servers for the given room id. diff --git a/talk/examples/objc/AppRTCDemo/ARDAppClient.m b/talk/examples/objc/AppRTCDemo/ARDAppClient.m index 5539adbb4d..695b86e8f0 100644 --- a/talk/examples/objc/AppRTCDemo/ARDAppClient.m +++ b/talk/examples/objc/AppRTCDemo/ARDAppClient.m @@ -45,20 +45,20 @@ #import "RTCVideoCapturer.h" #import "RTCVideoTrack.h" -static NSString *kARDDefaultSTUNServerUrl = +static NSString * const kARDDefaultSTUNServerUrl = @"stun:stun.l.google.com:19302"; // TODO(tkchin): figure out a better username for CEOD statistics. -static NSString *kARDTurnRequestUrl = +static NSString * const kARDTurnRequestUrl = @"https://computeengineondemand.appspot.com" @"/turn?username=iapprtc&key=4080218913"; -static NSString *kARDAppClientErrorDomain = @"ARDAppClient"; -static NSInteger kARDAppClientErrorUnknown = -1; -static NSInteger kARDAppClientErrorRoomFull = -2; -static NSInteger kARDAppClientErrorCreateSDP = -3; -static NSInteger kARDAppClientErrorSetSDP = -4; -static NSInteger kARDAppClientErrorInvalidClient = -5; -static NSInteger kARDAppClientErrorInvalidRoom = -6; +static NSString * const kARDAppClientErrorDomain = @"ARDAppClient"; +static NSInteger const kARDAppClientErrorUnknown = -1; +static NSInteger const kARDAppClientErrorRoomFull = -2; +static NSInteger const kARDAppClientErrorCreateSDP = -3; +static NSInteger const kARDAppClientErrorSetSDP = -4; +static NSInteger const kARDAppClientErrorInvalidClient = -5; +static NSInteger const kARDAppClientErrorInvalidRoom = -6; @implementation ARDAppClient @@ -81,6 +81,16 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; @synthesize defaultPeerConnectionConstraints = _defaultPeerConnectionConstraints; +- (instancetype)init { + if (self = [super init]) { + _roomServerClient = [[ARDAppEngineClient alloc] init]; + NSURL *turnRequestURL = [NSURL URLWithString:kARDTurnRequestUrl]; + _turnClient = [[ARDCEODTURNClient alloc] initWithURL:turnRequestURL]; + [self configure]; + } + return self; +} + - (instancetype)initWithDelegate:(id)delegate { if (self = [super init]) { _roomServerClient = [[ARDAppEngineClient alloc] init]; @@ -219,6 +229,8 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; switch (message.type) { case kARDSignalingMessageTypeOffer: case kARDSignalingMessageTypeAnswer: + // Offers and answers must be processed before any other message, so we + // place them at the front of the queue. _hasReceivedSdp = YES; [_messageQueue insertObject:message atIndex:0]; break; @@ -226,6 +238,7 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; [_messageQueue addObject:message]; break; case kARDSignalingMessageTypeBye: + // Disconnects can be processed immediately. [self processSignalingMessage:message]; return; } @@ -249,6 +262,8 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; } #pragma mark - RTCPeerConnectionDelegate +// Callbacks for this delegate occur on non-main thread and need to be +// dispatched back to main queue as needed. - (void)peerConnection:(RTCPeerConnection *)peerConnection signalingStateChanged:(RTCSignalingState)stateChanged { @@ -305,6 +320,8 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; } #pragma mark - RTCSessionDescriptionDelegate +// Callbacks for this delegate occur on non-main thread and need to be +// dispatched back to main queue as needed. - (void)peerConnection:(RTCPeerConnection *)peerConnection didCreateSessionDescription:(RTCSessionDescription *)sdp @@ -364,6 +381,11 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; return _clientId.length; } +// Begins the peer connection connection process if we have both joined a room +// on the room server and tried to obtain a TURN server. Otherwise does nothing. +// A peer connection object will be created with a stream that contains local +// audio and video capture. If this client is the caller, an offer is created as +// well, otherwise the client will wait for an offer to arrive. - (void)startSignalingIfReady { if (!_isTurnComplete || !self.hasJoinedRoomServerRoom) { return; @@ -375,24 +397,24 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; _peerConnection = [_factory peerConnectionWithICEServers:_iceServers constraints:constraints delegate:self]; + // Create AV media stream and add it to the peer connection. RTCMediaStream *localStream = [self createLocalMediaStream]; [_peerConnection addStream:localStream]; if (_isInitiator) { - [self sendOffer]; + // Send offer. + [_peerConnection createOfferWithDelegate:self + constraints:[self defaultOfferConstraints]]; } else { - [self waitForAnswer]; + // Check if we've received an offer. + [self drainMessageQueueIfReady]; } } -- (void)sendOffer { - [_peerConnection createOfferWithDelegate:self - constraints:[self defaultOfferConstraints]]; -} - -- (void)waitForAnswer { - [self drainMessageQueueIfReady]; -} - +// Processes the messages that we've received from the room server and the +// signaling channel. The offer or answer message must be processed before other +// signaling messages, however they can arrive out of order. Hence, this method +// only processes pending messages if there is a peer connection object and +// if we have received either an offer or answer. - (void)drainMessageQueueIfReady { if (!_peerConnection || !_hasReceivedSdp) { return; @@ -403,6 +425,7 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; [_messageQueue removeAllObjects]; } +// Processes the given signaling message based on its type. - (void)processSignalingMessage:(ARDSignalingMessage *)message { NSParameterAssert(_peerConnection || message.type == kARDSignalingMessageTypeBye); @@ -431,6 +454,9 @@ static NSInteger kARDAppClientErrorInvalidRoom = -6; } } +// Sends a signaling message to the other client. The caller will send messages +// through the room server, whereas the callee will send messages over the +// signaling channel. - (void)sendSignalingMessage:(ARDSignalingMessage *)message { if (_isInitiator) { __weak ARDAppClient *weakSelf = self; diff --git a/talk/examples/objc/AppRTCDemo/ARDAppEngineClient.m b/talk/examples/objc/AppRTCDemo/ARDAppEngineClient.m index c647ecb97f..0f3ad0ac68 100644 --- a/talk/examples/objc/AppRTCDemo/ARDAppEngineClient.m +++ b/talk/examples/objc/AppRTCDemo/ARDAppEngineClient.m @@ -33,17 +33,17 @@ #import "ARDUtilities.h" // TODO(tkchin): move these to a configuration object. -static NSString *kARDRoomServerHostUrl = +static NSString * const kARDRoomServerHostUrl = @"https://apprtc.appspot.com"; -static NSString *kARDRoomServerJoinFormat = +static NSString * const kARDRoomServerJoinFormat = @"https://apprtc.appspot.com/join/%@"; -static NSString *kARDRoomServerMessageFormat = +static NSString * const kARDRoomServerMessageFormat = @"https://apprtc.appspot.com/message/%@/%@"; -static NSString *kARDRoomServerLeaveFormat = +static NSString * const kARDRoomServerLeaveFormat = @"https://apprtc.appspot.com/leave/%@/%@"; -static NSString *kARDAppEngineClientErrorDomain = @"ARDAppEngineClient"; -static NSInteger kARDAppEngineClientErrorBadResponse = -1; +static NSString * const kARDAppEngineClientErrorDomain = @"ARDAppEngineClient"; +static NSInteger const kARDAppEngineClientErrorBadResponse = -1; @implementation ARDAppEngineClient diff --git a/talk/examples/objc/AppRTCDemo/tests/ARDAppClientTest.mm b/talk/examples/objc/AppRTCDemo/tests/ARDAppClientTest.mm index 2cdc2d7b3d..396f64f6cf 100644 --- a/talk/examples/objc/AppRTCDemo/tests/ARDAppClientTest.mm +++ b/talk/examples/objc/AppRTCDemo/tests/ARDAppClientTest.mm @@ -25,7 +25,6 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ - #import #import