Allow MojoAsyncResourceHandler::OnWillRead to complete asyncronously

content/browser/loader/mojo_async_resource_handler_unittest.cc

Index: content/browser/loader/mojo_async_resource_handler_unittest.cc
diff --git a/content/browser/loader/mojo_async_resource_handler_unittest.cc b/content/browser/loader/mojo_async_resource_handler_unittest.cc
index 2a29d61f35740c778053c7a7be643e7cb5bccd49..46c4cf05e278d5d7288fccdd2830dcfc85d56581 100644
--- a/content/browser/loader/mojo_async_resource_handler_unittest.cc
+++ b/content/browser/loader/mojo_async_resource_handler_unittest.cc
@@ -585,8 +585,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
     contents.append(buffer, read_size);
   }
   EXPECT_EQ(data, contents);
-  EXPECT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
-            mock_loader_->status());
+  EXPECT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->status());
 }
 
 TEST_F(MojoAsyncResourceHandlerTest,
@@ -723,8 +722,10 @@ TEST_F(MojoAsyncResourceHandlerTest, OutOfBandCancelDuringBodyTransmission) {
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
   std::string data(mock_loader_->io_buffer_size(), 'a');
-  ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
+  ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnReadCompleted(data));
+  ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
+            mock_loader_->OnWillRead());
   url_loader_client_.RunUntilResponseBodyArrived();
   ASSERT_TRUE(url_loader_client_.response_body().is_valid());
 
@@ -764,45 +765,122 @@ TEST_F(MojoAsyncResourceHandlerTest, BeginWriteFailsOnWillRead) {
 
 TEST_F(MojoAsyncResourceHandlerTest, BeginWriteReturnsShouldWaitOnWillRead) {
   handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
+
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
+  EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
+
+  // Bytes are read one-at-a-time, and each OnWillRead() call completes
+  // asynchronously. Note that this loop runs 4 times (once for the terminal
+  // '\0').
+  const char kReadData[] = "ABC";
+  for (const char read_char : kReadData) {
+    handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
+    ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
+              mock_loader_->OnWillRead());
+    EXPECT_EQ(1, rdh_.num_in_flight_requests_for_testing());
+
+    handler_->ResetBeginWriteExpectation();
+    handler_->OnWritableForTesting();
+    EXPECT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->status());
+
+    ASSERT_EQ(MockResourceLoader::Status::IDLE,
+              mock_loader_->OnReadCompleted(std::string(1, read_char)));
+    url_loader_client_.RunUntilResponseBodyArrived();
+
+    // Keep on trying to read the data until it succeeds.
+    while (true) {
+      char buffer[16];
+      uint32_t read_size = sizeof(buffer);
+      MojoResult result =
+          mojo::ReadDataRaw(url_loader_client_.response_body(), buffer,
+                            &read_size, MOJO_READ_DATA_FLAG_NONE);
+      if (result != MOJO_RESULT_SHOULD_WAIT) {
+        ASSERT_EQ(MOJO_RESULT_OK, result);
+        ASSERT_EQ(1u, read_size);
+        EXPECT_EQ(read_char, buffer[0]);
+        break;
+      }
+
+      base::RunLoop().RunUntilIdle();
+    }
+  }
+
+  // Should only count as one in-flight request.
+  EXPECT_EQ(1, rdh_.num_in_flight_requests_for_testing());
+
+  ASSERT_EQ(MockResourceLoader::Status::IDLE,
+            mock_loader_->OnResponseCompleted(
+                net::URLRequestStatus::FromError(net::OK)));
+
+  url_loader_client_.RunUntilComplete();
+  EXPECT_TRUE(url_loader_client_.has_received_completion());
+
+  handler_.reset();
+  EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
 }
 
+// Same as above, but after the first OnWriteable() call, BeginWrite() indicates
+// should wait again. Unclear if this can happen in practice, but seems best to
+// support it.
 TEST_F(MojoAsyncResourceHandlerTest,
-       BeginWriteReturnsShouldWaitOnWillReadAndThenReturnsOK) {
+       BeginWriteReturnsShouldWaitTwiceOnWillRead) {
   handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
+
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
-  size_t written = 0;
-  while (true) {
-    ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
-    MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
-        std::string(mock_loader_->io_buffer_size(), 'X'));
-    written += mock_loader_->io_buffer_size();
-    if (result == MockResourceLoader::Status::CALLBACK_PENDING)
-      break;
-    ASSERT_EQ(MockResourceLoader::Status::IDLE, result);
-  }
+  EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
 
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
-  handler_->ResetBeginWriteExpectation();
-  handler_->OnWritableForTesting();
+  // Bytes are read one-at-a-time, and each OnWillRead() call completes
+  // asynchronously. Note that this loop runs 4 times (once for the terminal
+  // '\0').
+  const char kReadData[] = "ABC";
+  for (const char read_char : kReadData) {
+    handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
+    ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
+              mock_loader_->OnWillRead());
+    EXPECT_EQ(1, rdh_.num_in_flight_requests_for_testing());
+
+    handler_->OnWritableForTesting();
+    EXPECT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
+              mock_loader_->status());
+
+    handler_->ResetBeginWriteExpectation();
+    handler_->OnWritableForTesting();
+    EXPECT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->status());
+
+    ASSERT_EQ(MockResourceLoader::Status::IDLE,
+              mock_loader_->OnReadCompleted(std::string(1, read_char)));
+    url_loader_client_.RunUntilResponseBodyArrived();
+
+    // Keep on trying to read the data until it succeeds.
+    while (true) {
+      char buffer[16];
+      uint32_t read_size = sizeof(buffer);
+      MojoResult result =
+          mojo::ReadDataRaw(url_loader_client_.response_body(), buffer,
+                            &read_size, MOJO_READ_DATA_FLAG_NONE);
+      if (result != MOJO_RESULT_SHOULD_WAIT) {
+        ASSERT_EQ(MOJO_RESULT_OK, result);
+        ASSERT_EQ(1u, read_size);
+        EXPECT_EQ(read_char, buffer[0]);
+        break;
+      }
 
-  std::string actual;
-  while (actual.size() < written) {
-    char buf[16];
-    uint32_t read_size = sizeof(buf);
-    MojoResult result =
-        mojo::ReadDataRaw(url_loader_client_.response_body(), buf, &read_size,
-                          MOJO_READ_DATA_FLAG_NONE);
-    ASSERT_TRUE(result == MOJO_RESULT_OK || result == MOJO_RESULT_SHOULD_WAIT);
-    if (result == MOJO_RESULT_OK)
-      actual.append(buf, read_size);
-    base::RunLoop().RunUntilIdle();
+      base::RunLoop().RunUntilIdle();
+    }
   }
 
-  EXPECT_EQ(std::string(written, 'X'), actual);
-  EXPECT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->status());
+  // Should only count as one in-flight request.
+  EXPECT_EQ(1, rdh_.num_in_flight_requests_for_testing());
+
+  ASSERT_EQ(MockResourceLoader::Status::IDLE,
+            mock_loader_->OnResponseCompleted(
+                net::URLRequestStatus::FromError(net::OK)));
+
+  url_loader_client_.RunUntilComplete();
+  EXPECT_TRUE(url_loader_client_.has_received_completion());
+
+  handler_.reset();
+  EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
 }
 
 TEST_F(MojoAsyncResourceHandlerTest,
@@ -869,7 +947,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
   mock_loader_->WaitUntilIdleOrCanceled();
   EXPECT_FALSE(url_loader_client_.has_received_completion());
   EXPECT_EQ(MockResourceLoader::Status::CANCELED, mock_loader_->status());
-  EXPECT_EQ(net::ERR_FAILED, mock_loader_->error_code());
+  EXPECT_EQ(net::ERR_INSUFFICIENT_RESOURCES, mock_loader_->error_code());
 }
 
 TEST_F(MojoAsyncResourceHandlerUploadTest, UploadProgressHandling) {
@@ -942,20 +1020,18 @@ TEST_F(MojoAsyncResourceHandlerTest, SetPriority) {
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        OnWillReadWithLongContents) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
+
   std::string expected;
   for (int i = 0; i < 3 * mock_loader_->io_buffer_size() + 2; ++i)
     expected += ('A' + i % 26);
 
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnReadCompleted(0));
-
   size_t written = 0;
   std::string actual;
   while (actual.size() < expected.size()) {
     while (written < expected.size() &&
            mock_loader_->status() == MockResourceLoader::Status::IDLE) {
       ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
-      const size_t to_be_written =
+      size_t to_be_written =
           std::min(static_cast<size_t>(mock_loader_->io_buffer_size()),
                    expected.size() - written);
 
@@ -993,10 +1069,16 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
 
+  // Whether the next OnReadCompleted call or OnWillRead returns the error
+  // depends on whether or not an intermediary buffer is being used by the
+  // MojoAsyncResourceHandler.
   handler_->set_begin_write_expectation(MOJO_RESULT_UNKNOWN);
-  ASSERT_EQ(MockResourceLoader::Status::CANCELED,
-            mock_loader_->OnReadCompleted(
-                std::string(mock_loader_->io_buffer_size(), 'A')));
+  MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
+      std::string(mock_loader_->io_buffer_size(), 'A'));
+  if (result == MockResourceLoader::Status::CANCELED)
+    return;
+  ASSERT_EQ(MockResourceLoader::Status::IDLE, result);
+  ASSERT_EQ(MockResourceLoader::Status::CANCELED, mock_loader_->OnWillRead());
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
@@ -1004,21 +1086,32 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
 
+  // Whether the next OnReadCompleted call or OnWillRead call completes
+  // asynchronously depends on whether or not an intermediary buffer is being
+  // used by the MojoAsyncResourceHandler.
   handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
+  MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
+      std::string(mock_loader_->io_buffer_size() - 1, 'A'));
+  if (result == MockResourceLoader::Status::CALLBACK_PENDING)
+    return;
+
   ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
-            mock_loader_->OnReadCompleted(
-                std::string(mock_loader_->io_buffer_size(), 'A')));
+            mock_loader_->OnWillRead());
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        BeginWriteFailsOnResume) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnReadCompleted(0));
 
   while (true) {
-    ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
-    MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
+    // Whether the next OnReadCompleted call or OnWillRead call completes
+    // asynchronously depends on whether or not an intermediary buffer is being
+    // used by the MojoAsyncResourceHandler.
+    MockResourceLoader::Status result = mock_loader_->OnWillRead();
+    if (result == MockResourceLoader::Status::CALLBACK_PENDING)
+      break;
+    ASSERT_EQ(MockResourceLoader::Status::IDLE, result);
+    result = mock_loader_->OnReadCompleted(
         std::string(mock_loader_->io_buffer_size(), 'A'));
     if (result == MockResourceLoader::Status::CALLBACK_PENDING)
       break;
@@ -1039,16 +1132,25 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
     base::RunLoop().RunUntilIdle();
   }
 
+  if (mock_loader_->status() == MockResourceLoader::Status::IDLE)
+    EXPECT_EQ(MockResourceLoader::Status::CANCELED, mock_loader_->OnWillRead());
+
   EXPECT_FALSE(url_loader_client_.has_received_completion());
-  EXPECT_EQ(net::ERR_FAILED, mock_loader_->error_code());
+  EXPECT_EQ(net::ERR_INSUFFICIENT_RESOURCES, mock_loader_->error_code());
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest, CancelWhileWaiting) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   while (true) {
-    ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead());
-    MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
+    // Whether the next OnReadCompleted call or OnWillRead call completes
+    // asynchronously depends on whether or not an intermediary buffer is being
+    // used by the MojoAsyncResourceHandler.
+    MockResourceLoader::Status result = mock_loader_->OnWillRead();
+    if (result == MockResourceLoader::Status::CALLBACK_PENDING)
+      break;
+    ASSERT_EQ(MockResourceLoader::Status::IDLE, result);
+    result = mock_loader_->OnReadCompleted(
         std::string(mock_loader_->io_buffer_size(), 'A'));
     if (result == MockResourceLoader::Status::CALLBACK_PENDING)
       break;