PeerConnection(iOS): make ARC-clean talk/.../objc* and talk/examples/ios

- Removes a strong-reference cycle between RTCPeerConnection and
  RTCPeerConnectionObserver
- Gives RTCPeerConnectionObserver a virtual dtor
- Ensures RTCPeerConnectionTest tears down correctly
- Ensures AppRTCDemo tears down correctly

This is the talk/ half; the webrtc/ half is in https://webrtc-codereview.appspot.com/10539005

BUG=3054,3055,3100
R=noahric@google.com

Review URL: https://webrtc-codereview.appspot.com/10499005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5771 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
fischman@webrtc.org
2014-03-25 05:16:29 +00:00
parent e68102e046
commit 385a722646
8 changed files with 57 additions and 17 deletions

View File

@ -39,7 +39,9 @@ class RTCPeerConnectionObserver : public PeerConnectionObserver {
public: public:
explicit RTCPeerConnectionObserver(id<RTCPeerConnectionDelegate> delegate); explicit RTCPeerConnectionObserver(id<RTCPeerConnectionDelegate> delegate);
virtual ~RTCPeerConnectionObserver();
// |peerConnection| owns |this|, so outlives it.
void SetPeerConnection(RTCPeerConnection *peerConnection); void SetPeerConnection(RTCPeerConnection *peerConnection);
virtual void OnError() OVERRIDE; virtual void OnError() OVERRIDE;
@ -73,7 +75,9 @@ class RTCPeerConnectionObserver : public PeerConnectionObserver {
private: private:
id<RTCPeerConnectionDelegate> _delegate; id<RTCPeerConnectionDelegate> _delegate;
RTCPeerConnection *_peerConnection; // __unsafe_unretained is in fact safe because |_peerConnection| owns |this|;
// see comment on SetPeerConnection() above.
__unsafe_unretained RTCPeerConnection *_peerConnection;
}; };
} // namespace webrtc } // namespace webrtc

View File

@ -42,6 +42,8 @@ RTCPeerConnectionObserver::RTCPeerConnectionObserver(
_delegate = delegate; _delegate = delegate;
} }
RTCPeerConnectionObserver::~RTCPeerConnectionObserver() {}
void RTCPeerConnectionObserver::SetPeerConnection( void RTCPeerConnectionObserver::SetPeerConnection(
RTCPeerConnection* peerConnection) { RTCPeerConnection* peerConnection) {
_peerConnection = peerConnection; _peerConnection = peerConnection;

View File

@ -59,7 +59,7 @@
videoTrackID:(NSString*)videoTrackID videoTrackID:(NSString*)videoTrackID
audioTrackID:(NSString*)audioTrackID; audioTrackID:(NSString*)audioTrackID;
- (void)testCompleteSession; - (void)testCompleteSessionWithFactory:(RTCPeerConnectionFactory*)factory;
@end @end
@ -93,8 +93,7 @@
return localMediaStream; return localMediaStream;
} }
- (void)testCompleteSession { - (void)testCompleteSessionWithFactory:(RTCPeerConnectionFactory*)factory {
RTCPeerConnectionFactory* factory = [[RTCPeerConnectionFactory alloc] init];
RTCMediaConstraints* constraints = [[RTCMediaConstraints alloc] init]; RTCMediaConstraints* constraints = [[RTCMediaConstraints alloc] init];
RTCPeerConnectionSyncObserver* offeringExpectations = RTCPeerConnectionSyncObserver* offeringExpectations =
[[RTCPeerConnectionSyncObserver alloc] init]; [[RTCPeerConnectionSyncObserver alloc] init];
@ -216,7 +215,24 @@
[[NSRunLoop currentRunLoop] [[NSRunLoop currentRunLoop]
runUntilDate:[NSDate dateWithTimeIntervalSinceNow:2]]; runUntilDate:[NSDate dateWithTimeIntervalSinceNow:2]];
// TODO(hughv): Implement orderly shutdown. [offeringExpectations expectICEConnectionChange:RTCICEConnectionClosed];
[answeringExpectations expectICEConnectionChange:RTCICEConnectionClosed];
[offeringExpectations expectSignalingChange:RTCSignalingClosed];
[answeringExpectations expectSignalingChange:RTCSignalingClosed];
[pcOffer close];
[pcAnswer close];
[offeringExpectations waitForAllExpectationsToBeSatisfied];
[answeringExpectations waitForAllExpectationsToBeSatisfied];
capturer = nil;
videoSource = nil;
pcOffer = nil;
pcAnswer = nil;
// TODO(fischman): be stricter about shutdown checks; ensure thread
// counts return to where they were before the test kicked off, and
// that all objects have in fact shut down.
} }
@end @end
@ -225,8 +241,20 @@
// RTCPeerConnectionTest and avoid the appearance of RTCPeerConnectionTest being // RTCPeerConnectionTest and avoid the appearance of RTCPeerConnectionTest being
// a TestBase since it's not. // a TestBase since it's not.
TEST(RTCPeerConnectionTest, SessionTest) { TEST(RTCPeerConnectionTest, SessionTest) {
@autoreleasepool {
talk_base::InitializeSSL(); talk_base::InitializeSSL();
// Since |factory| will own the signaling & worker threads, it's important
// that it outlive the created PeerConnections since they self-delete on the
// signaling thread, and if |factory| is freed first then a last refcount on
// the factory will expire during this teardown, causing the signaling
// thread to try to Join() with itself. This is a hack to ensure that the
// factory outlives RTCPeerConnection:dealloc.
// See https://code.google.com/p/webrtc/issues/detail?id=3100.
RTCPeerConnectionFactory* factory = [[RTCPeerConnectionFactory alloc] init];
@autoreleasepool {
RTCPeerConnectionTest* pcTest = [[RTCPeerConnectionTest alloc] init]; RTCPeerConnectionTest* pcTest = [[RTCPeerConnectionTest alloc] init];
[pcTest testCompleteSession]; [pcTest testCompleteSessionWithFactory:factory];
}
talk_base::CleanupSSL(); talk_base::CleanupSSL();
}
} }

View File

@ -27,6 +27,10 @@
#include "talk/base/gunit.h" #include "talk/base/gunit.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
int main(int argc, char* argv[]) { int main(int argc, char* argv[]) {
testing::InitGoogleTest(&argc, argv); testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS(); return RUN_ALL_TESTS();

View File

@ -30,7 +30,7 @@
#import "GAEChannelClient.h" #import "GAEChannelClient.h"
#import "APPRTCAppClient.h" #import "APPRTCAppClient.h"
#import "RTCSessionDescriptonDelegate.h" #import "RTCSessionDescriptonDelegate.h"
#import "RTCVideoSource.h"
// Used to send a message to an apprtc.appspot.com "room". // Used to send a message to an apprtc.appspot.com "room".
@protocol APPRTCSendMessage<NSObject> @protocol APPRTCSendMessage<NSObject>
@ -43,7 +43,7 @@
@class RTCVideoTrack; @class RTCVideoTrack;
// The main application class of the AppRTCDemo iOS app demonstrating // The main application class of the AppRTCDemo iOS app demonstrating
// interoperability between the Objcective C implementation of PeerConnection // interoperability between the Objective C implementation of PeerConnection
// and the apprtc.appspot.com demo webapp. // and the apprtc.appspot.com demo webapp.
@interface APPRTCAppDelegate : UIResponder<ICEServerDelegate, @interface APPRTCAppDelegate : UIResponder<ICEServerDelegate,
GAEMessageHandler, GAEMessageHandler,
@ -53,6 +53,7 @@
@property(strong, nonatomic) UIWindow* window; @property(strong, nonatomic) UIWindow* window;
@property(strong, nonatomic) APPRTCViewController* viewController; @property(strong, nonatomic) APPRTCViewController* viewController;
@property (strong, nonatomic) RTCVideoSource* videoSource;
- (void)closeVideoUI; - (void)closeVideoUI;

View File

@ -246,12 +246,12 @@
RTCVideoCapturer* capturer = RTCVideoCapturer* capturer =
[RTCVideoCapturer capturerWithDeviceName:cameraID]; [RTCVideoCapturer capturerWithDeviceName:cameraID];
RTCVideoSource* videoSource = [self.peerConnectionFactory self.videoSource = [self.peerConnectionFactory
videoSourceWithCapturer:capturer videoSourceWithCapturer:capturer
constraints:self.client.videoConstraints]; constraints:self.client.videoConstraints];
RTCVideoTrack* localVideoTrack = RTCVideoTrack* localVideoTrack =
[self.peerConnectionFactory videoTrackWithID:@"ARDAMSv0" [self.peerConnectionFactory videoTrackWithID:@"ARDAMSv0"
source:videoSource]; source:self.videoSource];
if (localVideoTrack) { if (localVideoTrack) {
[lms addVideoTrack:localVideoTrack]; [lms addVideoTrack:localVideoTrack];
} }
@ -485,9 +485,10 @@
sendData:[@"{\"type\": \"bye\"}" dataUsingEncoding:NSUTF8StringEncoding]]; sendData:[@"{\"type\": \"bye\"}" dataUsingEncoding:NSUTF8StringEncoding]];
[self.peerConnection close]; [self.peerConnection close];
self.peerConnection = nil; self.peerConnection = nil;
self.peerConnectionFactory = nil;
self.pcObserver = nil; self.pcObserver = nil;
self.client = nil; self.client = nil;
self.videoSource = nil;
self.peerConnectionFactory = nil;
[RTCPeerConnectionFactory deinitializeSSL]; [RTCPeerConnectionFactory deinitializeSSL];
} }
@ -523,8 +524,8 @@
#pragma mark - public methods #pragma mark - public methods
- (void)closeVideoUI { - (void)closeVideoUI {
[self disconnect];
[self.viewController resetUI]; [self.viewController resetUI];
[self disconnect];
} }
@end @end

View File

@ -77,7 +77,7 @@
[_remoteVideoView removeFromSuperview]; [_remoteVideoView removeFromSuperview];
self.remoteVideoView = nil; self.remoteVideoView = nil;
[_remoteVideoView renderVideoTrackInterface:nil]; [_localVideoView renderVideoTrackInterface:nil];
[_localVideoView removeFromSuperview]; [_localVideoView removeFromSuperview];
self.localVideoView = nil; self.localVideoView = nil;
} }