Stop using SpawnedTestServer in DownloadContentTest.*

content/public/test/test_download_request_handler.cc

+// Copyright 2015 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 "content/public/test/test_download_request_handler.h"
+
+#include "base/logging.h"
+#include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/base/io_buffer.h"
+#include "net/http/http_request_headers.h"
+#include "net/http/http_response_headers.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_interceptor.h"
+
+namespace content {
+
+namespace {
+
+using Parameters = TestDownloadRequestHandler::Parameters;
+using InjectedError = TestDownloadRequestHandler::InjectedError;
+
+class Interceptor : public net::URLRequestInterceptor {

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

nit, suggestion: Use a more fully qualified name so that codesearch/*tags can
more easily find this class?

[asanka, 2015/11/13 21:40:01]

Done.

+ public:
+  static base::WeakPtr<Interceptor> Register(const GURL& url);
+
+  using JobFactory =
+      base::Callback<net::URLRequestJob*(net::URLRequest*,
+                                         net::NetworkDelegate*,
+                                         base::WeakPtr<Interceptor>)>;
+
+  ~Interceptor() override;
+  void Unregister();
+  void SetJobFactory(const JobFactory& factory);
+  void GetAndResetCompletedRequests(

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

General comment throughout this file: Comments as to thread homes?  Those
comments can be for the whole class, or per method if useful, but given that (I
think?) everything on TestDownloadRequestHandler expects to be called on the UI
thread, and this class is pure IO thread, it seems worthwhile a line calling out
the usage.

[asanka, 2015/11/13 21:40:01]

Done.

+      TestDownloadRequestHandler::CompletedRequests* requests);
+  void AddCompletedRequest(
+      const TestDownloadRequestHandler::CompletedRequest& request_info);

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

nit: Use parallel variable names for these two functions?  (I.e. request{,s} or
request_info{,s}?)

[asanka, 2015/11/13 21:40:01]

Done.

+
+ private:
+  Interceptor(const GURL& url);
+
+  // net::URLRequestInterceptor
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override;
+
+  TestDownloadRequestHandler::CompletedRequests completed_requests_;
+  GURL url_;
+  JobFactory job_factory_;
+  mutable base::WeakPtrFactory<Interceptor> weak_ptr_factory_;
+  DISALLOW_COPY_AND_ASSIGN(Interceptor);
+};
+
+class PartialResponseJob : public net::URLRequestJob {
+ public:
+  static net::URLRequestJob* Factory(const Parameters& parameters,
+                                     net::URLRequest* request,
+                                     net::NetworkDelegate* delegate,
+                                     base::WeakPtr<Interceptor> interceptor);
+
+  // URLRequestJob
+  void Start() override;
+  void GetResponseInfo(net::HttpResponseInfo* response_info) override;
+  int64 GetTotalReceivedBytes() const override;
+  bool GetMimeType(std::string* mime_type) const override;
+  int GetResponseCode() const override;
+  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+
+ private:
+  PartialResponseJob(scoped_ptr<Parameters> parameters,
+                     base::WeakPtr<Interceptor> interceptor,
+                     net::URLRequest* url_request,
+                     net::NetworkDelegate* network_delegate);
+
+  ~PartialResponseJob() override;
+  void ReportCompletedRequest(int64_t transferred_content_length);
+
+  scoped_ptr<Parameters> parameters_;
+
+  base::WeakPtr<Interceptor> interceptor_;
+  net::HttpResponseInfo response_info_;
+  int64_t offset_ = -1;
+  int64_t offset_begin_ = -1;
+  int64_t offset_end_ = -1;
+  base::WeakPtrFactory<PartialResponseJob> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(PartialResponseJob);
+};
+
+class StaticResponseJob : public net::URLRequestJob {

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

nit: Single line comment as to purpose?  (In the context of this .cc file
PartialResponseJob was self-explanatory, but I'm not sure of what
StaticResponseJob does, and none of the methods in it give any hints.)

[asanka, 2015/11/13 21:40:02]

This got removed.

+ public:
+  static net::URLRequestJob* Factory(const std::string& headers,
+                                     net::URLRequest* request,
+                                     net::NetworkDelegate* delegate,
+                                     base::WeakPtr<Interceptor> interceptor);
+  // URLRequestJob
+  void Start() override;
+  void GetResponseInfo(net::HttpResponseInfo* response_info) override;
+
+ private:
+  StaticResponseJob(const std::string& headers,
+                    net::URLRequest* url_request,
+                    net::NetworkDelegate* network_delegate);
+  ~StaticResponseJob() override;
+
+  net::HttpResponseInfo response_info_;
+  base::WeakPtrFactory<StaticResponseJob> weak_factory_;
+  DISALLOW_COPY_AND_ASSIGN(StaticResponseJob);
+};
+
+// static
+net::URLRequestJob* PartialResponseJob::Factory(
+    const Parameters& parameters,
+    net::URLRequest* request,
+    net::NetworkDelegate* delegate,
+    base::WeakPtr<Interceptor> interceptor) {
+  return new PartialResponseJob(make_scoped_ptr(new Parameters(parameters)),
+                                interceptor, request, delegate);
+}
+
+PartialResponseJob::PartialResponseJob(scoped_ptr<Parameters> parameters,
+                                       base::WeakPtr<Interceptor> interceptor,
+                                       net::URLRequest* request,
+                                       net::NetworkDelegate* network_delegate)
+    : net::URLRequestJob(request, network_delegate),
+      parameters_(parameters.Pass()),
+      interceptor_(interceptor),
+      weak_factory_(this) {
+  DCHECK(parameters_.get());
+  DCHECK_LT(0, parameters_->size);
+  DCHECK_NE(-1, parameters_->pattern_generator_seed);
+}
+
+PartialResponseJob::~PartialResponseJob() {}
+
+void PartialResponseJob::Start() {
+  DVLOG(1) << "Starting request for " << request()->url().spec();

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

Just confirming that you wanted to land the log entries?

[asanka, 2015/11/13 21:40:01]

Yeah. They are D* logs that only result in code for debug builds and don't spew
any output unless explicitly requested via a --vmodule flag. I left them in
since they were pretty useful during debugging for me and I thought it would be
useful for others as well.

+  SetStatus(net::URLRequestStatus());
+  std::stringstream response_headers;
+
+  const net::HttpRequestHeaders extra_headers =
+      request()->extra_request_headers();
+
+  DCHECK(request()->method() == "GET") << "PartialResponseJob only "
+                                          "knows how to respond to GET "
+                                          "requests";
+  offset_begin_ = 0;
+  offset_end_ = parameters_->size - 1;
+  std::string value;
+  std::string range_header;
+  std::vector<net::HttpByteRange> byte_ranges;
+
+  if (parameters_->support_byte_ranges &&

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

I feel like a comment or two would go a long way here.

[asanka, 2015/11/13 21:40:02]

Yup. It looks a bit dense now that I look at it again. So I refactored it into
several functions instead.

+      extra_headers.GetHeader(net::HttpRequestHeaders::kIfRange, &value) &&
+      !value.empty() && value == parameters_->etag &&
+      extra_headers.GetHeader(net::HttpRequestHeaders::kRange, &range_header) &&
+      net::HttpUtil::ParseRangeHeader(range_header, &byte_ranges) &&
+      byte_ranges.size() > 0) {
+    if (byte_ranges[0].ComputeBounds(parameters_->size)) {
+      offset_begin_ = byte_ranges[0].first_byte_position();
+      offset_end_ = byte_ranges[0].last_byte_position();

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

Does this need to be the min of the parameters_->size - 1 and the
byte_ranges[0].last_byte_position()?

Also, I'm inclined to think the class should error out in some fashion if there
are multiple ranges?

[asanka, 2015/11/13 21:40:02]

The HttpByteRange::ComputeBounds() method takes care of this. I.e. it adjusts
the range so that it fits inside 0..size-1.
Handled in the new code.

+      response_headers << "HTTP/1.1 206 Partial content\r\n";
+      response_headers << "Content-Range: bytes " << offset_begin_ << "-"
+                       << offset_end_ << "/" << parameters_->size << "\r\n";
+      response_headers << "Content-Length: " << offset_end_ - offset_begin_ + 1
+                       << "\r\n";
+    } else {
+      response_headers << "HTTP/1.1 416 Range not satisfiable\r\n";
+      response_headers << "Content-Range: bytes */" << parameters_->size
+                       << "\r\n";
+    }
+  } else {
+    response_headers << "HTTP/1.1 200 Success\r\n";
+    response_headers << "Content-Length: " << parameters_->size << "\r\n";
+  }
+  if (!parameters_->content_type.empty())
+    response_headers << "Content-Type: " << parameters_->content_type << "\r\n";
+
+  if (parameters_->support_byte_ranges)
+    response_headers << "Accept-Ranges: bytes\r\n";
+
+  if (!parameters_->etag.empty())
+    response_headers << "ETag: " << parameters_->etag << "\r\n";
+
+  if (!parameters_->last_modified.empty())
+    response_headers << "Last-Modified: " << parameters_->last_modified
+                     << "\r\n";
+
+  std::string raw_headers = response_headers.str();
+  response_info_.headers =
+      new net::HttpResponseHeaders(net::HttpUtil::AssembleRawHeaders(
+          raw_headers.c_str(), raw_headers.size()));
+  offset_ = offset_begin_;
+
+  while (!parameters_->injected_errors.empty() &&
+         parameters_->injected_errors.front().offset <= offset_)
+    parameters_->injected_errors.pop();
+
+  DVLOG(1) << "Notifying response headers complete with headers:\n"
+           << raw_headers;
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(&PartialResponseJob::NotifyHeadersComplete,
+                            weak_factory_.GetWeakPtr()));
+}
+
+void PartialResponseJob::GetResponseInfo(net::HttpResponseInfo* response_info) {
+  *response_info = response_info_;
+}
+
+int64 PartialResponseJob::GetTotalReceivedBytes() const {
+  return offset_ - offset_begin_;
+}
+
+bool PartialResponseJob::GetMimeType(std::string* mime_type) const {
+  *mime_type = parameters_->content_type;
+  return !parameters_->content_type.empty();
+}
+
+int PartialResponseJob::GetResponseCode() const {
+  return response_info_.headers.get() ? response_info_.headers->response_code()
+                                      : 0;
+}
+
+bool PartialResponseJob::ReadRawData(net::IOBuffer* buf,
+                                     int buf_size,
+                                     int* bytes_read) {
+  *bytes_read = 0;
+  DVLOG(1) << "Reading " << buf_size << " bytes";
+  if (offset_ > offset_end_) {
+    ReportCompletedRequest(offset_end_ - offset_begin_ + 1);
+    DVLOG(1) << "Done reading.";
+    return true;
+  }
+
+  int64_t range_end = std::min(offset_end_, offset_ + buf_size - 1);
+  if (!parameters_->injected_errors.empty()) {
+    const InjectedError& injected_error = parameters_->injected_errors.front();
+    if (offset_ == injected_error.offset) {
+      int error = injected_error.error;
+      SetStatus(net::URLRequestStatus(net::URLRequestStatus::FAILED, error));
+      DVLOG(1) << "Returning error " << net::ErrorToString(error);
+      ReportCompletedRequest(injected_error.offset - offset_begin_);
+      parameters_->injected_errors.pop();
+      return false;
+    }
+
+    if (offset_ < injected_error.offset && injected_error.offset <= range_end)
+      range_end = injected_error.offset - 1;
+  }
+  int bytes_to_copy = (range_end - offset_) + 1;
+
+  TestDownloadRequestHandler::GetPatternBytes(
+      parameters_->pattern_generator_seed, offset_, bytes_to_copy, buf->data());
+  DVLOG(1) << "Read " << bytes_to_copy << " bytes at offset " << offset_;
+  offset_ += bytes_to_copy;
+  *bytes_read = bytes_to_copy;
+  return true;
+}
+
+void PartialResponseJob::ReportCompletedRequest(
+    int64_t transferred_content_length) {
+  if (interceptor_.get()) {
+    TestDownloadRequestHandler::CompletedRequest request_info;
+    request_info.transferred_content_length = transferred_content_length;
+    request_info.request_headers = request()->extra_request_headers();
+    interceptor_->AddCompletedRequest(request_info);
+  }
+}
+
+// static
+net::URLRequestJob* StaticResponseJob::Factory(
+    const std::string& headers,
+    net::URLRequest* request,
+    net::NetworkDelegate* delegate,
+    base::WeakPtr<Interceptor> interceptor) {
+  return new StaticResponseJob(headers, request, delegate);
+}
+
+StaticResponseJob::StaticResponseJob(const std::string& headers,
+                                     net::URLRequest* url_request,
+                                     net::NetworkDelegate* network_delegate)
+    : URLRequestJob(url_request, network_delegate), weak_factory_(this) {
+  response_info_.headers = new net::HttpResponseHeaders(
+      net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size()));
+}
+
+StaticResponseJob::~StaticResponseJob() {}
+
+void StaticResponseJob::Start() {
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(&StaticResponseJob::NotifyHeadersComplete,
+                            weak_factory_.GetWeakPtr()));
+  return;
+}
+
+void StaticResponseJob::GetResponseInfo(net::HttpResponseInfo* response_info) {
+  *response_info = response_info_;
+}
+
+template <class T>
+void StoreValueAndInvokeClosure(const base::Closure& quit_closure,
+                                T* value_receiver,
+                                T value) {
+  *value_receiver = value;
+  quit_closure.Run();
+}
+
+// Xorshift* PRNG from https://en.wikipedia.org/wiki/Xorshift
+uint64_t XorShift64StarWithIndex(uint64_t seed, uint64_t index) {
+  const uint64_t kMultiplier = UINT64_C(2685821657736338717);
+  uint64_t x = seed * kMultiplier + index;
+  x ^= x >> 12;
+  x ^= x << 25;
+  x ^= x >> 27;
+  return x * kMultiplier;
+}
+
+// static
+base::WeakPtr<Interceptor> Interceptor::Register(const GURL& url) {
+  DCHECK(url.is_valid());
+  scoped_ptr<Interceptor> interceptor(new Interceptor(url));
+  base::WeakPtr<Interceptor> weak_reference =
+      interceptor->weak_ptr_factory_.GetWeakPtr();
+  net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+  filter->AddUrlInterceptor(url, interceptor.Pass());
+  return weak_reference;
+}
+
+void Interceptor::Unregister() {
+  net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+  filter->RemoveUrlHandler(url_);
+}
+
+void Interceptor::SetJobFactory(const JobFactory& job_factory) {
+  job_factory_ = job_factory;
+}
+
+void Interceptor::GetAndResetCompletedRequests(
+    TestDownloadRequestHandler::CompletedRequests* requests) {
+  requests->clear();
+  completed_requests_.swap(*requests);
+}
+
+void Interceptor::AddCompletedRequest(
+    const TestDownloadRequestHandler::CompletedRequest& request_info) {
+  completed_requests_.push_back(request_info);
+}
+
+Interceptor::Interceptor(const GURL& url)
+    : url_(url), weak_ptr_factory_(this) {}
+
+Interceptor::~Interceptor() {}
+
+net::URLRequestJob* Interceptor::MaybeInterceptRequest(
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) const {
+  DVLOG(1) << "Intercepting request for " << request->url()
+           << " with headers:\n" << request->extra_request_headers().ToString();
+  if (job_factory_.is_null())
+    return nullptr;
+  return job_factory_.Run(request, network_delegate,
+                          weak_ptr_factory_.GetWeakPtr());
+}
+
+}  // namespace
+
+struct TestDownloadRequestHandler::InterceptorProxy {
+  base::WeakPtr<Interceptor> interceptor_;
+};
+
+TestDownloadRequestHandler::InjectedError::InjectedError(int64_t offset,
+                                                         net::Error error)
+    : offset(offset), error(error) {}
+
+// static
+Parameters TestDownloadRequestHandler::Parameters::WithSingleInterruption() {
+  Parameters parameters;
+  parameters.injected_errors.push(
+      InjectedError(parameters.size / 2, net::ERR_CONNECTION_RESET));
+  return parameters;
+}
+
+TestDownloadRequestHandler::Parameters::Parameters(const Parameters& other)
+    : etag(other.etag),
+      last_modified(other.last_modified),
+      content_type(other.content_type),
+      size(other.size),
+      pattern_generator_seed(other.pattern_generator_seed),
+      support_byte_ranges(other.support_byte_ranges),
+      injected_errors(other.injected_errors) {}
+
+TestDownloadRequestHandler::Parameters::Parameters()
+    : etag("abcd"),
+      last_modified("Tue, 15 Nov 1994 12:45:26 GMT"),
+      content_type("application/octet-stream"),
+      size(102400),
+      pattern_generator_seed(1),
+      support_byte_ranges(true) {}
+
+TestDownloadRequestHandler::Parameters::Parameters(
+    const std::string& etag,
+    const std::string& last_modified,
+    const std::string& content_type,
+    int64_t size,
+    int pattern_generator_seed,
+    bool support_byte_ranges)
+    : etag(etag),
+      last_modified(last_modified),
+      content_type(content_type),
+      size(size),
+      pattern_generator_seed(pattern_generator_seed),
+      support_byte_ranges(support_byte_ranges) {}
+
+TestDownloadRequestHandler::Parameters::~Parameters() {}
+
+void TestDownloadRequestHandler::Parameters::ClearInjectedErrors() {
+  std::queue<InjectedError> empty_error_list;
+  injected_errors.swap(empty_error_list);
+}
+
+TestDownloadRequestHandler::TestDownloadRequestHandler()
+    : TestDownloadRequestHandler(GURL("http://example.com/download")) {}
+
+TestDownloadRequestHandler::TestDownloadRequestHandler(const GURL& url)
+    : url_(url) {
+  interceptor_proxy_.reset(new InterceptorProxy);
+  base::RunLoop run_loop;
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::IO, FROM_HERE, base::Bind(&Interceptor::Register, url_),
+      base::Bind(&StoreValueAndInvokeClosure<base::WeakPtr<Interceptor>>,
+                 run_loop.QuitClosure(), &interceptor_proxy_->interceptor_));
+  run_loop.Run();
+}
+
+void TestDownloadRequestHandler::StartServing(const Parameters& parameters) {
+  Interceptor::JobFactory job_factory =
+      base::Bind(&PartialResponseJob::Factory, parameters);
+  BrowserThread::PostTask(

[Randy Smith (Not in Mondays), 2015/11/12 00:57:57]

Comment about why a simple call to a setter starts the serving?  (Because before
that the interceptor wasn't claiming any requests, I presume.)

[asanka, 2015/11/13 21:40:01]

It's a bit unclear, but StartServing(const Parameters&) starts serving based on
the given parameters. It doesn't mean that it wasn't serving before.

On the implementation side, it just sets the job factory to use based on the new
parameters. The Interceptor is RAII and is already intercepting requests, but
doesn't do anything until it has a factory to work with. I'll add a comment to
explain.

+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&Interceptor::SetJobFactory, interceptor_proxy_->interceptor_,
+                 job_factory));
+}
+
+void TestDownloadRequestHandler::StartServingStaticResponse(
+    const base::StringPiece& headers) {
+  Interceptor::JobFactory job_factory =
+      base::Bind(&StaticResponseJob::Factory, headers.as_string());
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&Interceptor::SetJobFactory, interceptor_proxy_->interceptor_,
+                 job_factory));
+}
+
+// static
+void TestDownloadRequestHandler::GetPatternBytes(int seed,
+                                                 int64_t starting_offset,
+                                                 int length,
+                                                 char* buffer) {
+  int64_t seed_offset = starting_offset / sizeof(int64_t);
+  int64_t first_byte_position = starting_offset % sizeof(int64_t);
+  while (length > 0) {
+    uint64_t data = XorShift64StarWithIndex(seed, seed_offset);
+    int length_to_copy =
+        std::min(length, static_cast<int>(sizeof(data) - first_byte_position));
+    memcpy(buffer, reinterpret_cast<char*>(&data) + first_byte_position,
+           length_to_copy);
+    buffer += length_to_copy;
+    length -= length_to_copy;
+    ++seed_offset;
+    first_byte_position = 0;
+  }
+}
+
+TestDownloadRequestHandler::~TestDownloadRequestHandler() {
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&Interceptor::Unregister, interceptor_proxy_->interceptor_));
+}
+
+void TestDownloadRequestHandler::GetCompletedRequestInfo(
+    TestDownloadRequestHandler::CompletedRequests* requests) {
+  base::RunLoop run_loop;
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&Interceptor::GetAndResetCompletedRequests,
+                 interceptor_proxy_->interceptor_, requests),
+      run_loop.QuitClosure());
+  run_loop.Run();
+}
+
+}  // namespace content