Added alternative configuration for the data reduction proxy

Added alternative configuration for the data reduction proxy

Added a second configuration that can support two http proxies and one https proxy. In the process, added a new DataReductionProxyParams object to encapsulate static configuration details and made DataReductionSettingsAndroid a BrowserKeyedContextService.

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

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

[bengr, 2014-05-20 21:03:31]

sgurun: android_webview/*
nyquist: chrome/android/*
thestig: chrome/browser/io_thread.*
mef: chrome/browser/net/predictor.cc
marq: chrome/browser/net/spdyproxy/*
      components/components_tests.gyp
      components/data_reduction_proxy.gypi
      components/data_reduction_proxy/*
mmenke: chrome/browser/profiles/profile_impl_io_data.cc

[sgurun-gerrit only, 2014-05-20 23:55:21]

aw lgtm

https://codereview.chromium.org/286013002/diff/140001/android_webview/browser...
File android_webview/browser/aw_browser_context.cc (right):

https://codereview.chromium.org/286013002/diff/140001/android_webview/browser...
android_webview/browser/aw_browser_context.cc:101: true, false, false, false)));
nit: I think this large list of booleans makes it easier to miss any
configuration issues. I like explicitly stating,              
datareductionproxyparams->setAllowed(true).

consider explicitly configuring.

https://codereview.chromium.org/286013002/diff/140001/android_webview/browser...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/286013002/diff/140001/android_webview/browser...
android_webview/browser/net/aw_url_request_context_getter.cc:198: true, false,
false, false);
nit: same comment about the bool list.

[mmenke, 2014-05-21 16:00:19]

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
File chrome/browser/net/spdyproxy/proxy_advisor.cc (right):

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
chrome/browser/net/spdyproxy/proxy_advisor.cc:25: // TODO(marq): Remove this
class because it is not being used.
Which class?

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
chrome/browser/net/spdyproxy/proxy_advisor.cc:126: true, true, false, true);
Suggest either enums instead of bools, or adding comments with the variable
names here after each of the bools.  A bunch of bools can be hard to follow.

[mmenke, 2014-05-21 16:10:17]

Oops...looked at the wrong file.  profiles/ LGTM.

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/profiles...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/profiles...
chrome/browser/profiles/profile_impl_io_data.cc:53: #include
"components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
Is this actually needed?  We don't dereference a pointer to a params object,
just pass a pointer around.

[sgurun-gerrit only, 2014-05-21 16:21:29]

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
File chrome/browser/net/spdyproxy/proxy_advisor.cc (right):

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
chrome/browser/net/spdyproxy/proxy_advisor.cc:126: true, true, false, true);
I like the idea of using enums. It will be a lot more readable.

On 2014/05/21 16:00:20, mmenke wrote:
> Suggest either enums instead of bools, or adding comments with the variable
> names here after each of the bools.  A bunch of bools can be hard to follow.

[nyquist, 2014-05-21 17:37:08]

chrome/android lgtm

[Lei Zhang, 2014-05-21 19:41:28]

c/b/io_threads.* lgtm

https://codereview.chromium.org/286013002/diff/140001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/286013002/diff/140001/build/common.gypi#newco...
build/common.gypi:556: 'data_reduction_proxy_alt_fallback_origin%' : '',
I'm a little concerned by the number of gyp variables here. Can you:

- Add some documentation to briefly explain how these work and how they relate?
Surely there aren't 2^9 possible build configs based on these variables, right?

- Add a newline to separate your list of gyp variables from the unrelated
service discovery ones below.

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/io_threa...
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/io_threa...
chrome/browser/io_thread.cc:101: #include
"components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
nit: not really needed since you already included it in io_thread.h.

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/io_threa...
chrome/browser/io_thread.cc:910: if
(globals_->data_reduction_proxy_params.get()) {
Have you considered wrapping this block in an ifdef since it's effectively a
no-op on desktop? Android folks have been excluding desktop-only code/resources
to save space on mobile. I think it's fair to do the reverse too.

[mef, 2014-05-23 18:39:35]

chrome/browser/net/predictor.cc lgtm

[marq (ping after 24h), 2014-05-27 22:31:46]

lgtm, although there will be some work to merge on iOS.

[bengr, 2014-05-28 01:25:32]

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
File chrome/browser/net/spdyproxy/proxy_advisor.cc (right):

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
chrome/browser/net/spdyproxy/proxy_advisor.cc:25: // TODO(marq): Remove this
class because it is not being used.
On 2014/05/21 16:00:20, mmenke wrote:
> Which class?

This ProxyAdvisor class.

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/net/spdy...
chrome/browser/net/spdyproxy/proxy_advisor.cc:126: true, true, false, true);
On 2014/05/21 16:21:29, sgurun wrote:
> I like the idea of using enums. It will be a lot more readable.
> 
> On 2014/05/21 16:00:20, mmenke wrote:
> > Suggest either enums instead of bools, or adding comments with the variable
> > names here after each of the bools.  A bunch of bools can be hard to follow.
> 

Done.

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/profiles...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

https://codereview.chromium.org/286013002/diff/140001/chrome/browser/profiles...
chrome/browser/profiles/profile_impl_io_data.cc:53: #include
"components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
On 2014/05/21 16:10:17, mmenke wrote:
> Is this actually needed?  We don't dereference a pointer to a params object,
> just pass a pointer around.

Done.

[bengr, 2014-05-28 07:35:15]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-28 07:36:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/260001

[commit-bot: I haz the power, 2014-05-28 11:15:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-05-28 11:16:39]

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

[commit-bot: I haz the power, 2014-05-28 11:16:40]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bengr, 2014-05-28 14:33:09]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-28 14:33:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/260001

[commit-bot: I haz the power, 2014-05-28 18:32:02]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-05-28 18:33:58]

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

[commit-bot: I haz the power, 2014-05-28 18:33:59]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bengr, 2014-05-28 21:06:04]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-28 21:08:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/260001

[commit-bot: I haz the power, 2014-05-29 00:29:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-05-29 00:32:06]

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

[commit-bot: I haz the power, 2014-05-29 00:32:07]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bengr, 2014-05-29 03:37:30]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-29 03:39:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/260001

[bengr, 2014-05-29 03:39:58]

The CQ bit was unchecked by bengr@chromium.org

[bengr, 2014-05-29 05:20:49]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-29 05:22:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/300001

[bengr, 2014-05-29 06:42:12]

The CQ bit was unchecked by bengr@chromium.org

[bengr, 2014-05-29 07:45:13]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-29 07:47:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/340001

[commit-bot: I haz the power, 2014-05-29 09:21:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-05-29 09:23:50]

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

[commit-bot: I haz the power, 2014-05-29 09:23:51]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[bengr, 2014-05-29 17:06:14]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-29 17:10:03]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/360001

[commit-bot: I haz the power, 2014-05-29 18:53:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[bengr, 2014-05-29 21:26:59]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-29 21:29:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/380001

[commit-bot: I haz the power, 2014-05-29 22:03:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-29 22:40:16]

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

[commit-bot: I haz the power, 2014-05-29 22:40:17]

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

[bengr, 2014-05-30 08:08:23]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-30 08:09:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/470001

[commit-bot: I haz the power, 2014-05-30 13:06:30]

Change committed as 273810

[bengr, 2014-05-31 05:00:09]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-31 05:02:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/286013002/490001

[commit-bot: I haz the power, 2014-05-31 22:19:51]

Change committed as 274067