Remove kAllowed and kFallbackAllowed from data reduction proxy

Remove kAllowed and kFallbackAllowed from data reduction proxy

Data reduction proxy is always allowed with primary and fallback
mechanism.

BUG=654438
Review-Url: https://codereview.chromium.org/2626963003
Cr-Commit-Position: refs/heads/master@{#443497}
Committed: https://chromium.googlesource.com/chromium/src/+/bea6ff95471b7a3da8030b0903d73e48434cc18a

[Raj, 2017-01-11 18:54:16]

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

[Raj, 2017-01-11 18:54:17]

ptal

[tbansal1, 2017-01-11 22:26:33]

Thanks for doing this.

https://codereview.chromium.org/2626963003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/net/spdyproxy/DataReductionProxySettings.java
(right):

https://codereview.chromium.org/2626963003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/net/spdyproxy/DataReductionProxySettings.java:130:
public boolean isDataReductionProxyAllowed() {
Why not just remove this method? I do not think we are ever going to remove
false here.

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc
(right):

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc:153:
.WithParamsFlags(0)
Should not this be kPromoAllowed?

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc
(right):

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc:137:
.WithParamsFlags(0)
Is it not possible to skip calling WithParamsFlags() when the argument is 0?

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:55:
lo_fi_mode_active_(false),
I think 
promo_allowed_(false),
got accidentally removed.

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:323:
void DataReductionProxySettings::RecordDataReductionInit() {
While you are here, it seems this method, and the one below can be const.

[Raj, 2017-01-11 23:49:21]

https://codereview.chromium.org/2626963003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/net/spdyproxy/DataReductionProxySettings.java
(right):

https://codereview.chromium.org/2626963003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/net/spdyproxy/DataReductionProxySettings.java:130:
public boolean isDataReductionProxyAllowed() {
On 2017/01/11 22:26:33, tbansal1 wrote:
> Why not just remove this method? I do not think we are ever going to remove
> false here.

Done.

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc
(right):

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc:153:
.WithParamsFlags(0)
On 2017/01/11 22:26:33, tbansal1 wrote:
> Should not this be kPromoAllowed?

Oops.

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc
(right):

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc:137:
.WithParamsFlags(0)
On 2017/01/11 22:26:33, tbansal1 wrote:
> Is it not possible to skip calling WithParamsFlags() when the argument is 0?

By default param flags is kPromoAllowed. So 0 should be explicitly set.
Do you suggest a constant kDefault instead of 0?

https://cs.chromium.org/chromium/src/components/data_reduction_proxy/core/bro...

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:55:
lo_fi_mode_active_(false),
On 2017/01/11 22:26:33, tbansal1 wrote:
> I think 
> promo_allowed_(false),
> got accidentally removed.

Yes. Fixed

https://codereview.chromium.org/2626963003/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:323:
void DataReductionProxySettings::RecordDataReductionInit() {
On 2017/01/11 22:26:33, tbansal1 wrote:
> While you are here, it seems this method, and the one below can be const.

Done.

[Raj, 2017-01-12 18:08:01]

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
(right):

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.h:184:
static const unsigned int kPromoAllowed = (1 << 2);
Wondering if the remaining flags can be moved up (in separate CL).
Any idea if these are persisted in config ?

[tbansal1, 2017-01-12 19:39:05]

lgtm % 1 nit. Thanks for doing this.

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
(right):

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.h:184:
static const unsigned int kPromoAllowed = (1 << 2);
On 2017/01/12 18:08:01, Raj wrote:
> Wondering if the remaining flags can be moved up (in separate CL).
> Any idea if these are persisted in config ?

They are not persisted. You can move them up, but lets not do that since these
flags are actually not useful, and it is better to just remove them. I can do
that.

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
(right):

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc:100:
const char kCarrierTestOrigin[] =
why was the static removed?

[Raj, 2017-01-12 22:24:48]

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

[commit-bot: I haz the power, 2017-01-12 22:25:35]

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, 2017-01-12 23:24:24]

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

[commit-bot: I haz the power, 2017-01-12 23:24:25]

Dry run: This issue passed the CQ dry run.

[Raj, 2017-01-13 00:07:01]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2017-01-13 00:07:01]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2626963003/#ps40001
(title: "Addressed nits")

[Raj, 2017-01-13 00:07:08]

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
(right):

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.h:184:
static const unsigned int kPromoAllowed = (1 << 2);
On 2017/01/12 19:39:05, tbansal1 wrote:
> On 2017/01/12 18:08:01, Raj wrote:
> > Wondering if the remaining flags can be moved up (in separate CL).
> > Any idea if these are persisted in config ?
> 
> They are not persisted. You can move them up, but lets not do that since these
> flags are actually not useful, and it is better to just remove them. I can do
> that.

Acknowledged.

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
(right):

https://codereview.chromium.org/2626963003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc:100:
const char kCarrierTestOrigin[] =
On 2017/01/12 19:39:05, tbansal1 wrote:
> why was the static removed?

Done.

[commit-bot: I haz the power, 2017-01-13 00:08:34]

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, 2017-01-13 00:24:01]

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

[commit-bot: I haz the power, 2017-01-13 00:24:02]

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

[Raj, 2017-01-13 00:24:59]

rajendrant@chromium.org changed reviewers:
+ dfalcantara@chromium.org, lazyboy@chromium.org

[Raj, 2017-01-13 00:24:59]

lazyboy: ptal DataReductionProxySettings.java and MainPreferences.java

dfalcantara: ptal render_view_context_menu_unittest.cc

[lazyboy, 2017-01-13 00:32:49]

render_view_context_menu_unittest.cc lgtm.

[gone, 2017-01-13 01:06:09]

java files lgtm.

[Raj, 2017-01-13 05:28:00]

The CQ bit was checked by rajendrant@chromium.org

[commit-bot: I haz the power, 2017-01-13 05:28:19]

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, 2017-01-13 05:33:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484285280460760,
"parent_rev": "ad3a8033e026174e6341e2f7bb08054c93dc2b43", "commit_rev":
"bea6ff95471b7a3da8030b0903d73e48434cc18a"}

[commit-bot: I haz the power, 2017-01-13 05:34:32]

Description was changed from

==========
Remove kAllowed and kFallbackAllowed from data reduction proxy

Data reduction proxy is always allowed with primary and fallback
mechanism.

BUG=654438
==========

to

==========
Remove kAllowed and kFallbackAllowed from data reduction proxy

Data reduction proxy is always allowed with primary and fallback
mechanism.

BUG=654438

Review-Url: https://codereview.chromium.org/2626963003
Cr-Commit-Position: refs/heads/master@{#443497}
Committed:
https://chromium.googlesource.com/chromium/src/+/bea6ff95471b7a3da8030b0903d7...
==========

[commit-bot: I haz the power, 2017-01-13 05:34:34]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bea6ff95471b7a3da8030b0903d7...

[Ted C, 2017-01-17 17:11:02]

On 2017/01/13 05:34:34, commit-bot: I haz the power wrote:
> Committed patchset #3 (id:40001) as
>
https://chromium.googlesource.com/chromium/src/+/bea6ff95471b7a3da8030b0903d7...

This breaks cronet:
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...

../../components/cronet/android/cronet_data_reduction_proxy.cc: In constructor
'cronet::CronetDataReductionProxy::CronetDataReductionProxy(const string&, const
string&, const string&, const string&, const string&,
scoped_refptr<base::SingleThreadTaskRunner>, net::NetLog*)':
../../components/cronet/android/cronet_data_reduction_proxy.cc:93:7: error:
'kAllowed' is not a member of 'data_reduction_proxy::DataReductionProxyParams'
       data_reduction_proxy::DataReductionProxyParams::kAllowed |
       ^
../../components/cronet/android/cronet_data_reduction_proxy.cc:94:11: error:
'kFallbackAllowed' is not a member of
'data_reduction_proxy::DataReductionProxyParams'
           data_reduc