Fixing SecurityKeySocket unit test issues and cleanup

remoting/host/security_key/security_key_socket.cc

 // Copyright 2016 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/host/security_key/security_key_socket.h"
 
 #include <utility>
 
 #include "base/callback_helpers.h"
 #include "base/macros.h"
@@ skipping 24 matching lines @@
   timer_.reset(new base::Timer(false, false));
   timer_->Start(FROM_HERE, timeout, timeout_callback);
 }
 
 SecurityKeySocket::~SecurityKeySocket() {}
 
 bool SecurityKeySocket::GetAndClearRequestData(std::string* data_out) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(read_completed_);
 
-  if (!read_completed_)
+  if (!read_completed_) {
     return false;
-  if (!IsRequestComplete() || IsRequestTooLarge())
+  }
+  if (!IsRequestComplete() || IsRequestTooLarge()) {
     return false;
+  }
   // The request size is not part of the data; don't send it.
   data_out->assign(request_data_.begin() + kRequestSizeBytes,
                    request_data_.end());
   request_data_.clear();
   return true;
 }
 
 void SecurityKeySocket::SendResponse(const std::string& response_data) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!write_buffer_);
 
   std::string response_length_string = GetResponseLengthAsBytes(response_data);
   int response_len = response_length_string.size() + response_data.size();
   std::unique_ptr<std::string> response(
       new std::string(response_length_string + response_data));
   write_buffer_ = new net::DrainableIOBuffer(
       new net::StringIOBuffer(std::move(response)), response_len);
+
+  DCHECK(write_buffer_->BytesRemaining());
   DoWrite();
 }
 
 void SecurityKeySocket::SendSshError() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   SendResponse(std::string(kSshError, arraysize(kSshError)));
 }
 
 void SecurityKeySocket::StartReadingRequest(
     const base::Closure& request_received_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(request_received_callback_.is_null());
 
   request_received_callback_ = request_received_callback;
   DoRead();
 }
 
 void SecurityKeySocket::OnDataWritten(int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(write_buffer_);
 
   if (result < 0) {
     LOG(ERROR) << "Error sending response: " << result;
     return;
   }
   ResetTimer();
   write_buffer_->DidConsume(result);
+
+  if (!write_buffer_->BytesRemaining()) {
+    write_buffer_ = nullptr;
+    return;
+  }
+
   DoWrite();
 }
 
 void SecurityKeySocket::DoWrite() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(write_buffer_);
 
-  if (!write_buffer_->BytesRemaining()) {
-    write_buffer_ = nullptr;
-    return;
-  }
   int result = socket_->Write(
       write_buffer_.get(), write_buffer_->BytesRemaining(),
       base::Bind(&SecurityKeySocket::OnDataWritten, base::Unretained(this)));
-  if (result != net::ERR_IO_PENDING)
+  if (result != net::ERR_IO_PENDING) {
     OnDataWritten(result);
+  }
 }
 
 void SecurityKeySocket::OnDataRead(int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (result <= 0) {
-    if (result < 0)
+    if (result < 0) {
       LOG(ERROR) << "Error reading request: " << result;
+    }
     read_completed_ = true;
     base::ResetAndReturn(&request_received_callback_).Run();
     return;
   }
 
   ResetTimer();
   request_data_.insert(request_data_.end(), read_buffer_->data(),
                        read_buffer_->data() + result);
   if (IsRequestComplete()) {
     read_completed_ = true;
     base::ResetAndReturn(&request_received_callback_).Run();
     return;
   }
 
   DoRead();
 }
 
 void SecurityKeySocket::DoRead() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   int result = socket_->Read(
       read_buffer_.get(), kRequestReadBufferLength,
       base::Bind(&SecurityKeySocket::OnDataRead, base::Unretained(this)));
-  if (result != net::ERR_IO_PENDING)
+  if (result != net::ERR_IO_PENDING) {
     OnDataRead(result);
+  }
 }
 
 bool SecurityKeySocket::IsRequestComplete() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (request_data_.size() < kRequestSizeBytes)
+  if (request_data_.size() < kRequestSizeBytes) {
     return false;
+  }
   return GetRequestLength() <= request_data_.size();
 }
 
 bool SecurityKeySocket::IsRequestTooLarge() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (request_data_.size() < kRequestSizeBytes)
+  if (request_data_.size() < kRequestSizeBytes) {
     return false;
+  }
   return GetRequestLength() > kMaxRequestLength;
 }
 
 size_t SecurityKeySocket::GetRequestLength() const {
   DCHECK(request_data_.size() >= kRequestSizeBytes);
 
   return ((request_data_[0] & 255) << 24) + ((request_data_[1] & 255) << 16) +
          ((request_data_[2] & 255) << 8) + (request_data_[3] & 255) +
          kRequestSizeBytes;
 }
 
 std::string SecurityKeySocket::GetResponseLengthAsBytes(
     const std::string& response) const {
   std::string response_len;
   response_len.reserve(kRequestSizeBytes);
   int len = response.size();
 
   response_len.push_back((len >> 24) & 255);
   response_len.push_back((len >> 16) & 255);
   response_len.push_back((len >> 8) & 255);
   response_len.push_back(len & 255);
 
   return response_len;
 }
 
 void SecurityKeySocket::ResetTimer() {
-  if (timer_->IsRunning())
+  if (timer_->IsRunning()) {
     timer_->Reset();
+  }
 }
 
 }  // namespace remoting