Add more ResourceLoaderTests.

In particular, check the cases of async URLRequest::Read successes.

Also add ResourceHandler re-entrancy checks to TestResourceHandler,
which the current API is designed to avoid.

Review-Url: https://codereview.chromium.org/2552463002
Cr-Commit-Position: refs/heads/master@{#437586}

content/browser/loader/test_resource_handler.cc

@@ -11,6 +11,28 @@
 
 namespace content {
 
+namespace {
+
+class ScopedCallDepthTracker {
+ public:
+  explicit ScopedCallDepthTracker(int* call_depth) : call_depth_(call_depth) {
+    EXPECT_EQ(0, *call_depth_);
+    (*call_depth_)++;
+  }
+
+  ~ScopedCallDepthTracker() {
+    EXPECT_EQ(1, *call_depth_);
+    (*call_depth_)--;
+  }
+
+ private:
+  int* const call_depth_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedCallDepthTracker);
+};
+
+}  // namespace
+
 TestResourceHandler::TestResourceHandler(net::URLRequestStatus* request_status,
                                          std::string* body)
     : ResourceHandler(nullptr),
@@ -37,6 +59,8 @@ bool TestResourceHandler::OnRequestRedirected(
   EXPECT_EQ(1, on_will_start_called_);
   EXPECT_EQ(0, on_response_started_called_);
   EXPECT_EQ(0, on_response_completed_called_);
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
+
   ++on_request_redirected_called_;
 
   if (!on_request_redirected_result_) {
@@ -57,6 +81,8 @@ bool TestResourceHandler::OnResponseStarted(ResourceResponse* response,
   EXPECT_EQ(1, on_will_start_called_);
   EXPECT_EQ(0, on_response_started_called_);
   EXPECT_EQ(0, on_response_completed_called_);
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
+
   ++on_response_started_called_;
 
   EXPECT_FALSE(resource_response_);
@@ -79,6 +105,8 @@ bool TestResourceHandler::OnWillStart(const GURL& url, bool* defer) {
   EXPECT_EQ(0, on_response_started_called_);
   EXPECT_EQ(0, on_will_start_called_);
   EXPECT_EQ(0, on_response_completed_called_);
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
+
   ++on_will_start_called_;
 
   start_url_ = url;
@@ -100,6 +128,8 @@ bool TestResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
   EXPECT_FALSE(canceled_);
   EXPECT_FALSE(expect_on_data_downloaded_);
   EXPECT_EQ(0, on_response_completed_called_);
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
+
   ++on_will_read_called_;
 
   *buf = buffer_;
@@ -117,6 +147,7 @@ bool TestResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
   EXPECT_EQ(1, on_response_started_called_);
   EXPECT_EQ(0, on_response_completed_called_);
   EXPECT_EQ(0, on_read_eof_);
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
 
   ++on_read_completed_called_;
   if (bytes_read == 0)
@@ -127,8 +158,7 @@ bool TestResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
     body_ptr_->append(buffer_->data(), bytes_read);
   body_.append(buffer_->data(), bytes_read);
 
-  if (!on_read_completed_result_ ||
-      (!on_on_read_eof_result_ && bytes_read == 0)) {
+  if (!on_read_completed_result_ || (!on_read_eof_result_ && bytes_read == 0)) {
     canceled_ = true;
     return false;
   }
@@ -147,6 +177,8 @@ bool TestResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
 void TestResourceHandler::OnResponseCompleted(
     const net::URLRequestStatus& status,
     bool* defer) {
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
+
   EXPECT_EQ(0, on_response_completed_called_);
   if (status.is_success() && !expect_on_data_downloaded_ && expect_eof_read_)
     EXPECT_EQ(1, on_read_eof_);
@@ -170,10 +202,12 @@ void TestResourceHandler::OnDataDownloaded(int bytes_downloaded) {
 }
 
 void TestResourceHandler::Resume() {
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
   controller_->Resume();
 }
 
 void TestResourceHandler::CancelWithError(net::Error net_error) {
+  ScopedCallDepthTracker call_depth_tracker(&call_depth_);
   canceled_ = true;
   controller_->CancelWithError(net_error);
 }

content/browser/loader/test_resource_handler.h

@@ -83,8 +83,8 @@ class TestResourceHandler : public ResourceHandler {
   void set_on_read_completed_result(bool on_read_completed_result) {
     on_read_completed_result_ = on_read_completed_result;
   }
-  void set_on_on_read_eof_result(bool on_on_read_eof_result) {
-    on_on_read_eof_result_ = on_on_read_eof_result;
+  void set_on_read_eof_result(bool on_read_eof_result) {
+    on_read_eof_result_ = on_read_eof_result;
   }
 
   // Cause |defer| to be set to true when the specified method is invoked. The
@@ -146,6 +146,9 @@ class TestResourceHandler : public ResourceHandler {
   const std::string& body() const { return body_; }
   net::URLRequestStatus final_status() const { return final_status_; }
 
+  // Returns the current number of |this|'s methods on the callstack.
+  int call_depth() const { return call_depth_; }
+
   // Spins the message loop until the request is deferred.  Using this is
   // optional, but if used, must use it exclusively to wait for the request. If
   // the request was deferred and then resumed/canceled without calling this
@@ -169,7 +172,7 @@ class TestResourceHandler : public ResourceHandler {
   bool on_response_started_result_ = true;
   bool on_will_read_result_ = true;
   bool on_read_completed_result_ = true;
-  bool on_on_read_eof_result_ = true;
+  bool on_read_eof_result_ = true;
 
   bool defer_on_will_start_ = false;
   bool defer_on_request_redirected_ = false;
@@ -198,6 +201,9 @@ class TestResourceHandler : public ResourceHandler {
       net::URLRequestStatus::FromError(net::ERR_UNEXPECTED);
   bool canceled_ = false;
 
+  // Tracks recursive calls, which aren't allowed.
+  int call_depth_ = 0;
+
   std::unique_ptr<base::RunLoop> deferred_run_loop_;
 
   base::RunLoop response_complete_run_loop_;

net/url_request/url_request_test_job.cc

@@ -44,21 +44,31 @@ class TestJobProtocolHandler : public URLRequestJobFactory::ProtocolHandler {
 GURL URLRequestTestJob::test_url_1() {
   return GURL("test:url1");
 }
+
 GURL URLRequestTestJob::test_url_2() {
   return GURL("test:url2");
 }
+
 GURL URLRequestTestJob::test_url_3() {
   return GURL("test:url3");
 }
+
 GURL URLRequestTestJob::test_url_4() {
   return GURL("test:url4");
 }
+
+GURL URLRequestTestJob::test_url_auto_advance_async_reads_1() {
+  return GURL("test:url_auto_advance_async_reads_1");
+}
+
 GURL URLRequestTestJob::test_url_error() {
   return GURL("test:error");
 }
+
 GURL URLRequestTestJob::test_url_redirect_to_url_1() {
   return GURL("test:redirect_to_1");
 }
+
 GURL URLRequestTestJob::test_url_redirect_to_url_2() {
   return GURL("test:redirect_to_2");
 }
@@ -133,15 +143,7 @@ URLRequestTestJob::CreateProtocolHandler() {
 
 URLRequestTestJob::URLRequestTestJob(URLRequest* request,
                                      NetworkDelegate* network_delegate)
-    : URLRequestJob(request, network_delegate),
-      auto_advance_(false),
-      stage_(WAITING),
-      priority_(DEFAULT_PRIORITY),
-      offset_(0),
-      async_buf_(NULL),
-      async_buf_size_(0),
-      response_headers_length_(0),
-      weak_factory_(this) {}
+    : URLRequestTestJob(request, network_delegate, false) {}
 
 URLRequestTestJob::URLRequestTestJob(URLRequest* request,
                                      NetworkDelegate* network_delegate,
@@ -154,6 +156,7 @@ URLRequestTestJob::URLRequestTestJob(URLRequest* request,
       async_buf_(NULL),
       async_buf_size_(0),
       response_headers_length_(0),
+      async_reads_(false),
       weak_factory_(this) {}
 
 URLRequestTestJob::URLRequestTestJob(URLRequest* request,
@@ -167,12 +170,13 @@ URLRequestTestJob::URLRequestTestJob(URLRequest* request,
       priority_(DEFAULT_PRIORITY),
       response_data_(response_data),
       offset_(0),
-      async_buf_(NULL),
+      async_buf_(nullptr),
       async_buf_size_(0),
       response_headers_(new net::HttpResponseHeaders(
           net::HttpUtil::AssembleRawHeaders(response_headers.c_str(),
                                             response_headers.size()))),
       response_headers_length_(response_headers.size()),
+      async_reads_(false),
       weak_factory_(this) {}
 
 URLRequestTestJob::~URLRequestTestJob() {
@@ -204,18 +208,22 @@ void URLRequestTestJob::Start() {
 void URLRequestTestJob::StartAsync() {
   if (!response_headers_.get()) {
     SetResponseHeaders(test_headers());
-    if (request_->url().spec() == test_url_1().spec()) {
+    if (request_->url() == test_url_1()) {
       response_data_ = test_data_1();
       stage_ = DATA_AVAILABLE;  // Simulate a synchronous response for this one.
-    } else if (request_->url().spec() == test_url_2().spec()) {
+    } else if (request_->url() == test_url_2()) {
       response_data_ = test_data_2();
-    } else if (request_->url().spec() == test_url_3().spec()) {
+    } else if (request_->url() == test_url_3()) {
       response_data_ = test_data_3();
-    } else if (request_->url().spec() == test_url_4().spec()) {
+    } else if (request_->url() == test_url_4()) {
       response_data_ = test_data_4();
-    } else if (request_->url().spec() == test_url_redirect_to_url_1().spec()) {
+    } else if (request_->url() == test_url_auto_advance_async_reads_1()) {
+      response_data_ = test_data_1();
+      stage_ = DATA_AVAILABLE;  // Data is available immediately.
+      async_reads_ = true;      // All reads complete asynchronously.
+    } else if (request_->url() == test_url_redirect_to_url_1()) {
       SetResponseHeaders(test_redirect_to_url_1_headers());
-    } else if (request_->url().spec() == test_url_redirect_to_url_2().spec()) {
+    } else if (request_->url() == test_url_redirect_to_url_2()) {
       SetResponseHeaders(test_redirect_to_url_2_headers());
     } else {
       AdvanceJob();
@@ -243,24 +251,33 @@ void URLRequestTestJob::SetResponseHeaders(
   response_headers_length_ = response_headers.size();
 }
 
+int URLRequestTestJob::CopyDataForRead(IOBuffer* buf, int buf_size) {
+  int bytes_read = 0;
+  if (offset_ < static_cast<int>(response_data_.length())) {
+    bytes_read = buf_size;
+    if (bytes_read + offset_ > static_cast<int>(response_data_.length()))
+      bytes_read = static_cast<int>(response_data_.length()) - offset_;
+
+    memcpy(buf->data(), &response_data_.c_str()[offset_], bytes_read);
+    offset_ += bytes_read;
+  }
+  return bytes_read;
+}
+
 int URLRequestTestJob::ReadRawData(IOBuffer* buf, int buf_size) {
-  if (stage_ == WAITING) {
+  if (stage_ == WAITING || async_reads_) {
     async_buf_ = buf;
     async_buf_size_ = buf_size;
+    if (stage_ != WAITING) {
+      stage_ = WAITING;
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::Bind(&URLRequestTestJob::ProcessNextOperation,
+                                weak_factory_.GetWeakPtr()));
+    }
     return ERR_IO_PENDING;
   }
 
-  if (offset_ >= static_cast<int>(response_data_.length()))
-    return 0;  // done reading
-
-  int to_read = buf_size;
-  if (to_read + offset_ > static_cast<int>(response_data_.length()))
-    to_read = static_cast<int>(response_data_.length()) - offset_;
-
-  memcpy(buf->data(), &response_data_.c_str()[offset_], to_read);
-  offset_ += to_read;
-
-  return to_read;
+  return CopyDataForRead(buf, buf_size);
 }
 
 void URLRequestTestJob::GetResponseInfo(HttpResponseInfo* info) {
@@ -322,7 +339,7 @@ void URLRequestTestJob::ProcessNextOperation() {
       stage_ = DATA_AVAILABLE;
       // OK if ReadRawData wasn't called yet.
       if (async_buf_) {
-        int result = ReadRawData(async_buf_, async_buf_size_);
+        int result = CopyDataForRead(async_buf_, async_buf_size_);
         if (result < 0)
           NOTREACHED() << "Reads should not fail in DATA_AVAILABLE.";
         if (NextReadAsync()) {

net/url_request/url_request_test_job.h

@@ -63,13 +63,28 @@ class NET_EXPORT_PRIVATE URLRequestTestJob : public URLRequestJob {
 
   // The canned URLs this handler will respond to without having been
   // explicitly initialized with response headers and data.
-  // FIXME(brettw): we should probably also have a redirect one
+
+  // URL that, by default, automatically advances through each state.  Reads
+  // complete synchronously.
   static GURL test_url_1();
+
+  // URLs that, by default, must be manually advanced through each state.
   static GURL test_url_2();
   static GURL test_url_3();
   static GURL test_url_4();
+
+  // URL that, by default, automatically advances through each state.  Reads
+  // complete asynchronously. Has same response body as test_url_1(), which is
+  // (test_data_1()).
+  static GURL test_url_auto_advance_async_reads_1();
+
+  // URL that fails with ERR_INVALID_URL.
   static GURL test_url_error();
+
+  // Redirects to test_url_1().
   static GURL test_url_redirect_to_url_1();
+
+  // Redirects to test_url_2().
   static GURL test_url_redirect_to_url_2();
 
   // The data that corresponds to each of the URLs above
@@ -151,6 +166,10 @@ class NET_EXPORT_PRIVATE URLRequestTestJob : public URLRequestJob {
   // Assigns |response_headers_| and |response_headers_length_|.
   void SetResponseHeaders(const std::string& response_headers);
 
+  // Copies as much of the response body as will into |buf|, and returns number
+  // of bytes written.
+  int CopyDataForRead(IOBuffer* buf, int buf_size);
+
   bool auto_advance_;
 
   Stage stage_;
@@ -178,6 +197,8 @@ class NET_EXPORT_PRIVATE URLRequestTestJob : public URLRequestJob {
   // Original size in bytes of the response headers before decoding.
   int response_headers_length_;
 
+  bool async_reads_;
+
   base::WeakPtrFactory<URLRequestTestJob> weak_factory_;
 };