Add content browser tests for PeerConnection DataChannels.

content/test/data/media/peerconnection-call.html

Index: content/test/data/media/peerconnection-call.html
diff --git a/content/test/data/media/peerconnection-call.html b/content/test/data/media/peerconnection-call.html
index be3387b2232971266c6c011dc9f660d1305c677a..60a4f3ea1a64fa2ef34d8ffb1d798e4de153e691 100644
--- a/content/test/data/media/peerconnection-call.html
+++ b/content/test/data/media/peerconnection-call.html
@@ -8,16 +8,112 @@
   var gFirstConnection = null;
   var gSecondConnection = null;
   var gTestWithoutMsidAndBundle = false;
-
+  
+  // Number of conditions to meet before the test pass. When the test pass, the 
+  // document title change to OK.

[phoglund_chromium, 2013/01/09 12:40:36]

I think a better name here is gNumberOfEvents and gExpectedNumberOfEvents.

[perkj_chrome, 2013/01/09 13:56:21]

Done.

+  var gNumberOfExitConditions = 0;
+  
+  // Number of conditions that currently have been met.  
+  var gNumberOfExitConditionsMet = 0;  
+        
+  // Test that we can setup call with an audio and video track.
   function call(constraints) {
+    createConnections(null);
     navigator.webkitGetUserMedia(constraints, okCallback, failedCallback);
+    waitForVideo($('remote-view'), 320, 240);
   }
   
+  // Test that we can setup call with an audio and video track and 
+  // simulate that the remote peer don't support MSID.
   function callWithoutMsidAndBundle() {
+    createConnections(null);
     gTestWithoutMsidAndBundle = true;
     navigator.webkitGetUserMedia({audio:true, video:true}, okCallback,
         failedCallback);
+    waitForVideo($('remote-view'), 320, 240);
+  }
+  
+  // Test only a data channel. 
+  function callWithDataOnly() {
+    createConnections({optional:[{RtpDataChannels: true}]});
+    setupDataChannel();
+    gFirstConnection.createOffer(onOfferCreated);
+  }
+  
+  // Test call with audio, video and a data channel.
+  function callWithDataAndMedia() {    
+    createConnections({optional:[{RtpDataChannels: true}]});        
+    setupDataChannel();
+    navigator.webkitGetUserMedia({audio:true, video:true}, okCallback,
+      failedCallback);
+    waitForVideo($('remote-view'), 320, 240);
   }
+  
+  // Test call with a data channel and later add audio and video.
+  function callWithDataAndLaterAddMedia() {
+    // TODO(perkj): This is needed for now until 
+    // https://code.google.com/p/webrtc/issues/detail?id=1203 is fixed.
+    gTestWithoutMsidAndBundle = true;
+    
+    createConnections({optional:[{RtpDataChannels: true}]});        
+    setupDataChannel();
+    gFirstConnection.createOffer(onOfferCreated);
+    
+    navigator.webkitGetUserMedia({audio:true, video:true}, okCallback,
+      failedCallback);
+    waitForVideo($('remote-view'), 320, 240);
+  }
+  

[phoglund_chromium, 2013/01/09 12:40:36]

Perhaps it would be nice to number the events, like 
1. Create data channel
2. Send data
3. ...

and spread the comments out in the code when they happen. For instance, in the
dataChannel.onMessage, a comment

// 4. Received reply: done.

Basically, i'd like it to be clearer in which order all the callbacks happen.

[perkj_chrome, 2013/01/09 13:56:21]

Done.

+  // Create a data channel on |gFirstConnection| and sends data to 

[phoglund_chromium, 2013/01/09 12:40:36]

Nit: line length

[perkj_chrome, 2013/01/09 13:56:21]

Done.

+  // |gSecondConnection|. When data is received on |gSecondConnection| a message 
+  // is sent to |gFirstConnection|.
+  // When data is received on |gFirstConnection|, the data 
+  // channel is closed again. The test pass when |dataChannel.readyState| has 
+  // transitioned to closed after an offer/answer exchange.
+  function setupDataChannel() {
+    var sendDataString = "send some text on a data channel."     
+    dataChannel = gFirstConnection.createDataChannel(
+        "sendDataChannel", {reliable : false});
+    expectEquals('connecting', dataChannel.readyState);
+    
+    dataChannel.onmessage = function(event) {
+      expectEquals(event.data, sendDataString);
+      dataChannel.close();
+      gFirstConnection.createOffer(onOfferCreated);
+    }
+        
+    dataChannel.onopen = function() {
+      expectEquals('open', dataChannel.readyState);
+      dataChannel.send(sendDataString);  
+    }
+    
+    gSecondConnection.ondatachannel = function (event) {

[phoglund_chromium, 2013/01/09 12:40:36]

This shadows a variable in the outer scope, which is very confusing. Either
rename the variable, or (better) extract this closure to a proper function.

[perkj_chrome, 2013/01/09 13:56:21]

renamed. I would prefer to not break out anyting since I think this the test is
more readable when the behavior is all defined in this functions.

On 2013/01/09 12:40:36, phoglund wrote:

[phoglund_chromium, 2013/01/09 16:36:45]

Ok, fair enough.

+      var dataChannel = event.channel;
+      var messageReceived = false;
+      dataChannel.onmessage = function(event) {
+        expectEquals(event.data, sendDataString);
+        // TODO(perkj): Currently we sometimes can't send a message here since
+        // the the |dataChannel.readyState| has not transitioned to open yet.
+        // http://code.google.com/p/webrtc/issues/detail?id=1262 
+        messageReceived = true;

[phoglund_chromium, 2013/01/09 12:40:36]

Hm. Ok, so this whole messageReceived thing is used in the case when the
connection isn't open yet and we need to wait until it opens until we can send
our reply?

Could this be written as

if (dataChannel.readyState == "open") {
  dataChannel.send(sendDataString);
} else {
  sendWhenChannelOpens(sendDataString);
}

where sendWhenOpen(sendDataString) {
  dataChannel.onopen = function(event) {
    expectEquals('open', dataChannel.readyState);
      if (messageReceived) {
        dataChannel.send(sendDataString);
      }           
   }
}

Because you don't need to hook up the onopen if the connection is already open,
right? This way you get rid of the variable and it becomes a lot clearer what
the code tries to do.

[perkj_chrome, 2013/01/09 13:56:21]

Done.

+        if (dataChannel.readyState == "open") {
+          dataChannel.send(sendDataString);
+        }         
+      }
+      dataChannel.onopen = function(event) {
+        expectEquals('open', dataChannel.readyState);
+        if (messageReceived) {
+          dataChannel.send(sendDataString);
+        }           
+      }
+    }
+    
+    addExitCondition();
+    dataChannel.onclose = function() {
+      expectEquals('closed', dataChannel.readyState);
+      testSuccessful();
+    }
+  }  
 
   function failedCallback(error) {
     document.title = 'getUserMedia request failed with code ' + error.code;
@@ -30,31 +126,33 @@
     callUsingStream(localStream);
   }
 
-  function callUsingStream(localStream) {
-    gFirstConnection = new webkitRTCPeerConnection(null, null);
+  function createConnections(constraints) {
+    gFirstConnection = new webkitRTCPeerConnection(null, constraints);
     gFirstConnection.onicecandidate = onIceCandidateToFirst;
+  
+    gSecondConnection = new webkitRTCPeerConnection(null, constraints);
+    gSecondConnection.onicecandidate = onIceCandidateToSecond;
+    gSecondConnection.onaddstream = onRemoteStream;       
+  }
+  
+  function callUsingStream(localStream) {    
     gFirstConnection.addStream(localStream);
     gFirstConnection.createOffer(onOfferCreated);
   }
-
+  
   function onOfferCreated(offer) {
     gFirstConnection.setLocalDescription(offer);
-
-    receiveCall(offer.sdp);
+    receiveOffer(offer.sdp);
   }
-
-  function receiveCall(offerSdp) {
+  
+  function receiveOffer(offerSdp) {
     if (gTestWithoutMsidAndBundle) {
       offerSdp = removeMsidAndBundle(offerSdp);
-    }
-    gSecondConnection = new webkitRTCPeerConnection(null, null);
-    gSecondConnection.onicecandidate = onIceCandidateToSecond;
-    gSecondConnection.onaddstream = onRemoteStream;
+    }    
 
     var parsedOffer = new RTCSessionDescription({ type: 'offer',
                                                   sdp: offerSdp });
     gSecondConnection.setRemoteDescription(parsedOffer);
-
     gSecondConnection.createAnswer(onAnswerCreated);
   }
   
@@ -93,22 +191,20 @@
   }
 
   function onRemoteStream(e) {
-    var remoteStreamUrl = webkitURL.createObjectURL(e.stream);
-    var remoteVideo = $('remote-view');
-    remoteVideo.src = remoteStreamUrl;
-    
     if (gTestWithoutMsidAndBundle && e.stream.label != "default") {
       document.title = 'a default remote stream was expected but instead ' + 
           e.stream.label + ' was received.';
       return;
     }
-
-    waitForVideo(remoteVideo, 320, 240);
+    var remoteStreamUrl = webkitURL.createObjectURL(e.stream);
+    var remoteVideo = $('remote-view');
+    remoteVideo.src = remoteStreamUrl;
   }
 
   // TODO(phoglund): perhaps use the video detector in chrome/test/data/webrtc/?
   function waitForVideo(videoElement, width, height) {
     document.title = 'Waiting for video...';

[phoglund_chromium, 2013/01/09 12:40:36]

Use addExitConditions.

[perkj_chrome, 2013/01/09 13:56:21]

Done.

+    ++gNumberOfExitConditions;
     var canvas = $('canvas');
     setInterval(function() {
       var context = canvas.getContext('2d');
@@ -133,9 +229,27 @@
     }
     return false;
   }
+    
+  
+  // This function matches |left| and |right| and throws an exception if the 
+  // values don't match. 
+  function expectEquals(left, right) {
+    if (left != right) {
+      var s = "expectEquals failed left: " + left + " right: " + right;  
+      document.title = s;
+      throw s;
+    }
+  }
 
+  function addExitCondition() {
+    ++gNumberOfExitConditions;
+  }
+  
   function testSuccessful() {

[phoglund_chromium, 2013/01/09 12:40:36]

Why is testSuccessful an exit condition?...

[perkj_chrome, 2013/01/09 13:56:21]

Renamed to exitConditionMet. This is called when an exit event occur.

On 2013/01/09 12:40:36, phoglund wrote:

-    document.title = 'OK';
+    ++gNumberOfExitConditionsMet;
+    if (gNumberOfExitConditionsMet == gNumberOfExitConditions) {
+      document.title = 'OK';
+    }
   }
   </script>
 </head>