Refactor: simplify interface of SenderRtcpSession and CastTransport.

media/cast/net/rtcp/sender_rtcp_session.h

Index: media/cast/net/rtcp/sender_rtcp_session.h
diff --git a/media/cast/net/rtcp/sender_rtcp_session.h b/media/cast/net/rtcp/sender_rtcp_session.h
index d32a1ade86fd60014332933e4730954b97909176..00db54f9ebe470d46863c09e3a57c8eb21661d59 100644
--- a/media/cast/net/rtcp/sender_rtcp_session.h
+++ b/media/cast/net/rtcp/sender_rtcp_session.h
@@ -11,6 +11,7 @@
 
 #include "base/containers/hash_tables.h"
 #include "base/time/time.h"
+#include "media/cast/net/cast_transport.h"
 #include "media/cast/net/pacing/paced_sender.h"
 #include "media/cast/net/rtcp/rtcp_defines.h"
 #include "media/cast/net/rtcp/rtcp_session.h"
@@ -19,31 +20,28 @@
 namespace media {
 namespace cast {
 
-typedef std::pair<uint32_t, base::TimeTicks> RtcpSendTimePair;
-typedef std::map<uint32_t, base::TimeTicks> RtcpSendTimeMap;
-typedef std::queue<RtcpSendTimePair> RtcpSendTimeQueue;
+using RtcpSendTimePair = std::pair<uint32_t, base::TimeTicks>;
+using RtcpSendTimeMap = std::map<uint32_t, base::TimeTicks>;
+using RtcpSendTimeQueue = std::queue<RtcpSendTimePair>;
 
 // This class represents a RTCP session on a RTP sender. It provides an
 // interface to send RTCP sender report (SR). RTCP SR packets allow
 // receiver to maintain clock offsets and synchronize between audio and video.
 //
 // RTCP session on sender handles the following incoming RTCP reports
-// from receiver:
+// from receiver and passes the information to the Client:

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

s/the Client/a SenderRtcpObserver/

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

Done.

 // - Receiver reference time report: Helps with tracking largest timestamp
 //   seen and as a result rejecting old RTCP reports.
-// - Receiver logs: The sender receives log events from the receiver and
-//   invokes a callback passed.
-// - cast message: Receives feedback from receiver on missing packets/frames
-//   and last frame id received and invokes a callback passed.
+// - Receiver logs: The sender receives log events from the receiver.
+// - cast message: Receives feedback from receiver on missing packets/frames,
+//   later frames received, and last frame id.
 // - Last report: The receiver provides feedback on delay since last report
-//   received which helps it compute round trip time and invoke a callback.
+//   received which helps it compute round trip time.
+// - PLI: Receiver sends PLI when decoding error exists on ultra-low latency
+//   applications.
 class SenderRtcpSession : public RtcpSession {
  public:
-  // TODO(xjz): Simplify the interface. http://crbug.com/588275.
-  SenderRtcpSession(const RtcpCastMessageCallback& cast_callback,
-                    const RtcpRttCallback& rtt_callback,
-                    const RtcpLogMessageCallback& log_callback,
-                    const RtcpPliCallback pli_callback,
+  SenderRtcpSession(std::unique_ptr<SenderRtcpObserver> observer,

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

The object ownership graph is a bit weird around this: First, CastTransportImpl
creates a CastTransportImpl::RtcpObserver that contains a raw pointer back to
the CastTransportImpl.  Then, it passes ownership of the observer to
SenderRtcpSession.

Generally, when the observer pattern is used, the observer is owned by the
client code, and it's the responsibility of client code to make sure the
observer outlives the observed (or use an Add/Remove observer scheme).  In this
case, it seems like CastTransportImpl should own the RtcpObserver it creates,
pass a raw pointer to the SenderRtcpSession ctor here, and then ensure the
RtcpObserver outlives the SenderRtcpSession.  It can easily do this since
CastTransportImpl owns the SenderRtcpSession instance.

Changing this also help cleans up issues I pointed out in other files...

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

Done.

                     base::TickClock* clock,            // Not owned.
                     PacedPacketSender* packet_sender,  // Not owned.
                     uint32_t local_ssrc,
@@ -98,11 +96,7 @@ class SenderRtcpSession : public RtcpSession {
   PacedPacketSender* packet_sender_;  // Not owned.
   const uint32_t local_ssrc_;
   const uint32_t remote_ssrc_;
-
-  const RtcpCastMessageCallback cast_callback_;
-  const RtcpRttCallback rtt_callback_;
-  const RtcpLogMessageCallback log_callback_;
-  const RtcpPliCallback pli_callback_;
+  const std::unique_ptr<SenderRtcpObserver> sender_rtcp_observer_;
 
   // Computed from RTCP RRTR report.
   base::TimeTicks largest_seen_timestamp_;