New issue
Advanced search Search tips

Issue 634061 link

Starred by 3 users
Status: Fixed
Owner:
tbansal@chromium.org
Closed: Dec 2016
Cc:
harringtond@google.com
ryansturm@chromium.org
bengr@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocked on:
issue 701062


Sign in to add a comment

Accept-Encoding: br not sent for data_reduction_proxy requests

Project Member Reported by harringtond@google.com, Aug 3 2016 
Chrome only adds Accept-Encoding: br for https requests. 

The data reduction proxy can encode content with Brotli, but because of the missing Accept-Encoding: br header, there is no way to know if the client has Brotli enabled.

Suggested fix is to add Accept-Encoding: br for requests going through the data reduction proxy.
Project Member

Comment 1 by sheriffbot@chromium.org, Aug 5 2016

Labels: Hotlist-Google

Comment 3 by harringtond@google.com, Aug 10 2016

Status: Fixed (was: Started)

Comment 4 by harringtond@google.com, Aug 23 2016

Status: Started (was: Fixed)
Changing the Accept-Encoding header for data reduction proxy requests is problematic. Local cache lookups will not have this header resulting in cache misses for Vary:Accept-Encoding resources.

For now, do not change Accept-Encoding header.

Comment 6 by harringtond@google.com, Sep 30 2016

Owner: bengr@chromium.org
Status: Assigned (was: Started)

Comment 7 by bengr@chromium.org, Oct 19 2016

Cc: ryansturm@chromium.org
Owner: tbansal@chromium.org

Comment 8 by tbansal@chromium.org, Oct 31 2016

Status: Started (was: Assigned)

Comment 9 by tbansal@chromium.org, Oct 31 2016

Labels: -Pri-3 M-56 Pri-2
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3a49eedb67a035baf453f1c5695ccc8d14b923c

commit d3a49eedb67a035baf453f1c5695ccc8d14b923c
Author: tbansal <tbansal@chromium.org>
Date: Tue Dec 13 22:05:32 2016

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
Cr-Commit-Position: refs/heads/master@{#438307}

[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/d3a49eedb67a035baf453f1c5695ccc8d14b923c/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc

Comment 11 by tbansal@chromium.org, Dec 14 2016

Cc: harringtond@google.com
CL in #10 got released in 57.0.2951.0. So, beginning 57.0.2951.0, Chrome will add "br" to the accept-encoding header if the request is being fetched via a secure data reduction proxy.

Comment 12 by tbansal@chromium.org, Dec 14 2016

Labels: -M-56 M-57
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d20801e2b126cfec1d66cb78c05082d0f90b3ba

commit 1d20801e2b126cfec1d66cb78c05082d0f90b3ba
Author: tbansal <tbansal@chromium.org>
Date: Fri Dec 16 01:49:46 2016

Cleanup DataReductionProxy (DRP) Brotli code

Add thread checkers to request options class.

Force the generation of the chrome-proxy header, and use it later when
generating the MockWrite for the sockets in tests.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG= 634061 

Review-Url: https://codereview.chromium.org/2571993002
Cr-Commit-Position: refs/heads/master@{#438978}

[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7e7022d9679740ace361f373f754a506f9e72e1

commit b7e7022d9679740ace361f373f754a506f9e72e1
Author: mathp <mathp@chromium.org>
Date: Fri Dec 16 14:54:17 2016

Revert of Cleanup DataReductionProxy (DRP) Brotli code (patchset #3 id:140001 of https://codereview.chromium.org/2571993002/ )

Reason for revert:
Reverting because it's hitting a DCHECK on top of tree (android). It's going to make it hard for people to work.

Filed a bug at https://bugs.chromium.org/p/chromium/issues/detail?id=674918

Original issue's description:
> Cleanup DataReductionProxy (DRP) Brotli code
>
> Add thread checkers to request options class.
>
> Force the generation of the chrome-proxy header, and use it later when
> generating the MockWrite for the sockets in tests.
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
>
> BUG= 634061 
>
> Committed: https://crrev.com/1d20801e2b126cfec1d66cb78c05082d0f90b3ba
> Cr-Commit-Position: refs/heads/master@{#438978}

TBR=ryansturm@chromium.org,tbansal@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 634061 

Review-Url: https://codereview.chromium.org/2577413002
Cr-Commit-Position: refs/heads/master@{#439103}

[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/b7e7022d9679740ace361f373f754a506f9e72e1/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 17 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eede5513b6b631647b3cf84e42436ca8b4694294

commit eede5513b6b631647b3cf84e42436ca8b4694294
Author: tbansal <tbansal@chromium.org>
Date: Sat Dec 17 00:45:17 2016

Cleanup DataReductionProxy (DRP) Brotli code

Add thread checkers to request options class.

Force the generation of the chrome-proxy header, and use it later when
generating the MockWrite for the sockets in tests.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG= 634061 

Review-Url: https://codereview.chromium.org/2581313002
Cr-Commit-Position: refs/heads/master@{#439260}

[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/eede5513b6b631647b3cf84e42436ca8b4694294/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h

Comment 16 by tbansal@chromium.org, Dec 19 2016

Status: Fixed (was: Started)
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee71e66f483b63c57c67fe350722a049579bd2ec

commit ee71e66f483b63c57c67fe350722a049579bd2ec
Author: tbansal <tbansal@chromium.org>
Date: Mon Jan 09 19:30:27 2017

Add Vary: Accept encoding header to DRP Brotli tests

The additional header ensures that even if the proxy server
adds Vary accept encoding header, the second request is still served
by the cache.

BUG= 634061 

Review-Url: https://codereview.chromium.org/2617333002
Cr-Commit-Position: refs/heads/master@{#442309}

[modify] https://crrev.com/ee71e66f483b63c57c67fe350722a049579bd2ec/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d41aee024940ce81f8f63f245a97da89e6771668

commit d41aee024940ce81f8f63f245a97da89e6771668
Author: tbansal <tbansal@chromium.org>
Date: Wed Jan 18 00:17:08 2017

Add Brotli to the experiment config

Add "allow_brotli" as a param for data reduction proxy server experiment.
"prefetch" has been removed since that experiment has been turned down.

BUG= 634061 

Review-Url: https://codereview.chromium.org/2635273004
Cr-Commit-Position: refs/heads/master@{#444198}

[modify] https://crrev.com/d41aee024940ce81f8f63f245a97da89e6771668/testing/variations/fieldtrial_testing_config.json

Comment 19 by tbansal@chromium.org, Mar 13 2017

Blockedon: 701062
►

Sign in to add a comment