New issue
Advanced search Search tips

Issue 625270 link

Starred by 4 users
Status: Fixed
Owner:
rajendrant@chromium.org
Closed: Oct 2016
Cc:
wifiprefetch-reviews@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

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

Project Member Reported by rajendrant@chromium.org, Jul 1 2016 
The current behavior is that precache task will stop if size of a resource exceeds max_bytes_per_resource or if total size of all resources exceeds max_bytes_total.

But there are resources which are sent gzipped, and they are stored in cache in gzipped. So the max bytes cap should check the bytes used in network instead of the resource size.

Comment 1 by bengr@chromium.org, Jul 8 2016 

I don't understand.

Comment 2 by rajendrant@chromium.org, Jul 8 2016 

1. Gzipped resources are not tallied based on network bytes.
2. Not sure what happens to 304 responses. Size of resource in cache(may be gzipped) should be considered in that case.
3. The URLRequest should be cancelable in the mid if max cap reached, since we are downloading from third party servers..


Pls see Devin's comments from CL:
https://codereview.chromium.org/2115043002

Comment 3 by rajendrant@chromium.org, Jul 14 2016 

I checked that compressed resource sizes are accounted for in other places for
200 and 304 responses. There could be an improvement for 304 response
where it uses few network bytes for validating, and that is not taken into
account.

The minor open issue is that OnURLFetchDownloadProgress() could cancel gzipped
resources which are large than max_bytes_per_resource cap in uncompressed state. But the network usage and cache usage of the resource will only be its
compressed size. I was not able to find a simple fix for this issue.

Comment 4 by rajendrant@chromium.org, Jul 19 2016 

Summary:
* OnURLFetchDownloadProgress() uses uncompressed bytes for the per-resource cap. No easy fix for this.

* In all other places compressed bytes are accounted for.

More info in CL: https://codereview.chromium.org/2115043002

Comment 5 by rajendrant@chromium.org, Jul 19 2016

Status: WontFix (was: Assigned)

Comment 6 by rajendrant@chromium.org, Aug 17 2016 

Reopening since cacnelled resources are overcounted to the Precache.Fetch.ResponseBytes.Network UMA.

For example, a gzipped resource with size uncompressed=1000KB, compressed=200KB is prefetched, it gets cancelled when 500KB of uncompressed content is retrieved. At that time, network is used for only ~100KB(for 500KB of uncompressed). But Precache.Fetch.ResponseBytes.Network UMA is accounted for 500KB.

Comment 7 by rajendrant@chromium.org, Aug 17 2016

Status: Assigned (was: WontFix)

Comment 8 by rajendrant@chromium.org, Aug 18 2016

Cc: wifiprefetch-reviews@google.com

Comment 9 by rajendrant@chromium.org, Sep 1 2016

Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a30b7788400d638152f4727173e44a5b362903a2

commit a30b7788400d638152f4727173e44a5b362903a2
Author: rajendrant <rajendrant@chromium.org>
Date: Wed Sep 14 19:54:22 2016

Report the wasted network bytes when resource prefetches hit max limit

BUG= 625270 

Review-Url: https://codereview.chromium.org/2278793002
Cr-Commit-Position: refs/heads/master@{#418646}

[modify] https://crrev.com/a30b7788400d638152f4727173e44a5b362903a2/components/precache/core/precache_fetcher.cc
[modify] https://crrev.com/a30b7788400d638152f4727173e44a5b362903a2/tools/metrics/histograms/histograms.xml

Comment 11 by rajendrant@chromium.org, Sep 14 2016

Labels: M-54

Comment 12 by rajendrant@chromium.org, Sep 20 2016

Labels: Merge-Request-54

Comment 13 by dimu@chromium.org, Sep 20 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 20 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f7fdbf8756420e1a836d644c9a25b86f16e21f9

commit 0f7fdbf8756420e1a836d644c9a25b86f16e21f9
Author: rajendrant <rajendrant@chromium.org>
Date: Tue Sep 20 19:01:44 2016

Report the wasted network bytes when resource prefetches hit max limit

BUG= 625270 

Review-Url: https://codereview.chromium.org/2278793002
Cr-Commit-Position: refs/heads/master@{#418646}
(cherry picked from commit a30b7788400d638152f4727173e44a5b362903a2)

Review URL: https://codereview.chromium.org/2350803004 .

Cr-Commit-Position: refs/branch-heads/2840@{#447}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/0f7fdbf8756420e1a836d644c9a25b86f16e21f9/components/precache/core/precache_fetcher.cc
[modify] https://crrev.com/0f7fdbf8756420e1a836d644c9a25b86f16e21f9/tools/metrics/histograms/histograms.xml
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc12c3dfd755611528561c6e48dec23ae59dea5f

commit dc12c3dfd755611528561c6e48dec23ae59dea5f
Author: rajendrant <rajendrant@chromium.org>
Date: Tue Sep 27 04:20:11 2016

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}

[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/policy/core/common/cloud/external_policy_data_fetcher.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/policy/core/common/cloud/external_policy_data_fetcher.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/policy/core/common/cloud/external_policy_data_fetcher_unittest.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/policy/core/common/cloud/external_policy_data_updater_unittest.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/precache/core/fetcher_pool_unittest.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/precache/core/precache_fetcher.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/precache/core/precache_fetcher.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/search_provider_logos/logo_tracker.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/search_provider_logos/logo_tracker.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/sync/core/http_bridge.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/sync/core/http_bridge.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/update_client/url_fetcher_downloader.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/components/update_client/url_fetcher_downloader.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/content/browser/speech/speech_recognition_engine.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/content/browser/speech/speech_recognition_engine.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/content/browser/speech/speech_recognition_engine_unittest.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/content/test/mock_google_streaming_server.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/google_apis/drive/base_requests.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/google_apis/drive/base_requests.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/tools/get_server_time/get_server_time.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/url_request/test_url_fetcher_factory.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/url_request/url_fetcher_core.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/url_request/url_fetcher_core.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/url_request/url_fetcher_delegate.cc
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/url_request/url_fetcher_delegate.h
[modify] https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f/net/url_request/url_fetcher_impl_unittest.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9db9657f57cad43fd011f96cf855b7f4da37c362

commit 9db9657f57cad43fd011f96cf855b7f4da37c362
Author: rajendrant <rajendrant@chromium.org>
Date: Wed Sep 28 18:32:11 2016

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}
(cherry picked from commit dc12c3dfd755611528561c6e48dec23ae59dea5f)

Review URL: https://codereview.chromium.org/2378173002 .

Cr-Commit-Position: refs/branch-heads/2840@{#565}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_fetcher.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_fetcher.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_fetcher_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_updater_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/precache/core/fetcher_pool_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/precache/core/precache_fetcher.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/precache/core/precache_fetcher.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/search_provider_logos/logo_tracker.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/search_provider_logos/logo_tracker.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/sync/core/http_bridge.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/sync/core/http_bridge.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/update_client/url_fetcher_downloader.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/update_client/url_fetcher_downloader.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/browser/speech/speech_recognition_engine.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/browser/speech/speech_recognition_engine.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/browser/speech/speech_recognition_engine_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/test/mock_google_streaming_server.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/google_apis/drive/base_requests.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/google_apis/drive/base_requests.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/tools/get_server_time/get_server_time.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/test_url_fetcher_factory.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_core.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_core.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_delegate.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_delegate.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_impl_unittest.cc

Comment 17 by rajendrant@chromium.org, Oct 5 2016

Status: Fixed (was: Started)
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f7fdbf8756420e1a836d644c9a25b86f16e21f9

commit 0f7fdbf8756420e1a836d644c9a25b86f16e21f9
Author: rajendrant <rajendrant@chromium.org>
Date: Tue Sep 20 19:01:44 2016

Report the wasted network bytes when resource prefetches hit max limit

BUG= 625270 

Review-Url: https://codereview.chromium.org/2278793002
Cr-Commit-Position: refs/heads/master@{#418646}
(cherry picked from commit a30b7788400d638152f4727173e44a5b362903a2)

Review URL: https://codereview.chromium.org/2350803004 .

Cr-Commit-Position: refs/branch-heads/2840@{#447}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/0f7fdbf8756420e1a836d644c9a25b86f16e21f9/components/precache/core/precache_fetcher.cc
[modify] https://crrev.com/0f7fdbf8756420e1a836d644c9a25b86f16e21f9/tools/metrics/histograms/histograms.xml
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9db9657f57cad43fd011f96cf855b7f4da37c362

commit 9db9657f57cad43fd011f96cf855b7f4da37c362
Author: rajendrant <rajendrant@chromium.org>
Date: Wed Sep 28 18:32:11 2016

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}
(cherry picked from commit dc12c3dfd755611528561c6e48dec23ae59dea5f)

Review URL: https://codereview.chromium.org/2378173002 .

Cr-Commit-Position: refs/branch-heads/2840@{#565}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_fetcher.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_fetcher.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_fetcher_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/policy/core/common/cloud/external_policy_data_updater_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/precache/core/fetcher_pool_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/precache/core/precache_fetcher.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/precache/core/precache_fetcher.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/search_provider_logos/logo_tracker.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/search_provider_logos/logo_tracker.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/sync/core/http_bridge.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/sync/core/http_bridge.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/update_client/url_fetcher_downloader.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/components/update_client/url_fetcher_downloader.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/browser/speech/speech_recognition_engine.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/browser/speech/speech_recognition_engine.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/browser/speech/speech_recognition_engine_unittest.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/content/test/mock_google_streaming_server.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/google_apis/drive/base_requests.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/google_apis/drive/base_requests.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/tools/get_server_time/get_server_time.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/test_url_fetcher_factory.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_core.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_core.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_delegate.cc
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_delegate.h
[modify] https://crrev.com/9db9657f57cad43fd011f96cf855b7f4da37c362/net/url_request/url_fetcher_impl_unittest.cc
►

Sign in to add a comment