Avoid error messages from UDP sockets in Chromoting client.

remoting/client/plugin/pepper_packet_socket_factory.cc

 // Copyright (c) 2012 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 "remoting/client/plugin/pepper_packet_socket_factory.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "net/base/io_buffer.h"
 #include "ppapi/cpp/net_address.h"
 #include "ppapi/cpp/udp_socket.h"
 #include "ppapi/utility/completion_callback_factory.h"
 #include "remoting/client/plugin/pepper_util.h"
 #include "third_party/libjingle/source/talk/base/asyncpacketsocket.h"
 
 namespace remoting {
 
 namespace {
 
 // Size of the buffer to allocate for RecvFrom().
 const int kReceiveBufferSize = 65536;
 
 // Maximum amount of data in the send buffers. This is necessary to
 // prevent out-of-memory crashes if the caller sends data faster than
 // Pepper's UDP API can handle it. This maximum should never be
 // reached under normal conditions.
 const int kMaxSendBufferSize = 256 * 1024;
 
+// Returns true if |error| must be ignored when returned from sendto().
+bool IsTransientError(int error) {

[Wez, 2014/06/16 22:36:36]

Are all of these errors really transient per-packet? Are none of them per-socket
transient, or even permanent (e.g. PP_ERROR_NOACCESS)?

[Sergey Ulanov, 2014/06/17 00:53:35]

This list is based on what chrome does for sockets in WebRTC (see
content/browser/renderer_host/p2p/socket_host_udp.cc).
PP_ERROR_NOACCESS can returned when the packet was blocked by the local firewall
(on OSX), see https://code.google.com/p/webrtc/issues/detail?id=1207.

+  return error == PP_ERROR_ADDRESS_UNREACHABLE ||
+         error == PP_ERROR_ADDRESS_INVALID || error == PP_ERROR_NOACCESS ||
+         error == PP_ERROR_CONNECTION_RESET || error == PP_ERROR_NOMEMORY;

[Wez, 2014/06/16 22:36:36]

I think it'd be good to clarify why this particular errors can/should/must be
ignored the first time we see them.

[Sergey Ulanov, 2014/06/17 00:53:35]

Done. Also added |retry| flag. It makes sense to retry sending only for some
errors, but not the others.

+}
+
 class UdpPacketSocket : public talk_base::AsyncPacketSocket {
  public:
   explicit UdpPacketSocket(const pp::InstanceHandle& instance);
   virtual ~UdpPacketSocket();
 
   // |min_port| and |max_port| are set to zero if the port number
   // should be assigned by the OS.
   bool Init(const talk_base::SocketAddress& local_address,
             int min_port,
             int max_port);
@@ skipping 15 matching lines @@
   virtual void SetError(int error) OVERRIDE;
 
  private:
   struct PendingPacket {
     PendingPacket(const void* buffer,
                   int buffer_size,
                   const pp::NetAddress& address);
 
     scoped_refptr<net::IOBufferWithSize> data;
     pp::NetAddress address;
+    bool retried;
   };
 
   void OnBindCompleted(int error);
 
   void DoSend();
   void OnSendCompleted(int result);
 
   void DoRead();
   void OnReadCompleted(int result, pp::NetAddress address);
   void HandleReadResult(int result, pp::NetAddress address);
@@ skipping 21 matching lines @@
   pp::CompletionCallbackFactory<UdpPacketSocket> callback_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UdpPacketSocket);
 };
 
 UdpPacketSocket::PendingPacket::PendingPacket(
     const void* buffer,
     int buffer_size,
     const pp::NetAddress& address)
     : data(new net::IOBufferWithSize(buffer_size)),
-      address(address) {
+      address(address),
+      retried(true) {
   memcpy(data->data(), buffer, buffer_size);
 }
 
 UdpPacketSocket::UdpPacketSocket(const pp::InstanceHandle& instance)
     : instance_(instance),
       socket_(instance),
       state_(STATE_CLOSED),
       error_(0),
       min_port_(0),
       max_port_(0),
@@ skipping 54 matching lines @@
     ++min_port_;
     pp::NetAddress pp_local_address;
     if (SocketAddressToPpNetAddressWithPort(
             instance_, local_address_, &pp_local_address, min_port_)) {
       pp::CompletionCallback callback =
           callback_factory_.NewCallback(&UdpPacketSocket::OnBindCompleted);
       int result = socket_.Bind(pp_local_address, callback);
       DCHECK_EQ(result, PP_OK_COMPLETIONPENDING);
     }
   } else {
-    LOG(ERROR) << "Failed to bind UDP socket: " << result;
+    LOG(ERROR) << "Failed to bind UDP socket to " << local_address_.ToString()
+               << ", error: " << result;
   }
 }
 
 talk_base::SocketAddress UdpPacketSocket::GetLocalAddress() const {
   DCHECK_EQ(state_, STATE_BOUND);
   return local_address_;
 }
 
 talk_base::SocketAddress UdpPacketSocket::GetRemoteAddress() const {
   // UDP sockets are not connected - this method should never be called.
@@ skipping 83 matching lines @@
   if (result == PP_ERROR_ABORTED) {
     // Send is aborted when the socket is being destroyed.
     // |send_queue_| may be already destroyed, it's not safe to access
     // it here.
     return;
   }
 
   send_pending_ = false;
 
   if (result < 0) {
-    LOG(ERROR) << "Send failed on a UDP socket: " << result;
+    if (!IsTransientError(result)) {
+      LOG(ERROR) << "Send failed on a UDP socket: " << result;
+      return;
+    }
 
-    // OS (e.g. OSX) may return EHOSTUNREACH when the peer has the
-    // same subnet address as the local host but connected to a
-    // different network. That error must be ingored because the
-    // socket may still be useful for other ICE canidadates (e.g. for
-    // STUN candidates with a different address). Unfortunately pepper
-    // interface currently returns PP_ERROR_FAILED for any error (see
-    // crbug.com/136406). It's not possible to distinguish that case
-    // from other errors and so we have to ingore all of them. This
-    // behavior matchers the libjingle's AsyncUDPSocket used by the
-    // host.
-    //
-    // TODO(sergeyu): Once implementation of the Pepper UDP interface
-    // is fixed, uncomment the code below, but ignore
-    // host-unreacheable error.
-
-    // error_ = EINVAL;
-    // return;
+    // If this is a transient error and we haven't tried resending the packet
+    // yet, then try resending it.
+    if (!send_queue_.front().retried) {
+      send_queue_.front().retried = true;
+      DoSend();
+      return;
+    }
   }
 
   send_queue_size_ -= send_queue_.front().data->size();
   send_queue_.pop_front();
   DoSend();
 }
 
 void UdpPacketSocket::DoRead() {
   receive_buffer_.resize(kReceiveBufferSize);
   pp::CompletionCallbackWithOutput<pp::NetAddress> callback =
@@ skipping 63 matching lines @@
   return NULL;
 }
 
 talk_base::AsyncResolverInterface*
 PepperPacketSocketFactory::CreateAsyncResolver() {
   NOTREACHED();
   return NULL;
 }
 
 }  // namespace remoting