Refactor: simplify interface of SenderRtcpSession and CastTransport.

media/cast/net/rtcp/rtcp_unittest.cc

Index: media/cast/net/rtcp/rtcp_unittest.cc
diff --git a/media/cast/net/rtcp/rtcp_unittest.cc b/media/cast/net/rtcp/rtcp_unittest.cc
index ef28072831afb5a6ce16f7793f8dd7fd93bcb07c..aef4dce7b56445cfc4ee0b2dec9de01c78833a5f 100644
--- a/media/cast/net/rtcp/rtcp_unittest.cc
+++ b/media/cast/net/rtcp/rtcp_unittest.cc
@@ -8,6 +8,7 @@
 
 #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"
@@ -31,8 +32,6 @@ media::cast::RtcpTimeData CreateRtcpTimeData(base::TimeTicks now) {
   return ret;
 }
 
-}  // namespace
-
 using testing::_;
 
 static const uint32_t kSenderSsrc = 0x10203;
@@ -75,24 +74,35 @@ class FakeRtcpTransport : public PacedPacketSender {
   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),
@@ -135,6 +145,8 @@ class RtcpTest : public ::testing::Test {
     }
   }
 
+  void OnReceivedPli() { received_pli_ = true; }
+
   PacketRef BuildRtcpPacketFromRtpReceiver(
       const RtcpTimeData& time_data,
       const RtcpCastMessage* cast_message,
@@ -182,10 +194,8 @@ class RtcpTest : public ::testing::Test {
     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_;
@@ -200,6 +210,24 @@ class RtcpTest : public ::testing::Test {
   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.