Forward the SignalFirstPacketReceived to RtpReceiver.

webrtc/api/peerconnection_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 128 matching lines @@
                                  std::string& msg) = 0;
   virtual void ReceiveIceMessage(const std::string& sdp_mid,
                                  int sdp_mline_index,
                                  const std::string& msg) = 0;
 
  protected:
   SignalingMessageReceiver() {}
   virtual ~SignalingMessageReceiver() {}
 };
 
+class MockRtpReceiverObserver : public webrtc::RtpReceiverObserverInterface {
+ public:
+  MockRtpReceiverObserver()

[pthatcher1, 2016/06/08 17:35:55]

I'd suggest just passing the kind (or expected cricket::MediaType; see below)
into the constructor.

+      : is_first_audio_packet_recevied_(false),
+        is_first_video_packet_recevied_(false),
+        kind_("unset") {}
+
+  void OnFirstPacketReceived(cricket::MediaType media_type) override {
+    if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+      is_first_audio_packet_recevied_ = true;
+    } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
+      is_first_video_packet_recevied_ = true;
+    }

[pthatcher1, 2016/06/08 17:35:55]

Given the unlikelyhood of getting the wrong media type, an
ASSERT_EQ(expected_media_type_, media_type) here might make more sense.  Then
you just have store "first_packet_received_ = true" here and the checking code
will be much more simple.

+  }
+
+  void SetKind(std::string kind) { kind_ = kind; }
+
+  std::string kind() { return kind_; }
+
+  bool IsFirstAudioPacketReceived() { return is_first_audio_packet_recevied_; }

[pthatcher1, 2016/06/08 17:35:55]

Using lowercase first_x_packet_received() is probably a better fit.  The "Is"
prefix sounds awkward, and the simple reading of a member variable matches the
lowercase style more.

+
+  bool IsFirstVideoPacketReceived() { return is_first_video_packet_recevied_; }
+
+  virtual ~MockRtpReceiverObserver() {}
+
+ private:
+  bool is_first_audio_packet_recevied_;
+  bool is_first_video_packet_recevied_;

[pthatcher1, 2016/06/08 17:35:55]

You can use "= false" here and avoid needing to set it in the constructor.

[pthatcher1, 2016/06/08 17:35:55]

Just "first_x_packet_received_" would be enough.  The "is_" sounds awkward.

+  std::string kind_;

[pthatcher1, 2016/06/08 17:35:55]

I think it might make more sense to store the expected_media_type_ here, going
along with the comment above about using an ASSERT_EQ.

+};
+
 class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
                                  public SignalingMessageReceiver,
                                  public ObserverInterface {
  public:
   static PeerConnectionTestClient* CreateClientWithDtlsIdentityStore(
       const std::string& id,
       const MediaConstraintsInterface* constraints,
       const PeerConnectionFactory::Options* options,
       std::unique_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store,
       bool prefer_constraint_apis,
@@ skipping 611 matching lines @@
   }
 
   webrtc::PeerConnectionInterface::IceConnectionState ice_connection_state() {
     return pc()->ice_connection_state();
   }
 
   webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state() {
     return pc()->ice_gathering_state();
   }
 
+  std::vector<std::unique_ptr<MockRtpReceiverObserver>> const&
+  rtp_receiver_observers() {
+    return rtp_receiver_observers_;
+  }
+
  private:
   class DummyDtmfObserver : public DtmfSenderObserverInterface {
    public:
     DummyDtmfObserver() : completed_(false) {}
 
     // Implements DtmfSenderObserverInterface.
     void OnToneChange(const std::string& tone) override {
       tones_.push_back(tone);
       if (tone.empty()) {
         completed_ = true;
@@ skipping 70 matching lines @@
 
   void HandleIncomingOffer(const std::string& msg) {
     LOG(INFO) << id_ << "HandleIncomingOffer ";
     if (NumberOfLocalMediaStreams() == 0 && auto_add_stream_) {
       // If we are not sending any streams ourselves it is time to add some.
       AddMediaStream(true, true);
     }
     std::unique_ptr<SessionDescriptionInterface> desc(
         webrtc::CreateSessionDescription("offer", msg, nullptr));
     EXPECT_TRUE(DoSetRemoteDescription(desc.release()));
+    // Register the RtpReceiverObserver after receivers are created.
+    SetRtpReceiverObservers();
     std::unique_ptr<SessionDescriptionInterface> answer;
     EXPECT_TRUE(DoCreateAnswer(&answer));
     std::string sdp;
     EXPECT_TRUE(answer->ToString(&sdp));
     EXPECT_TRUE(DoSetLocalDescription(answer.release()));
     if (signaling_message_receiver_) {
       signaling_message_receiver_->ReceiveSdpMessage(
           webrtc::SessionDescriptionInterface::kAnswer, sdp);
     }
   }
 
   void HandleIncomingAnswer(const std::string& msg) {
     LOG(INFO) << id_ << "HandleIncomingAnswer";
     std::unique_ptr<SessionDescriptionInterface> desc(
         webrtc::CreateSessionDescription("answer", msg, nullptr));
     EXPECT_TRUE(DoSetRemoteDescription(desc.release()));
+    // Register the RtpReceiverObserver after receivers are created.
+    SetRtpReceiverObservers();
   }
 
   bool DoCreateOfferAnswer(std::unique_ptr<SessionDescriptionInterface>* desc,
                            bool offer) {
     rtc::scoped_refptr<MockCreateSessionDescriptionObserver>
         observer(new rtc::RefCountedObject<
             MockCreateSessionDescriptionObserver>());
     if (prefer_constraint_apis_) {
       if (offer) {
         pc()->CreateOffer(observer, &offer_answer_constraints_);
@@ skipping 37 matching lines @@
     // ProcessMessages waits at least 1ms but processes all messages before
     // returning. Since this test is synchronous and send messages to the remote
     // peer whenever a callback is invoked, this can lead to messages being
     // sent to the remote peer in the wrong order.
     // TODO(perkj): Find a way to check the result without risking that the
     // order of sent messages are changed. Ex- by posting all messages that are
     // sent to the remote peer.
     return true;
   }
 
+  void SetRtpReceiverObservers() {
+    for (auto receiver : pc()->GetReceivers()) {
+      std::unique_ptr<MockRtpReceiverObserver> observer(
+          new MockRtpReceiverObserver());
+      observer->SetKind(receiver->track()->kind());
+      receiver->SetObserver(observer.get());
+      rtp_receiver_observers_.push_back(std::move(observer));
+    }
+  }
+
   bool DoSetRemoteDescription(SessionDescriptionInterface* desc) {
     rtc::scoped_refptr<MockSetSessionDescriptionObserver>
         observer(new rtc::RefCountedObject<
             MockSetSessionDescriptionObserver>());
     LOG(INFO) << id_ << "SetRemoteDescription ";
     pc()->SetRemoteDescription(observer, desc);
     EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs);
     return observer->result();
   }
 
@@ skipping 63 matching lines @@
   bool remove_bundle_ =
       false;  // True if bundle should be removed in received SDP.
   bool remove_sdes_ =
       false;  // True if a=crypto should be removed in received SDP.
   // |remove_cvo_| is true if extension urn:3gpp:video-orientation should be
   // removed in the received SDP.
   bool remove_cvo_ = false;
 
   rtc::scoped_refptr<DataChannelInterface> data_channel_;
   std::unique_ptr<MockDataChannelObserver> data_observer_;
+
+  std::vector<std::unique_ptr<MockRtpReceiverObserver>> rtp_receiver_observers_;
 };
 
 class P2PTestConductor : public testing::Test {
  public:
   P2PTestConductor()
       : network_thread_(rtc::Thread::CreateWithSocketServer()),
         worker_thread_(rtc::Thread::Create()),
         pss_(new rtc::PhysicalSocketServer),
         ss_(new rtc::VirtualSocketServer(pss_.get())),
         ss_scope_(ss_.get()) {
@@ skipping 269 matching lines @@
 
   // Set the |receiving_client_| to the |client| passed in and return the
   // original |receiving_client_|.
   PeerConnectionTestClient* set_receiving_client(
       PeerConnectionTestClient* client) {
     PeerConnectionTestClient* old = receiving_client_.release();
     receiving_client_.reset(client);
     return old;
   }
 
+  void CheckRtpReceiverObserverAreCalled(PeerConnectionTestClient* client) {
+    for (auto const& observer : client->rtp_receiver_observers()) {
+      if (observer->kind() == webrtc::MediaStreamTrackInterface::kAudioKind) {
+        EXPECT_TRUE_WAIT(observer->IsFirstAudioPacketReceived(),
+                         kMaxWaitForFramesMs);
+        EXPECT_FALSE(observer->IsFirstVideoPacketReceived());
+      } else if (observer->kind() ==
+                 webrtc::MediaStreamTrackInterface::kVideoKind) {
+        EXPECT_FALSE(observer->IsFirstAudioPacketReceived());
+        EXPECT_TRUE_WAIT(observer->IsFirstVideoPacketReceived(),
+                         kMaxWaitForFramesMs);
+      } else {
+        EXPECT_TRUE(false) << "Unknown RtpReceiver kind.";
+      }

[pthatcher1, 2016/06/08 17:35:55]

With my other suggestion about ASSERT_EQ(media_type, ....), this could just be:

EXPECT_TRUE_WAIT(observer->first_packet_received(), kMaxWaitForFrameMs);

[Zhi Huang, 2016/06/09 00:37:36]

This make the code much cleaner.

+    }
+  }
+
  private:
   // |worker_thread_| is used by both |initiating_client_| and
   // |receiving_client_|. Must be destroyed last.
   std::unique_ptr<rtc::Thread> network_thread_;
   std::unique_ptr<rtc::Thread> worker_thread_;
   std::unique_ptr<rtc::PhysicalSocketServer> pss_;
   std::unique_ptr<rtc::VirtualSocketServer> ss_;
   rtc::SocketServerScope ss_scope_;
   std::unique_ptr<PeerConnectionTestClient> initiating_client_;
   std::unique_ptr<PeerConnectionTestClient> receiving_client_;
   bool prefer_constraint_apis_ = true;
 };
 
 // Disable for TSan v2, see
 // https://code.google.com/p/webrtc/issues/detail?id=1205 for details.
 #if !defined(THREAD_SANITIZER)
 
+TEST_F(P2PTestConductor, TestRtpReceiverObserverCallbackFunction) {
+  ASSERT_TRUE(CreateTestClients());
+  LocalP2PTest();
+  CheckRtpReceiverObserverAreCalled(initializing_client());
+  CheckRtpReceiverObserverAreCalled(receiving_client());
+}
+
 // This test sets up a Jsep call between two parties and test Dtmf.
 // TODO(holmer): Disabled due to sometimes crashing on buildbots.
 // See issue webrtc/2378.
 TEST_F(P2PTestConductor, DISABLED_LocalP2PTestDtmf) {
   ASSERT_TRUE(CreateTestClients());
   LocalP2PTest();
   VerifyDtmf();
 }
 
 // This test sets up a Jsep call between two parties and test that we can get a
@@ skipping 869 matching lines @@
   server.urls.push_back("turn:hostname2");
   servers.push_back(server);
   EXPECT_TRUE(webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_));
   EXPECT_EQ(2U, turn_servers_.size());
   EXPECT_NE(turn_servers_[0].priority, turn_servers_[1].priority);
 }
 
 #endif // if !defined(THREAD_SANITIZER)
 
 }  // namespace