Precache per-resource cap should be applied on network bytes used

Currently precache fetches are cancelled when response length reaches the per-resource limit. This is incorrect for gzipped resources. BUG=625270 Review-Url: https://codereview.chromium.org/2303653002 Cr-Commit-Position: refs/heads/master@{#421101}
Denger-Network · Sep 27, 2016 · dc12c3d · dc12c3d
1 parent 930e066
commit dc12c3d
Show file tree

Hide file tree

Showing 29 changed files with 83 additions and 51 deletions.
diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
@@ -108,7 +108,8 @@ void WriteFromUrlOperation::OnURLFetchUploadProgress(
 void WriteFromUrlOperation::OnURLFetchDownloadProgress(
     const net::URLFetcher* source,
     int64_t current,
-    int64_t total) {
+    int64_t total,
+    int64_t current_network_bytes) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   if (IsCancelled()) {

diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
@@ -56,7 +56,8 @@ class WriteFromUrlOperation : public Operation, public net::URLFetcherDelegate {
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
   void OnURLFetchUploadProgress(const net::URLFetcher* source,
                                 int64_t current,
                                 int64_t total) override;

diff --git a/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc b/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc
@@ -243,7 +243,8 @@ TEST_F(ComponentCloudPolicyUpdaterTest, PolicyDataInvalid) {
   EXPECT_EQ(GURL(kTestDownload2), fetcher->GetOriginalURL());
 
   // Indicate that the policy data size will exceed allowed maximum.
-  fetcher->delegate()->OnURLFetchDownloadProgress(fetcher, 6 * 1024 * 1024, -1);
+  fetcher->delegate()->OnURLFetchDownloadProgress(fetcher, 6 * 1024 * 1024, -1,
+                                                  6 * 1024 * 1024);
   task_runner_->RunUntilIdle();
 
   // Verify that the third download has been started.

diff --git a/components/policy/core/common/cloud/external_policy_data_fetcher.cc b/components/policy/core/common/cloud/external_policy_data_fetcher.cc
@@ -253,7 +253,8 @@ void ExternalPolicyDataFetcherBackend::OnURLFetchComplete(
 void ExternalPolicyDataFetcherBackend::OnURLFetchDownloadProgress(
     const net::URLFetcher* source,
     int64_t current,
-    int64_t total) {
+    int64_t total,
+    int64_t current_network_bytes) {
   DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
   auto it = job_map_.find(source);
   DCHECK(it != job_map_.end());

diff --git a/components/policy/core/common/cloud/external_policy_data_fetcher.h b/components/policy/core/common/cloud/external_policy_data_fetcher.h
@@ -163,7 +163,8 @@ class POLICY_EXPORT ExternalPolicyDataFetcherBackend
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
 
  private:
   scoped_refptr<base::SequencedTaskRunner> io_task_runner_;

diff --git a/components/policy/core/common/cloud/external_policy_data_fetcher_unittest.cc b/components/policy/core/common/cloud/external_policy_data_fetcher_unittest.cc
@@ -158,9 +158,8 @@ TEST_F(ExternalPolicyDataFetcherTest, MaxSizeExceeded) {
 
   // Indicate that the data size will exceed maximum allowed.
   fetcher->delegate()->OnURLFetchDownloadProgress(
-      fetcher,
-      kExternalPolicyDataMaxSize + 1,
-      -1);
+      fetcher, kExternalPolicyDataMaxSize + 1, -1,
+      kExternalPolicyDataMaxSize + 1);
 
   // Verify that the fetch is no longer running.
   EXPECT_FALSE(fetcher_factory_.GetFetcherByID(0));

diff --git a/components/policy/core/common/cloud/external_policy_data_updater_unittest.cc b/components/policy/core/common/cloud/external_policy_data_updater_unittest.cc
@@ -178,9 +178,8 @@ TEST_F(ExternalPolicyDataUpdaterTest, PayloadSizeExceedsLimit) {
 
   // Indicate that the payload size will exceed allowed maximum.
   fetcher->delegate()->OnURLFetchDownloadProgress(
-      fetcher,
-      kExternalPolicyDataMaxSize + 1,
-      -1);
+      fetcher, kExternalPolicyDataMaxSize + 1, -1,
+      kExternalPolicyDataMaxSize + 1);
   backend_task_runner_->RunPendingTasks();
   io_task_runner_->RunUntilIdle();
 

diff --git a/components/precache/core/fetcher_pool_unittest.cc b/components/precache/core/fetcher_pool_unittest.cc
@@ -39,8 +39,11 @@ class MockURLFetcherDelegate : public net::URLFetcherDelegate {
   virtual ~MockURLFetcherDelegate() {};
 
   MOCK_METHOD1(OnURLFetchComplete, void(const URLFetcher*));
-  MOCK_METHOD3(OnURLFetchDownloadProgress,
-               void(const URLFetcher* source, int64_t current, int64_t total));
+  MOCK_METHOD4(OnURLFetchDownloadProgress,
+               void(const URLFetcher* source,
+                    int64_t current,
+                    int64_t total,
+                    int64_t current_network_bytes));
   MOCK_METHOD3(OnURLFetchUploadProgress,
                void(const URLFetcher* source, int64_t current, int64_t total));
 };

diff --git a/components/precache/core/precache_fetcher.cc b/components/precache/core/precache_fetcher.cc
@@ -286,23 +286,25 @@ void PrecacheFetcher::Fetcher::LoadFromNetwork() {
 void PrecacheFetcher::Fetcher::OnURLFetchDownloadProgress(
     const net::URLFetcher* source,
     int64_t current,
-    int64_t total) {
-  // If going over the per-resource download cap.
+    int64_t total,
+    int64_t current_network_bytes) {
+  // If network bytes going over the per-resource download cap.
   if (fetch_stage_ == FetchStage::NETWORK &&
-      // |current| is guaranteed to be non-negative, so this cast is safe.
-      static_cast<size_t>(std::max(current, total)) > max_bytes_) {
+      // |current_network_bytes| is guaranteed to be non-negative, so this cast
+      // is safe.
+      static_cast<size_t>(current_network_bytes) > max_bytes_) {
     VLOG(1) << "Cancelling " << url_ << ": (" << current << "/" << total
             << ") is over " << max_bytes_;
 
     // Call the completion callback, to attempt the next download, or to trigger
     // cleanup in precache_delegate_->OnDone().
-    response_bytes_ = network_response_bytes_ = current;
+    response_bytes_ = current;
+    network_response_bytes_ = current_network_bytes;
     was_cached_ = source->WasCached();
 
     UMA_HISTOGRAM_CUSTOM_COUNTS("Precache.Fetch.ResponseBytes.NetworkWasted",
                                 network_response_bytes_, 1,
                                 1024 * 1024 /* 1 MB */, 100);
-
     // Cancel the download.
     network_url_fetcher_.reset();
     callback_.Run(*this);

diff --git a/components/precache/core/precache_fetcher.h b/components/precache/core/precache_fetcher.h
@@ -264,7 +264,8 @@ class PrecacheFetcher::Fetcher : public net::URLFetcherDelegate {
   ~Fetcher() override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   int64_t response_bytes() const { return response_bytes_; }
   int64_t network_response_bytes() const { return network_response_bytes_; }

diff --git a/components/search_provider_logos/logo_tracker.cc b/components/search_provider_logos/logo_tracker.cc
@@ -341,7 +341,8 @@ void LogoTracker::OnURLFetchComplete(const net::URLFetcher* source) {
 
 void LogoTracker::OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                              int64_t current,
-                                             int64_t total) {
+                                             int64_t total,
+                                             int64_t current_network_bytes) {
   if (total > kMaxDownloadBytes || current > kMaxDownloadBytes) {
     LOG(WARNING) << "Search provider logo exceeded download size limit";
     ReturnToIdle(DOWNLOAD_OUTCOME_DOWNLOAD_FAILED);

diff --git a/components/search_provider_logos/logo_tracker.h b/components/search_provider_logos/logo_tracker.h
@@ -204,7 +204,8 @@ class LogoTracker : public net::URLFetcherDelegate {
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
 
   // The URL from which the logo is fetched.
   GURL logo_url_;

diff --git a/components/sync/core/http_bridge.cc b/components/sync/core/http_bridge.cc
@@ -370,7 +370,8 @@ void HttpBridge::OnURLFetchComplete(const net::URLFetcher* source) {
 
 void HttpBridge::OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                             int64_t current,
-                                            int64_t total) {
+                                            int64_t total,
+                                            int64_t current_network_bytes) {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
   // Reset the delay when forward progress is made.
   base::AutoLock lock(fetch_state_lock_);

diff --git a/components/sync/core/http_bridge.h b/components/sync/core/http_bridge.h
@@ -76,7 +76,8 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>,
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
   void OnURLFetchUploadProgress(const net::URLFetcher* source,
                                 int64_t current,
                                 int64_t total) override;

diff --git a/components/update_client/url_fetcher_downloader.cc b/components/update_client/url_fetcher_downloader.cc
@@ -98,7 +98,8 @@ void UrlFetcherDownloader::OnURLFetchComplete(const net::URLFetcher* source) {
 void UrlFetcherDownloader::OnURLFetchDownloadProgress(
     const net::URLFetcher* source,
     int64_t current,
-    int64_t total) {
+    int64_t total,
+    int64_t current_network_bytes) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   downloaded_bytes_ = current;

diff --git a/components/update_client/url_fetcher_downloader.h b/components/update_client/url_fetcher_downloader.h
@@ -46,7 +46,8 @@ class UrlFetcherDownloader : public CrxDownloader,
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
   std::unique_ptr<net::URLFetcher> url_fetcher_;
   net::URLRequestContextGetter* context_getter_;
 

diff --git a/content/browser/speech/speech_recognition_engine.cc b/content/browser/speech/speech_recognition_engine.cc
@@ -136,7 +136,8 @@ void SpeechRecognitionEngine::OnURLFetchComplete(const URLFetcher* source) {
 void SpeechRecognitionEngine::OnURLFetchDownloadProgress(
     const URLFetcher* source,
     int64_t current,
-    int64_t total) {
+    int64_t total,
+    int64_t current_network_bytes) {
   const bool kPartialResponse = false;
   DispatchHTTPResponse(source, kPartialResponse);
 }

diff --git a/content/browser/speech/speech_recognition_engine.h b/content/browser/speech/speech_recognition_engine.h
@@ -119,7 +119,8 @@ class CONTENT_EXPORT SpeechRecognitionEngine
   void OnURLFetchComplete(const net::URLFetcher* source) override;
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
 
  private:
   Delegate* delegate_;

diff --git a/content/browser/speech/speech_recognition_engine_unittest.cc b/content/browser/speech/speech_recognition_engine_unittest.cc
@@ -509,9 +509,8 @@ void SpeechRecognitionEngineTest::ProvideMockProtoResultDownstream(
   response_buffer_.append(response_string);
   downstream_fetcher->SetResponseString(response_buffer_);
   downstream_fetcher->delegate()->OnURLFetchDownloadProgress(
-      downstream_fetcher,
-      response_buffer_.size(),
-      -1 /* total response length not used */);
+      downstream_fetcher, response_buffer_.size(),
+      -1 /* total response length not used */, response_buffer_.size());
 }
 
 void SpeechRecognitionEngineTest::ProvideMockResultDownstream(

diff --git a/content/test/mock_google_streaming_server.cc b/content/test/mock_google_streaming_server.cc
@@ -137,7 +137,7 @@ void MockGoogleStreamingServer::SimulateServerResponse(
       net::URLRequestStatus::FromError(success ? net::OK : net::ERR_FAILED));
   fetcher->set_response_code(success ? 200 : 500);
   fetcher->SetResponseString(http_response);
-  fetcher->delegate()->OnURLFetchDownloadProgress(fetcher, 0, 0);
+  fetcher->delegate()->OnURLFetchDownloadProgress(fetcher, 0, 0, 0);
 }
 
 // Can return NULL if the SpeechRecognizer has not requested the connection yet.

diff --git a/google_apis/drive/base_requests.cc b/google_apis/drive/base_requests.cc
@@ -984,7 +984,8 @@ void DownloadFileRequestBase::GetOutputFilePath(
 void DownloadFileRequestBase::OnURLFetchDownloadProgress(
     const URLFetcher* source,
     int64_t current,
-    int64_t total) {
+    int64_t total,
+    int64_t current_network_bytes) {
   if (!progress_callback_.is_null())
     progress_callback_.Run(current, total);
 }

diff --git a/google_apis/drive/base_requests.h b/google_apis/drive/base_requests.h
@@ -630,7 +630,8 @@ class DownloadFileRequestBase : public UrlFetchRequestBase {
   // net::URLFetcherDelegate overrides.
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override;
+                                  int64_t total,
+                                  int64_t current_network_bytes) override;
 
  private:
   const DownloadActionCallback download_action_callback_;

diff --git a/net/tools/get_server_time/get_server_time.cc b/net/tools/get_server_time/get_server_time.cc
@@ -83,7 +83,8 @@ class QuitDelegate : public net::URLFetcherDelegate {
 
   void OnURLFetchDownloadProgress(const net::URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override {
+                                  int64_t total,
+                                  int64_t current_network_bytes) override {
     NOTREACHED();
   }
 

diff --git a/net/url_request/test_url_fetcher_factory.cc b/net/url_request/test_url_fetcher_factory.cc
@@ -398,7 +398,7 @@ void FakeURLFetcher::RunDelegate() {
   // this with a weak pointer, and only call OnURLFetchComplete if this still
   // exists.
   auto weak_this = weak_factory_.GetWeakPtr();
-  delegate()->OnURLFetchDownloadProgress(this, response_bytes_,
+  delegate()->OnURLFetchDownloadProgress(this, response_bytes_, response_bytes_,
                                          response_bytes_);
   if (weak_this.get())
     delegate()->OnURLFetchComplete(this);

diff --git a/net/url_request/url_fetcher_core.cc b/net/url_request/url_fetcher_core.cc
@@ -932,16 +932,19 @@ void URLFetcherCore::InformDelegateDownloadProgress() {
   delegate_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(
-          &URLFetcherCore::InformDelegateDownloadProgressInDelegateThread,
-          this, current_response_bytes_, total_response_bytes_));
+          &URLFetcherCore::InformDelegateDownloadProgressInDelegateThread, this,
+          current_response_bytes_, total_response_bytes_,
+          request_->GetTotalReceivedBytes()));
 }
 
 void URLFetcherCore::InformDelegateDownloadProgressInDelegateThread(
     int64_t current,
-    int64_t total) {
+    int64_t total,
+    int64_t current_network_bytes) {
   DCHECK(delegate_task_runner_->BelongsToCurrentThread());
   if (delegate_)
-    delegate_->OnURLFetchDownloadProgress(fetcher_, current, total);
+    delegate_->OnURLFetchDownloadProgress(fetcher_, current, total,
+                                          current_network_bytes);
 }
 
 void URLFetcherCore::AssertHasNoUploadData() const {

diff --git a/net/url_request/url_fetcher_core.h b/net/url_request/url_fetcher_core.h
@@ -220,8 +220,10 @@ class URLFetcherCore : public base::RefCountedThreadSafe<URLFetcherCore>,
   void InformDelegateUploadProgressInDelegateThread(int64_t current,
                                                     int64_t total);
   void InformDelegateDownloadProgress();
-  void InformDelegateDownloadProgressInDelegateThread(int64_t current,
-                                                      int64_t total);
+  void InformDelegateDownloadProgressInDelegateThread(
+      int64_t current,
+      int64_t total,
+      int64_t current_network_bytes);
 
   // Check if any upload data is set or not.
   void AssertHasNoUploadData() const;

diff --git a/net/url_request/url_fetcher_delegate.cc b/net/url_request/url_fetcher_delegate.cc
@@ -6,9 +6,11 @@
 
 namespace net {
 
-void URLFetcherDelegate::OnURLFetchDownloadProgress(const URLFetcher* source,
-                                                    int64_t current,
-                                                    int64_t total) {}
+void URLFetcherDelegate::OnURLFetchDownloadProgress(
+    const URLFetcher* source,
+    int64_t current,
+    int64_t total,
+    int64_t current_network_bytes) {}
 
 void URLFetcherDelegate::OnURLFetchUploadProgress(const URLFetcher* source,
                                                   int64_t current,

diff --git a/net/url_request/url_fetcher_delegate.h b/net/url_request/url_fetcher_delegate.h
@@ -26,9 +26,12 @@ class NET_EXPORT URLFetcherDelegate {
   // This will be called when some part of the response is read. |current|
   // denotes the number of bytes received up to the call, and |total| is the
   // expected total size of the response (or -1 if not determined).
+  // |current_network_bytes| denotes the number of network bytes received
+  // up to the call, excluding redirect bodies, SSL and proxy handshakes.
   virtual void OnURLFetchDownloadProgress(const URLFetcher* source,
                                           int64_t current,
-                                          int64_t total);
+                                          int64_t total,
+                                          int64_t current_network_bytes);
 
   // This will be called when uploading of POST or PUT requests proceeded.
   // |current| denotes the number of bytes sent so far, and |total| is the

diff --git a/net/url_request/url_fetcher_impl_unittest.cc b/net/url_request/url_fetcher_impl_unittest.cc
@@ -127,7 +127,8 @@ class WaitingURLFetcherDelegate : public URLFetcherDelegate {
 
   void OnURLFetchDownloadProgress(const URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override {
+                                  int64_t total,
+                                  int64_t current_network_bytes) override {
     // Note that the current progress may be greater than the previous progress,
     // in the case of retrying the request.
     EXPECT_FALSE(did_complete_);
@@ -924,10 +925,11 @@ class CheckDownloadProgressDelegate : public WaitingURLFetcherDelegate {
 
   void OnURLFetchDownloadProgress(const URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override {
+                                  int64_t total,
+                                  int64_t current_network_bytes) override {
     // Run default checks.
-    WaitingURLFetcherDelegate::OnURLFetchDownloadProgress(source, current,
-                                                          total);
+    WaitingURLFetcherDelegate::OnURLFetchDownloadProgress(
+        source, current, total, current_network_bytes);
 
     EXPECT_LE(last_seen_progress_, current);
     EXPECT_EQ(file_size_, total);
@@ -1011,7 +1013,8 @@ class CancelOnDownloadProgressDelegate : public WaitingURLFetcherDelegate {
 
   void OnURLFetchDownloadProgress(const URLFetcher* source,
                                   int64_t current,
-                                  int64_t total) override {
+                                  int64_t total,
+                                  int64_t current_network_bytes) override {
     CancelFetch();
   }