AppRTCDemo(iOS): remote-video reliability fixes

Previously GAE Channel callbacks would be handled by JS string-encoding the
payload into a URL.  Unfortunately this is limited to the (undocumented,
silently problematic) maximum URL length UIWebView supports.  Replaced this
scheme by a notification from JS to ObjC and a getter from ObjC to JS (which
happens out-of-line to avoid worrying about UIWebView's re-entrancy, or lack
thereof).  Part of this change also moved from a combination of: JSON,
URL-escaping, and ad-hoc :-separated values to simply JSON.

Also incidentally:
- Removed outdated TODO about onRenegotiationNeeded, which is unneeded
- Move handling of PeerConnection callbacks to the main queue to avoid having
  to think about concurrency too hard.
- Replaced a bunch of NSOrderedSame with isEqualToString for clearer code and
  not having to worry about the fact that [nil compare:@"foo"]==NSOrderedSame
  is always true (yay ObjC!).
- Auto-scroll messages view.

BUG=3117
R=noahric@google.com

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5814 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/examples/ios/AppRTCDemo/APPRTCAppDelegate.m

@@ -64,19 +64,23 @@
 }
 
 - (void)peerConnectionOnError:(RTCPeerConnection*)peerConnection {
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
       NSLog(@"PCO onError.");
       NSAssert(NO, @"PeerConnection failed.");
+  });
 }
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
     signalingStateChanged:(RTCSignalingState)stateChanged {
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
       NSLog(@"PCO onSignalingStateChange: %d", stateChanged);
+  });
 }
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
            addedStream:(RTCMediaStream*)stream {
-  NSLog(@"PCO onAddStream.");
   dispatch_async(dispatch_get_main_queue(), ^(void) {
+      NSLog(@"PCO onAddStream.");
       NSAssert([stream.audioTracks count] <= 1,
                @"Expected at most 1 audio stream");
       NSAssert([stream.videoTracks count] <= 1,
@@ -90,16 +94,20 @@
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
          removedStream:(RTCMediaStream*)stream {
-  NSLog(@"PCO onRemoveStream.");
+  dispatch_async(dispatch_get_main_queue(),
+                 ^(void) { NSLog(@"PCO onRemoveStream."); });
 }
 
 - (void)peerConnectionOnRenegotiationNeeded:(RTCPeerConnection*)peerConnection {
-  NSLog(@"PCO onRenegotiationNeeded.");
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
-  // TODO(hughv): Handle this.
+      NSLog(@"PCO onRenegotiationNeeded - ignoring because AppRTC has a "
+             "predefined negotiation strategy");
+  });
 }
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
        gotICECandidate:(RTCICECandidate*)candidate {
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
       NSLog(@"PCO onICECandidate.\n  Mid[%@] Index[%d] Sdp[%@]",
             candidate.sdpMid,
             candidate.sdpMLineIndex,
@@ -120,19 +128,23 @@
                  @"Unable to serialize JSON object with error: %@",
                  error.localizedDescription);
       }
+  });
 }
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
     iceGatheringChanged:(RTCICEGatheringState)newState {
-  NSLog(@"PCO onIceGatheringChange. %d", newState);
+  dispatch_async(dispatch_get_main_queue(),
+                 ^(void) { NSLog(@"PCO onIceGatheringChange. %d", newState); });
 }
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
     iceConnectionChanged:(RTCICEConnectionState)newState {
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
       NSLog(@"PCO onIceConnectionChange. %d", newState);
       if (newState == RTCICEConnectionConnected)
         [self displayLogMessage:@"ICE Connection Connected."];
       NSAssert(newState != RTCICEConnectionFailed, @"ICE Connection failed!");
+  });
 }
 
 - (void)displayLogMessage:(NSString*)message {
@@ -198,6 +210,7 @@
 }
 
 - (void)displayLogMessage:(NSString*)message {
+  NSAssert([NSThread isMainThread], @"Called off main thread!");
   NSLog(@"%@", message);
   [self.viewController displayText:message];
 }
@@ -263,7 +276,7 @@
 
   [lms addAudioTrack:[self.peerConnectionFactory audioTrackWithID:@"ARDAMSa0"]];
   [self.peerConnection addStream:lms constraints:constraints];
-  [self displayLogMessage:@"onICEServers - add local stream."];
+  [self displayLogMessage:@"onICEServers - added local stream."];
 }
 
 #pragma mark - GAEMessageHandler methods
@@ -286,24 +299,15 @@
   [self displayLogMessage:@"PC - createOffer."];
 }
 
-- (void)onMessage:(NSString*)data {
+- (void)onMessage:(NSDictionary*)messageData {
-  NSString* message = [self unHTMLifyString:data];
+  NSString* type = messageData[@"type"];
-  NSError* error;
+  NSAssert(type, @"Missing type: %@", messageData);
-  NSDictionary* objects = [NSJSONSerialization
-      JSONObjectWithData:[message dataUsingEncoding:NSUTF8StringEncoding]
-                 options:0
-                   error:&error];
-  NSAssert(!error,
-           @"%@",
-           [NSString stringWithFormat:@"Error: %@", error.description]);
-  NSAssert([objects count] > 0, @"Invalid JSON object");
-  NSString* value = [objects objectForKey:@"type"];
   [self displayLogMessage:[NSString stringWithFormat:@"GAE onMessage type - %@",
-                                                     value]];
+                                                     type]];
-  if ([value compare:@"candidate"] == NSOrderedSame) {
+  if ([type isEqualToString:@"candidate"]) {
-    NSString* mid = [objects objectForKey:@"id"];
+    NSString* mid = messageData[@"id"];
-    NSNumber* sdpLineIndex = [objects objectForKey:@"label"];
+    NSNumber* sdpLineIndex = messageData[@"label"];
-    NSString* sdp = [objects objectForKey:@"candidate"];
+    NSString* sdp = messageData[@"candidate"];
     RTCICECandidate* candidate =
         [[RTCICECandidate alloc] initWithMid:mid
                                        index:sdpLineIndex.intValue
@@ -313,16 +317,16 @@
     } else {
       [self.peerConnection addICECandidate:candidate];
     }
-  } else if (([value compare:@"offer"] == NSOrderedSame) ||
+  } else if ([type isEqualToString:@"offer"] ||
-             ([value compare:@"answer"] == NSOrderedSame)) {
+             [type isEqualToString:@"answer"]) {
-    NSString* sdpString = [objects objectForKey:@"sdp"];
+    NSString* sdpString = messageData[@"sdp"];
     RTCSessionDescription* sdp = [[RTCSessionDescription alloc]
-        initWithType:value
+        initWithType:type
                  sdp:[APPRTCAppDelegate preferISAC:sdpString]];
     [self.peerConnection setRemoteDescriptionWithDelegate:self
                                        sessionDescription:sdp];
     [self displayLogMessage:@"PC - setRemoteDescription."];
-  } else if ([value compare:@"bye"] == NSOrderedSame) {
+  } else if ([type isEqualToString:@"bye"]) {
     [self closeVideoUI];
     UIAlertView* alertView =
         [[UIAlertView alloc] initWithTitle:@"Remote end hung up"
@@ -332,7 +336,7 @@
                          otherButtonTitles:nil];
     [alertView show];
   } else {
-    NSAssert(NO, @"Invalid message: %@", data);
+    NSAssert(NO, @"Invalid message: %@", messageData);
   }
 }
 
@@ -342,8 +346,8 @@
 }
 
 - (void)onError:(int)code withDescription:(NSString*)description {
-  [self displayLogMessage:[NSString stringWithFormat:@"GAE onError:  %@",
+  [self displayLogMessage:[NSString stringWithFormat:@"GAE onError: %d, %@",
-                                                     description]];
+                                    code, description]];
   [self closeVideoUI];
 }
 
@@ -400,8 +404,8 @@
   [newMLine addObject:[origMLineParts objectAtIndex:origPartIndex++]];
   [newMLine addObject:isac16kRtpMap];
   for (; origPartIndex < [origMLineParts count]; ++origPartIndex) {
-    if ([isac16kRtpMap compare:[origMLineParts objectAtIndex:origPartIndex]] !=
+    if (![isac16kRtpMap
-        NSOrderedSame) {
+            isEqualToString:[origMLineParts objectAtIndex:origPartIndex]]) {
       [newMLine addObject:[origMLineParts objectAtIndex:origPartIndex]];
     }
   }
@@ -415,20 +419,20 @@
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
     didCreateSessionDescription:(RTCSessionDescription*)origSdp
                           error:(NSError*)error {
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
       if (error) {
         [self displayLogMessage:@"SDP onFailure."];
         NSAssert(NO, error.description);
         return;
       }
-
       [self displayLogMessage:@"SDP onSuccess(SDP) - set local description."];
       RTCSessionDescription* sdp = [[RTCSessionDescription alloc]
           initWithType:origSdp.type
                    sdp:[APPRTCAppDelegate preferISAC:origSdp.description]];
       [self.peerConnection setLocalDescriptionWithDelegate:self
                                         sessionDescription:sdp];
+
       [self displayLogMessage:@"PC setLocalDescription."];
-  dispatch_async(dispatch_get_main_queue(), ^(void) {
       NSDictionary* json = @{@"type" : sdp.type, @"sdp" : sdp.description};
       NSError* error;
       NSData* data =
@@ -442,6 +446,7 @@
 
 - (void)peerConnection:(RTCPeerConnection*)peerConnection
     didSetSessionDescriptionWithError:(NSError*)error {
+  dispatch_async(dispatch_get_main_queue(), ^(void) {
       if (error) {
         [self displayLogMessage:@"SDP onFailure."];
         NSAssert(NO, error.description);
@@ -449,7 +454,6 @@
       }
 
       [self displayLogMessage:@"SDP onSuccess() - possibly drain candidates"];
-  dispatch_async(dispatch_get_main_queue(), ^(void) {
       if (!self.client.initiator) {
         if (self.peerConnection.remoteDescription &&
             !self.peerConnection.localDescription) {

talk/examples/ios/AppRTCDemo/APPRTCViewController.h

@@ -32,9 +32,9 @@
 // The view controller that is displayed when AppRTCDemo is loaded.
 @interface APPRTCViewController : UIViewController<UITextFieldDelegate>
 
-@property(weak, nonatomic) IBOutlet UITextField* textField;
+@property(weak, nonatomic) IBOutlet UITextField* roomInput;
-@property(weak, nonatomic) IBOutlet UITextView* textInstructions;
+@property(weak, nonatomic) IBOutlet UITextView* instructionsView;
-@property(weak, nonatomic) IBOutlet UITextView* textOutput;
+@property(weak, nonatomic) IBOutlet UITextView* logView;
 @property(weak, nonatomic) IBOutlet UIView* blackView;
 
 @property(nonatomic, strong) APPRTCVideoView* remoteVideoView;

talk/examples/ios/AppRTCDemo/APPRTCViewController.m

@@ -41,8 +41,8 @@
   [super viewDidLoad];
   self.statusBarOrientation =
       [UIApplication sharedApplication].statusBarOrientation;
-  self.textField.delegate = self;
+  self.roomInput.delegate = self;
-  [self.textField becomeFirstResponder];
+  [self.roomInput becomeFirstResponder];
 }
 
 - (void)viewDidLayoutSubviews {
@@ -59,18 +59,20 @@
 - (void)displayText:(NSString*)text {
   dispatch_async(dispatch_get_main_queue(), ^(void) {
       NSString* output =
-          [NSString stringWithFormat:@"%@\n%@", self.textOutput.text, text];
+          [NSString stringWithFormat:@"%@\n%@", self.logView.text, text];
-      self.textOutput.text = output;
+      self.logView.text = output;
+      [self.logView
+          scrollRangeToVisible:NSMakeRange([self.logView.text length], 0)];
   });
 }
 
 - (void)resetUI {
-  [self.textField resignFirstResponder];
+  [self.roomInput resignFirstResponder];
-  self.textField.text = nil;
+  self.roomInput.text = nil;
-  self.textField.hidden = NO;
+  self.roomInput.hidden = NO;
-  self.textInstructions.hidden = NO;
+  self.instructionsView.hidden = NO;
-  self.textOutput.hidden = YES;
+  self.logView.hidden = YES;
-  self.textOutput.text = nil;
+  self.logView.text = nil;
   self.blackView.hidden = YES;
 
   [_remoteVideoView renderVideoTrackInterface:nil];
@@ -145,8 +147,8 @@ enum {
     return;
   }
   textField.hidden = YES;
-  self.textInstructions.hidden = YES;
+  self.instructionsView.hidden = YES;
-  self.textOutput.hidden = NO;
+  self.logView.hidden = NO;
   // TODO(hughv): Instead of launching a URL with apprtc scheme, change to
   // prepopulating the textField with a valid URL missing the room.  This allows
   // the user to have the simplicity of just entering the room or the ability to

talk/examples/ios/AppRTCDemo/GAEChannelClient.h

@@ -33,7 +33,7 @@
 @protocol GAEMessageHandler<NSObject>
 
 - (void)onOpen;
-- (void)onMessage:(NSString *)data;
+- (void)onMessage:(NSDictionary*)data;
 - (void)onClose;
 - (void)onError:(int)code withDescription:(NSString *)description;
 

talk/examples/ios/AppRTCDemo/GAEChannelClient.m

@@ -63,41 +63,54 @@
 
 #pragma mark - UIWebViewDelegate method
 
++ (NSDictionary*)jsonStringToDictionary:(NSString*)str {
+  NSData* data = [str dataUsingEncoding:NSUTF8StringEncoding];
+  NSError* error;
+  NSDictionary* dict =
+      [NSJSONSerialization JSONObjectWithData:data options:0 error:&error];
+  NSAssert(!error, @"Invalid JSON? %@", str);
+  return dict;
+}
+
 - (BOOL)webView:(UIWebView*)webView
     shouldStartLoadWithRequest:(NSURLRequest*)request
                 navigationType:(UIWebViewNavigationType)navigationType {
   NSString* scheme = [request.URL scheme];
-  if ([scheme compare:@"js-frame"] != NSOrderedSame) {
+  NSAssert(scheme, @"scheme is nil: %@", request);
+  if (![scheme isEqualToString:@"js-frame"]) {
     return YES;
   }
-  NSString* resourceSpecifier = [request.URL resourceSpecifier];
+
-  NSRange range = [resourceSpecifier rangeOfString:@":"];
-  NSString* method;
-  NSString* message;
-  if (range.length == 0 && range.location == NSNotFound) {
-    method = resourceSpecifier;
-  } else {
-    method = [resourceSpecifier substringToIndex:range.location];
-    message = [resourceSpecifier substringFromIndex:range.location + 1];
-  }
   dispatch_async(dispatch_get_main_queue(), ^(void) {
-      if ([method compare:@"onopen"] == NSOrderedSame) {
+      NSString* queuedMessage = [webView
+          stringByEvaluatingJavaScriptFromString:@"popQueuedMessage();"];
+      NSAssert([queuedMessage length], @"Empty queued message from JS");
+
+      NSDictionary* queuedMessageDict =
+          [GAEChannelClient jsonStringToDictionary:queuedMessage];
+      NSString* method = queuedMessageDict[@"type"];
+      NSAssert(method, @"Missing method: %@", queuedMessageDict);
+      NSDictionary* payload = queuedMessageDict[@"payload"];  // May be nil.
+
+      if ([method isEqualToString:@"onopen"]) {
         [self.delegate onOpen];
-      } else if ([method compare:@"onmessage"] == NSOrderedSame) {
+      } else if ([method isEqualToString:@"onmessage"]) {
-        [self.delegate onMessage:message];
+        NSDictionary* payloadData =
-      } else if ([method compare:@"onclose"] == NSOrderedSame) {
+            [GAEChannelClient jsonStringToDictionary:payload[@"data"]];
+        [self.delegate onMessage:payloadData];
+      } else if ([method isEqualToString:@"onclose"]) {
         [self.delegate onClose];
-      } else if ([method compare:@"onerror"] == NSOrderedSame) {
+      } else if ([method isEqualToString:@"onerror"]) {
-        // TODO(hughv): Get error.
+        NSNumber* codeNumber = payload[@"code"];
-        int code = -1;
+        int code = [codeNumber intValue];
-        NSString* description = message;
+        NSAssert([codeNumber isEqualToNumber:[NSNumber numberWithInt:code]],
-        [self.delegate onError:code withDescription:description];
+                 @"Unexpected non-integral code: %@", payload);
+        [self.delegate onError:code withDescription:payload[@"description"]];
       } else {
-        NSAssert(
+        NSAssert(NO, @"Invalid message sent from UIWebView: %@", queuedMessage);
-            NO, @"Invalid message sent from UIWebView: %@", resourceSpecifier);
       }
   });
-  return YES;
+  return NO;
 }
 
 @end

talk/examples/ios/AppRTCDemo/en.lproj/APPRTCViewController.xib

@@ -178,7 +178,7 @@
 				</object>
 				<object class="IBConnectionRecord">
 					<object class="IBCocoaTouchOutletConnection" key="connection">
-						<string key="label">textField</string>
+						<string key="label">roomInput</string>
 						<reference key="source" ref="372490531"/>
 						<reference key="destination" ref="546385578"/>
 					</object>
@@ -186,7 +186,7 @@
 				</object>
 				<object class="IBConnectionRecord">
 					<object class="IBCocoaTouchOutletConnection" key="connection">
-						<string key="label">textInstructions</string>
+						<string key="label">instructionsView</string>
 						<reference key="source" ref="372490531"/>
 						<reference key="destination" ref="176994284"/>
 					</object>
@@ -194,7 +194,7 @@
 				</object>
 				<object class="IBConnectionRecord">
 					<object class="IBCocoaTouchOutletConnection" key="connection">
-						<string key="label">textOutput</string>
+						<string key="label">logView</string>
 						<reference key="source" ref="372490531"/>
 						<reference key="destination" ref="634862110"/>
 					</object>
@@ -660,25 +660,25 @@
 					<string key="superclassName">UIViewController</string>
 					<dictionary class="NSMutableDictionary" key="outlets">
 						<string key="blackView">UIView</string>
-						<string key="textField">UITextField</string>
+						<string key="roomInput">UITextField</string>
-						<string key="textInstructions">UITextView</string>
+						<string key="instructionsView">UITextView</string>
-						<string key="textOutput">UITextView</string>
+						<string key="logView">UITextView</string>
 					</dictionary>
 					<dictionary class="NSMutableDictionary" key="toOneOutletInfosByName">
 						<object class="IBToOneOutletInfo" key="blackView">
 							<string key="name">blackView</string>
 							<string key="candidateClassName">UIView</string>
 						</object>
-						<object class="IBToOneOutletInfo" key="textField">
+						<object class="IBToOneOutletInfo" key="roomInput">
-							<string key="name">textField</string>
+							<string key="name">roomInput</string>
 							<string key="candidateClassName">UITextField</string>
 						</object>
-						<object class="IBToOneOutletInfo" key="textInstructions">
+						<object class="IBToOneOutletInfo" key="instructionsView">
-							<string key="name">textInstructions</string>
+							<string key="name">instructionsView</string>
 							<string key="candidateClassName">UITextView</string>
 						</object>
-						<object class="IBToOneOutletInfo" key="textOutput">
+						<object class="IBToOneOutletInfo" key="logView">
-							<string key="name">textOutput</string>
+							<string key="name">logView</string>
 							<string key="candidateClassName">UITextView</string>
 						</object>
 					</dictionary>

talk/examples/ios/AppRTCDemo/ios_channel.html

@@ -6,10 +6,11 @@
   Helper HTML that redirects Google AppEngine's Channel API to Objective C.
   This is done by hosting this page in an iOS application.  The hosting
   class creates a UIWebView control and implements the UIWebViewDelegate
-  protocol.  Then when there is a channel message, it is encoded in an IFRAME.
+  protocol.  Then when there is a channel message it is queued in JS,
-  That IFRAME is added to the DOM which triggers a navigation event
+  and an IFRAME is added to the DOM, triggering a navigation event
-  |shouldStartLoadWithRequest| in Objective C which can then be routed in the
+  |shouldStartLoadWithRequest| in Objective C which can then fetch the
-  application as desired.
+  message using |popQueuedMessage|.  This queuing is necessary to avoid URL
+  length limits in UIWebView (which are undocumented).
   -->
   <body onbeforeunload="closeSocket()" onload="openSocket()">
     <script type="text/javascript">
@@ -38,6 +39,10 @@
 
       var channel = null;
       var socket = null;
+      // In-order queue of messages to be delivered to ObjectiveC.
+      // Each is a JSON.stringify()'d dictionary containing a 'type'
+      // field and optionally a 'payload'.
+      var messageQueue = [];
 
       function openSocket() {
         if (!QueryString.token || !QueryString.token.match(/^[A-z0-9_-]+$/)) {
@@ -52,17 +57,13 @@
             sendMessageToObjC("onopen");
           },
           'onmessage': function(msg) {
-            sendMessageToObjC("onmessage:" +
+            sendMessageToObjC("onmessage", msg);
-                              encodeURIComponent(JSON.stringify(msg.data)));
           },
           'onclose': function() {
             sendMessageToObjC("onclose");
           },
           'onerror': function(err) {
-            sendMessageToObjC("onerror:" +
+            sendMessageToObjC("onerror", err);
-                              encodeURIComponent(JSON.stringify(err.code)) +
-                              ":message:" +
-                              encodeURIComponent(JSON.stringify(err.description)));
           }
         });
       }
@@ -73,9 +74,10 @@
 
       // Add an IFRAME to the DOM to trigger a navigation event.  Then remove
       // it as it is no longer needed.  Only one event is generated.
-      function sendMessageToObjC(message) {
+      function sendMessageToObjC(type, payload) {
+        messageQueue.push(JSON.stringify({'type': type, 'payload': payload}));
         var iframe = document.createElement("IFRAME");
-        iframe.setAttribute("src", "js-frame:" + message);
+        iframe.setAttribute("src", "js-frame:");
         // For some reason we need to set a non-empty size for the iOS6
         // simulator...
         iframe.setAttribute("height", "1px");
@@ -83,6 +85,10 @@
         document.documentElement.appendChild(iframe);
         iframe.parentNode.removeChild(iframe);
       }
+
+      function popQueuedMessage() {
+        return messageQueue.shift();
+      }
     </script>
   </body>
 </html>