Enable QUIC for proxies only when QUIC is enabled

Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|.
This change brings QUIC proxy code closer to
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change
 DRP will use a QUIC proxy only when it is initialized using client
config service (and not statically configured proxies).

BUG=609177
Committed: https://crrev.com/9bf2667a46fd3ba3f7c4f4c69d2217ca68959a45
Cr-Commit-Position: refs/heads/master@{#393002}

[tbansal1, 2016-05-05 05:54:04]

Patchset #1 (id:1) has been deleted

[tbansal1, 2016-05-05 21:02:17]

Patchset #1 (id:20001) has been deleted

[tbansal1, 2016-05-05 21:04:33]

Description was changed from

==========
Enable Quic for proxies only when QUIC is enabled.

BUG=
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled.

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere.
2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

BUG=
==========

[tbansal1, 2016-05-05 21:19:36]

Patchset #1 (id:40001) has been deleted

[tbansal1, 2016-05-05 21:19:58]

Patchset #1 (id:60001) has been deleted

[tbansal1, 2016-05-05 21:22:05]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled.

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere.
2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

BUG=
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled.

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This would make it easier to
deprecate d_r_p_params in future.

BUG=
==========

[tbansal1, 2016-05-05 21:22:27]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled.

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This would make it easier to
deprecate d_r_p_params in future.

BUG=
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled.

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=
==========

[tbansal1, 2016-05-05 21:23:56]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled.

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=
==========

[tbansal1, 2016-05-05 21:24:47]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=609177
==========

[tbansal1, 2016-05-05 21:34:00]

Patchset #1 (id:80001) has been deleted

[tbansal1, 2016-05-05 21:34:42]

Patchset #1 (id:100001) has been deleted

[tbansal1, 2016-05-05 21:35:11]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|
everywhere. This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=609177
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. This change brings QUIC
proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=609177
==========

[tbansal1, 2016-05-05 21:36:11]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. This change brings QUIC
proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=609177
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=609177
==========

[tbansal1, 2016-05-05 21:38:24]

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

[tbansal1, 2016-05-05 21:38:25]

sclittle: PTAL at components/d_r_p and 
chrome/browser/net/spdyproxy/.

Thanks.

[sclittle, 2016-05-10 01:40:24]

components/d_r_p and chrome/browser/net/spdyproxy/ LGTM % nits

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc
(right):

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc:146:
url_request_context_getter->GetURLRequestContext() &&
nit: IIUC this can only safely be called on the IO thread, right? Could you add
a comment to that effect?

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
(right):

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc:444:
// Tests that QUIC proxy is used when all conditions are satisfied.
nit: Could you give this test a slightly different comment than the last? It's
not clear how it's different.

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
(left):

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.h:187:
// If true, uses QUIC instead of SPDY to connect to proxies that use TLS.
IIUC, this change makes it impossible to use the DRPParams to configure QUIC for
the DRP, and instead it's only possible to enable it using the ConfigService,
right?

[tbansal1, 2016-05-10 01:48:51]

I am going to address other comments when I upload the next patch, but wanted to
reply back to one of the questions in the comments.

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
(left):

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.h:187:
// If true, uses QUIC instead of SPDY to connect to proxies that use TLS.
On 2016/05/10 01:40:24, sclittle wrote:
> IIUC, this change makes it impossible to use the DRPParams to configure QUIC
for
> the DRP, and instead it's only possible to enable it using the ConfigService,
> right?

Yes, that code path will be deprecated/removed as soon as Cronet DRP integration
is fixed.

[tbansal1, 2016-05-10 20:14:12]

Patchset #2 (id:140001) has been deleted

[tbansal1, 2016-05-10 20:16:33]

tbansal@chromium.org changed reviewers:
+ rch@chromium.org, sgurun@chromium.org

[tbansal1, 2016-05-10 20:16:34]

PTAL
rch: ios/*, net/*, chrome/browser/io_thread*
sgurun: android_webview/browser/aw_browser_context.cc
Thanks!

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc
(right):

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc:146:
url_request_context_getter->GetURLRequestContext() &&
On 2016/05/10 01:40:24, sclittle wrote:
> nit: IIUC this can only safely be called on the IO thread, right? Could you
add
> a comment to that effect?

Done.

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
(right):

https://codereview.chromium.org/1862193002/diff/120001/components/data_reduct...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc:444:
// Tests that QUIC proxy is used when all conditions are satisfied.
On 2016/05/10 01:40:24, sclittle wrote:
> nit: Could you give this test a slightly different comment than the last? It's
> not clear how it's different.

Done.

[Ryan Hamilton, 2016-05-10 21:31:14]

https://codereview.chromium.org/1862193002/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.cc (left):

https://codereview.chromium.org/1862193002/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:1368: 
This means that the only way to enable QUIC for flywheel is to opt into the QUIC
field trial. Is that really what you want? It seems like that will mean that
quic-team will be enabling/disabling QUIC in flywheel?

[tbansal1, 2016-05-10 21:43:46]

https://codereview.chromium.org/1862193002/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.cc (left):

https://codereview.chromium.org/1862193002/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:1368: 
On 2016/05/10 21:31:14, Ryan Hamilton wrote:
> This means that the only way to enable QUIC for flywheel is to opt into the
QUIC
> field trial. Is that really what you want? It seems like that will mean that
> quic-team will be enabling/disabling QUIC in flywheel?

Yes, I think that's the right approach in longer term. It avoids any weird bugs
that may pop up when enable_quic != enable_quic_for_proxies. wdyt?

[sgurun-gerrit only, 2016-05-10 21:47:12]

aw lgtm

[Ryan Hamilton, 2016-05-10 22:41:24]

lgtm

https://codereview.chromium.org/1862193002/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.cc (left):

https://codereview.chromium.org/1862193002/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:1368: 
On 2016/05/10 21:43:46, tbansal1 wrote:
> On 2016/05/10 21:31:14, Ryan Hamilton wrote:
> > This means that the only way to enable QUIC for flywheel is to opt into the
> QUIC
> > field trial. Is that really what you want? It seems like that will mean that
> > quic-team will be enabling/disabling QUIC in flywheel?
> 
> Yes, I think that's the right approach in longer term. It avoids any weird
bugs
> that may pop up when enable_quic != enable_quic_for_proxies. wdyt?

Discussed offline. LGTM.

[tbansal1, 2016-05-10 22:42:41]

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

[tbansal1, 2016-05-10 22:42:42]

mmenke: PTAL at chrome/browser/profiles/profile_impl_io_data.cc and
components/cronet/.

Thanks.

[tbansal1, 2016-05-11 01:26:34]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

With this change DRP will use a QUIC proxy only when it 
is using client config service (and not statically 
configured proxies). This makes it easier to deprecate 
d_r_p_params in future.

BUG=609177
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change DRP will use a
QUIC proxy only when it is initialized using client 
config service (and not statically configured proxies).

BUG=609177
==========

[tbansal1, 2016-05-11 01:27:07]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change DRP will use a
QUIC proxy only when it is initialized using client 
config service (and not statically configured proxies).

BUG=609177
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change
 DRP will use a QUIC proxy only when it is initialized using client 
config service (and not statically configured proxies).

BUG=609177
==========

[mmenke, 2016-05-11 15:04:29]

On 2016/05/10 22:42:42, tbansal1 wrote:
> mmenke: PTAL at chrome/browser/profiles/profile_impl_io_data.cc and
> components/cronet/.
> 
> Thanks.

LGTM, rubberstamp, deferring to Ryan.

[tbansal1, 2016-05-11 18:26:16]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-05-11 18:26:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
sclittle@chromium.org
Link to the patchset: https://codereview.chromium.org/1862193002/#ps160001
(title: "Addressed sclittle comments")

[commit-bot: I haz the power, 2016-05-11 18:27:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1862193002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1862193002/160001

[commit-bot: I haz the power, 2016-05-11 18:32:56]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change
 DRP will use a QUIC proxy only when it is initialized using client 
config service (and not statically configured proxies).

BUG=609177
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change
 DRP will use a QUIC proxy only when it is initialized using client 
config service (and not statically configured proxies).

BUG=609177
==========

[commit-bot: I haz the power, 2016-05-11 18:32:58]

Committed patchset #2 (id:160001)

[commit-bot: I haz the power, 2016-05-11 18:34:28]

Description was changed from

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|. 
This change brings QUIC proxy code closer to 
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from 
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change
 DRP will use a QUIC proxy only when it is initialized using client 
config service (and not statically configured proxies).

BUG=609177
==========

to

==========
Enable QUIC for proxies only when QUIC is enabled

1. Remove |enable_quic_for_proxies| from
HttpNetworkSession::Params. Instead, use |enable_quic|.
This change brings QUIC proxy code closer to
the global QUIC code.

2. Do not plumb |enable_quic| down to Data Reduction
Proxy (DRP) IO Data. Instead, DRP reads |enable_quic| from
HttpNetworkSession::Params directly.

3. Chrome will use QUIC to connect to DRP if QUIC is enabled
globally and Chrome is in DRP QUIC field trial. With this change
 DRP will use a QUIC proxy only when it is initialized using client
config service (and not statically configured proxies).

BUG=609177

Committed: https://crrev.com/9bf2667a46fd3ba3f7c4f4c69d2217ca68959a45
Cr-Commit-Position: refs/heads/master@{#393002}
==========

[commit-bot: I haz the power, 2016-05-11 18:34:30]

Patchset 2 (id:??) landed as
https://crrev.com/9bf2667a46fd3ba3f7c4f4c69d2217ca68959a45
Cr-Commit-Position: refs/heads/master@{#393002}