Refactor: simplify interface of SenderRtcpSession and CastTransport.

media/cast/net/rtcp/sender_rtcp_session.h

 // Copyright 2015 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.
 
 #ifndef MEDIA_CAST_NET_RTCP_SENDER_RTCP_SESSION_H_
 #define MEDIA_CAST_NET_RTCP_SENDER_RTCP_SESSION_H_
 
 #include <map>
 #include <queue>
 #include <utility>
 
 #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"
 #include "media/cast/net/rtcp/rtcp_utility.h"
 
 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,
                     uint32_t remote_ssrc);
 
   ~SenderRtcpSession() override;
 
   // If greater than zero, this is the last measured network round trip time.
   base::TimeDelta current_round_trip_time() const {
     return current_round_trip_time_;
@@ skipping 34 matching lines @@
   // Save last sent NTP time on RTPC SR. This helps map the sent time when a
   // last report is received from RTP receiver to compute RTT.
   void SaveLastSentNtpTime(const base::TimeTicks& now,
                            uint32_t last_ntp_seconds,
                            uint32_t last_ntp_fraction);
 
   base::TickClock* const clock_;      // Not owned.
   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_;
 
   // The RTCP packet parser is re-used when parsing each RTCP packet.  It
   // remembers state about prior RTP timestamps and other sequence values to
   // re-construct "expanded" values.
   RtcpParser parser_;
 
   // For extending received ACK frame IDs from 8-bit to 32-bit.
@@ skipping 14 matching lines @@
   RtcpSendTimeMap last_reports_sent_map_;
   RtcpSendTimeQueue last_reports_sent_queue_;
 
   DISALLOW_COPY_AND_ASSIGN(SenderRtcpSession);
 };
 
 }  // namespace cast
 }  // namespace media
 
 #endif  // MEDIA_CAST_NET_RTCP_SENDER_RTCP_SESSION_H_