Fix DCHECK in UDPSocket::RecvFrom().

Before this change the DCHECK in RecvFrom() may fail there is data in the
read buffer, and recvfrom() reads data instead of blocking.

BUG=None
TEST=None

Review URL: http://codereview.chromium.org/6693005

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

4 files changed, 91 insertions, 57 deletions

diff --git a/net/udp/udp_socket_libevent.cc b/net/udp/udp_socket_libevent.cc
index 0396ea9..6604e7d 100644
--- a/net/udp/udp_socket_libevent.cc
+++ b/net/udp/udp_socket_libevent.cc
@@ -125,13 +125,21 @@ int UDPSocketLibevent::GetLocalAddress(IPEndPoint* address) const {
 int UDPSocketLibevent::Read(IOBuffer* buf,
                             int buf_len,
                             CompletionCallback* callback) {
+  return RecvFrom(buf, buf_len, NULL, callback);
+}
+
+int UDPSocketLibevent::RecvFrom(IOBuffer* buf,
+                                int buf_len,
+                                IPEndPoint* address,
+                                CompletionCallback* callback) {
   DCHECK(CalledOnValidThread());
   DCHECK_NE(kInvalidSocket, socket_);
   DCHECK(!read_callback_);
+  DCHECK(!recv_from_address_);
   DCHECK(callback);  // Synchronous operation not supported
   DCHECK_GT(buf_len, 0);
 
-  int nread = InternalRead(buf, buf_len);
+  int nread = InternalRecvFrom(buf, buf_len, address);
   if (nread != ERR_IO_PENDING)
     return nread;
 
@@ -144,37 +152,35 @@ int UDPSocketLibevent::Read(IOBuffer* buf,
 
   read_buf_ = buf;
   read_buf_len_ = buf_len;
+  recv_from_address_ = address;
   read_callback_ = callback;
   return ERR_IO_PENDING;
 }
 
-int UDPSocketLibevent::RecvFrom(IOBuffer* buf,
-                                int buf_len,
-                                IPEndPoint* address,
-                                CompletionCallback* callback) {
-  DCHECK(!recv_from_address_);
-  recv_from_address_ = address;
-  return Read(buf, buf_len, callback);
+int UDPSocketLibevent::Write(IOBuffer* buf,
+                             int buf_len,
+                             CompletionCallback* callback) {
+  return SendToOrWrite(buf, buf_len, NULL, callback);
 }
 
 int UDPSocketLibevent::SendTo(IOBuffer* buf,
                               int buf_len,
                               const IPEndPoint& address,
                               CompletionCallback* callback) {
-  send_to_address_.reset(new IPEndPoint(address));
-  return Write(buf, buf_len, callback);
+  return SendToOrWrite(buf, buf_len, &address, callback);
 }
 
-int UDPSocketLibevent::Write(IOBuffer* buf,
-                             int buf_len,
-                             CompletionCallback* callback) {
+int UDPSocketLibevent::SendToOrWrite(IOBuffer* buf,
+                                     int buf_len,
+                                     const IPEndPoint* address,
+                                     CompletionCallback* callback) {
   DCHECK(CalledOnValidThread());
   DCHECK_NE(kInvalidSocket, socket_);
   DCHECK(!write_callback_);
   DCHECK(callback);  // Synchronous operation not supported
   DCHECK_GT(buf_len, 0);
 
-  int nwrite = InternalWrite(buf, buf_len);
+  int nwrite = InternalSendTo(buf, buf_len, address);
   if (nwrite >= 0) {
     static base::StatsCounter write_bytes("udp.write_bytes");
     write_bytes.Add(nwrite);
@@ -192,6 +198,10 @@ int UDPSocketLibevent::Write(IOBuffer* buf,
 
   write_buf_ = buf;
   write_buf_len_ = buf_len;
+  DCHECK(!send_to_address_.get());
+  if (address) {
+    send_to_address_.reset(new IPEndPoint(*address));
+  }
   write_callback_ = callback;
   return ERR_IO_PENDING;
 }
@@ -245,7 +255,6 @@ void UDPSocketLibevent::DoReadCallback(int rv) {
   // since Run may result in Read being called, clear read_callback_ up front.
   CompletionCallback* c = read_callback_;
   read_callback_ = NULL;
-  recv_from_address_ = NULL;
   c->Run(rv);
 }
 
@@ -256,15 +265,15 @@ void UDPSocketLibevent::DoWriteCallback(int rv) {
   // since Run may result in Write being called, clear write_callback_ up front.
   CompletionCallback* c = write_callback_;
   write_callback_ = NULL;
-  send_to_address_.reset();
   c->Run(rv);
 }
 
 void UDPSocketLibevent::DidCompleteRead() {
-  int result = InternalRead(read_buf_, read_buf_len_);
+  int result = InternalRecvFrom(read_buf_, read_buf_len_, recv_from_address_);
   if (result != ERR_IO_PENDING) {
     read_buf_ = NULL;
     read_buf_len_ = 0;
+    recv_from_address_ = NULL;
     bool ok = read_socket_watcher_.StopWatchingFileDescriptor();
     DCHECK(ok);
     DoReadCallback(result);
@@ -284,7 +293,8 @@ int UDPSocketLibevent::CreateSocket(const IPEndPoint& address) {
 }
 
 void UDPSocketLibevent::DidCompleteWrite() {
-  int result = InternalWrite(write_buf_, write_buf_len_);
+  int result = InternalSendTo(write_buf_, write_buf_len_,
+                              send_to_address_.get());
   if (result >= 0) {
     static base::StatsCounter write_bytes("udp.write_bytes");
     write_bytes.Add(result);
@@ -295,12 +305,14 @@ void UDPSocketLibevent::DidCompleteWrite() {
   if (result != ERR_IO_PENDING) {
     write_buf_ = NULL;
     write_buf_len_ = 0;
+    send_to_address_.reset();
     write_socket_watcher_.StopWatchingFileDescriptor();
     DoWriteCallback(result);
   }
 }
 
-int UDPSocketLibevent::InternalRead(IOBuffer* buf, int buf_len) {
+int UDPSocketLibevent::InternalRecvFrom(IOBuffer* buf, int buf_len,
+                                        IPEndPoint* address) {
   int bytes_transferred;
   int flags = 0;
 
@@ -320,8 +332,8 @@ int UDPSocketLibevent::InternalRead(IOBuffer* buf, int buf_len) {
     result = bytes_transferred;
     static base::StatsCounter read_bytes("udp.read_bytes");
     read_bytes.Add(bytes_transferred);
-    if (recv_from_address_) {
-      if (!recv_from_address_->FromSockAddr(addr, addr_len))
+    if (address) {
+      if (!address->FromSockAddr(addr, addr_len))
         result = ERR_FAILED;
     }
   } else {
@@ -330,16 +342,17 @@ int UDPSocketLibevent::InternalRead(IOBuffer* buf, int buf_len) {
   return result;
 }
 
-int UDPSocketLibevent::InternalWrite(IOBuffer* buf, int buf_len) {
+int UDPSocketLibevent::InternalSendTo(IOBuffer* buf, int buf_len,
+                                      const IPEndPoint* address) {
   struct sockaddr_storage addr_storage;
   size_t addr_len = sizeof(addr_storage);
   struct sockaddr* addr = reinterpret_cast<struct sockaddr*>(&addr_storage);
 
-  if (!send_to_address_.get()) {
+  if (!address) {
     addr = NULL;
     addr_len = 0;
   } else {
-    if (!send_to_address_->ToSockAddr(addr, &addr_len))
+    if (!address->ToSockAddr(addr, &addr_len))
       return ERR_FAILED;
   }
 
diff --git a/net/udp/udp_socket_libevent.h b/net/udp/udp_socket_libevent.h
index 1a9fd6e..e51dc22 100644
--- a/net/udp/udp_socket_libevent.h
+++ b/net/udp/udp_socket_libevent.h
@@ -142,8 +142,16 @@ class UDPSocketLibevent : public base::NonThreadSafe {
   // Returns the OS error code (or 0 on success).
   int CreateSocket(const IPEndPoint& address);
 
-  int InternalRead(IOBuffer* buf, int buf_len);
-  int InternalWrite(IOBuffer* buf, int buf_len);
+  // Same as SendTo(), except that address is passed by pointer
+  // instead of by reference. It is called from Write() with |address|
+  // set to NULL.
+  int SendToOrWrite(IOBuffer* buf,
+                    int buf_len,
+                    const IPEndPoint* address,
+                    CompletionCallback* callback);
+
+  int InternalRecvFrom(IOBuffer* buf, int buf_len, IPEndPoint* address);
+  int InternalSendTo(IOBuffer* buf, int buf_len, const IPEndPoint* address);
 
   int socket_;
 
diff --git a/net/udp/udp_socket_win.cc b/net/udp/udp_socket_win.cc
index a34fb0c..328f379 100644
--- a/net/udp/udp_socket_win.cc
+++ b/net/udp/udp_socket_win.cc
@@ -66,7 +66,6 @@ void UDPSocketWin::Close() {
   read_callback_ = NULL;
   recv_from_address_ = NULL;
   write_callback_ = NULL;
-  send_to_address_.reset();
 
   read_watcher_.StopWatching();
   write_watcher_.StopWatching();
@@ -119,51 +118,57 @@ int UDPSocketWin::GetLocalAddress(IPEndPoint* address) const {
   return OK;
 }
 
+int UDPSocketWin::Read(IOBuffer* buf,
+                       int buf_len,
+                       CompletionCallback* callback) {
+  return RecvFrom(buf, buf_len, NULL, callback);
+}
+
 int UDPSocketWin::RecvFrom(IOBuffer* buf,
                            int buf_len,
                            IPEndPoint* address,
                            CompletionCallback* callback) {
-  DCHECK(!recv_from_address_);
-  recv_from_address_ = address;
-  return Read(buf, buf_len, callback);
-}
-
-int UDPSocketWin::Read(IOBuffer* buf,
-                       int buf_len,
-                       CompletionCallback* callback) {
   DCHECK(CalledOnValidThread());
   DCHECK_NE(INVALID_SOCKET, socket_);
   DCHECK(!read_callback_);
+  DCHECK(!recv_from_address_);
   DCHECK(callback);  // Synchronous operation not supported.
   DCHECK_GT(buf_len, 0);
 
-  int nread = InternalRead(buf, buf_len);
+  int nread = InternalRecvFrom(buf, buf_len, address);
   if (nread != ERR_IO_PENDING)
     return nread;
 
   read_iobuffer_ = buf;
   read_callback_ = callback;
+  recv_from_address_ = address;
   return ERR_IO_PENDING;
 }
 
+int UDPSocketWin::Write(IOBuffer* buf,
+                        int buf_len,
+                        CompletionCallback* callback) {
+  return SendToOrWrite(buf, buf_len, NULL, callback);
+}
+
 int UDPSocketWin::SendTo(IOBuffer* buf,
                          int buf_len,
                          const IPEndPoint& address,
                          CompletionCallback* callback) {
-  send_to_address_.reset(new IPEndPoint(address));
-  return Write(buf, buf_len, callback);
+  return SendToOrWrite(buf, buf_len, &address, callback);
 }
 
-int UDPSocketWin::Write(IOBuffer* buf,
-                        int buf_len,
-                        CompletionCallback* callback) {
+int UDPSocketWin::SendToOrWrite(IOBuffer* buf,
+                                int buf_len,
+                                const IPEndPoint* address,
+                                CompletionCallback* callback) {
   DCHECK(CalledOnValidThread());
   DCHECK_NE(INVALID_SOCKET, socket_);
   DCHECK(!write_callback_);
   DCHECK(callback);  // Synchronous operation not supported.
   DCHECK_GT(buf_len, 0);
 
-  int nwrite = InternalWrite(buf, buf_len);
+  int nwrite = InternalSendTo(buf, buf_len, address);
   if (nwrite != ERR_IO_PENDING)
     return nwrite;
 
@@ -229,7 +234,6 @@ void UDPSocketWin::DoReadCallback(int rv) {
   // since Run may result in Read being called, clear read_callback_ up front.
   CompletionCallback* c = read_callback_;
   read_callback_ = NULL;
-  recv_from_address_ = NULL;
   c->Run(rv);
 }
 
@@ -240,7 +244,6 @@ void UDPSocketWin::DoWriteCallback(int rv) {
   // since Run may result in Write being called, clear write_callback_ up front.
   CompletionCallback* c = write_callback_;
   write_callback_ = NULL;
-  send_to_address_.reset();
   c->Run(rv);
 }
 
@@ -251,22 +254,23 @@ void UDPSocketWin::DidCompleteRead() {
   WSAResetEvent(read_overlapped_.hEvent);
   int result = ok ? num_bytes : MapSystemError(WSAGetLastError());
   if (ok) {
-    if (!ProcessSuccessfulRead(num_bytes))
+    if (!ProcessSuccessfulRead(num_bytes, recv_from_address_))
       result = ERR_FAILED;
   }
   read_iobuffer_ = NULL;
+  recv_from_address_ = NULL;
   DoReadCallback(result);
 }
 
-bool UDPSocketWin::ProcessSuccessfulRead(int num_bytes) {
+bool UDPSocketWin::ProcessSuccessfulRead(int num_bytes, IPEndPoint* address) {
   static base::StatsCounter read_bytes("udp.read_bytes");
   read_bytes.Add(num_bytes);
 
   // Convert address.
-  if (recv_from_address_) {
+  if (address) {
     struct sockaddr* addr =
         reinterpret_cast<struct sockaddr*>(&recv_addr_storage_);
-    if (!recv_from_address_->FromSockAddr(addr, recv_addr_len_))
+    if (!address->FromSockAddr(addr, recv_addr_len_))
       return false;
   }
 
@@ -290,7 +294,8 @@ void UDPSocketWin::ProcessSuccessfulWrite(int num_bytes) {
   write_bytes.Add(num_bytes);
 }
 
-int UDPSocketWin::InternalRead(IOBuffer* buf, int buf_len) {
+int UDPSocketWin::InternalRecvFrom(IOBuffer* buf, int buf_len,
+                                   IPEndPoint* address) {
   recv_addr_len_ = sizeof(recv_addr_storage_);
   struct sockaddr* addr =
       reinterpret_cast<struct sockaddr*>(&recv_addr_storage_);
@@ -313,7 +318,7 @@ int UDPSocketWin::InternalRead(IOBuffer* buf, int buf_len) {
       // false error reports.
       // See bug 5297.
       base::MemoryDebug::MarkAsInitialized(read_buffer.buf, num);
-      if (!ProcessSuccessfulRead(num))
+      if (!ProcessSuccessfulRead(num, address))
         return ERR_FAILED;
       return static_cast<int>(num);
     }
@@ -326,17 +331,18 @@ int UDPSocketWin::InternalRead(IOBuffer* buf, int buf_len) {
   return ERR_IO_PENDING;
 }
 
-int UDPSocketWin::InternalWrite(IOBuffer* buf, int buf_len) {
+int UDPSocketWin::InternalSendTo(IOBuffer* buf, int buf_len,
+                                 const IPEndPoint* address) {
   struct sockaddr_storage addr_storage;
   size_t addr_len = sizeof(addr_storage);
   struct sockaddr* addr = reinterpret_cast<struct sockaddr*>(&addr_storage);
 
   // Convert address.
-  if (!send_to_address_.get()) {
+  if (!address) {
     addr = NULL;
     addr_len = 0;
   } else {
-    if (!send_to_address_->ToSockAddr(addr, &addr_len))
+    if (!address->ToSockAddr(addr, &addr_len))
       return ERR_FAILED;
   }
 
diff --git a/net/udp/udp_socket_win.h b/net/udp/udp_socket_win.h
index d9e1dee..fde423e 100644
--- a/net/udp/udp_socket_win.h
+++ b/net/udp/udp_socket_win.h
@@ -124,14 +124,22 @@ class UDPSocketWin : public base::NonThreadSafe {
   void DoWriteCallback(int rv);
   void DidCompleteRead();
   void DidCompleteWrite();
-  bool ProcessSuccessfulRead(int num_bytes);
+  bool ProcessSuccessfulRead(int num_bytes, IPEndPoint* address);
   void ProcessSuccessfulWrite(int num_bytes);
 
   // Returns the OS error code (or 0 on success).
   int CreateSocket(const IPEndPoint& address);
 
-  int InternalRead(IOBuffer* buf, int buf_len);
-  int InternalWrite(IOBuffer* buf, int buf_len);
+  // Same as SendTo(), except that address is passed by pointer
+  // instead of by reference. It is called from Write() with |address|
+  // set to NULL.
+  int SendToOrWrite(IOBuffer* buf,
+                    int buf_len,
+                    const IPEndPoint* address,
+                    CompletionCallback* callback);
+
+  int InternalRecvFrom(IOBuffer* buf, int buf_len, IPEndPoint* address);
+  int InternalSendTo(IOBuffer* buf, int buf_len, const IPEndPoint* address);
 
   SOCKET socket_;
 
@@ -160,7 +168,6 @@ class UDPSocketWin : public base::NonThreadSafe {
 
   // The buffer used by InternalWrite() to retry Write requests
   scoped_refptr<IOBuffer> write_iobuffer_;
-  scoped_ptr<IPEndPoint> send_to_address_;
 
   // External callback; called when read is complete.
   CompletionCallback* read_callback_;