Added a callback function OnAddTrack to PeerConnectionObserver

webrtc/api/peerconnectioninterface_unittest.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 73 matching lines @@
     "a=ssrc:1 mslabel:stream1\r\n"
     "a=ssrc:1 label:audiotrack0\r\n"
     "m=video 1 RTP/AVPF 120\r\n"
     "a=mid:video\r\n"
     "a=sendrecv\r\n"
     "a=rtpmap:120 VP8/90000\r\n"
     "a=ssrc:2 cname:stream1\r\n"
     "a=ssrc:2 mslabel:stream1\r\n"
     "a=ssrc:2 label:videotrack0\r\n";
 
+// Reference SDP with a MediaStream with label "stream1" and audio track with
+// id "audio_1";
+static const char kSdpStringWithStream1AudioTrackOnly[] =
+    "v=0\r\n"
+    "o=- 0 0 IN IP4 127.0.0.1\r\n"
+    "s=-\r\n"
+    "t=0 0\r\n"
+    "a=ice-ufrag:e5785931\r\n"
+    "a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
+    "a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
+    "BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
+    "m=audio 1 RTP/AVPF 103\r\n"
+    "a=mid:audio\r\n"
+    "a=sendrecv\r\n"
+    "a=rtpmap:103 ISAC/16000\r\n"
+    "a=ssrc:1 cname:stream1\r\n"
+    "a=ssrc:1 mslabel:stream1\r\n"
+    "a=ssrc:1 label:audiotrack0\r\n";
+
 // Reference SDP with two MediaStreams with label "stream1" and "stream2. Each
 // MediaStreams have one audio track and one video track.
 // This uses MSID.
 static const char kSdpStringWithStream1And2[] =
     "v=0\r\n"
     "o=- 0 0 IN IP4 127.0.0.1\r\n"
     "s=-\r\n"
     "t=0 0\r\n"
     "a=ice-ufrag:e5785931\r\n"
     "a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
@@ skipping 388 matching lines @@
   }
   void OnRenegotiationNeeded() override { renegotiation_needed_ = true; }
   void OnDataChannel(
       rtc::scoped_refptr<DataChannelInterface> data_channel) override {
     last_datachannel_ = data_channel;
   }
 
   void OnIceConnectionChange(
       PeerConnectionInterface::IceConnectionState new_state) override {
     EXPECT_EQ(pc_->ice_connection_state(), new_state);
-    callback_triggered = true;
+    callback_triggered_ = true;
   }
   void OnIceGatheringChange(
       PeerConnectionInterface::IceGatheringState new_state) override {
     EXPECT_EQ(pc_->ice_gathering_state(), new_state);
     ice_complete_ = new_state == PeerConnectionInterface::kIceGatheringComplete;
-    callback_triggered = true;
+    callback_triggered_ = true;
   }
   void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override {
     EXPECT_NE(PeerConnectionInterface::kIceGatheringNew,
               pc_->ice_gathering_state());
 
     std::string sdp;
     EXPECT_TRUE(candidate->ToString(&sdp));
     EXPECT_LT(0u, sdp.size());
     last_candidate_.reset(webrtc::CreateIceCandidate(candidate->sdp_mid(),
         candidate->sdp_mline_index(), sdp, NULL));
     EXPECT_TRUE(last_candidate_.get() != NULL);
-    callback_triggered = true;
+    callback_triggered_ = true;
   }
 
   void OnIceCandidatesRemoved(
       const std::vector<cricket::Candidate>& candidates) override {
-    callback_triggered = true;
+    callback_triggered_ = true;
   }
 
   void OnIceConnectionReceivingChange(bool receiving) override {
-    callback_triggered = true;
+    callback_triggered_ = true;
+  }
+
+  void OnAddTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface> receiver,
+                  std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>
+                      streams) override {
+    EXPECT_TRUE(receiver != nullptr);
+    num_added_track_++;
+    last_added_track_label_ = receiver->id();
   }
 
   // Returns the label of the last added stream.
   // Empty string if no stream have been added.
   std::string GetLastAddedStreamLabel() {
     if (last_added_stream_.get())
       return last_added_stream_->label();
     return "";
   }
   std::string GetLastRemovedStreamLabel() {
     if (last_removed_stream_.get())
       return last_removed_stream_->label();
     return "";
   }
 
   rtc::scoped_refptr<PeerConnectionInterface> pc_;
   PeerConnectionInterface::SignalingState state_;
   std::unique_ptr<IceCandidateInterface> last_candidate_;
   rtc::scoped_refptr<DataChannelInterface> last_datachannel_;
   rtc::scoped_refptr<StreamCollection> remote_streams_;
   bool renegotiation_needed_ = false;
   bool ice_complete_ = false;
-  bool callback_triggered = false;
+  bool callback_triggered_ = false;
+  int num_added_track_ = 0;

[Taylor Brandstetter, 2016/11/17 00:34:23]

nit: I'd change this to "num_added_tracks_", plural

[Zhi Huang, 2016/11/17 01:42:06]

Done.

I also removed the initialization of last_added_track_label which is not
necessary.

+  std::string last_added_track_label_ = "";
 
  private:
   rtc::scoped_refptr<MediaStreamInterface> last_added_stream_;
   rtc::scoped_refptr<MediaStreamInterface> last_removed_stream_;
 };
 
 }  // namespace
 
 // The PeerConnectionMediaConfig tests below verify that configuration
 // and constraints are propagated into the MediaConfig passed to
@@ skipping 500 matching lines @@
 // PeerConnection is closed.
 TEST_F(PeerConnectionInterfaceTest, CloseAndTestCallbackFunctions) {
   rtc::scoped_refptr<PeerConnectionInterface> pc(
       pc_factory_for_test_->CreatePeerConnection(
           PeerConnectionInterface::RTCConfiguration(), nullptr, nullptr,
           nullptr, &observer_));
   observer_.SetPeerConnectionInterface(pc.get());
   pc->Close();
 
   // No callbacks is expected to be called.
-  observer_.callback_triggered = false;
+  observer_.callback_triggered_ = false;
   std::vector<cricket::Candidate> candidates;
   pc_factory_for_test_->transport_controller->SignalGatheringState(
       cricket::IceGatheringState{});
   pc_factory_for_test_->transport_controller->SignalCandidatesGathered(
       "", candidates);
   pc_factory_for_test_->transport_controller->SignalConnectionState(
       cricket::IceConnectionState{});
   pc_factory_for_test_->transport_controller->SignalCandidatesRemoved(
       candidates);
   pc_factory_for_test_->transport_controller->SignalReceiving(false);
-  EXPECT_FALSE(observer_.callback_triggered);
+  EXPECT_FALSE(observer_.callback_triggered_);
 }
 
 // Generate different CNAMEs when PeerConnections are created.
 // The CNAMEs are expected to be generated randomly. It is possible
 // that the test fails, though the possibility is very low.
 TEST_F(PeerConnectionInterfaceTest, CnameGenerationInOffer) {
   std::unique_ptr<SessionDescriptionInterface> offer1 =
       CreateOfferWithOneAudioStream();
   std::unique_ptr<SessionDescriptionInterface> offer2 =
       CreateOfferWithOneAudioStream();
@@ skipping 1538 matching lines @@
 
   auto new_senders = pc_->GetSenders();
   // Should be the same senders as before, but with updated stream id.
   // Note that this behavior is subject to change in the future.
   // We may decide the PC should ignore existing tracks in AddStream.
   EXPECT_EQ(senders, new_senders);
   EXPECT_TRUE(ContainsSender(new_senders, kAudioTracks[0], kStreams[1]));
   EXPECT_TRUE(ContainsSender(new_senders, kVideoTracks[0], kStreams[1]));
 }
 
+// This tests the PeerConnectionObserver::OnAddTrack are correctly called.

[Taylor Brandstetter, 2016/11/17 00:34:23]

nit: "the" -> "that", "are" -> "is"

[Zhi Huang, 2016/11/17 01:42:06]

Done.

+TEST_F(PeerConnectionInterfaceTest, OnAddTrackCallback) {
+  FakeConstraints constraints;
+  constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
+                           true);
+  CreatePeerConnection(&constraints);
+  observer_.num_added_track_ = 0;
+  observer_.last_added_track_label_ = "";

[Taylor Brandstetter, 2016/11/17 00:34:23]

These don't need to be initialized here since they're initialized in the
constructor.

[Zhi Huang, 2016/11/17 01:42:06]

Done.

+
+  CreateAndSetRemoteOffer(kSdpStringWithStream1AudioTrackOnly);
+  EXPECT_EQ(observer_.num_added_track_, 1);
+  EXPECT_EQ(observer_.last_added_track_label_, kAudioTracks[0]);
+  // Create and set the updated remote SDP.
+  CreateAndSetRemoteOffer(kSdpStringWithStream1);
+  EXPECT_EQ(observer_.num_added_track_, 2);
+  EXPECT_EQ(observer_.last_added_track_label_, kVideoTracks[0]);
+
+  // Clear the observer stats.
+  observer_.num_added_track_ = 0;
+  observer_.last_added_track_label_ = "";

[Taylor Brandstetter, 2016/11/17 00:34:23]

This isn't needed, since each test case completely reconstructs the test object.

P.S.: This does make you wonder, "then what is the SetUp method for?" The answer
is "for things you can't do in a constructor"; see
https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md#shoul....
This surprised me when I first learned about it.

[Zhi Huang, 2016/11/17 01:42:06]

Good to know. Thanks for the link!

+}
+
 class PeerConnectionMediaConfigTest : public testing::Test {
  protected:
   void SetUp() override {
     pcf_ = new rtc::RefCountedObject<PeerConnectionFactoryForTest>();
     pcf_->Initialize();
   }
   const cricket::MediaConfig& TestCreatePeerConnection(
       const PeerConnectionInterface::RTCConfiguration& config,
       const MediaConstraintsInterface *constraints) {
     pcf_->create_media_controller_called_ = false;
@@ skipping 243 matching lines @@
   FakeConstraints updated_answer_c;
   answer_c.SetMandatoryReceiveAudio(false);
   answer_c.SetMandatoryReceiveVideo(false);
 
   cricket::MediaSessionOptions updated_answer_options;
   EXPECT_TRUE(
       ParseConstraintsForAnswer(&updated_answer_c, &updated_answer_options));
   EXPECT_TRUE(updated_answer_options.has_audio());
   EXPECT_TRUE(updated_answer_options.has_video());
 }