Set the Flywheel promo rate for Android One devices to be 100%.

Set the Flywheel promo rate for Android One devices to be 100%.

BUG=431362
Committed: https://crrev.com/b6c97ae6c541988f5c38c96549d1281879e450ee
Cr-Commit-Position: refs/heads/master@{#309981}

[jeremyim, 2014-12-16 19:38:25]

jeremyim@chromium.org changed reviewers:
+ bengr@chromium.org

[jeremyim, 2014-12-16 19:38:26]

PTAL =)

[bengr, 2014-12-16 23:47:49]

You will need a reviewer for io_thread.cc

https://codereview.chromium.org/806053003/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/806053003/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:93:
bool DataReductionProxyParams::IsIncludedInAndroidOnePromoFieldTrial(
As trivial as this method is, please add a test.

[jeremyim, 2014-12-17 01:43:19]

jeremyim@chromium.org changed reviewers:
+ mmenke@chromium.org

[jeremyim, 2014-12-17 01:43:19]

mmenke => io_globals.cc

Thank you!

https://codereview.chromium.org/806053003/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/806053003/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:93:
bool DataReductionProxyParams::IsIncludedInAndroidOnePromoFieldTrial(
On 2014/12/16 23:47:49, bengr wrote:
> As trivial as this method is, please add a test.

Done.

[jeremyim, 2014-12-17 01:43:33]

Err io_thread.cc

On 2014/12/17 01:43:19, jeremyim wrote:
> mmenke => io_globals.cc
> 
> Thank you!
> 
>
https://codereview.chromium.org/806053003/diff/1/components/data_reduction_pr...
> File
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
> (right):
> 
>
https://codereview.chromium.org/806053003/diff/1/components/data_reduction_pr...
> components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:93:
> bool DataReductionProxyParams::IsIncludedInAndroidOnePromoFieldTrial(
> On 2014/12/16 23:47:49, bengr wrote:
> > As trivial as this method is, please add a test.
> 
> Done.

[mmenke, 2014-12-17 16:32:50]

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

https://codereview.chromium.org/806053003/diff/2/chrome/browser/io_thread.cc#...
chrome/browser/io_thread.cc:1182: flags |=
data_reduction_proxy::DataReductionProxyParams::kPromoAllowed;
We already have
data_reduction_proxy::DataReductionProxyParams::IsIncludedInPromoFieldTrial().

Calling into DRP code multiple times to check for different field trials seems
weird.  And then we also have another field trial check just above...

Can we just merge these into one method?  Or since DRP already knows about these
field trials anyways, put the checks in DataReductionProxyParams's constructor?

[jeremyim, 2014-12-17 17:52:49]

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

https://codereview.chromium.org/806053003/diff/2/chrome/browser/io_thread.cc#...
chrome/browser/io_thread.cc:1182: flags |=
data_reduction_proxy::DataReductionProxyParams::kPromoAllowed;
On 2014/12/17 16:32:50, mmenke wrote:
> We already have
> data_reduction_proxy::DataReductionProxyParams::IsIncludedInPromoFieldTrial().
> 
> Calling into DRP code multiple times to check for different field trials seems
> weird.  And then we also have another field trial check just above...
> 
> Can we just merge these into one method?  Or since DRP already knows about
these
> field trials anyways, put the checks in DataReductionProxyParams's
constructor?

This will get cleaned up/refactored in bengr's CL
https://codereview.chromium.org/778463002/

[mmenke, 2015-01-05 15:25:00]

On 2014/12/17 17:52:49, jeremyim wrote:
> https://codereview.chromium.org/806053003/diff/2/chrome/browser/io_thread.cc
> File chrome/browser/io_thread.cc (right):
> 
>
https://codereview.chromium.org/806053003/diff/2/chrome/browser/io_thread.cc#...
> chrome/browser/io_thread.cc:1182: flags |=
> data_reduction_proxy::DataReductionProxyParams::kPromoAllowed;
> On 2014/12/17 16:32:50, mmenke wrote:
> > We already have
> >
data_reduction_proxy::DataReductionProxyParams::IsIncludedInPromoFieldTrial().
> > 
> > Calling into DRP code multiple times to check for different field trials
seems
> > weird.  And then we also have another field trial check just above...
> > 
> > Can we just merge these into one method?  Or since DRP already knows about
> these
> > field trials anyways, put the checks in DataReductionProxyParams's
> constructor?
> 
> This will get cleaned up/refactored in bengr's CL
> https://codereview.chromium.org/778463002/

LGTM (Sorry, forgot to sign off on this before leaving, and ended up not
checking my work email over the break).

[mmenke, 2015-01-05 15:25:01]

On 2014/12/17 17:52:49, jeremyim wrote:
> https://codereview.chromium.org/806053003/diff/2/chrome/browser/io_thread.cc
> File chrome/browser/io_thread.cc (right):
> 
>
https://codereview.chromium.org/806053003/diff/2/chrome/browser/io_thread.cc#...
> chrome/browser/io_thread.cc:1182: flags |=
> data_reduction_proxy::DataReductionProxyParams::kPromoAllowed;
> On 2014/12/17 16:32:50, mmenke wrote:
> > We already have
> >
data_reduction_proxy::DataReductionProxyParams::IsIncludedInPromoFieldTrial().
> > 
> > Calling into DRP code multiple times to check for different field trials
seems
> > weird.  And then we also have another field trial check just above...
> > 
> > Can we just merge these into one method?  Or since DRP already knows about
> these
> > field trials anyways, put the checks in DataReductionProxyParams's
> constructor?
> 
> This will get cleaned up/refactored in bengr's CL
> https://codereview.chromium.org/778463002/

LGTM (Sorry, forgot to sign off on this before leaving, and ended up not
checking my work email over the break).

[jeremyim, 2015-01-05 16:42:03]

The CQ bit was checked by jeremyim@chromium.org

[commit-bot: I haz the power, 2015-01-05 16:42:48]

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

[commit-bot: I haz the power, 2015-01-05 16:48:41]

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

[commit-bot: I haz the power, 2015-01-05 16:48:42]

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

[jeremyim, 2015-01-05 16:53:12]

The CQ bit was checked by jeremyim@chromium.org

[commit-bot: I haz the power, 2015-01-05 16:53:35]

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

[commit-bot: I haz the power, 2015-01-05 16:59:33]

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

[commit-bot: I haz the power, 2015-01-05 16:59:34]

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

[bengr, 2015-01-05 18:21:48]

lgtm with formatting nits

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

https://codereview.chromium.org/806053003/diff/30001/chrome/browser/io_thread...
chrome/browser/io_thread.cc:1192: IsIncludedInAndroidOnePromoFieldTrial(
indent 4?

https://codereview.chromium.org/806053003/diff/30001/chrome/browser/net/spdyp...
File
chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.cc
(right):

https://codereview.chromium.org/806053003/diff/30001/chrome/browser/net/spdyp...
chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.cc:68:
base::android::BuildInfo::GetInstance()->android_build_fp())) {
indent 4 more

[jeremyim, 2015-01-05 21:36:05]

Thanks all!

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

https://codereview.chromium.org/806053003/diff/30001/chrome/browser/io_thread...
chrome/browser/io_thread.cc:1192: IsIncludedInAndroidOnePromoFieldTrial(
On 2015/01/05 18:21:48, bengr wrote:
> indent 4?

Done.

https://codereview.chromium.org/806053003/diff/30001/chrome/browser/net/spdyp...
File
chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.cc
(right):

https://codereview.chromium.org/806053003/diff/30001/chrome/browser/net/spdyp...
chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.cc:68:
base::android::BuildInfo::GetInstance()->android_build_fp())) {
On 2015/01/05 18:21:48, bengr wrote:
> indent 4 more

Done.

[jeremyim, 2015-01-05 21:36:09]

The CQ bit was checked by jeremyim@chromium.org

[commit-bot: I haz the power, 2015-01-05 21:37:27]

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

[commit-bot: I haz the power, 2015-01-05 22:57:44]

Committed patchset #4 (id:50001)

[commit-bot: I haz the power, 2015-01-05 22:59:15]

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