Added support for 'Chrome-Proxy: block-once' header

Added support for 'Chrome-Proxy: block-once' header

Added support for a new instruction 'block-once' that, when present in the 'Chrome-Proxy' HTTP response header, causes data reduction proxies to be bypassed when retrying the current request (if it's idempotent), but does not cause data reduction proxies to be bypassed for future requests.

BUG=325328
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289469

[sclittle, 2014-08-12 20:21:17]

bengr, marq: *

Thanks in advance!

[bengr, 2014-08-12 20:57:11]

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:148: if
(headers->HasHeaderValue(kChromeProxyHeader,
If we believe that this will become the dominant instruction, then would it make
sense to put it first?

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:212:
data_reduction_proxy_info->mark_proxies_as_bad = true;
Please add a comment to explain why this gets set to true here.

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc
(right):

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc:294:
"Chrome-Proxy: block-once=\n"
So block-once= and block-once=foo are both ignored? I guess that's ok. I wonder
what would happen if a later version supported block-once=foo. I can think of
reasons why this version should ignore such an instruction, and I can think of
reasons why this version should interpret block-once=foo as block-once.

[sclittle, 2014-08-12 21:24:44]

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/common/data_reduction_proxy_headers.cc
(right):

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:148: if
(headers->HasHeaderValue(kChromeProxyHeader,
On 2014/08/12 20:57:10, bengr1 wrote:
> If we believe that this will become the dominant instruction, then would it
make
> sense to put it first?

I'm not certain what the priorities should be. The reason I put 'block-once'
last was because the effects of 'block-once' are a subset of the effects of
'block' (and also 'bypass'), but I can switch them around if you'd prefer.

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers.cc:212:
data_reduction_proxy_info->mark_proxies_as_bad = true;
On 2014/08/12 20:57:11, bengr1 wrote:
> Please add a comment to explain why this gets set to true here.

Done.

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc
(right):

https://codereview.chromium.org/467823002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc:294:
"Chrome-Proxy: block-once=\n"
On 2014/08/12 20:57:11, bengr1 wrote:
> So block-once= and block-once=foo are both ignored? I guess that's ok. I
wonder
> what would happen if a later version supported block-once=foo. I can think of
> reasons why this version should ignore such an instruction, and I can think of
> reasons why this version should interpret block-once=foo as block-once. 

Acknowledged.

[sclittle, 2014-08-13 16:38:47]

Ping?

[bengr, 2014-08-13 16:40:36]

On 2014/08/13 16:38:47, sclittle wrote:
> Ping?

lgtm

[sclittle, 2014-08-13 17:09:48]

The CQ bit was checked by sclittle@chromium.org

[commit-bot: I haz the power, 2014-08-13 17:11:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sclittle@chromium.org/467823002/40001

[commit-bot: I haz the power, 2014-08-13 23:14:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-14 01:00:22]

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

[commit-bot: I haz the power, 2014-08-14 01:00:23]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)

[sclittle, 2014-08-14 01:21:21]

The CQ bit was unchecked by sclittle@chromium.org

[sclittle, 2014-08-14 01:21:21]

The CQ bit was checked by sclittle@chromium.org

[commit-bot: I haz the power, 2014-08-14 01:24:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sclittle@chromium.org/467823002/40001

[commit-bot: I haz the power, 2014-08-14 05:32:53]

Committed patchset #3 (40001) as 289469