Updating SecurityKeyAuthHandlerPosix socket lifetime management

remoting/host/security_key/security_key_socket.cc

Index: remoting/host/security_key/security_key_socket.cc
diff --git a/remoting/host/security_key/security_key_socket.cc b/remoting/host/security_key/security_key_socket.cc
index 88239ad4008d4bdd0d812dab8c0cd9eb4a70d6b6..569e53cf12e4d9430e7fdfd8134d936ef5a6613e 100644
--- a/remoting/host/security_key/security_key_socket.cc
+++ b/remoting/host/security_key/security_key_socket.cc
@@ -30,7 +30,6 @@ SecurityKeySocket::SecurityKeySocket(std::unique_ptr<net::StreamSocket> socket,
                                      base::TimeDelta timeout,
                                      const base::Closure& timeout_callback)
     : socket_(std::move(socket)),
-      read_completed_(false),
       read_buffer_(new net::IOBufferWithSize(kRequestReadBufferLength)) {
   timer_.reset(new base::Timer(false, false));
   timer_->Start(FROM_HERE, timeout, timeout_callback);
@@ -40,11 +39,8 @@ SecurityKeySocket::~SecurityKeySocket() {}
 
 bool SecurityKeySocket::GetAndClearRequestData(std::string* data_out) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(read_completed_);
+  DCHECK(!waiting_for_request_);
 
-  if (!read_completed_) {
-    return false;
-  }
   if (!IsRequestComplete() || IsRequestTooLarge()) {
     return false;
   }
@@ -81,7 +77,9 @@ void SecurityKeySocket::StartReadingRequest(
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(request_received_callback_.is_null());
 
+  waiting_for_request_ = true;
   request_received_callback_ = request_received_callback;
+
   DoRead();
 }
 
@@ -122,17 +120,22 @@ void SecurityKeySocket::OnDataRead(int result) {
   if (result <= 0) {
     if (result < 0) {
       LOG(ERROR) << "Error reading request: " << result;
+      socket_read_error_ = true;
     }
-    read_completed_ = true;
+    waiting_for_request_ = false;
     base::ResetAndReturn(&request_received_callback_).Run();
     return;
   }
 
   ResetTimer();
+  // TODO(joedow): If there are multiple requests in a burst, it is possible
+  // that we could read too many bytes from the buffer (e.g. all of request #1
+  // and some of request #2).  We should consider using the request header to
+  // determine the request length and only read that amount from buffer.

[Sergey Ulanov, 2016/12/20 00:54:23]

I think it would be easier to keep the current reading logic and update the code
that handles compelte requests: when extra bytes are read they need to be kept
in the buffer and handled later.

[joedow, 2016/12/20 02:24:03]

That's reasonable.  I'll work on that tomorrow and get a CL out for it.

   request_data_.insert(request_data_.end(), read_buffer_->data(),
                        read_buffer_->data() + result);
   if (IsRequestComplete()) {
-    read_completed_ = true;
+    waiting_for_request_ = false;
     base::ResetAndReturn(&request_received_callback_).Run();
     return;
   }