Adds a test for an one-way media PeerConnection.

webrtc/api/peerconnection_unittest.cc

Index: webrtc/api/peerconnection_unittest.cc
diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc
index d0fb9ad4c5a098a8d615506a8d4a04feb2aa2678..abf2a0e87b5900297272ad11b705605fb9b2c07f 100644
--- a/webrtc/api/peerconnection_unittest.cc
+++ b/webrtc/api/peerconnection_unittest.cc
@@ -415,7 +415,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     return data_observer_.get();
   }
 
-  webrtc::PeerConnectionInterface* pc() { return peer_connection_.get(); }
+  webrtc::PeerConnectionInterface* pc() const { return peer_connection_.get(); }
 
   void StopVideoCapturers() {
     for (std::vector<cricket::VideoCapturer*>::iterator it =
@@ -440,7 +440,8 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
       if (decoders.empty()) {
         return number_of_frames <= 0;
       }
-
+      // Note - this checks that EACH decoder has the requisite number
+      // of frames. The video_frames_received() function sums them.
       for (FakeWebRtcVideoDecoder* decoder : decoders) {
         if (number_of_frames > decoder->GetNumFramesReceived()) {
           return false;
@@ -660,7 +661,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     return pc()->remote_streams()->count();
   }
 
-  StreamCollectionInterface* remote_streams() {
+  StreamCollectionInterface* remote_streams() const {
     if (!pc()) {
       ADD_FAILURE();
       return nullptr;
@@ -919,22 +920,38 @@ class P2PTestConductor : public testing::Test {
            receiving_client_->SessionActive();
   }
 
-  // Return true if the number of frames provided have been received or it is
-  // known that that will never occur (e.g. no frames will be sent or
-  // captured).
-  bool FramesNotPending(int audio_frames_to_receive,
-                        int video_frames_to_receive) {
-    return VideoFramesReceivedCheck(video_frames_to_receive) &&
-        AudioFramesReceivedCheck(audio_frames_to_receive);
-  }
-  bool AudioFramesReceivedCheck(int frames_received) {
-    return initiating_client_->AudioFramesReceivedCheck(frames_received) &&
-        receiving_client_->AudioFramesReceivedCheck(frames_received);
-  }
-  bool VideoFramesReceivedCheck(int frames_received) {
-    return initiating_client_->VideoFramesReceivedCheck(frames_received) &&
-        receiving_client_->VideoFramesReceivedCheck(frames_received);
+  // Return true if the number of frames provided have been received
+  // on the video and audio tracks provided.
+  bool FramesHaveArrived(int audio_frames_to_receive,
+                         int video_frames_to_receive) {
+    bool all_good = true;
+    // Note: We assume that all tracks contain audio and video,
+    // unless one of the parties has can_receive_video() = false.
+    if (initiating_client_->local_streams()->count() > 0) {
+      if (receiving_client_->can_receive_audio()) {
+        all_good &= receiving_client_->AudioFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+      if (receiving_client_->can_receive_video() &&
+          initiating_client_->can_receive_video()) {
+        all_good &= receiving_client_->VideoFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+    }
+    if (receiving_client_->local_streams()->count() > 0) {
+      if (initiating_client_->can_receive_audio()) {
+        all_good &= initiating_client_->AudioFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+      if (initiating_client_->can_receive_video() &&
+          receiving_client_->can_receive_video()) {
+        all_good &= initiating_client_->VideoFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+    }
+    return all_good;
   }
+
   void VerifyDtmf() {
     initiating_client_->VerifyDtmf();
     receiving_client_->VerifyDtmf();
@@ -1039,51 +1056,46 @@ class P2PTestConductor : public testing::Test {
     VerifySessionDescriptions();
 
     int audio_frame_count = kEndAudioFrameCount;
+    int video_frame_count = kEndVideoFrameCount;
     // TODO(ronghuawu): Add test to cover the case of sendonly and recvonly.
     if (!initiating_client_->can_receive_audio() ||

[perkj_webrtc, 2016/03/09 14:44:41]

these can be removed now? 

[hta-webrtc, 2016/03/09 16:37:52]

With one more tweak, yes.

         !receiving_client_->can_receive_audio()) {
       audio_frame_count = -1;
     }
-    int video_frame_count = kEndVideoFrameCount;
     if (!initiating_client_->can_receive_video() ||

[perkj_webrtc, 2016/03/09 14:44:41]

dito?

[hta-webrtc, 2016/03/09 16:37:52]

Done.

         !receiving_client_->can_receive_video()) {
       video_frame_count = -1;
     }
 
-    if (audio_frame_count != -1 || video_frame_count != -1) {
-      // Audio or video is expected to flow, so both clients should reach the
-      // Connected state, and the offerer (ICE controller) should proceed to
-      // Completed.
-      // Note: These tests have been observed to fail under heavy load at
-      // shorter timeouts, so they may be flaky.
-      EXPECT_EQ_WAIT(
-          webrtc::PeerConnectionInterface::kIceConnectionCompleted,
-          initiating_client_->ice_connection_state(),
-          kMaxWaitForFramesMs);
-      EXPECT_EQ_WAIT(
-          webrtc::PeerConnectionInterface::kIceConnectionConnected,
-          receiving_client_->ice_connection_state(),
-          kMaxWaitForFramesMs);
+    if ((!initiating_client_->can_receive_audio() &&
+         !initiating_client_->can_receive_video()) ||
+        (!receiving_client_->can_receive_audio() &&
+         !receiving_client_->can_receive_video())) {
+      // Neither audio nor video will flow, so connections won't be
+      // established. There's nothing more to check.

[perkj_webrtc, 2016/03/09 14:17:17]

Is this really true? What about data channels only?

[hta-webrtc, 2016/03/09 14:31:28]

Datachannels will cause transports to be established, yes. The old logic ignored
that case, so I reproduced that. Need a way to detect the presence of
datachannels.

This whole logic is not valid under Plan A + non-Bundle anyway (we should be
counting media sections of each type, not whether constraints have been set).

[perkj_webrtc, 2016/03/09 14:44:41]

I mean- should the below checks now work even if you just remove all if cases
here and above?

[hta-webrtc, 2016/03/09 16:37:52]

The checks that wait for ICE to complete won't work if ICE doesn't start, and
ICE won't start if there are no transports, and there won't be any transports
when there is no media or datachannel.
So this IF test is necessary. The rest are gone.

I added a TODO about the data channel case.

+      return;
     }
 
-    if (initiating_client_->can_receive_audio() ||
-        initiating_client_->can_receive_video()) {
-      // The initiating client can receive media, so it must produce candidates
-      // that will serve as destinations for that media.
-      // TODO(bemasc): Understand why the state is not already Complete here, as
-      // seems to be the case for the receiving client. This may indicate a bug
-      // in the ICE gathering system.
-      EXPECT_NE(webrtc::PeerConnectionInterface::kIceGatheringNew,
-                initiating_client_->ice_gathering_state());
-    }
-    if (receiving_client_->can_receive_audio() ||
-        receiving_client_->can_receive_video()) {
-      EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete,
-                     receiving_client_->ice_gathering_state(),
-                     kMaxWaitForFramesMs);
-    }
+    // Audio or video is expected to flow, so both clients should reach the
+    // Connected state, and the offerer (ICE controller) should proceed to
+    // Completed.
+    // Note: These tests have been observed to fail under heavy load at
+    // shorter timeouts, so they may be flaky.
+    EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
+                   initiating_client_->ice_connection_state(),
+                   kMaxWaitForFramesMs);
+    EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
+                   receiving_client_->ice_connection_state(),
+                   kMaxWaitForFramesMs);
+
+    EXPECT_NE(webrtc::PeerConnectionInterface::kIceGatheringNew,

[perkj_webrtc, 2016/03/09 14:17:17]

Should you leave the todo here unless you know that this is the correct
behaviour? 

[hta-webrtc, 2016/03/09 14:31:28]

I know it's not correct behavior according to current spec (and the bug is
already filed a year ago, I think), but the WG seems about to revise its state
machine so that the kIceGatheringComplete state changes meaning, so it doesn't
make sense to fixate upon changing this now.

The check is correct, and will remain correct after spec revision.

[hta-webrtc, 2016/03/09 16:37:52]

Added a TODO and explanatory text.

+              initiating_client_->ice_gathering_state());
+    EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete,
+                   receiving_client_->ice_gathering_state(),
+                   kMaxWaitForFramesMs);
 
-    EXPECT_TRUE_WAIT(FramesNotPending(audio_frame_count, video_frame_count),
+    // Check that the expected number of frames have arrived.
+    EXPECT_TRUE_WAIT(FramesHaveArrived(audio_frame_count, video_frame_count),
                      kMaxWaitForFramesMs);
   }
 
@@ -1213,6 +1225,14 @@ TEST_F(P2PTestConductor, LocalP2PTestDtls) {
   SetupAndVerifyDtlsCall();
 }
 
+// This test sets up an one-way call, with media only from initiator to
+// responder.
+TEST_F(P2PTestConductor, OneWayMediaCall) {
+  ASSERT_TRUE(CreateTestClients());
+  receiving_client()->set_auto_add_stream(false);
+  LocalP2PTest();
+}
+
 // This test sets up a audio call initially and then upgrades to audio/video,
 // using DTLS.
 TEST_F(P2PTestConductor, LocalP2PTestDtlsRenegotiate) {
@@ -1819,7 +1839,7 @@ TEST_F(P2PTestConductor, EarlyWarmupTest) {
       audio_sender->SetTrack(initializing_client()->CreateLocalAudioTrack("")));
   EXPECT_TRUE(
       video_sender->SetTrack(initializing_client()->CreateLocalVideoTrack("")));
-  EXPECT_TRUE_WAIT(FramesNotPending(kEndAudioFrameCount, kEndVideoFrameCount),
+  EXPECT_TRUE_WAIT(FramesHaveArrived(kEndAudioFrameCount, kEndVideoFrameCount),
                    kMaxWaitForFramesMs);
 }