Android device manager should not drop WebSocket frame data even if it arrives together with handsh…

chrome/browser/devtools/device/android_device_manager.cc

 // Copyright 2014 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 "chrome/browser/devtools/device/android_device_manager.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "net/base/io_buffer.h"
@@ skipping 34 matching lines @@
     const std::string& response) {
   response_message_loop->PostTask(FROM_HERE,
                                   base::Bind(callback, result, response));
 }
 
 static void PostHttpUpgradeCallback(
     scoped_refptr<base::MessageLoopProxy> response_message_loop,
     const AndroidDeviceManager::HttpUpgradeCallback& callback,
     int result,
     const std::string& extensions,
+    const std::string& body_head,
     scoped_ptr<net::StreamSocket> socket) {
   response_message_loop->PostTask(
       FROM_HERE,
-      base::Bind(callback, result, extensions, base::Passed(&socket)));
+      base::Bind(callback, result, extensions, body_head,
+                 base::Passed(&socket)));
 }
 
 class HttpRequest {
  public:
   typedef AndroidDeviceManager::CommandCallback CommandCallback;
   typedef AndroidDeviceManager::HttpUpgradeCallback HttpUpgradeCallback;
 
   static void CommandRequest(const std::string& request,
                              const CommandCallback& callback,
                              int result,
                              scoped_ptr<net::StreamSocket> socket) {
     if (result != net::OK) {
       callback.Run(result, std::string());
       return;
     }
     new HttpRequest(socket.Pass(), request, callback);
   }
 
   static void HttpUpgradeRequest(const std::string& request,
                                  const HttpUpgradeCallback& callback,
                                  int result,
                                  scoped_ptr<net::StreamSocket> socket) {
     if (result != net::OK) {
-      callback.Run(result, "", make_scoped_ptr<net::StreamSocket>(nullptr));
+      callback.Run(
+          result, "", "", make_scoped_ptr<net::StreamSocket>(nullptr));
       return;
     }
     new HttpRequest(socket.Pass(), request, callback);
   }
 
  private:
   HttpRequest(scoped_ptr<net::StreamSocket> socket,
               const std::string& request,
               const CommandCallback& callback)
       : socket_(socket.Pass()),
         command_callback_(callback),
-        body_pos_(0) {
+        response_total_size_(-1),
+        response_header_size_(0) {
     SendRequest(request);
   }
 
   HttpRequest(scoped_ptr<net::StreamSocket> socket,
               const std::string& request,
               const HttpUpgradeCallback& callback)
     : socket_(socket.Pass()),
       http_upgrade_callback_(callback),
-      body_pos_(0) {
+      response_total_size_(-1),
+      response_header_size_(0) {
     SendRequest(request);
   }
 
   ~HttpRequest() {
   }
 
   void SendRequest(const std::string& request) {
     scoped_refptr<net::StringIOBuffer> request_buffer =
         new net::StringIOBuffer(request);
 
     int result = socket_->Write(
         request_buffer.get(),
         request_buffer->size(),
         base::Bind(&HttpRequest::ReadResponse, base::Unretained(this)));
     if (result != net::ERR_IO_PENDING)
       ReadResponse(result);
   }
 
   void ReadResponse(int result) {
     if (!CheckNetResultOrDie(result))
       return;
-    scoped_refptr<net::IOBuffer> response_buffer =
-        new net::IOBuffer(kBufferSize);
+
+    response_buffer_ = new net::IOBuffer(kBufferSize);
 
     result = socket_->Read(
-        response_buffer.get(),
+        response_buffer_.get(),
         kBufferSize,
-        base::Bind(&HttpRequest::OnResponseData, base::Unretained(this),
-                  response_buffer,
-                  -1));
+        base::Bind(&HttpRequest::OnResponseData, base::Unretained(this)));
     if (result != net::ERR_IO_PENDING)
-      OnResponseData(response_buffer, -1, result);
+      OnResponseData(result);
   }
 
-  void OnResponseData(scoped_refptr<net::IOBuffer> response_buffer,
-                      int bytes_total,
-                      int result) {
+  void OnResponseData(int result) {
     if (!CheckNetResultOrDie(result))
       return;
     if (result == 0) {
       CheckNetResultOrDie(net::ERR_CONNECTION_CLOSED);
       return;
     }
 
-    response_ += std::string(response_buffer->data(), result);
-    int expected_length = 0;
-    if (bytes_total < 0) {
+    response_.append(response_buffer_->data(), result);
+    if (response_total_size_ < 0) {
+      int expected_length = 0;
+
       // TODO(kaznacheev): Use net::HttpResponseHeader to parse the header.
       std::string content_length = ExtractHeader("Content-Length:");
       if (!content_length.empty()) {
         if (!base::StringToInt(content_length, &expected_length)) {
           CheckNetResultOrDie(net::ERR_FAILED);
           return;
         }
       }
 
-      body_pos_ = response_.find("\r\n\r\n");
-      if (body_pos_ != std::string::npos) {
-        body_pos_ += 4;
-        bytes_total = body_pos_ + expected_length;
+      response_header_size_ = response_.find("\r\n\r\n");
+      if (response_header_size_ != std::string::npos) {
+        response_header_size_ += 4;
+        response_total_size_ = response_header_size_ + expected_length;
       }
     }
 
-    if (bytes_total == static_cast<int>(response_.length())) {
+    // WebSocket handshake doesn't contain the Content-Length. For this case,
+    // |response_total_size_| is set to the size of the header (handshake).
+    // Some WebSocket frames can be already received into |response_|.
+    if (static_cast<int>(response_.length()) >= response_total_size_) {
+      const std::string& body = response_.substr(response_header_size_);
       if (!command_callback_.is_null()) {
-        command_callback_.Run(net::OK, response_.substr(body_pos_));
+        command_callback_.Run(net::OK, body);
       } else {
+        // Pass the WebSocket frames (in |body|), too.
         http_upgrade_callback_.Run(net::OK,
-            ExtractHeader("Sec-WebSocket-Extensions:"), socket_.Pass());
+            ExtractHeader("Sec-WebSocket-Extensions:"), body, socket_.Pass());
       }
       delete this;
       return;
     }
 
     result = socket_->Read(
-        response_buffer.get(),
+        response_buffer_.get(),
         kBufferSize,
-        base::Bind(&HttpRequest::OnResponseData,
-                   base::Unretained(this),
-                   response_buffer,
-                   bytes_total));
+        base::Bind(&HttpRequest::OnResponseData, base::Unretained(this)));
     if (result != net::ERR_IO_PENDING)
-      OnResponseData(response_buffer, bytes_total, result);
+      OnResponseData(result);
   }
 
   std::string ExtractHeader(const std::string& header) {
     size_t start_pos = response_.find(header);
     if (start_pos == std::string::npos)
       return std::string();
 
     size_t endline_pos = response_.find("\n", start_pos);
     if (endline_pos == std::string::npos)
       return std::string();
 
     std::string value = response_.substr(
         start_pos + header.length(), endline_pos - start_pos - header.length());
     base::TrimWhitespace(value, base::TRIM_ALL, &value);
     return value;
   }
 
   bool CheckNetResultOrDie(int result) {
     if (result >= 0)
       return true;
     if (!command_callback_.is_null()) {
       command_callback_.Run(result, std::string());
     } else {
       http_upgrade_callback_.Run(
-          result, "", make_scoped_ptr<net::StreamSocket>(nullptr));
+          result, "", "", make_scoped_ptr<net::StreamSocket>(nullptr));

[dgozman, 2015/04/09 14:05:22]

nit: "" -> std::string()

[tyoshino (SeeGerritForStatus), 2015/04/14 12:24:32]

Done.

     }
     delete this;
     return false;
   }
 
   scoped_ptr<net::StreamSocket> socket_;
   std::string response_;
   CommandCallback command_callback_;
   HttpUpgradeCallback http_upgrade_callback_;
-  size_t body_pos_;
+
+  scoped_refptr<net::IOBuffer> response_buffer_;
+  // Initially -1. Once the end of the header is seen:
+  // - If the Content-Length header is included, this variable is set to the
+  //   sum of the header size (including the last two CRLFs) and the value of
+  //   the header.
+  // - Otherwise, this variable is set to the size of the header (including the
+  //   last two CRLFs).
+  int response_total_size_;

[dgozman, 2015/04/09 14:05:22]

Consider renaming to |expected_size_| to emphasize that we wait for this number
of bytes, not that it's exactly the response size.

[tyoshino (SeeGerritForStatus), 2015/04/14 12:24:32]

Done.

+  // Set to the size of the header part in |response_|.
+  size_t response_header_size_;

[dgozman, 2015/04/09 14:05:22]

I'd just call this |header_size_|, since this class does nothing except for
response handling.

[tyoshino (SeeGerritForStatus), 2015/04/14 12:24:32]

Done.

 };
 
 class DevicesRequest : public base::RefCountedThreadSafe<DevicesRequest> {
  public:
   typedef AndroidDeviceManager::DeviceInfo DeviceInfo;
   typedef AndroidDeviceManager::DeviceProvider DeviceProvider;
   typedef AndroidDeviceManager::DeviceProviders DeviceProviders;
   typedef AndroidDeviceManager::DeviceDescriptors DeviceDescriptors;
   typedef base::Callback<void(scoped_ptr<DeviceDescriptors>)>
       DescriptorsCallback;
@@ skipping 291 matching lines @@
           it->provider, it->serial);
     } else {
       device = found->second.get();
     }
     response.push_back(device);
     new_devices[it->serial] = device->weak_factory_.GetWeakPtr();
   }
   devices_.swap(new_devices);
   callback.Run(response);
 }