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

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
Committed: https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f
Cr-Commit-Position: refs/heads/master@{#421101}

[Raj, 2016-09-01 05:36:14]

rajendrant@chromium.org changed reviewers:
+ bengr@chromium.org, mmenke@chromium.org

[Raj, 2016-09-01 05:36:14]

mmenke: ptal net/url_request/url_fetcher_core*

An alternative is to add an argument current_network_bytes to
OnURLFetchDownloadProgress(), and let InformDelegateDownloadProgress() calculate
the network bytes and send it.

I will add possible tests, once you are fine with the change.

[mmenke, 2016-09-01 14:52:01]

On 2016/09/01 05:36:14, Raj wrote:
> mmenke: ptal net/url_request/url_fetcher_core*
> 
> An alternative is to add an argument current_network_bytes to
> OnURLFetchDownloadProgress(), and let InformDelegateDownloadProgress()
calculate
> the network bytes and send it.
> 
> I will add possible tests, once you are fine with the change.

NOT LGTM.  This code is not threadsafe, since URLFetcher is a cross-thread
interface.  We could add it to InformDelegateDownloadProgress, as you suggest,
though I'm a bit reluctant to bloat up the URLFetcher API - it's supposed to be
the "simple" alternative to URLRequest, and the more we bloat it up, the less
that's true.

[Raj, 2016-09-01 18:36:16]

mmenke: ptal net/*
Once you are OK, I will add some unittests and other reviewers.

[bengr, 2016-09-08 00:00:41]

On 2016/09/01 18:36:16, Raj wrote:
> mmenke: ptal net/*
> Once you are OK, I will add some unittests and other reviewers.

I'll wait for mmenke@ to ok the approach before reviewing.

[mmenke, 2016-09-08 17:57:38]

On 2016/09/08 00:00:41, bengr wrote:
> On 2016/09/01 18:36:16, Raj wrote:
> > mmenke: ptal net/*
> > Once you are OK, I will add some unittests and other reviewers.
> 
> I'll wait for mmenke@ to ok the approach before reviewing.

I'm sorry, completely forgot about this one.  net/ LGTM.

[Raj, 2016-09-08 18:12:08]

rajendrant@chromium.org changed reviewers:
+ cschuet@chromium.org, haven@chromium.org, justincohen@chromium.org,
maxbogue@chromium.org, pfeldman@chromium.org, waffles@chromium.org,
yoshiki@chromium.org

[Raj, 2016-09-08 18:12:09]

cschuet: ptal components/policy/core/common/cloud/*

justincohen: ptal components/search_provider_logos/*

yoshiki: ptal google_apis/drive/*

waffles: ptal components/update_client/*

haven: ptal chrome/browser/extensions/api/image_writer_private/*

maxbogue: ptal components/sync/core/*

pfeldman: ptal content/browser/speech/*

[justincohen, 2016-09-08 18:13:40]

lgtm

[maxbogue, 2016-09-08 18:20:32]

//components/sync lgtm

[yoshiki, 2016-09-09 16:08:21]

google_apis/drive/* lgtm

[cschuet (SLOW), 2016-09-14 00:09:58]

Description was changed from

==========
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
==========

to

==========
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
==========

[cschuet (SLOW), 2016-09-14 00:09:58]

cschuet@chromium.org changed reviewers:
+ tnagel@google.com
- cschuet@chromium.org

[pfeldman, 2016-09-14 00:32:46]

content/ lgtm

[Raj, 2016-09-14 02:34:00]

rajendrant@chromium.org changed reviewers:
+ tnagel@chromium.org

[Raj, 2016-09-14 02:34:01]

tnagel: ptal components/policy/*

[waffles, 2016-09-15 17:00:01]

https://codereview.chromium.org/2303653002/diff/20001/net/url_request/url_fet...
File net/url_request/url_fetcher_delegate.h (right):

https://codereview.chromium.org/2303653002/diff/20001/net/url_request/url_fet...
net/url_request/url_fetcher_delegate.h:28: // expected total size of the
response (or -1 if not determined).
I believe this comment needs to be updated.

[Raj, 2016-09-15 18:46:33]

Patchset #3 (id:40001) has been deleted

[Raj, 2016-09-15 18:56:17]

waffles: ptal

https://codereview.chromium.org/2303653002/diff/20001/net/url_request/url_fet...
File net/url_request/url_fetcher_delegate.h (right):

https://codereview.chromium.org/2303653002/diff/20001/net/url_request/url_fet...
net/url_request/url_fetcher_delegate.h:28: // expected total size of the
response (or -1 if not determined).
On 2016/09/15 17:00:01, waffles wrote:
> I believe this comment needs to be updated.

Done.

[waffles, 2016-09-15 19:24:54]

components/update_client lgtm

[Raj, 2016-09-22 21:55:02]

rajendrant@chromium.org changed reviewers:
+ asargent@chromium.org

[Raj, 2016-09-22 21:55:03]

asargent: ptal
chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
since haven@chromium.org is not reachable.

[asargent_no_longer_on_chrome, 2016-09-22 22:15:23]

chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
lgtm

[bengr, 2016-09-23 20:17:33]

components/precache lgtm

[Thiemo Nagel, 2016-09-26 14:13:18]

components/policy lgtm

[Raj, 2016-09-26 17:20:43]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 17:21:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 17:31:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 17:31:19]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Raj, 2016-09-26 17:52:09]

Patchset #4 (id:80001) has been deleted

[Raj, 2016-09-26 17:54:18]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 17:54:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 18:04:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 18:04:17]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Raj, 2016-09-26 18:32:18]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 18:33:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 18:43:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 18:43:08]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Raj, 2016-09-26 20:16:09]

Patchset #5 (id:120001) has been deleted

[Raj, 2016-09-26 20:16:17]

Patchset #4 (id:100001) has been deleted

[Raj, 2016-09-26 20:17:19]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 20:18:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 20:40:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 20:40:07]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raj, 2016-09-26 20:45:47]

Patchset #4 (id:140001) has been deleted

[Raj, 2016-09-26 20:47:13]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 20:47:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raj, 2016-09-26 21:05:42]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 21:06:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raj, 2016-09-26 21:10:24]

Patchset #5 (id:180001) has been deleted

[Raj, 2016-09-26 21:10:33]

Patchset #4 (id:160001) has been deleted

[Raj, 2016-09-26 21:14:24]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 21:15:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 21:34:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 21:34:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raj, 2016-09-26 22:05:48]

Patchset #4 (id:200001) has been deleted

[Raj, 2016-09-26 22:09:32]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 22:10:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 22:25:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 22:25:18]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Raj, 2016-09-26 22:37:07]

Patchset #4 (id:220001) has been deleted

[Raj, 2016-09-26 22:37:34]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 22:38:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 22:51:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 22:51:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raj, 2016-09-27 00:03:20]

Patchset #4 (id:240001) has been deleted

[Raj, 2016-09-27 00:03:34]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 00:04:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 00:19:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 00:19:04]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Raj, 2016-09-27 01:24:02]

Patchset #4 (id:260001) has been deleted

[Raj, 2016-09-27 01:24:49]

The CQ bit was checked by rajendrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 01:25:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 02:55:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 02:55:14]

Dry run: This issue passed the CQ dry run.

[Raj, 2016-09-27 04:15:00]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2016-09-27 04:15:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
justincohen@chromium.org, mmenke@chromium.org, maxbogue@chromium.org,
yoshiki@chromium.org, pfeldman@chromium.org, waffles@chromium.org,
tnagel@chromium.org, bengr@chromium.org, asargent@chromium.org
Link to the patchset: https://codereview.chromium.org/2303653002/#ps280001
(title: "rebased")

[commit-bot: I haz the power, 2016-09-27 04:15:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 04:20:32]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-09-27 04:20:34]

Committed patchset #4 (id:280001)

[commit-bot: I haz the power, 2016-09-27 04:24:00]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f
Cr-Commit-Position: refs/heads/master@{#421101}
==========

[commit-bot: I haz the power, 2016-09-27 04:24:01]

Patchset 4 (id:??) landed as
https://crrev.com/dc12c3dfd755611528561c6e48dec23ae59dea5f
Cr-Commit-Position: refs/heads/master@{#421101}