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(). |retry|
+// is set when sendto() should be called for the same packet again.
+bool IsTransientError(int error, bool* retry) {

[Wez, 2014/06/17 01:13:26]

Adding a "retry" member to a function called IsTransientError ends up being
confusing because it makes the caller wonder why would you ever _not_ retry if
the error is transient - surely it might go away?

The point here is that some errors should be treated as packet drops, and others
are likely one-offs that should trigger a retry - the rest are real errors.

[Wez, 2014/06/17 01:13:26]

This would be cleaner as a GetErrorAction(int error) that returns a
KErrorActionFail, kErrorActionIgnore, kErrorActionRetry enum

[Sergey Ulanov, 2014/06/17 02:12:02]

Done.

[Sergey Ulanov, 2014/06/17 02:12:03]

Done.

+  // Following error codes are returned when one of the previous datagrams was
+  // sent to an unreachable address.

[Wez, 2014/06/17 01:13:26]

What's the point of retrying if we tried to send to an unreachable address?
What's the likelihood that it will suddenly become reachable within the retry
interval?

[Sergey Ulanov, 2014/06/17 02:12:03]

PP_ERROR_ADDRESS_UNREACHABLE and PP_ERROR_CONNECTION_RESET are returned in
response to an ICMP packets from a remote host that failed to deliver a message
we sent before. They indicate that at least one packet that was sent from this
socket in the past was sent to an unreachable address. That doesn't mean that
the current packet couldn't be sent.

Actually, now that I looked at this list again it doesn't make sense to try
resending after PP_ERROR_ADDRESS_INVALID, fixed now.

+  if (error == PP_ERROR_ADDRESS_UNREACHABLE ||
+      error == PP_ERROR_ADDRESS_INVALID ||
+      error == PP_ERROR_CONNECTION_RESET) {
+    *retry = true;
+    return true;
+  }
+
+  // PP_ERROR_NOACCESS may be returned when the packet is blocked by local
+  // firewall (see https://code.google.com/p/webrtc/issues/detail?id=1207). The
+  // error is transient because the firewall may block only some target
+  // addresses.
+  //
+  // PP_ERROR_NOMEMORY indicates that the buffer in the network adapter is full.
+  if (error == PP_ERROR_NOACCESS || error == PP_ERROR_NOMEMORY) {
+    *retry = false;
+    return true;
+  }
+
+  return false;
+}
+
 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;
+    bool retry;
+    if (!IsTransientError(result, &retry)) {
+      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 (retry && !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