Added flag to enable the data reduction proxy alternative

Added flag to enable the data reduction proxy alternative

This flags has the same effect as being part of the enabled
group of the DataCompressionProxyAlternativeConfiguration
field trial.

BUG=415185
Committed: https://crrev.com/2989fef8c7173769f4361c5bb55d991fa7c0ac26
Cr-Commit-Position: refs/heads/master@{#295563}

[bengr, 2014-09-17 18:43:29]

Patchset #1 (id:1) has been deleted

[bengr, 2014-09-17 18:47:15]

bengr@chromium.org changed reviewers:
+ asvitkine@chromium.org, marq@chromium.org, mmenke@chromium.org

[bengr, 2014-09-17 18:47:16]

mmenke: io_thread.cc
marq: chrome/browser/net/spdyproxy and components/data_reduction_proxy
asvitkine: histograms.xml

[mmenke, 2014-09-17 19:12:13]

https://codereview.chromium.org/578883004/diff/20001/chrome/browser/io_thread.cc
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/578883004/diff/20001/chrome/browser/io_thread...
chrome/browser/io_thread.cc:647: drp_flags |=
data_reduction_proxy::DataReductionProxyParams::kHoldback;
Any plans to no longer duplicate this code?  Duplicating code seems really
regression prone to me.  I know I've asked this before, but I have a short term
memory.  :(

[bengr, 2014-09-17 20:11:21]

On 2014/09/17 19:12:13, mmenke wrote:
>
https://codereview.chromium.org/578883004/diff/20001/chrome/browser/io_thread.cc
> File chrome/browser/io_thread.cc (right):
> 
>
https://codereview.chromium.org/578883004/diff/20001/chrome/browser/io_thread...
> chrome/browser/io_thread.cc:647: drp_flags |=
> data_reduction_proxy::DataReductionProxyParams::kHoldback;
> Any plans to no longer duplicate this code?  Duplicating code seems really
> regression prone to me.  I know I've asked this before, but I have a short
term
> memory.  :(

Yes, but not before branch. these are for different request contexts, but since
they're always set to the same values, they should be shared.

[Alexei Svitkine (slow), 2014-09-17 20:15:07]

https://codereview.chromium.org/578883004/diff/20001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/578883004/diff/20001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_params.cc:44:
"DataCompressionProxyAlternativeConfiguration") == kEnabled);
Use the pattern recommended by go/finch-and-flags instead, which allows you to
be able to distinguish users who have the flag on the dashboard (vs. this way,
where they wouldn't touch the trial and thus wouldn't be tagged).

[mmenke, 2014-09-17 20:56:52]

On 2014/09/17 20:15:07, Alexei Svitkine wrote:
>
https://codereview.chromium.org/578883004/diff/20001/components/data_reductio...
> File components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
> (right):
> 
>
https://codereview.chromium.org/578883004/diff/20001/components/data_reductio...
> components/data_reduction_proxy/browser/data_reduction_proxy_params.cc:44:
> "DataCompressionProxyAlternativeConfiguration") == kEnabled);
> Use the pattern recommended by go/finch-and-flags instead, which allows you to
> be able to distinguish users who have the flag on the dashboard (vs. this way,
> where they wouldn't touch the trial and thus wouldn't be tagged).

io_thread LGTM.

[bengr, 2014-09-18 17:39:46]

https://codereview.chromium.org/578883004/diff/20001/chrome/browser/io_thread.cc
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/578883004/diff/20001/chrome/browser/io_thread...
chrome/browser/io_thread.cc:647: drp_flags |=
data_reduction_proxy::DataReductionProxyParams::kHoldback;
On 2014/09/17 19:12:13, mmenke wrote:
> Any plans to no longer duplicate this code?  Duplicating code seems really
> regression prone to me.  I know I've asked this before, but I have a short
term
> memory.  :(

See my previous reply. I agree.

https://codereview.chromium.org/578883004/diff/20001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/578883004/diff/20001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_params.cc:44:
"DataCompressionProxyAlternativeConfiguration") == kEnabled);
On 2014/09/17 20:15:07, Alexei Svitkine wrote:
> Use the pattern recommended by go/finch-and-flags instead, which allows you to
> be able to distinguish users who have the flag on the dashboard (vs. this way,
> where they wouldn't touch the trial and thus wouldn't be tagged).

Done.

[Alexei Svitkine (slow), 2014-09-18 17:46:41]

https://codereview.chromium.org/578883004/diff/40001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/578883004/diff/40001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_params.cc:45:
"DataCompressionProxyAlternativeConfiguration") == kForcedEnabled);
The go/finch-and-flags page actually doesn't say to do it this way as a best
practice. Though it will mostly work, it misses an edge case - which is when the
flag is on but the Finch config is not available. So it's best to check the
flag, but after you query the group from the API - the page has a code snippet
example.

[marq (ping after 24h), 2014-09-18 17:47:49]

LGTM as long as this flag/trial isn't expected to work on iOS.

[bengr, 2014-09-18 19:28:33]

https://codereview.chromium.org/578883004/diff/40001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/578883004/diff/40001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_params.cc:45:
"DataCompressionProxyAlternativeConfiguration") == kForcedEnabled);
On 2014/09/18 17:46:40, Alexei Svitkine wrote:
> The go/finch-and-flags page actually doesn't say to do it this way as a best
> practice. Though it will mostly work, it misses an edge case - which is when
the
> flag is on but the Finch config is not available. So it's best to check the
> flag, but after you query the group from the API - the page has a code snippet
> example.

Done. I think.

[Alexei Svitkine (slow), 2014-09-18 20:05:17]

lgtm

[bengr, 2014-09-18 20:19:27]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-09-18 20:19:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/578883004/100001

[commit-bot: I haz the power, 2014-09-18 21:47:45]

Committed patchset #5 (id:100001) as 53811bd842946dc18471055c20801ffc1d2e8676

[commit-bot: I haz the power, 2014-09-18 21:48:25]

Patchset 5 (id:??) landed as
https://crrev.com/2989fef8c7173769f4361c5bb55d991fa7c0ac26
Cr-Commit-Position: refs/heads/master@{#295563}