New issue
Advanced search Search tips

Issue 839140 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

When the DRP proxy config changes, in-progress requests to the previous DRP targets aren't classified as being from the DRP.

Project Member Reported by sclit...@chromium.org, May 2 2018

Issue description

Suppose the following sequence of events happen:
1) Chrome sends a request for an image resource to the currently configured DRP.
2) Chrome updates the DRP proxy config, causing a new and different list of proxies to be used for the DRP.
3) The response for that image comes back from the previously-configured DRP.
4) Since DataReductionProxyConfig::IsDataReductionProxy() only checks the proxy_server against the currently configured DRP targets, which doesn't include that old DRP target, Chrome doesn't treat that image response as having come from the DRP.

The exception to this is that most of the DRP bypass logic would still apply, since there's currently a hack in the bypass logic to treat any response with the DRP Via header from any proxy as if it was from a DRP proxy. This happens a tiny but non-zero number of times, see the DataReductionProxy.ResponseProxyServerStatus histogram.

I'm removing this hack, though, so we should actually solve this problem, such as by having the DataReductionProxyConfig keep track of the last N lists of configured DRP targets, and treat any proxy that matches a proxy on those lists as a DRP.

N = 1 will probably be enough.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 8 2018

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

commit b24aa115f3f292570ce856615dc6a0c8bee0ebfc
Author: Scott Little <sclittle@chromium.org>
Date: Tue May 08 17:28:44 2018

Use DRP logic for recent previously configured DRP servers.

Before this CL, if the list of configured proxies to use as Data
Reduction Proxies changes while a request to a previously configured
Data Reduction Proxy server is in-progress, then DRP-specific bypass
logic and metrics wouldn't be applied to that response since its proxy
server doesn't match the currently configured proxies.

To solve this, this CL changes Chrome to keep track of the two most
recent previous proxy configurations, such that it treats any proxy that
matches the current configuration or these old configurations as a Data
Reduction Proxy.

This CL also includes some clean up and minor miscellaneous
refactoring/optimizations.

Bug:  839140 
Change-Id: I93ea5c45723aabea2f6a2a4960ad72132f5c30b9
Reviewed-on: https://chromium-review.googlesource.com/1045025
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556850}
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values_unittest.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/common/BUILD.gn
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/common/data_reduction_proxy_config_values.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
[add] https://crrev.com/b24aa115f3f292570ce856615dc6a0c8bee0ebfc/components/data_reduction_proxy/core/common/data_reduction_proxy_type_info.h

Status: Fixed (was: Started)

Sign in to add a comment