[Mojo-Loading] OnStartLoadingResponseBody should be called after OnReceiveResponse

content/browser/loader/mojo_async_resource_handler_unittest.cc

 // Copyright 2016 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/browser/loader/mojo_async_resource_handler.h"
 
 #include <string.h>
 
 #include <utility>
 #include <vector>
@@ skipping 439 matching lines @@
   EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnWillReadAndOnReadCompleted) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   // The buffer size that the mime sniffer requires implicitly.
   ASSERT_GE(mock_loader_->io_buffer_size(),
             kSizeMimeSnifferRequiresForFirstOnWillRead);
 
+  ASSERT_EQ(MockResourceLoader::Status::IDLE,
+            mock_loader_->OnReadCompleted("AB"));
+
   url_loader_client_.RunUntilResponseBodyArrived();
   ASSERT_TRUE(url_loader_client_.response_body().is_valid());
 
-  ASSERT_EQ(MockResourceLoader::Status::IDLE,
-            mock_loader_->OnReadCompleted("AB"));
-
   std::string contents;
   while (contents.size() < 2) {
     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) {
       base::RunLoop().RunUntilIdle();
       continue;
     }
     contents.append(buffer, read_size);
   }
   EXPECT_EQ("AB", contents);
 }
 
 TEST_F(MojoAsyncResourceHandlerTest,
        OnWillReadAndOnReadCompletedWithInsufficientInitialCapacity) {
   MojoAsyncResourceHandler::SetAllocationSizeForTesting(2);
 
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   // The buffer size that the mime sniffer requires implicitly.
   ASSERT_GE(mock_loader_->io_buffer_size(),
             kSizeMimeSnifferRequiresForFirstOnWillRead);
 
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
-
   const std::string data("abcdefgh");
   ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
             mock_loader_->OnReadCompleted(data));
 
+  url_loader_client_.RunUntilResponseBodyArrived();
+  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
+
   std::string contents;
   while (contents.size() < data.size()) {
     // This is needed for Resume to be called.
     base::RunLoop().RunUntilIdle();
     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)
@@ skipping 103 matching lines @@
   url_loader_client_.RunUntilComplete();
   EXPECT_TRUE(url_loader_client_.has_received_completion());
   EXPECT_EQ(net::ERR_TIMED_OUT,
             url_loader_client_.completion_status().error_code);
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, ResponseCompletionShouldCloseDataPipe) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
+  ASSERT_EQ(MockResourceLoader::Status::IDLE,
+            mock_loader_->OnReadCompleted("AB"));
   url_loader_client_.RunUntilResponseBodyArrived();
   ASSERT_TRUE(url_loader_client_.response_body().is_valid());
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnReadCompleted(0));
 
   net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, net::OK);
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnResponseCompleted(status));
 
   url_loader_client_.RunUntilComplete();
   EXPECT_TRUE(url_loader_client_.has_received_completion());
   EXPECT_EQ(net::OK, url_loader_client_.completion_status().error_code);
 
   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_FAILED_PRECONDITION)
       break;
-    ASSERT_EQ(result, MOJO_RESULT_SHOULD_WAIT);
+    ASSERT_TRUE(result == MOJO_RESULT_SHOULD_WAIT || result == MOJO_RESULT_OK);
   }
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OutOfBandCancelDuringBodyTransmission) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
   std::string data(mock_loader_->io_buffer_size(), 'a');
   ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
             mock_loader_->OnReadCompleted(data));
+  url_loader_client_.RunUntilResponseBodyArrived();
+  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
 
   net::URLRequestStatus status(net::URLRequestStatus::FAILED, net::ERR_FAILED);
   ASSERT_EQ(
       MockResourceLoader::Status::IDLE,
       mock_loader_->OnResponseCompletedFromExternalOutOfBandCancel(status));
 
   url_loader_client_.RunUntilComplete();
   EXPECT_TRUE(url_loader_client_.has_received_completion());
   EXPECT_EQ(net::ERR_FAILED, url_loader_client_.completion_status().error_code);
 
   std::string actual;
   while (true) {
     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);
     if (result == MOJO_RESULT_FAILED_PRECONDITION)
       break;
     if (result == MOJO_RESULT_SHOULD_WAIT) {
       base::RunLoop().RunUntilIdle();
       continue;
     }
     EXPECT_EQ(MOJO_RESULT_OK, result);
     actual.append(buf, read_size);
   }
   EXPECT_EQ(data, actual);
 }
 
-// In this case, an error is notified after OnWillRead, before OnReadCompleted.
-TEST_F(MojoAsyncResourceHandlerTest, ResponseErrorDuringBodyTransmission2) {

[mmenke, 2017/01/18 16:26:12]

Why did you remove this test?

[yhirano, 2017/01/19 03:25:50]

This test doesn't have an OnReadCompleted call. With this change
OnResponseBodyArrived doesn't arrive and so the test makes no sense.

-  ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
-
-  ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
-  net::URLRequestStatus status(net::URLRequestStatus::FAILED, net::ERR_FAILED);
-  ASSERT_EQ(MockResourceLoader::Status::IDLE,
-            mock_loader_->OnResponseCompleted(status));
-
-  url_loader_client_.RunUntilComplete();
-  EXPECT_TRUE(url_loader_client_.has_received_completion());
-  EXPECT_EQ(net::ERR_FAILED, url_loader_client_.completion_status().error_code);
-
-  while (true) {
-    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);
-    if (result == MOJO_RESULT_FAILED_PRECONDITION)
-      break;
-    ASSERT_EQ(MOJO_RESULT_SHOULD_WAIT, result);
-    base::RunLoop().RunUntilIdle();
-  }
-}
-
 TEST_F(MojoAsyncResourceHandlerTest, BeginWriteFailsOnWillRead) {
   handler_->set_begin_write_expectation(MOJO_RESULT_UNKNOWN);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::CANCELED, mock_loader_->OnWillRead(-1));
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, BeginWriteReturnsShouldWaitOnWillRead) {
   handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ skipping 64 matching lines @@
   ASSERT_EQ(MockResourceLoader::Status::CANCELED,
             mock_loader_->OnReadCompleted(
                 std::string(mock_loader_->io_buffer_size(), 'w')));
 }
 
 TEST_F(MojoAsyncResourceHandlerTest,
        EndWriteFailsOnResumeWithInsufficientInitialCapacity) {
   MojoAsyncResourceHandler::SetAllocationSizeForTesting(8);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
 
   while (true) {
     MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
         std::string(mock_loader_->io_buffer_size(), 'A'));
     if (result == MockResourceLoader::Status::CALLBACK_PENDING)
       break;
     ASSERT_EQ(MockResourceLoader::Status::IDLE, result);
 
     ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   }
 
+  url_loader_client_.RunUntilResponseBodyArrived();
+  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
+
   while (true) {
     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);
     if (result == MOJO_RESULT_SHOULD_WAIT)
       break;
     ASSERT_EQ(MOJO_RESULT_OK, result);
   }
 
   handler_->set_end_write_expectation(MOJO_RESULT_SHOULD_WAIT);
   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());
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        OnWillReadWithLongContents) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   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));
 
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
-
   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(-1));
       const size_t to_be_written =
           std::min(static_cast<size_t>(mock_loader_->io_buffer_size()),
                    expected.size() - written);
 
       // Request should be resumed or paused.
       ASSERT_NE(MockResourceLoader::Status::CANCELED,
                 mock_loader_->OnReadCompleted(
                     expected.substr(written, to_be_written)));
 
       written += to_be_written;
     }
+    if (!url_loader_client_.response_body().is_valid()) {
+      url_loader_client_.RunUntilResponseBodyArrived();
+      ASSERT_TRUE(url_loader_client_.response_body().is_valid());
+    }
 
     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);
     if (result != MOJO_RESULT_SHOULD_WAIT) {
       ASSERT_EQ(MOJO_RESULT_OK, result);
       actual.append(buf, read_size);
     }
@@ skipping 25 matching lines @@
   ASSERT_EQ(MockResourceLoader::Status::CALLBACK_PENDING,
             mock_loader_->OnReadCompleted(
                 std::string(mock_loader_->io_buffer_size(), 'A')));
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        BeginWriteFailsOnResume) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnReadCompleted(0));
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
 
   while (true) {
     ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
     MockResourceLoader::Status result = mock_loader_->OnReadCompleted(
         std::string(mock_loader_->io_buffer_size(), 'A'));
     if (result == MockResourceLoader::Status::CALLBACK_PENDING)
       break;
     ASSERT_EQ(MockResourceLoader::Status::IDLE, result);
   }
+  url_loader_client_.RunUntilResponseBodyArrived();
+  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
+
   handler_->set_begin_write_expectation(MOJO_RESULT_UNKNOWN);
 
   while (mock_loader_->status() != MockResourceLoader::Status::CANCELED) {
     char buf[256];
     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);
     base::RunLoop().RunUntilIdle();
@@ skipping 126 matching lines @@
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnResponseStarted(
                 make_scoped_refptr(new ResourceResponse())));
 
   ASSERT_FALSE(url_loader_client_.has_received_response());
   url_loader_client_.RunUntilResponseReceived();
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
 
   ASSERT_FALSE(url_loader_client_.response_body().is_valid());
+
+  ASSERT_EQ(MockResourceLoader::Status::IDLE,
+            mock_loader_->OnReadCompleted("A"));
   url_loader_client_.RunUntilResponseBodyArrived();
   ASSERT_TRUE(url_loader_client_.response_body().is_valid());
 
-  ASSERT_EQ(MockResourceLoader::Status::IDLE,
-            mock_loader_->OnReadCompleted("A"));
   net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, net::OK);
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnResponseCompleted(status));
 
   ASSERT_FALSE(url_loader_client_.has_received_completion());
   url_loader_client_.RunUntilComplete();
   EXPECT_EQ(net::OK, url_loader_client_.completion_status().error_code);
 
   std::string body;
   while (true) {
@@ skipping 18 matching lines @@
 TEST_P(
     MojoAsyncResourceHandlerWithAllocationSizeTest,
     OnWillStartThenOnWillReadThenOnResponseStartedThenOnReadCompletedThenOnResponseCompleted) {
   rdh_delegate_.set_num_on_response_started_calls_expectation(1);
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnWillStart(request_->url()));
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
 
-  ASSERT_FALSE(url_loader_client_.response_body().is_valid());
-  url_loader_client_.RunUntilResponseBodyArrived();
-  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
-
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnResponseStarted(
                 make_scoped_refptr(new ResourceResponse())));
 
   ASSERT_FALSE(url_loader_client_.has_received_response());
   url_loader_client_.RunUntilResponseReceived();
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnReadCompleted("B"));
+
+  ASSERT_FALSE(url_loader_client_.response_body().is_valid());
+  url_loader_client_.RunUntilResponseBodyArrived();
+  ASSERT_TRUE(url_loader_client_.response_body().is_valid());
+
   net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, net::OK);
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnResponseCompleted(status));
 
   ASSERT_FALSE(url_loader_client_.has_received_completion());
   url_loader_client_.RunUntilComplete();
   EXPECT_EQ(net::OK, url_loader_client_.completion_status().error_code);
 
   std::string body;
   while (true) {
@@ skipping 12 matching lines @@
     }
   }
   EXPECT_EQ("B", body);
 }
 
 INSTANTIATE_TEST_CASE_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
                         MojoAsyncResourceHandlerWithAllocationSizeTest,
                         ::testing::Values(8, 32 * 2014));
 }  // namespace
 }  // namespace content