CastTransportIPC can send messages to uninitialized channel id.

chrome/renderer/media/cast_transport_ipc.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 "chrome/renderer/media/cast_transport_ipc.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/callback.h"
 #include "base/id_map.h"
 #include "chrome/common/cast_messages.h"
 #include "chrome/renderer/media/cast_ipc_dispatcher.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "media/cast/cast_sender.h"
 
 CastTransportIPC::CastTransportIPC(
     const net::IPEndPoint& local_end_point,
     const net::IPEndPoint& remote_end_point,
     std::unique_ptr<base::DictionaryValue> options,
     const media::cast::PacketReceiverCallback& packet_callback,
     const media::cast::CastTransportStatusCallback& status_cb,
     const media::cast::BulkRawEventsCallback& raw_events_cb)
-    : packet_callback_(packet_callback),
+    : channel_id_(-1),
+      packet_callback_(packet_callback),
       status_callback_(status_cb),
       raw_events_callback_(raw_events_cb) {
   if (CastIPCDispatcher::Get()) {

[miu, 2016/11/21 20:21:50]

Note: It's unfortunate that the original author of the code short-cutted this
here. (Really, CastIPCDispatcher should be provided as a ctor argument.) Could
you add a TODO comment here for future clean-up?

     channel_id_ = CastIPCDispatcher::Get()->AddSender(this);
+    Send(new CastHostMsg_New(channel_id_, local_end_point, remote_end_point,
+                             *options));
   }
-  Send(new CastHostMsg_New(channel_id_, local_end_point, remote_end_point,
-                           *options));
 }
 
 CastTransportIPC::~CastTransportIPC() {
-  Send(new CastHostMsg_Delete(channel_id_));
+  if (channel_id_ != -1) {
+    Send(new CastHostMsg_Delete(channel_id_));
+  }
   if (CastIPCDispatcher::Get()) {
     CastIPCDispatcher::Get()->RemoveSender(channel_id_);
   }
 }
 
 void CastTransportIPC::InitializeStream(
     const media::cast::CastTransportRtpConfig& config,
     std::unique_ptr<media::cast::RtcpObserver> rtcp_observer) {
   if (rtcp_observer) {
     DCHECK(clients_.find(config.ssrc) == clients_.end());
     clients_[config.ssrc] = std::move(rtcp_observer);
   }
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_InitializeStream(channel_id_, config));
 }
 
 void CastTransportIPC::InsertFrame(uint32_t ssrc,
                                    const media::cast::EncodedFrame& frame) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_InsertFrame(channel_id_, ssrc, frame));
 }
 
 void CastTransportIPC::SendSenderReport(
     uint32_t ssrc,
     base::TimeTicks current_time,
     media::cast::RtpTimeTicks current_time_as_rtp_timestamp) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_SendSenderReport(channel_id_, ssrc, current_time,
                                         current_time_as_rtp_timestamp));
 }
 
 void CastTransportIPC::CancelSendingFrames(
     uint32_t ssrc,
     const std::vector<media::cast::FrameId>& frame_ids) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_CancelSendingFrames(channel_id_, ssrc, frame_ids));
 }
 
 void CastTransportIPC::ResendFrameForKickstart(uint32_t ssrc,
                                                media::cast::FrameId frame_id) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_ResendFrameForKickstart(channel_id_, ssrc, frame_id));
 }
 
 void CastTransportIPC::AddValidRtpReceiver(uint32_t rtp_sender_ssrc,
                                            uint32_t rtp_receiver_ssrc) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_AddValidRtpReceiver(channel_id_, rtp_sender_ssrc,
                                            rtp_receiver_ssrc));
 }
 
 void CastTransportIPC::InitializeRtpReceiverRtcpBuilder(
     uint32_t rtp_receiver_ssrc,
     const media::cast::RtcpTimeData& time_data) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_InitializeRtpReceiverRtcpBuilder(
       channel_id_, rtp_receiver_ssrc, time_data));
 }
 
 void CastTransportIPC::AddCastFeedback(
     const media::cast::RtcpCastMessage& cast_message,
     base::TimeDelta target_delay) {
+  DCHECK(channel_id_ != -1);
   Send(
       new CastHostMsg_AddCastFeedback(channel_id_, cast_message, target_delay));
 }
 
 void CastTransportIPC::AddPli(const media::cast::RtcpPliMessage& pli_message) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_AddPli(channel_id_, pli_message));
 }
 
 void CastTransportIPC::AddRtcpEvents(
     const media::cast::ReceiverRtcpEventSubscriber::RtcpEvents& rtcp_events) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_AddRtcpEvents(channel_id_, rtcp_events));
 }
 
 void CastTransportIPC::AddRtpReceiverReport(
     const media::cast::RtcpReportBlock& rtp_receiver_report_block) {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_AddRtpReceiverReport(channel_id_,
                                             rtp_receiver_report_block));
 }
 
 void CastTransportIPC::SendRtcpFromRtpReceiver() {
+  DCHECK(channel_id_ != -1);
   Send(new CastHostMsg_SendRtcpFromRtpReceiver(channel_id_));
 }
 
 void CastTransportIPC::OnNotifyStatusChange(
     media::cast::CastTransportStatus status) {
   status_callback_.Run(status);
 }
 
 void CastTransportIPC::OnRawEvents(
     const std::vector<media::cast::PacketEvent>& packet_events,
@@ skipping 48 matching lines @@
     // TODO(hubbe): Perhaps an non-ownership-transferring cb here?
     std::unique_ptr<media::cast::Packet> packet_copy(
         new media::cast::Packet(packet));
     packet_callback_.Run(std::move(packet_copy));
   } else {
     DVLOG(1) << "CastIPCDispatcher::OnReceivedPacket no packet callback yet.";
   }
 }
 
 void CastTransportIPC::Send(IPC::Message* message) {
   if (CastIPCDispatcher::Get()) {

[miu, 2016/11/21 20:21:50]

Suggestion for simplifying: If you change this to the following, you can remove
all the checks for "channel_id_ != -1" in the dtor and methods above:

  if (CastIPCDispatcher::Get() && channel_id_ != -1) {
    ...

     CastIPCDispatcher::Get()->Send(message);
   } else {
     delete message;
   }
 }