Precache bytes cap should be based on total bytes fetched from network

components/precache/core/precache_fetcher.cc

Index: components/precache/core/precache_fetcher.cc
diff --git a/components/precache/core/precache_fetcher.cc b/components/precache/core/precache_fetcher.cc
index dc0d3a272327a9ce1002ba33b377b2abb2f7e058..fd1eb5e1e6f462ce8ad9dd6b1dfa69b76ca62a53 100644
--- a/components/precache/core/precache_fetcher.cc
+++ b/components/precache/core/precache_fetcher.cc
@@ -235,8 +235,7 @@ void PrecacheFetcher::Fetcher::OnURLFetchDownloadProgress(
     int64_t total) {
   // If 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_) {
+      source->GetTotalReceivedBytes() > max_bytes_) {

[twifkak, 2016/07/01 23:59:25]

Also, the proto says "This max applies only to new downloads; cached resources
are not capped." Is that still true? I don't have a strong opinion on what it
should be -- only that the documentation should agree.

[twifkak, 2016/07/01 23:59:25]

AFAICT this is only available after the request is finished, not during
download, so it won't let us cancel mid-stream:
https://cs.chromium.org/chromium/src/net/url_request/url_fetcher_core.cc?rcl=...

This is a problem since we're downloading from third-party servers and can't
trust that they won't send us a really big file.

[Raj, 2016/07/14 18:41:30]

Yes. You are right. This is updated only after request is finished. It does not
seem easy to implement this in net/ as well to return network bytes or
compressed size progressively.

[Raj, 2016/07/14 18:41:30]

The proto is correct. Only new downloads have FetchStage::NETWORK and
max_bytes_per_resource cap is applied only for those.

     VLOG(1) << "Cancelling " << url_ << ": (" << current << "/" << total
             << ") is over " << max_bytes_;
 
@@ -407,7 +406,7 @@ void PrecacheFetcher::StartNextResourceFetch() {
     const size_t max_bytes =
         std::min(unfinished_work_->config_settings().max_bytes_per_resource(),
                  unfinished_work_->config_settings().max_bytes_total() -
-                     unfinished_work_->total_bytes());
+                     unfinished_work_->network_bytes());
     VLOG(3) << "Fetching " << resource_urls_to_fetch_.front();
     pool_.Add(base::WrapUnique(
         new Fetcher(request_context_.get(), resource_urls_to_fetch_.front(),
@@ -447,7 +446,7 @@ void PrecacheFetcher::NotifyDone(
 void PrecacheFetcher::StartNextFetch() {
   DCHECK(unfinished_work_->has_config_settings());
   // If over the precache total size cap, then stop prefetching.
-  if (unfinished_work_->total_bytes() >
+  if (unfinished_work_->network_bytes() >

[twifkak, 2016/07/01 23:59:25]

Does this include the size of cache resources, too? It should, as our goal with
this cap is to keep from eating the whole cache (doing so incrementally isn't
much better). Per the proto, "this includes new downloads as well as cached
resources."

[Raj, 2016/07/14 18:41:30]

My code is wrong. It should not be changed. total_bytes() accounts for bytes in
the network or bytes in the cache, depending on http response 200 or 304.

       unfinished_work_->config_settings().max_bytes_total()) {
     size_t pending_manifests_in_pool = 0;
     size_t pending_resources_in_pool = 0;