Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy (DRP) server, Brotli is enabled globally,
and Brotli for data reduction proxy (DRP) is not disabled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061
Committed: https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c
Cr-Commit-Position: refs/heads/master@{#438307}

[tbansal1, 2016-12-09 22:27:45]

Patchset #1 (id:1) has been deleted

[tbansal1, 2016-12-09 22:30:06]

Patchset #1 (id:20001) has been deleted

[tbansal1, 2016-12-09 22:30:26]

Description was changed from

==========
Add Brotli to Accept Encoding header

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy server, Brotli is enabled globally,
and Brotli for data reduction proxy is not didsbled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

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

to

==========
Add Brotli to Accept Encoding header

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy server, Brotli is enabled globally,
and Brotli for data reduction proxy is not didsbled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061
==========

[tbansal1, 2016-12-09 22:30:30]

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

[tbansal1, 2016-12-09 22:31:11]

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

[tbansal1, 2016-12-09 22:31:11]

ryansturm: ptal. Thanks.

[commit-bot: I haz the power, 2016-12-09 22:33:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-12-09 22:34:25]

CC'ing mmenke@chromium.org, rdsmith@chromium.org in case they are interested.

[tbansal1, 2016-12-09 22:35:28]

Description was changed from

==========
Add Brotli to Accept Encoding header

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy server, Brotli is enabled globally,
and Brotli for data reduction proxy is not didsbled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061
==========

to

==========
Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy (DRP) server, Brotli is enabled globally,
and Brotli for data reduction proxy (DRP) is not disabled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061
==========

[commit-bot: I haz the power, 2016-12-10 03:38:53]

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

[commit-bot: I haz the power, 2016-12-10 03:38:53]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[RyanSturm, 2016-12-12 18:14:25]

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
(right):

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc:541:
DCHECK_EQ(std::string::npos, header_value.find(kBrotli));
Why is this true? Does this rely on the fact that the URL is originally HTTP?

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
(right):

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc:965:
std::unique_ptr<net::URLRequest> request_1(FetchURLRequest(
s/request_1/request/

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc:989:
std::unique_ptr<net::URLRequest> request_1(
s/request_1/request/

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:210:
bool IsBrotliAcceptEncodingEnabled() {
Change to a real enabled/control/disabled experiment. This will control memory
overhead better.

[tbansal1, 2016-12-12 23:57:40]

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

[commit-bot: I haz the power, 2016-12-12 23:58:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-12-12 23:59:34]

ryansturm: ptal. Thanks.

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
(right):

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc:541:
DCHECK_EQ(std::string::npos, header_value.find(kBrotli));
On 2016/12/12 18:14:24, Ryan Sturm wrote:
> Why is this true? Does this rely on the fact that the URL is originally HTTP?

Yes, added to the comment.

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
(right):

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc:965:
std::unique_ptr<net::URLRequest> request_1(FetchURLRequest(
On 2016/12/12 18:14:24, Ryan Sturm wrote:
> s/request_1/request/

Done.

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc:989:
std::unique_ptr<net::URLRequest> request_1(
On 2016/12/12 18:14:25, Ryan Sturm wrote:
> s/request_1/request/

Done.

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/2566773002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:210:
bool IsBrotliAcceptEncodingEnabled() {
On 2016/12/12 18:14:25, Ryan Sturm wrote:
> Change to a real enabled/control/disabled experiment. This will control memory
> overhead better.

Talked with harringtond. They are going to send brotli encoded content only for
users on certain channels for now. Also, memory is not a big concern since
Brotli is already used by HTTPS server. So, keeping this as it is.

[RyanSturm, 2016-12-13 00:14:14]

lgtm

[commit-bot: I haz the power, 2016-12-13 01:49:05]

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

[commit-bot: I haz the power, 2016-12-13 01:49:06]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2016-12-13 02:27:01]

Patchset #3 (id:80001) has been deleted

[tbansal1, 2016-12-13 02:27:10]

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

[commit-bot: I haz the power, 2016-12-13 02:27:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-12-13 04:06:22]

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

[tbansal1, 2016-12-13 04:06:26]

Patchset #3 (id:100001) has been deleted

[commit-bot: I haz the power, 2016-12-13 04:06:42]

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, 2016-12-13 06:29:40]

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

[commit-bot: I haz the power, 2016-12-13 06:29:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2016-12-13 17:11:39]

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

[commit-bot: I haz the power, 2016-12-13 17:12:03]

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, 2016-12-13 17:21:13]

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

[commit-bot: I haz the power, 2016-12-13 17:21:15]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tbansal1, 2016-12-13 17:26:39]

Patchset #3 (id:120001) has been deleted

[tbansal1, 2016-12-13 17:26:58]

Patchset #3 (id:140001) has been deleted

[tbansal1, 2016-12-13 17:27:06]

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

[commit-bot: I haz the power, 2016-12-13 17:27:37]

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, 2016-12-13 18:34:31]

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

[commit-bot: I haz the power, 2016-12-13 18:34:32]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2016-12-13 21:10:22]

Patchset #4 (id:180001) has been deleted

[tbansal1, 2016-12-13 21:10:34]

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

[commit-bot: I haz the power, 2016-12-13 21:11:14]

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, 2016-12-13 21:58:09]

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

[commit-bot: I haz the power, 2016-12-13 21:58:10]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2016-12-13 21:58:23]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-12-13 21:58:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
ryansturm@chromium.org
Link to the patchset: https://codereview.chromium.org/2566773002/#ps200001
(title: "Fix test")

[commit-bot: I haz the power, 2016-12-13 21:59:11]

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, 2016-12-13 22:05:40]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1481666303883450,
"parent_rev": "eea808ca961d9937dd2b24c60eec010514432074", "commit_rev":
"ab83831205042937f1eed9d6ebf10e6ab3817480"}

[commit-bot: I haz the power, 2016-12-13 22:06:05]

Description was changed from

==========
Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy (DRP) server, Brotli is enabled globally,
and Brotli for data reduction proxy (DRP) is not disabled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061
==========

to

==========
Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy (DRP) server, Brotli is enabled globally,
and Brotli for data reduction proxy (DRP) is not disabled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061

Review-Url: https://codereview.chromium.org/2566773002
==========

[commit-bot: I haz the power, 2016-12-13 22:06:07]

Committed patchset #4 (id:200001)

[commit-bot: I haz the power, 2016-12-13 22:09:22]

Description was changed from

==========
Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy (DRP) server, Brotli is enabled globally,
and Brotli for data reduction proxy (DRP) is not disabled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061

Review-Url: https://codereview.chromium.org/2566773002
==========

to

==========
Add Brotli to Accept Encoding header for secure DRP requests

Add Brotli to Accept Encoding header if the request is fetched from a
secure data reduction proxy (DRP) server, Brotli is enabled globally,
and Brotli for data reduction proxy (DRP) is not disabled via field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=634061

Committed: https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c
Cr-Commit-Position: refs/heads/master@{#438307}
==========

[commit-bot: I haz the power, 2016-12-13 22:09:23]

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