Refactor: simplify interface of SenderRtcpSession and CastTransport.

media/cast/net/rtcp/rtcp_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 #include <stdint.h>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "media/cast/net/cast_transport_config.h"
 #include "media/cast/net/pacing/paced_sender.h"
 #include "media/cast/net/rtcp/receiver_rtcp_session.h"
 #include "media/cast/net/rtcp/rtcp_builder.h"
 #include "media/cast/net/rtcp/rtcp_session.h"
 #include "media/cast/net/rtcp/rtcp_utility.h"
 #include "media/cast/net/rtcp/sender_rtcp_session.h"
 #include "media/cast/test/skewed_tick_clock.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace media {
 namespace cast {
 
 namespace {
 
 media::cast::RtcpTimeData CreateRtcpTimeData(base::TimeTicks now) {
   media::cast::RtcpTimeData ret;
   ret.timestamp = now;
   media::cast::ConvertTimeTicksToNtp(now, &ret.ntp_seconds, &ret.ntp_fraction);
   return ret;
 }
 
-}  // namespace
-
 using testing::_;
 
 static const uint32_t kSenderSsrc = 0x10203;
 static const uint32_t kReceiverSsrc = 0x40506;
 static const int kInitialReceiverClockOffsetSeconds = -5;
 static const uint16_t kTargetDelayMs = 100;
 
 class FakeRtcpTransport : public PacedPacketSender {
  public:
   explicit FakeRtcpTransport(base::SimpleTestTickClock* clock)
@@ skipping 22 matching lines @@
   void CancelSendingPacket(const PacketKey& packet_key) final {}
 
  private:
   base::SimpleTestTickClock* const clock_;
   base::TimeDelta packet_delay_;
   RtcpSession* rtcp_session_;  //  RTCP destination.
 
   DISALLOW_COPY_AND_ASSIGN(FakeRtcpTransport);
 };
 
+}  // namespace
+
 class RtcpTest : public ::testing::Test {
  protected:
+  class RtcpObserver : public SenderRtcpObserver {

[miu, 2016/04/25 23:23:53]

Seems like you only need this class because SenderRtcpSession has to own the
SenderRtcpObserver.  Otherwise, RtcpTest could implement the SenderRtcpObserver
interface itself.  See comments in sender_rtcp_session.h for more about observer
object ownership...

[xjz, 2016/04/29 19:15:49]

Done.

+   public:
+    explicit RtcpObserver(RtcpTest* rtcp_test) : rtcp_test_(rtcp_test) {}
+
+    void OnCastMessageReceived(const RtcpCastMessage& cast_message) override;
+    void OnRttReceived(base::TimeDelta round_trip_time) override;
+    void OnReceiverLogReceived(const RtcpReceiverLogMessage& log) override;
+    void OnPliReceived() override;
+
+   private:
+    RtcpTest* const rtcp_test_;
+
+    DISALLOW_COPY_AND_ASSIGN(RtcpObserver);
+  };
+
   RtcpTest()
       : sender_clock_(new base::SimpleTestTickClock()),
         receiver_clock_(new test::SkewedTickClock(sender_clock_.get())),
         rtp_sender_pacer_(sender_clock_.get()),
         rtp_receiver_pacer_(sender_clock_.get()),
-        rtcp_at_rtp_sender_(
-            base::Bind(&RtcpTest::OnReceivedCastFeedback,
-                       base::Unretained(this)),
-            base::Bind(&RtcpTest::OnMeasuredRoundTripTime,
-                       base::Unretained(this)),
-            base::Bind(&RtcpTest::OnReceivedLogs, base::Unretained(this)),
-            base::Bind(&RtcpTest::OnReceivedPli, base::Unretained(this)),
-            sender_clock_.get(),
-            &rtp_sender_pacer_,
-            kSenderSsrc,
-            kReceiverSsrc),
+        rtcp_at_rtp_sender_(base::WrapUnique(new RtcpObserver(this)),
+                            sender_clock_.get(),
+                            &rtp_sender_pacer_,
+                            kSenderSsrc,
+                            kReceiverSsrc),
         rtcp_at_rtp_receiver_(receiver_clock_.get(),
                               kReceiverSsrc,
                               kSenderSsrc),
         received_pli_(false) {
     sender_clock_->Advance(base::TimeTicks::Now() - base::TimeTicks());
     receiver_clock_->SetSkew(
         1.0,  // No skew.
         base::TimeDelta::FromSeconds(kInitialReceiverClockOffsetSeconds));
 
     rtp_sender_pacer_.set_rtcp_destination(&rtcp_at_rtp_receiver_);
@@ skipping 22 matching lines @@
         RtcpReceiverEventLogMessage event_log;
         event_log.type = event_log_msg.type;
         event_log.event_timestamp = event_log_msg.event_timestamp;
         event_log.delay_delta = event_log_msg.delay_delta;
         event_log.packet_id = event_log_msg.packet_id;
         last_logs_.back().event_log_messages_.push_back(event_log);
       }
     }
   }
 
+  void OnReceivedPli() { received_pli_ = true; }
+
   PacketRef BuildRtcpPacketFromRtpReceiver(
       const RtcpTimeData& time_data,
       const RtcpCastMessage* cast_message,
       const RtcpPliMessage* pli_message,
       base::TimeDelta target_delay,
       const ReceiverRtcpEventSubscriber::RtcpEvents* rtcp_events,
       const RtpReceiverStatistics* rtp_receiver_statistics) {
     RtcpBuilder builder(rtcp_at_rtp_receiver_.local_ssrc());
     builder.Start();
     RtcpReceiverReferenceTimeReport rrtr;
@@ skipping 27 matching lines @@
     }
     if (cast_message)
       builder.AddCast(*cast_message, target_delay);
     if (pli_message)
       builder.AddPli(*pli_message);
     if (rtcp_events)
       builder.AddReceiverLog(*rtcp_events);
     return builder.Finish();
   }
 
-  void OnReceivedPli() { received_pli_ = true; }
-
-  scoped_ptr<base::SimpleTestTickClock> sender_clock_;
-  scoped_ptr<test::SkewedTickClock> receiver_clock_;
+  std::unique_ptr<base::SimpleTestTickClock> sender_clock_;
+  std::unique_ptr<test::SkewedTickClock> receiver_clock_;
   FakeRtcpTransport rtp_sender_pacer_;
   FakeRtcpTransport rtp_receiver_pacer_;
   SenderRtcpSession rtcp_at_rtp_sender_;
   ReceiverRtcpSession rtcp_at_rtp_receiver_;
 
   base::TimeDelta current_round_trip_time_;
   RtcpCastMessage last_cast_message_;
   RtcpReceiverLogMessage last_logs_;
   bool received_pli_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(RtcpTest);
 };
 
+void RtcpTest::RtcpObserver::OnCastMessageReceived(

[miu, 2016/04/25 23:23:53]

Feel free to inline these methods in the class definition.  They're so simple,
IMHO, it'd be better for readability to have them there.

[xjz, 2016/04/29 19:15:49]

Done.

+    const RtcpCastMessage& cast_message) {
+  rtcp_test_->OnReceivedCastFeedback(cast_message);
+}
+
+void RtcpTest::RtcpObserver::OnRttReceived(base::TimeDelta round_trip_time) {
+  rtcp_test_->OnMeasuredRoundTripTime(round_trip_time);
+}
+
+void RtcpTest::RtcpObserver::OnReceiverLogReceived(
+    const RtcpReceiverLogMessage& log) {
+  rtcp_test_->OnReceivedLogs(log);
+}
+
+void RtcpTest::RtcpObserver::OnPliReceived() {
+  rtcp_test_->OnReceivedPli();
+}
+
 TEST_F(RtcpTest, LipSyncGleanedFromSenderReport) {
   // Initially, expect no lip-sync info receiver-side without having first
   // received a RTCP packet.
   base::TimeTicks reference_time;
   RtpTimeTicks rtp_timestamp;
   ASSERT_FALSE(rtcp_at_rtp_receiver_.GetLatestLipSyncTimes(&rtp_timestamp,
                                                            &reference_time));
 
   // Send a Sender Report to the receiver.
   const base::TimeTicks reference_time_sent = sender_clock_->NowTicks();
@@ skipping 156 matching lines @@
   // Only 24 bits of event timestamp sent on wire.
   uint32_t event_ts =
       (event.timestamp - base::TimeTicks()).InMilliseconds() & 0xffffff;
   uint32_t log_msg_ts =
       (log_msg.event_timestamp - base::TimeTicks()).InMilliseconds() & 0xffffff;
   EXPECT_EQ(log_msg_ts, event_ts);
 }
 
 }  // namespace cast
 }  // namespace media