Simplify interface for media/cast: CastTransportSenderImpl

chrome/browser/media/cast_transport_host_filter.cc

Index: chrome/browser/media/cast_transport_host_filter.cc
diff --git a/chrome/browser/media/cast_transport_host_filter.cc b/chrome/browser/media/cast_transport_host_filter.cc
index 3f140734c2f728dcc3a414e4c29454de16f3a323..f2285923b7dc9266296c8b8ff6f418606fde9a5d 100644
--- a/chrome/browser/media/cast_transport_host_filter.cc
+++ b/chrome/browser/media/cast_transport_host_filter.cc
@@ -16,7 +16,7 @@ namespace {
 // How often to send raw events.
 const int kSendRawEventsIntervalSecs = 1;
 
-}
+}  // namespace
 
 namespace cast {
 
@@ -49,25 +49,22 @@ bool CastTransportHostFilter::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-void CastTransportHostFilter::ReceivedPacket(
-    int32 channel_id,
-    scoped_ptr<media::cast::Packet> packet) {
-  Send(new CastMsg_ReceivedPacket(channel_id, *packet));
-}
-
-void CastTransportHostFilter::NotifyStatusChange(
-    int32 channel_id,
+void CastTransportHostFilter::OnStatusChange(
     media::cast::CastTransportStatus status) {
-  Send(new CastMsg_NotifyStatusChange(channel_id, status));
+  Send(new CastMsg_NotifyStatusChange(channel_id_, status));
 }
 
-void CastTransportHostFilter::SendRawEvents(
-    int32 channel_id,
+void CastTransportHostFilter::OnReceivedLoggingEvents(
     scoped_ptr<std::vector<media::cast::FrameEvent>> frame_events,
     scoped_ptr<std::vector<media::cast::PacketEvent>> packet_events) {
   if (frame_events->empty() && packet_events->empty())
     return;
-  Send(new CastMsg_RawEvents(channel_id, *packet_events, *frame_events));
+  Send(new CastMsg_RawEvents(channel_id_, *packet_events, *frame_events));
+}
+
+void CastTransportHostFilter::OnReceivedPackets(
+    scoped_ptr<media::cast::Packet> packet) {
+  Send(new CastMsg_ReceivedPacket(channel_id_, *packet));
 }
 
 void CastTransportHostFilter::SendRtt(int32 channel_id,
@@ -102,24 +99,13 @@ void CastTransportHostFilter::OnNew(
     id_map_.Remove(channel_id);
   }
 
+  channel_id_ = channel_id;

[Irfan, 2015/12/16 19:44:34]

as we discussed, this creates a problem since we want to be able to manage
several channel ids in CastTransportHostFilter.

Two options:

- Push channel id to CastTransportSender and let it manage it. I agree with you
that channel id is an abstraction that CastTransportSender should probably not
know about.

- Go back to your approach of abstracting channel id in a separate class of its
own and pass ownership to CastTransportSender to manage the life time

[xjz, 2015/12/16 21:58:01]

I prefer the second approach, unless there are reasons to let the sender manager
the channel id. And it is more flexible to have its own class. For example, the
client might want to add some other parameters in the future.

+  media::cast::CastTransportSender::CreateParams transport_params(
+      g_browser_process->net_log(), &clock_, local_end_point, remote_end_point,
+      this, base::TimeDelta::FromSeconds(kSendRawEventsIntervalSecs),
+      make_scoped_ptr(options.DeepCopy()), base::ThreadTaskRunnerHandle::Get());
   scoped_ptr<media::cast::CastTransportSender> sender =
-      media::cast::CastTransportSender::Create(
-          g_browser_process->net_log(),
-          &clock_,
-          local_end_point,
-          remote_end_point,
-          make_scoped_ptr(options.DeepCopy()),
-          base::Bind(&CastTransportHostFilter::NotifyStatusChange,
-                     weak_factory_.GetWeakPtr(),
-                     channel_id),
-          base::Bind(&CastTransportHostFilter::SendRawEvents,
-                     weak_factory_.GetWeakPtr(),
-                     channel_id),
-          base::TimeDelta::FromSeconds(kSendRawEventsIntervalSecs),
-          base::Bind(&CastTransportHostFilter::ReceivedPacket,
-                     weak_factory_.GetWeakPtr(),
-                     channel_id),
-          base::ThreadTaskRunnerHandle::Get());
+      media::cast::CastTransportSender::Create(transport_params);
   id_map_.AddWithID(sender.release(), channel_id);
 }