Add client for background testing of HTTP pipelining.

chrome/browser/net/http_pipelining_compatibility_client.cc

Index: chrome/browser/net/http_pipelining_compatibility_client.cc
diff --git a/chrome/browser/net/http_pipelining_compatibility_client.cc b/chrome/browser/net/http_pipelining_compatibility_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..49a553c4aa8bf6b3cf719a6dd188ef66daa1e61b
--- /dev/null
+++ b/chrome/browser/net/http_pipelining_compatibility_client.cc
@@ -0,0 +1,163 @@
+// Copyright (c) 2012 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/net/http_pipelining_compatibility_client.h"
+
+#include "base/metrics/histogram.h"
+#include "base/stringprintf.h"
+#include "net/base/load_flags.h"
+#include "net/disk_cache/histogram_macros.h"
+
+namespace chrome_browser_net {
+
+HttpPipeliningCompatibilityClient::HttpPipeliningCompatibilityClient()
+    : num_finished_(0) {
+}
+
+void HttpPipeliningCompatibilityClient::Start(
+    const std::string& base_url,
+    std::vector<RequestInfo>& requests,
+    const net::CompletionCallback& callback,
+    net::URLRequestContext* url_request_context) {
+  finished_callback_ = callback;
+  for (size_t i = 0; i < requests.size(); ++i) {
+    requests_.push_back(new Request(i, base_url, requests[i], this,
+                                    url_request_context));
+  }
+}
+
+void HttpPipeliningCompatibilityClient::Finished(int request_id,
+                                                 Status status) {
+  // The CACHE_HISTOGRAM_* macros are used, because they allow dynamic metric
+  // names.
+  CACHE_HISTOGRAM_ENUMERATION(GetMetricName(request_id, "Status"),
+                              status, STATUS_MAX);
+  ++num_finished_;
+  if (num_finished_ == requests_.size()) {
+    finished_callback_.Run(0);
+  }
+}
+
+void HttpPipeliningCompatibilityClient::ReportNetworkError(int request_id,
+                                                           int error_code) {
+  CACHE_HISTOGRAM_ENUMERATION(GetMetricName(request_id, "NetworkError"),
+                              -error_code, 900);
+}
+
+void HttpPipeliningCompatibilityClient::ReportResponseCode(int request_id,
+                                                           int response_code) {
+  CACHE_HISTOGRAM_ENUMERATION(GetMetricName(request_id, "ResponseCode"),
+                              response_code, 600);
+}
+
+std::string HttpPipeliningCompatibilityClient::GetMetricName(
+    int request_id, const char* description) {
+  return base::StringPrintf("NetConnectivity.Pipeline.%d.%s",
+                            request_id, description);
+}
+
+HttpPipeliningCompatibilityClient::Request::Request(
+    int request_id,
+    const std::string& base_url,
+    const RequestInfo& info,
+    HttpPipeliningCompatibilityClient* client,
+    net::URLRequestContext* url_request_context)
+  : request_id_(request_id),

[mmenke, 2012/02/01 19:08:07]

nit:  4-space indent.

[James Simonsen, 2012/02/07 00:01:37]

Done.

+    request_(GURL(base_url + info.filename), this),
+    info_(info),
+    client_(client),
+    finished_(false) {
+  request_.set_context(url_request_context);
+  // TODO(simonjam): Force pipelining.
+  request_.set_load_flags(net::LOAD_BYPASS_CACHE |
+                          net::LOAD_DISABLE_CACHE |
+                          net::LOAD_DO_NOT_SAVE_COOKIES |
+                          net::LOAD_DO_NOT_SEND_COOKIES |
+                          net::LOAD_DO_NOT_SEND_AUTH_DATA);

[mmenke, 2012/02/01 19:08:07]

Think you might want to add net::DO_NOT_PROMPT_FOR_LOGIN.

[James Simonsen, 2012/02/07 00:01:37]

Done.

+  request_.Start();
+}
+
+void HttpPipeliningCompatibilityClient::Request::OnReceivedRedirect(
+    net::URLRequest* request,
+    const GURL& new_url,
+    bool* defer_redirect) {
+  *defer_redirect = true;
+  request->Cancel();
+  Finished(REDIRECTED);
+}
+
+void HttpPipeliningCompatibilityClient::Request::OnSSLCertificateError(
+    net::URLRequest* request,
+    const net::SSLInfo& ssl_info,
+    bool fatal) {
+  Finished(CERT_ERROR);
+}
+
+void HttpPipeliningCompatibilityClient::Request::OnResponseStarted(
+    net::URLRequest* request) {
+  if (finished_) {
+    return;
+  }
+  int response_code = request->GetResponseCode();
+  if (response_code > 0) {
+    client_->ReportResponseCode(request_id_, response_code);
+  }
+  if (response_code == 200) {
+    read_buffer_ = new net::IOBuffer(info_.expected_response.length());
+    DoRead();
+  } else {
+    Finished(BAD_RESPONSE_CODE);
+  }
+}
+
+void HttpPipeliningCompatibilityClient::Request::OnReadCompleted(
+    net::URLRequest* request,
+    int bytes_read) {
+  if (bytes_read == 0) {
+    DoReadFinished();
+  } else if (bytes_read < 0) {
+    Finished(NETWORK_ERROR);
+  } else {
+    response_.append(read_buffer_->data(), bytes_read);
+    if (response_.length() <= info_.expected_response.length()) {
+      DoRead();
+    } else {
+      Finished(TOO_LARGE);
+    }
+  }
+}
+
+void HttpPipeliningCompatibilityClient::Request::DoRead() {
+  int bytes_read = 0;
+  if (request_.Read(read_buffer_.get(), info_.expected_response.length(),
+                    &bytes_read)) {
+    OnReadCompleted(&request_, bytes_read);
+  }
+}
+
+void HttpPipeliningCompatibilityClient::Request::DoReadFinished() {
+  if (response_.length() != info_.expected_response.length()) {
+    Finished(TOO_SMALL);

[mmenke, 2012/02/01 19:08:07]

Is there a meaningful difference between when we get this and when we get
CONTENT_MISMATCH?  May want to check if the response is a subset of what we
expect.

Also, what if the response is too long?  I don't think this will happen, but the
Internet can be a scary place when you're facing non-compliant HTTP proxies,
armed with nothing but an RFC.

[James Simonsen, 2012/02/07 00:01:37]

In practice, it probably doesn't matter. I just thought it'd be interesting to
see if we were getting truncated as opposed to some other failure. The subset
idea is good, so I added that.
There's a Finished(TOO_LARGE) earlier that'll bail on reading if it gets more
bytes than it's expecting. We won't crash.

+  } else if (response_ == info_.expected_response) {
+    Finished(SUCCESS);
+  } else {
+    Finished(CONTENT_MISMATCH);
+  }
+}
+
+void HttpPipeliningCompatibilityClient::Request::Finished(Status result) {
+  if (finished_) {
+    return;
+  }
+  finished_ = true;
+  net::URLRequestStatus status = request_.status();
+  if (status.status() == net::URLRequestStatus::FAILED) {

[mmenke, 2012/02/01 19:08:07]

It seems a little weird to me that we're completely ignoring |result| here.

[James Simonsen, 2012/02/07 00:01:37]

I figured the network errors trump all others. For instance on a suddenly closed
connection while reading the body, I'd rather see NETWORK_ERROR than TOO_SMALL.

I think the distinction for me is whether we can detect problems with real
content. We can detect anything that falls under NETWORK_ERROR, but if content
is missing even though all the headers are correct (TOO_SMALL), then pipelining
will happily return broken content to WebKit and we'll break pages.

[mmenke, 2012/02/07 15:17:39]

Suggest you at least put a small comment here about that.

[James Simonsen, 2012/02/07 22:40:40]

Done.

+    client_->ReportNetworkError(request_id_, status.error());
+    client_->Finished(request_id_, NETWORK_ERROR);
+    return;
+  }
+  client_->Finished(request_id_, result);
+}
+
+}  // namespace chrome_browser_net