Add Pause/ResumeReadingBodyFromNet support to MojoAsyncResourceHandler.

This is needed in order to switch frame resource loading to use the new
safe_browsing::BrowserURLLoaderThrottle (once feature NavigationMojoResponse is
enabled).

Bug: 715673
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: If7919dbdd2ad0c8b09c38a0ba350f04c02ab5f3b
Reviewed-on: https://chromium-review.googlesource.com/887796
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533781}

content/browser/loader/mojo_async_resource_handler.cc

@@ -405,11 +405,11 @@ void MojoAsyncResourceHandler::SetPriority(net::RequestPriority priority,
 }
 
 void MojoAsyncResourceHandler::PauseReadingBodyFromNet() {
-  NOTREACHED();
+  ResourceHandler::PauseReadingBodyFromNet();
 }
 
 void MojoAsyncResourceHandler::ResumeReadingBodyFromNet() {
-  NOTREACHED();
+  ResourceHandler::ResumeReadingBodyFromNet();
 }
 
 void MojoAsyncResourceHandler::OnWritableForTesting() {

content/browser/loader/resource_handler.cc

@@ -14,6 +14,10 @@ ResourceHandler::Delegate::Delegate() {}
 
 ResourceHandler::Delegate::~Delegate() {}
 
+void ResourceHandler::Delegate::PauseReadingBodyFromNet() {}
+
+void ResourceHandler::Delegate::ResumeReadingBodyFromNet() {}
+
 void ResourceHandler::SetDelegate(Delegate* delegate) {
   delegate_ = delegate;
 }
@@ -52,6 +56,14 @@ void ResourceHandler::OutOfBandCancel(int error_code, bool tell_renderer) {
   delegate_->OutOfBandCancel(error_code, tell_renderer);
 }
 
+void ResourceHandler::PauseReadingBodyFromNet() {
+  delegate_->PauseReadingBodyFromNet();
+}
+
+void ResourceHandler::ResumeReadingBodyFromNet() {
+  delegate_->ResumeReadingBodyFromNet();
+}
+
 void ResourceHandler::GetNumericArg(const std::string& name, int* result) {
   const std::string& value =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(name);

content/browser/loader/resource_handler.h

@@ -67,6 +67,15 @@ class CONTENT_EXPORT ResourceHandler {
     // cancellation.
     virtual void OutOfBandCancel(int error_code, bool tell_renderer) = 0;
 
+    // Pauses/resumes reading response body if the resource is fetched from
+    // network. They could be no-ops if the resource is not fetched from
+    // network.
+    //
+    // It is allowed to call these methods before response body is available, or
+    // while the request is deferred.
+    virtual void PauseReadingBodyFromNet();
+    virtual void ResumeReadingBodyFromNet();
+
     DISALLOW_COPY_AND_ASSIGN(Delegate);
   };
 
@@ -162,6 +171,8 @@ class CONTENT_EXPORT ResourceHandler {
   // Cancels the request when the class does not currently have ownership of the
   // ResourceController.
   void OutOfBandCancel(int error_code, bool tell_renderer);
+  void PauseReadingBodyFromNet();
+  void ResumeReadingBodyFromNet();
 
   net::URLRequest* request() const { return request_; }
 

content/browser/loader/resource_loader.cc

@@ -45,6 +45,7 @@
 #include "net/url_request/url_request_status.h"
 #include "services/network/loader_util.h"
 #include "services/network/public/cpp/resource_response.h"
+#include "url/url_constants.h"
 
 using base::TimeDelta;
 using base::TimeTicks;
@@ -239,6 +240,25 @@ ResourceLoader::ResourceLoader(std::unique_ptr<net::URLRequest> request,
 }
 
 ResourceLoader::~ResourceLoader() {
+  if (update_body_read_before_paused_)
+    body_read_before_paused_ = request_->GetRawBodyBytes();
+  if (body_read_before_paused_ != -1) {
+    // Only record histograms for web schemes.
+    bool should_record_scheme = request_->url().SchemeIs(url::kHttpScheme) ||
+                                request_->url().SchemeIs(url::kHttpsScheme) ||
+                                request_->url().SchemeIs(url::kFtpScheme);
+    if (!request_->was_cached() && should_record_scheme) {
+      UMA_HISTOGRAM_COUNTS_1M("Network.URLLoader.BodyReadFromNetBeforePaused",
+                              body_read_before_paused_);
+    } else {
+      DVLOG(1) << "The request has been paused, but "
+               << "Network.URLLoader.BodyReadFromNetBeforePaused is not "
+               << "reported because the response body may not be from the "
+               << "network, or may be from cache. body_read_before_paused_: "
+               << body_read_before_paused_;
+    }
+  }
+
   if (login_delegate_.get())
     login_delegate_->OnRequestCancelled();
   ssl_client_auth_handler_.reset();
@@ -320,6 +340,44 @@ void ResourceLoader::OutOfBandCancel(int error_code, bool tell_renderer) {
   CancelRequestInternal(error_code, !tell_renderer);
 }
 
+void ResourceLoader::PauseReadingBodyFromNet() {
+  DVLOG(1) << "ResourceLoader pauses fetching response body for "
+           << request_->original_url().spec();
+
+  // Please note that reading the body is paused in all cases. Even if the URL
+  // request indicates that the response was cached, there could still be
+  // network activity involved. For example, the response was only partially
+  // cached. This also pauses things that don't come from the network (chrome
+  // URLs, file URLs, data URLs, etc.).
+  //
+  // On the other hand, BodyReadFromNetBeforePaused histogram is only reported
+  // when it is certain that the response body is read from the network and
+  // wasn't cached. This avoids polluting the histogram data.
+  should_pause_reading_body_ = true;
+
+  if (pending_read_) {
+    update_body_read_before_paused_ = true;
+  } else {
+    body_read_before_paused_ = request_->GetRawBodyBytes();
+  }
+}
+
+void ResourceLoader::ResumeReadingBodyFromNet() {
+  DVLOG(1) << "ResourceLoader resumes fetching response body for "
+           << request_->original_url().spec();
+
+  should_pause_reading_body_ = false;
+
+  if (read_more_body_supressed_) {
+    DCHECK(!is_deferred());
+    read_more_body_supressed_ = false;
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(&ResourceLoader::ReadMore,
+                                  weak_ptr_factory_.GetWeakPtr(),
+                                  false /* handle_result_asynchronously */));
+  }
+}
+
 void ResourceLoader::OnReceivedRedirect(net::URLRequest* unused,
                                         const net::RedirectInfo& redirect_info,
                                         bool* defer) {
@@ -449,6 +507,8 @@ void ResourceLoader::OnReadCompleted(net::URLRequest* unused, int bytes_read) {
   DVLOG(1) << "OnReadCompleted: \"" << request_->url().spec() << "\""
            << " bytes_read = " << bytes_read;
 
+  pending_read_ = false;
+
   // bytes_read == -1 always implies an error.
   if (bytes_read == -1 || !request_->status().is_success()) {
     ResponseCompleted();
@@ -716,6 +776,13 @@ void ResourceLoader::ReadMore(bool handle_result_async) {
   DCHECK(read_buffer_.get());
   DCHECK_GT(read_buffer_size_, 0);
 
+  if (should_pause_reading_body_) {
+    read_more_body_supressed_ = true;
+    return;
+  }
+
+  pending_read_ = true;
+
   int result = request_->Read(read_buffer_.get(), read_buffer_size_);
   // Have to do this after the Read call, to ensure it still has an outstanding
   // reference.
@@ -759,6 +826,11 @@ void ResourceLoader::CompleteRead(int bytes_read) {
   DCHECK(bytes_read >= 0);
   DCHECK(request_->status().is_success());
 
+  if (update_body_read_before_paused_) {
+    update_body_read_before_paused_ = false;
+    body_read_before_paused_ = request_->GetRawBodyBytes();
+  }
+
   ScopedDeferral scoped_deferral(
       this, bytes_read > 0 ? DEFERRED_READ : DEFERRED_RESPONSE_COMPLETE);
   handler_->OnReadCompleted(bytes_read, std::make_unique<Controller>(this));

content/browser/loader/resource_loader.h

@@ -60,6 +60,8 @@ class CONTENT_EXPORT ResourceLoader : public net::URLRequest::Delegate,
 
   // ResourceHandler::Delegate implementation:
   void OutOfBandCancel(int error_code, bool tell_renderer) override;
+  void PauseReadingBodyFromNet() override;
+  void ResumeReadingBodyFromNet() override;
 
  private:
   // ResourceController implementation for the ResourceLoader.
@@ -188,6 +190,24 @@ class CONTENT_EXPORT ResourceLoader : public net::URLRequest::Delegate,
 
   ResourceContext* resource_context_;
 
+  bool should_pause_reading_body_ = false;
+  // The request is not deferred (i.e., DEFERRED_NONE) and is ready to read more
+  // response body data. However, reading is paused because of
+  // PauseReadingBodyFromNet().
+  bool read_more_body_supressed_ = false;
+
+  // Whether to update |body_read_before_paused_| after the pending read is
+  // completed (or when this resource loader is destroyed).
+  bool update_body_read_before_paused_ = false;
+  // The number of bytes obtained by the reads initiated before the last
+  // PauseReadingBodyFromNet() call. -1 means the request hasn't been paused.
+  // The body may be read from cache or network. So even if this value is not
+  // -1, we still need to check whether it is from network before reporting it
+  // as BodyReadFromNetBeforePaused.
+  int64_t body_read_before_paused_ = -1;
+
+  bool pending_read_ = false;
+
   base::ThreadChecker thread_checker_;
 
   base::WeakPtrFactory<ResourceLoader> weak_ptr_factory_;

content/browser/loader/resource_loader_unittest.cc

@@ -16,6 +16,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -45,6 +46,7 @@
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_private_key.h"
 #include "net/test/cert_test_util.h"
+#include "net/test/embedded_test_server/controllable_http_response.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/test_data_directory.h"
 #include "net/test/url_request/url_request_failed_job.h"
@@ -62,6 +64,9 @@
 namespace content {
 namespace {
 
+constexpr char kBodyReadFromNetBeforePausedHistogram[] =
+    "Network.URLLoader.BodyReadFromNetBeforePaused";
+
 // Stub client certificate store that returns a preset list of certificates for
 // each request and records the arguments of the most recent request for later
 // inspection.
@@ -352,6 +357,26 @@ class NonChunkedUploadDataStream : public net::UploadDataStream {
   DISALLOW_COPY_AND_ASSIGN(NonChunkedUploadDataStream);
 };
 
+// Returns whether monitoring was successfully set up. If yes,
+// StopMonitorBodyReadFromNetBeforePausedHistogram() needs to be called later to
+// stop monitoring.
+//
+// |*output_sample| needs to stay valid until monitoring is stopped.
+WARN_UNUSED_RESULT bool StartMonitorBodyReadFromNetBeforePausedHistogram(
+    base::HistogramBase::Sample* output_sample) {
+  return base::StatisticsRecorder::SetCallback(
+      kBodyReadFromNetBeforePausedHistogram,
+      base::BindRepeating(
+          [](base::HistogramBase::Sample* output,
+             base::HistogramBase::Sample sample) { *output = sample; },
+          output_sample));
+}
+
+void StopMonitorBodyReadFromNetBeforePausedHistogram() {
+  base::StatisticsRecorder::ClearCallback(
+      kBodyReadFromNetBeforePausedHistogram);
+}
+
 }  // namespace
 
 class ResourceLoaderTest : public testing::Test,
@@ -1598,4 +1623,156 @@ TEST_F(EffectiveConnectionTypeResourceLoaderTest, DoesNotBelongToMainFrame) {
                                 net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN);
 }
 
+TEST_F(ResourceLoaderTest, PauseReadingBodyFromNetBeforeRespnoseHeaders) {
+  static constexpr char kPath[] = "/hello.html";
+  static constexpr char kBodyContents[] = "This is the data as you requested.";
+
+  base::HistogramBase::Sample output_sample = -1;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(&output_sample));
+
+  net::EmbeddedTestServer server;
+  net::test_server::ControllableHttpResponse response_controller(&server,
+                                                                 kPath);
+  ASSERT_TRUE(server.Start());
+
+  SetUpResourceLoaderForUrl(server.GetURL(kPath));
+  loader_->StartRequest();
+
+  // Pausing reading response body from network stops future reads from the
+  // underlying URLRequest. So no data should be sent using the response body
+  // data pipe.
+  loader_->PauseReadingBodyFromNet();
+
+  response_controller.WaitForRequest();
+  response_controller.Send(
+      "HTTP/1.1 200 OK\r\n"
+      "Content-Type: text/plain\r\n\r\n" +
+      std::string(kBodyContents));
+  response_controller.Done();
+
+  // We will still receive the response header, although there won't be any data
+  // available until ResumeReadBodyFromNet() is called.
+  raw_ptr_resource_handler_->WaitUntilResponseStarted();
+  EXPECT_EQ(1, raw_ptr_resource_handler_->on_response_started_called());
+  EXPECT_EQ(0, raw_ptr_resource_handler_->on_read_completed_called());
+
+  // Wait for a little amount of time so that if the loader mistakenly reads
+  // response body from the underlying URLRequest, it is easier to find out.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(),
+      base::TimeDelta::FromMilliseconds(100));
+  run_loop.Run();
+
+  EXPECT_TRUE(raw_ptr_resource_handler_->body().empty());
+
+  loader_->ResumeReadingBodyFromNet();
+  raw_ptr_resource_handler_->WaitUntilResponseComplete();
+
+  EXPECT_EQ(kBodyContents, raw_ptr_resource_handler_->body());
+
+  loader_.reset();
+  EXPECT_EQ(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
+}
+
+TEST_F(ResourceLoaderTest, PauseReadingBodyFromNetWhenReadIsPending) {
+  static constexpr char kPath[] = "/hello.html";
+  static constexpr char kBodyContentsFirstHalf[] = "This is the first half.";
+  static constexpr char kBodyContentsSecondHalf[] = "This is the second half.";
+
+  base::HistogramBase::Sample output_sample = -1;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(&output_sample));
+
+  net::EmbeddedTestServer server;
+  net::test_server::ControllableHttpResponse response_controller(&server,
+                                                                 kPath);
+  ASSERT_TRUE(server.Start());
+
+  SetUpResourceLoaderForUrl(server.GetURL(kPath));
+  loader_->StartRequest();
+
+  response_controller.WaitForRequest();
+  response_controller.Send(
+      "HTTP/1.1 200 OK\r\n"
+      "Content-Type: text/plain\r\n\r\n" +
+      std::string(kBodyContentsFirstHalf));
+
+  raw_ptr_resource_handler_->WaitUntilResponseStarted();
+  EXPECT_EQ(1, raw_ptr_resource_handler_->on_response_started_called());
+
+  loader_->PauseReadingBodyFromNet();
+
+  response_controller.Send(kBodyContentsSecondHalf);
+  response_controller.Done();
+
+  // It is uncertain how much data has been read before reading is actually
+  // paused, because if there is a pending read when PauseReadingBodyFromNet()
+  // arrives, the pending read won't be cancelled. Therefore, this test only
+  // checks that after ResumeReadingBodyFromNet() we should be able to get the
+  // whole response body.
+  loader_->ResumeReadingBodyFromNet();
+  raw_ptr_resource_handler_->WaitUntilResponseComplete();
+
+  EXPECT_EQ(std::string(kBodyContentsFirstHalf) +
+                std::string(kBodyContentsSecondHalf),
+            raw_ptr_resource_handler_->body());
+
+  loader_.reset();
+  EXPECT_LE(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
+}
+
+TEST_F(ResourceLoaderTest, MultiplePauseResumeReadingBodyFromNet) {
+  static constexpr char kPath[] = "/hello.html";
+  static constexpr char kBodyContentsFirstHalf[] = "This is the first half.";
+  static constexpr char kBodyContentsSecondHalf[] = "This is the second half.";
+
+  base::HistogramBase::Sample output_sample = -1;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(&output_sample));
+
+  net::EmbeddedTestServer server;
+  net::test_server::ControllableHttpResponse response_controller(&server,
+                                                                 kPath);
+  ASSERT_TRUE(server.Start());
+
+  SetUpResourceLoaderForUrl(server.GetURL(kPath));
+  loader_->StartRequest();
+
+  // It is okay to call ResumeReadingBodyFromNet() even if there is no prior
+  // PauseReadingBodyFromNet().
+  loader_->ResumeReadingBodyFromNet();
+
+  response_controller.WaitForRequest();
+  response_controller.Send(
+      "HTTP/1.1 200 OK\r\n"
+      "Content-Type: text/plain\r\n\r\n" +
+      std::string(kBodyContentsFirstHalf));
+
+  loader_->PauseReadingBodyFromNet();
+
+  raw_ptr_resource_handler_->WaitUntilResponseStarted();
+  EXPECT_EQ(1, raw_ptr_resource_handler_->on_response_started_called());
+
+  loader_->PauseReadingBodyFromNet();
+  loader_->PauseReadingBodyFromNet();
+
+  response_controller.Send(kBodyContentsSecondHalf);
+  response_controller.Done();
+
+  // One ResumeReadingBodyFromNet() call will resume reading even if there are
+  // multiple PauseReadingBodyFromNet() calls before it.
+  loader_->ResumeReadingBodyFromNet();
+
+  raw_ptr_resource_handler_->WaitUntilResponseComplete();
+
+  EXPECT_EQ(std::string(kBodyContentsFirstHalf) +
+                std::string(kBodyContentsSecondHalf),
+            raw_ptr_resource_handler_->body());
+
+  loader_.reset();
+  EXPECT_LE(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
+}
+
 }  // namespace content

content/browser/loader/test_resource_handler.cc

@@ -91,6 +91,8 @@ void TestResourceHandler::OnResponseStarted(
   EXPECT_FALSE(resource_response_);
   resource_response_ = response;
 
+  response_started_run_loop_.Quit();
+
   if (!on_response_started_result_) {
     canceled_ = true;
     controller->Cancel();
@@ -291,6 +293,10 @@ void TestResourceHandler::WaitUntilDeferred() {
   deferred_run_loop_.reset(new base::RunLoop());
 }
 
+void TestResourceHandler::WaitUntilResponseStarted() {
+  response_started_run_loop_.Run();
+}
+
 void TestResourceHandler::WaitUntilResponseComplete() {
   response_complete_run_loop_.Run();
 }

content/browser/loader/test_resource_handler.h

@@ -168,6 +168,7 @@ class TestResourceHandler : public ResourceHandler {
   // method, behavior is undefined.
   void WaitUntilDeferred();
 
+  void WaitUntilResponseStarted();
   void WaitUntilResponseComplete();
 
   // Returns a weak pointer to |this|.  Allows testing object lifetime.
@@ -226,6 +227,8 @@ class TestResourceHandler : public ResourceHandler {
 
   std::unique_ptr<base::RunLoop> deferred_run_loop_;
 
+  base::RunLoop response_started_run_loop_;
+
   base::RunLoop response_complete_run_loop_;
 
   base::WeakPtrFactory<TestResourceHandler> weak_ptr_factory_;

services/network/public/interfaces/url_loader.mojom

@@ -59,12 +59,21 @@ interface URLLoader {
   // intra priority value is maintained.
   SetPriority(RequestPriority priority, int32 intra_priority_value);
 
-  // If a URLLoader fetches the resource from network, it should handle these
-  // methods to pause/resume reading the response body from network. Otherwise,
-  // these methods could be no-ops.
+  // If the resource is being fetched from the network,
+  // PauseReadingBodyFromNet() pauses fetching the response body. It is okay to
+  // call this method before fetching the body starts, too.
+  // ResumeReadingBodyFromNet() resumes fetching the body if it has been paused.
   //
-  // It is allowed to call Pause/ResumeReadingBodyFromNet before response body
-  // is available.
+  // Note that PauseReadingBodyFromNet() is asynchronous and only gurantees to
+  // pause if the response body is fetched from the network. This means:
+  // - any data in flight before PauseReadingBodyFromNet() is processed will
+  //   still be passed to the client data pipe.
+  // - a response body not from the network, e.g. from blob, may not be paused
+  //   at all.
+  //
+  // Redundant calls to these methods are ingored. It is not required to match
+  // pause and resume calls. It is not an error to resume a non-paused request,
+  // or pause a request multiple times.
   PauseReadingBodyFromNet();
   ResumeReadingBodyFromNet();
 };

services/network/url_loader.cc

@@ -306,21 +306,8 @@ URLLoader::URLLoader(NetworkContext* context,
 }
 
 URLLoader::~URLLoader() {
+  RecordBodyReadFromNetBeforePausedIfNeeded();
   context_->DeregisterURLLoader(this);
-
-  if (update_body_read_before_paused_)
-    UpdateBodyReadBeforePaused();
-  if (body_read_before_paused_ != -1) {
-    if (!body_may_be_from_cache_) {
-      UMA_HISTOGRAM_COUNTS_1M("Network.URLLoader.BodyReadFromNetBeforePaused",
-                              body_read_before_paused_);
-    } else {
-      DVLOG(1) << "The request has been paused, but "
-               << "Network.URLLoader.BodyReadFromNetBeforePaused is not "
-               << "reported because the response body may be from cache. "
-               << "body_read_before_paused_: " << body_read_before_paused_;
-    }
-  }
 }
 
 void URLLoader::Cleanup() {
@@ -352,6 +339,10 @@ void URLLoader::PauseReadingBodyFromNet() {
   DVLOG(1) << "URLLoader pauses fetching response body for "
            << (url_request_ ? url_request_->original_url().spec()
                             : "a URL that has completed loading or failed.");
+
+  if (!url_request_)
+    return;
+
   // Please note that we pause reading body in all cases. Even if the URL
   // request indicates that the response was cached, there could still be
   // network activity involved. For example, the response was only partially
@@ -362,10 +353,12 @@ void URLLoader::PauseReadingBodyFromNet() {
   // avoids polluting the histogram data with data points from cached responses.
   should_pause_reading_body_ = true;
 
-  if (url_request_ && url_request_->status().is_io_pending()) {
+  // If the data pipe has been set up and the request is in IO pending state,
+  // there is a pending read for the response body.
+  if (HasDataPipe() && url_request_->status().is_io_pending()) {
     update_body_read_before_paused_ = true;
   } else {
-    UpdateBodyReadBeforePaused();
+    body_read_before_paused_ = url_request_->GetRawBodyBytes();
   }
 }
 
@@ -463,8 +456,6 @@ void URLLoader::OnResponseStarted(net::URLRequest* url_request, int net_error) {
     raw_response_headers_ = nullptr;
   }
 
-  body_may_be_from_cache_ = url_request_->was_cached();
-
   mojo::DataPipe data_pipe(kDefaultAllocationSize);
   response_body_stream_ = std::move(data_pipe.producer_handle);
   consumer_handle_ = std::move(data_pipe.consumer_handle);
@@ -542,7 +533,7 @@ void URLLoader::DidRead(int num_bytes, bool completed_synchronously) {
     pending_write_buffer_offset_ += num_bytes;
   if (update_body_read_before_paused_) {
     update_body_read_before_paused_ = false;
-    UpdateBodyReadBeforePaused();
+    body_read_before_paused_ = url_request_->GetRawBodyBytes();
   }
 
   bool complete_read = true;
@@ -646,14 +637,14 @@ void URLLoader::OnResponseBodyStreamReady(MojoResult result) {
 }
 
 void URLLoader::CloseResponseBodyStreamProducer() {
+  RecordBodyReadFromNetBeforePausedIfNeeded();
+
   url_request_.reset();
   peer_closed_handle_watcher_.Cancel();
   writable_handle_watcher_.Cancel();
   response_body_stream_.reset();
 
-  // |pending_write_buffer_offset_| is intentionally not reset, so that
-  // |total_written_bytes_ + pending_write_buffer_offset_| always reflects the
-  // total bytes read from |url_request_|.
+  pending_write_buffer_offset_ = 0;
   pending_write_ = nullptr;
 
   // Make sure if a ResumeReadingBodyFromNet() call is received later, we don't
@@ -664,8 +655,7 @@ void URLLoader::CloseResponseBodyStreamProducer() {
 }
 
 void URLLoader::DeleteIfNeeded() {
-  bool has_data_pipe = pending_write_.get() || response_body_stream_.is_valid();
-  if (!connected_ && !has_data_pipe)
+  if (!connected_ && !HasDataPipe())
     delete this;
 }
 
@@ -703,13 +693,6 @@ void URLLoader::SetRawResponseHeaders(
   raw_response_headers_ = headers;
 }
 
-void URLLoader::UpdateBodyReadBeforePaused() {
-  DCHECK_GE(pending_write_buffer_offset_ + total_written_bytes_,
-            body_read_before_paused_);
-  body_read_before_paused_ =
-      pending_write_buffer_offset_ + total_written_bytes_;
-}
-
 void URLLoader::SendUploadProgress(const net::UploadProgress& progress) {
   url_loader_client_->OnUploadProgress(
       progress.position(), progress.size(),
@@ -755,4 +738,27 @@ void URLLoader::OnCertificateRequestedResponse(
   }
 }
 
+bool URLLoader::HasDataPipe() const {
+  return pending_write_ || response_body_stream_.is_valid();
+}
+
+void URLLoader::RecordBodyReadFromNetBeforePausedIfNeeded() {
+  if (!url_request_)
+    return;
+
+  if (update_body_read_before_paused_)
+    body_read_before_paused_ = url_request_->GetRawBodyBytes();
+  if (body_read_before_paused_ != -1) {
+    if (!url_request_->was_cached()) {
+      UMA_HISTOGRAM_COUNTS_1M("Network.URLLoader.BodyReadFromNetBeforePaused",
+                              body_read_before_paused_);
+    } else {
+      DVLOG(1) << "The request has been paused, but "
+               << "Network.URLLoader.BodyReadFromNetBeforePaused is not "
+               << "reported because the response body may be from cache. "
+               << "body_read_before_paused_: " << body_read_before_paused_;
+    }
+  }
+}
+
 }  // namespace network

services/network/url_loader.h

@@ -85,7 +85,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
   void SendResponseToClient();
   void CompletePendingWrite();
   void SetRawResponseHeaders(scoped_refptr<const net::HttpResponseHeaders>);
-  void UpdateBodyReadBeforePaused();
   void SendUploadProgress(const net::UploadProgress& progress);
   void OnUploadProgressACK();
   void OnSSLCertificateErrorResponse(const net::SSLInfo& ssl_info,
@@ -95,6 +94,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
       const std::vector<uint16_t>& algorithm_preferences,
       mojom::SSLPrivateKeyPtr ssl_private_key,
       bool cancel_certificate_selection);
+  bool HasDataPipe() const;
+  void RecordBodyReadFromNetBeforePausedIfNeeded();
 
   NetworkContext* context_;
   int32_t options_;
@@ -130,8 +131,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
   // The response body stream is open, but transferring data is paused.
   bool paused_reading_body_ = false;
 
-  // Set to true if the response body may be read from cache.
-  bool body_may_be_from_cache_ = false;
   // Whether to update |body_read_before_paused_| after the pending read is
   // completed (or when the response body stream is closed).
   bool update_body_read_before_paused_ = false;

services/network/url_loader_unittest.cc

@@ -13,7 +13,9 @@
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
@@ -52,6 +54,9 @@ namespace network {
 
 namespace {
 
+constexpr char kBodyReadFromNetBeforePausedHistogram[] =
+    "Network.URLLoader.BodyReadFromNetBeforePaused";
+
 static ResourceRequest CreateResourceRequest(const char* method,
                                              const GURL& url) {
   ResourceRequest request;
@@ -172,6 +177,31 @@ class RequestInterceptor : public net::URLRequestInterceptor {
   DISALLOW_COPY_AND_ASSIGN(RequestInterceptor);
 };
 
+// Returns whether monitoring was successfully set up. If yes,
+// StopMonitorBodyReadFromNetBeforePausedHistogram() needs to be called later to
+// stop monitoring.
+//
+// |*output_sample| needs to stay valid until monitoring is stopped.
+WARN_UNUSED_RESULT bool StartMonitorBodyReadFromNetBeforePausedHistogram(
+    const base::RepeatingClosure& quit_closure,
+    base::HistogramBase::Sample* output_sample) {
+  return base::StatisticsRecorder::SetCallback(
+      kBodyReadFromNetBeforePausedHistogram,
+      base::BindRepeating(
+          [](const base::RepeatingClosure& quit_closure,
+             base::HistogramBase::Sample* output,
+             base::HistogramBase::Sample sample) {
+            *output = sample;
+            quit_closure.Run();
+          },
+          quit_closure, output_sample));
+}
+
+void StopMonitorBodyReadFromNetBeforePausedHistogram() {
+  base::StatisticsRecorder::ClearCallback(
+      kBodyReadFromNetBeforePausedHistogram);
+}
+
 }  // namespace
 
 class URLLoaderTest : public testing::Test {
@@ -754,6 +784,11 @@ TEST_F(URLLoaderTest, PauseReadingBodyFromNetBeforeRespnoseHeaders) {
   const char* const kPath = "/hello.html";
   const char* const kBodyContents = "This is the data as you requested.";
 
+  base::HistogramBase::Sample output_sample = -1;
+  base::RunLoop histogram_run_loop;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(
+      histogram_run_loop.QuitClosure(), &output_sample));
+
   net::EmbeddedTestServer server;
   net::test_server::ControllableHttpResponse response_controller(&server,
                                                                  kPath);
@@ -804,6 +839,12 @@ TEST_F(URLLoaderTest, PauseReadingBodyFromNetBeforeRespnoseHeaders) {
 
   available_data = ReadBody();
   EXPECT_EQ(kBodyContents, available_data);
+
+  loader.reset();
+  client()->Unbind();
+  histogram_run_loop.Run();
+  EXPECT_EQ(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
 }
 
 TEST_F(URLLoaderTest, PauseReadingBodyFromNetWhenReadIsPending) {
@@ -811,6 +852,11 @@ TEST_F(URLLoaderTest, PauseReadingBodyFromNetWhenReadIsPending) {
   const char* const kBodyContentsFirstHalf = "This is the first half.";
   const char* const kBodyContentsSecondHalf = "This is the second half.";
 
+  base::HistogramBase::Sample output_sample = -1;
+  base::RunLoop histogram_run_loop;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(
+      histogram_run_loop.QuitClosure(), &output_sample));
+
   net::EmbeddedTestServer server;
   net::test_server::ControllableHttpResponse response_controller(&server,
                                                                  kPath);
@@ -851,12 +897,23 @@ TEST_F(URLLoaderTest, PauseReadingBodyFromNetWhenReadIsPending) {
   EXPECT_EQ(std::string(kBodyContentsFirstHalf) +
                 std::string(kBodyContentsSecondHalf),
             ReadBody());
+
+  loader.reset();
+  client()->Unbind();
+  histogram_run_loop.Run();
+  EXPECT_LE(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
 }
 
 TEST_F(URLLoaderTest, ResumeReadingBodyFromNetAfterClosingConsumer) {
   const char* const kPath = "/hello.html";
   const char* const kBodyContentsFirstHalf = "This is the first half.";
 
+  base::HistogramBase::Sample output_sample = -1;
+  base::RunLoop histogram_run_loop;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(
+      histogram_run_loop.QuitClosure(), &output_sample));
+
   net::EmbeddedTestServer server;
   net::test_server::ControllableHttpResponse response_controller(&server,
                                                                  kPath);
@@ -890,6 +947,12 @@ TEST_F(URLLoaderTest, ResumeReadingBodyFromNetAfterClosingConsumer) {
   // made after the response body data pipe is closed.
   loader->ResumeReadingBodyFromNet();
   loader.FlushForTesting();
+
+  loader.reset();
+  client()->Unbind();
+  histogram_run_loop.Run();
+  EXPECT_EQ(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
 }
 
 TEST_F(URLLoaderTest, MultiplePauseResumeReadingBodyFromNet) {
@@ -897,6 +960,11 @@ TEST_F(URLLoaderTest, MultiplePauseResumeReadingBodyFromNet) {
   const char* const kBodyContentsFirstHalf = "This is the first half.";
   const char* const kBodyContentsSecondHalf = "This is the second half.";
 
+  base::HistogramBase::Sample output_sample = -1;
+  base::RunLoop histogram_run_loop;
+  EXPECT_TRUE(StartMonitorBodyReadFromNetBeforePausedHistogram(
+      histogram_run_loop.QuitClosure(), &output_sample));
+
   net::EmbeddedTestServer server;
   net::test_server::ControllableHttpResponse response_controller(&server,
                                                                  kPath);
@@ -943,6 +1011,12 @@ TEST_F(URLLoaderTest, MultiplePauseResumeReadingBodyFromNet) {
   EXPECT_EQ(std::string(kBodyContentsFirstHalf) +
                 std::string(kBodyContentsSecondHalf),
             ReadBody());
+
+  loader.reset();
+  client()->Unbind();
+  histogram_run_loop.Run();
+  EXPECT_LE(0, output_sample);
+  StopMonitorBodyReadFromNetBeforePausedHistogram();
 }
 
 TEST_F(URLLoaderTest, UploadBytes) {