Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(491)

Unified Diff: remoting/client/plugin/pepper_packet_socket_factory.cc

Issue 336113002: Avoid error messages from UDP sockets in Chromoting client. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: remoting/client/plugin/pepper_packet_socket_factory.cc
diff --git a/remoting/client/plugin/pepper_packet_socket_factory.cc b/remoting/client/plugin/pepper_packet_socket_factory.cc
index 90d7c61bd94cdb5b345ec0cf456be00776c1d66b..1c6cd432a0c08a92c8ad2cf67526cc1b44094ae4 100644
--- a/remoting/client/plugin/pepper_packet_socket_factory.cc
+++ b/remoting/client/plugin/pepper_packet_socket_factory.cc
@@ -26,6 +26,13 @@ const int kReceiveBufferSize = 65536;
// 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-packe
Sergey Ulanov 2014/06/17 00:53:35 This list is based on what chrome does for sockets
+ 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 particula
Sergey Ulanov 2014/06/17 00:53:35 Done. Also added |retry| flag. It makes sense to r
+}
+
class UdpPacketSocket : public talk_base::AsyncPacketSocket {
public:
explicit UdpPacketSocket(const pp::InstanceHandle& instance);
@@ -61,6 +68,7 @@ class UdpPacketSocket : public talk_base::AsyncPacketSocket {
scoped_refptr<net::IOBufferWithSize> data;
pp::NetAddress address;
+ bool retried;
};
void OnBindCompleted(int error);
@@ -102,7 +110,8 @@ UdpPacketSocket::PendingPacket::PendingPacket(
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);
}
@@ -177,7 +186,8 @@ void UdpPacketSocket::OnBindCompleted(int result) {
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;
}
}
@@ -281,25 +291,18 @@ void UdpPacketSocket::OnSendCompleted(int result) {
send_pending_ = false;
if (result < 0) {
- LOG(ERROR) << "Send failed on a UDP socket: " << result;
-
- // 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 (!IsTransientError(result)) {
+ LOG(ERROR) << "Send failed on a UDP socket: " << result;
+ 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();

Powered by Google App Engine
This is Rietveld 408576698