Cast: UDP socket operation should be more resilient

UDP socket has transient errors. In cast if we have an unsuccessful
read followed by an unsuccessful write then socket reading will stop.
This can cause the stream to be stopped until there is a successful
send again.

We should ignore any UDP socket errors and keep going.

Also changed the code location such that we start receiving
packets when CastTransportSender is created.

Testing

This change interacts directly with system UDP sockets so
it is not mockable. Instead I tested this locally with
streaming went smoothly.

BUG=396138

Review URL: https://codereview.chromium.org/409163003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286646 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 11 insertions, 35 deletions

diff --git a/media/cast/net/cast_transport_sender_impl.cc b/media/cast/net/cast_transport_sender_impl.cc
index a13be94..374b990 100644
--- a/media/cast/net/cast_transport_sender_impl.cc
+++ b/media/cast/net/cast_transport_sender_impl.cc
@@ -76,6 +76,9 @@ CastTransportSenderImpl::CastTransportSenderImpl(
     // The default DSCP value for cast is AF41. Which gives it a higher
     // priority over other traffic.
     transport_->SetDscp(net::DSCP_AF41);
+    transport_->StartReceiving(
+        base::Bind(&CastTransportSenderImpl::OnReceivedPacket,
+                   weak_factory_.GetWeakPtr()));
   }
 }
 
@@ -118,10 +121,6 @@ void CastTransportSenderImpl::InitializeAudio(
                config.feedback_ssrc,
                config.c_name));
   pacer_.RegisterAudioSsrc(config.ssrc);
-
-  // Only start receiving once.
-  if (!video_sender_)
-    StartReceiving();
   status_callback_.Run(TRANSPORT_AUDIO_INITIALIZED);
 }
 
@@ -154,10 +153,6 @@ void CastTransportSenderImpl::InitializeVideo(
                config.feedback_ssrc,
                config.c_name));
   pacer_.RegisterVideoSsrc(config.ssrc);
-
-  // Only start receiving once.
-  if (!audio_sender_)
-    StartReceiving();
   status_callback_.Run(TRANSPORT_VIDEO_INITIALIZED);
 }
 
@@ -248,14 +243,6 @@ void CastTransportSenderImpl::SendRawEvents() {
       raw_events_callback_interval_);
 }
 
-void CastTransportSenderImpl::StartReceiving() {
-  if (!transport_)
-    return;
-  transport_->StartReceiving(
-      base::Bind(&CastTransportSenderImpl::OnReceivedPacket,
-                 weak_factory_.GetWeakPtr()));
-}
-
 void CastTransportSenderImpl::OnReceivedPacket(scoped_ptr<Packet> packet) {
   if (audio_rtcp_session_ &&
       audio_rtcp_session_->IncomingRtcpPacket(&packet->front(),
diff --git a/media/cast/net/cast_transport_sender_impl.h b/media/cast/net/cast_transport_sender_impl.h
index 9cc3b46..cee8452 100644
--- a/media/cast/net/cast_transport_sender_impl.h
+++ b/media/cast/net/cast_transport_sender_impl.h
@@ -92,9 +92,6 @@ class CastTransportSenderImpl : public CastTransportSender {
   // by |event_subscriber_| since last call.
   void SendRawEvents();
 
-  // Start receiving packets.
-  void StartReceiving();
-
   // Called when a packet is received.
   void OnReceivedPacket(scoped_ptr<Packet> packet);
 
diff --git a/media/cast/net/udp_transport.cc b/media/cast/net/udp_transport.cc
index ae0593d..64bcb8f 100644
--- a/media/cast/net/udp_transport.cc
+++ b/media/cast/net/udp_transport.cc
@@ -131,7 +131,6 @@ void UdpTransport::ReceiveNextPacket(int length_or_status) {
     if (length_or_status < 0) {
       VLOG(1) << "Failed to receive packet: Status code is "
               << length_or_status;
-      status_callback_.Run(TRANSPORT_SOCKET_ERROR);
       receive_pending_ = false;
       return;
     }
@@ -170,8 +169,8 @@ bool UdpTransport::SendPacket(PacketRef packet, const base::Closure& cb) {
   if (next_dscp_value_ != net::DSCP_NO_CHANGE) {
     int result = udp_socket_->SetDiffServCodePoint(next_dscp_value_);
     if (result != net::OK) {
-      LOG(ERROR) << "Unable to set DSCP: " << next_dscp_value_
-                 << " to socket; Error: " << result;
+      VLOG(1) << "Unable to set DSCP: " << next_dscp_value_
+              << " to socket; Error: " << result;
     }
     // Don't change DSCP in next send.
     next_dscp_value_ = net::DSCP_NO_CHANGE;
@@ -199,21 +198,17 @@ bool UdpTransport::SendPacket(PacketRef packet, const base::Closure& cb) {
                                  remote_addr_,
                                  callback);
   } else {
+    VLOG(1) << "Failed to send packet; socket is neither bound nor "
+            << "connected.";
     return true;
   }
 
   if (result == net::ERR_IO_PENDING) {
     send_pending_ = true;
     return false;
-  } else if (result < 0) {
-    LOG(ERROR) << "Failed to send packet: " << result << ".";
-    status_callback_.Run(TRANSPORT_SOCKET_ERROR);
-    return true;
-  } else {
-    // Successful send, re-start reading if needed.
-    ScheduleReceiveNextPacket();
-    return true;
   }
+  OnSent(buf, packet, base::Closure(), result);
+  return true;
 }
 
 void UdpTransport::OnSent(const scoped_refptr<net::IOBuffer>& buf,
@@ -224,12 +219,9 @@ void UdpTransport::OnSent(const scoped_refptr<net::IOBuffer>& buf,
 
   send_pending_ = false;
   if (result < 0) {
-    LOG(ERROR) << "Failed to send packet: " << result << ".";
-    status_callback_.Run(TRANSPORT_SOCKET_ERROR);
-  } else {
-    // Successful send, re-start reading if needed.
-    ScheduleReceiveNextPacket();
+    VLOG(1) << "Failed to send packet: " << result << ".";
   }
+  ScheduleReceiveNextPacket();
 
   if (!cb.is_null()) {
     cb.Run();