Commit 563061cf authored by megjablon's avatar megjablon Committed by Commit bot

Move adding Lo-Fi directives from DRPRequestOptions to ContentLoFiDecider

All Lo-Fi directives for the "Chrome-Proxy" header should be added by the
ContentLoFiDecider. This includes "q=low" and "exp=lofi_active_control".

BUG=539934

Review URL: https://codereview.chromium.org/1463583003

Cr-Commit-Position: refs/heads/master@{#361386}
parent 6ccf5fd5
......@@ -4,7 +4,12 @@
#include "components/data_reduction_proxy/content/browser/content_lofi_decider.h"
#include <string>
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
#include "content/public/browser/resource_request_info.h"
#include "net/http/http_request_headers.h"
namespace data_reduction_proxy {
......@@ -15,8 +20,59 @@ ContentLoFiDecider::~ContentLoFiDecider() {}
bool ContentLoFiDecider::IsUsingLoFiMode(const net::URLRequest& request) const {
const content::ResourceRequestInfo* request_info =
content::ResourceRequestInfo::ForRequest(&request);
// The Lo-Fi directive should not be added for users in the Lo-Fi field
// trial "Control" group. Check that the user is in a group that can get
// "q=low".
bool lofi_enabled_via_flag_or_field_trial =
params::IsLoFiOnViaFlags() || params::IsIncludedInLoFiEnabledFieldTrial();
// Return if the user is using Lo-Fi and not part of the "Control" group.
if (request_info)
return request_info->IsUsingLoFi();
return request_info->IsUsingLoFi() && lofi_enabled_via_flag_or_field_trial;
return false;
}
bool ContentLoFiDecider::MaybeAddLoFiDirectiveToHeaders(
const net::URLRequest& request,
net::HttpRequestHeaders* headers) const {
const content::ResourceRequestInfo* request_info =
content::ResourceRequestInfo::ForRequest(&request);
if (!request_info)
return false;
// The Lo-Fi directive should not be added for users in the Lo-Fi field
// trial "Control" group. Check that the user is in a group that should
// get "q=low".
bool lofi_enabled_via_flag_or_field_trial =
params::IsLoFiOnViaFlags() || params::IsIncludedInLoFiEnabledFieldTrial();
std::string header_value;
// User is using Lo-Fi and not part of the "Control" group.
if (request_info->IsUsingLoFi() && lofi_enabled_via_flag_or_field_trial) {
if (headers->HasHeader(chrome_proxy_header())) {
headers->GetHeader(chrome_proxy_header(), &header_value);
headers->RemoveHeader(chrome_proxy_header());
header_value += ", ";
}
header_value += chrome_proxy_lo_fi_directive();
headers->SetHeader(chrome_proxy_header(), header_value);
return true;
}
// User is part of Lo-Fi active control experiment.
if (request_info->IsUsingLoFi() &&
params::IsIncludedInLoFiControlFieldTrial()) {
if (headers->HasHeader(chrome_proxy_header())) {
headers->GetHeader(chrome_proxy_header(), &header_value);
headers->RemoveHeader(chrome_proxy_header());
header_value += ", ";
}
header_value += chrome_proxy_lo_fi_experiment_directive();
headers->SetHeader(chrome_proxy_header(), header_value);
}
return false;
}
......
......@@ -9,6 +9,7 @@
#include "base/macros.h"
namespace net {
class HttpRequestHeaders;
class URLRequest;
}
......@@ -17,14 +18,19 @@ namespace data_reduction_proxy {
// Class responsible for deciding whether a request should be requested with low
// fidelity (Lo-Fi) or not. Relies on the Lo-Fi mode state stored in the
// request's content::ResourceRequestInfo, which must be fetched using
// content::ResourceRequestInfo::ForRequest. Owned by DataReductionProxyIOData
// and should be called on the IO thread.
// content::ResourceRequestInfo::ForRequest. Lo-Fi mode will not be enabled for
// requests that don't have a ResourceRequestInfo, such as background requests.
// Owned by DataReductionProxyIOData and should be called on the IO thread.
class ContentLoFiDecider : public LoFiDecider {
public:
ContentLoFiDecider();
~ContentLoFiDecider() override;
// LoFiDecider implementation:
bool IsUsingLoFiMode(const net::URLRequest& request) const override;
bool MaybeAddLoFiDirectiveToHeaders(
const net::URLRequest& request,
net::HttpRequestHeaders* headers) const override;
private:
DISALLOW_COPY_AND_ASSIGN(ContentLoFiDecider);
......
......@@ -13,6 +13,7 @@
#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h"
#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h"
#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
......@@ -29,8 +30,6 @@ namespace data_reduction_proxy {
namespace {
const char kChromeProxyHeader[] = "chrome-proxy";
#if defined(OS_ANDROID)
const Client kClient = Client::CHROME_ANDROID;
#elif defined(OS_IOS)
......@@ -119,11 +118,23 @@ class ContentLoFiDeciderTest : public testing::Test {
static void VerifyLoFiHeader(bool expected_lofi_used,
const net::HttpRequestHeaders& headers) {
EXPECT_TRUE(headers.HasHeader(kChromeProxyHeader));
EXPECT_TRUE(headers.HasHeader(chrome_proxy_header()));
std::string header_value;
headers.GetHeader(chrome_proxy_header(), &header_value);
EXPECT_EQ(
expected_lofi_used,
header_value.find(chrome_proxy_lo_fi_directive()) != std::string::npos);
}
static void VerifyLoFiExperimentHeader(
bool expected_lofi_experiment_used,
const net::HttpRequestHeaders& headers) {
EXPECT_TRUE(headers.HasHeader(chrome_proxy_header()));
std::string header_value;
headers.GetHeader(kChromeProxyHeader, &header_value);
EXPECT_EQ(expected_lofi_used,
header_value.find("q=low") != std::string::npos);
headers.GetHeader(chrome_proxy_header(), &header_value);
EXPECT_EQ(expected_lofi_experiment_used,
header_value.find(chrome_proxy_lo_fi_experiment_directive()) !=
std::string::npos);
}
protected:
......@@ -222,4 +233,104 @@ TEST_F(ContentLoFiDeciderTest, LoFiControlFieldTrial) {
}
}
TEST_F(ContentLoFiDeciderTest, AutoLoFi) {
const struct {
bool auto_lofi_enabled_group;
bool auto_lofi_control_group;
bool network_prohibitively_slow;
} tests[] = {
{false, false, false},
{false, false, true},
{true, false, false},
{true, false, true},
{false, true, false},
{false, true, true},
// Repeat this test data to simulate user moving out of Lo-Fi control
// experiment.
{false, true, false},
};
for (size_t i = 0; i < arraysize(tests); ++i) {
test_context_->config()->ResetLoFiStatusForTest();
// Lo-Fi header is expected only if session is part of Lo-Fi enabled field
// trial and network is prohibitively slow.
bool expect_lofi_header =
tests[i].auto_lofi_enabled_group && tests[i].network_prohibitively_slow;
bool expect_lofi_experiment_header =
tests[i].auto_lofi_control_group && tests[i].network_prohibitively_slow;
base::FieldTrialList field_trial_list(nullptr);
if (tests[i].auto_lofi_enabled_group) {
base::FieldTrialList::CreateFieldTrial(params::GetLoFiFieldTrialName(),
"Enabled");
}
if (tests[i].auto_lofi_control_group) {
base::FieldTrialList::CreateFieldTrial(params::GetLoFiFieldTrialName(),
"Control");
}
test_context_->config()->SetNetworkProhibitivelySlow(
tests[i].network_prohibitively_slow);
scoped_ptr<net::URLRequest> request =
CreateRequest(tests[i].network_prohibitively_slow);
net::HttpRequestHeaders headers;
NotifyBeforeSendProxyHeaders(&headers, request.get());
VerifyLoFiHeader(expect_lofi_header, headers);
VerifyLoFiExperimentHeader(expect_lofi_experiment_header, headers);
}
}
TEST_F(ContentLoFiDeciderTest, SlowConnectionsFlag) {
const struct {
bool slow_connections_flag_enabled;
bool network_prohibitively_slow;
bool auto_lofi_enabled_group;
} tests[] = {
{false, false, false}, {false, true, false}, {true, false, false},
{true, true, false}, {false, false, true}, {false, true, true},
{true, false, true}, {true, true, true},
};
for (size_t i = 0; i < arraysize(tests); ++i) {
test_context_->config()->ResetLoFiStatusForTest();
// For the purpose of this test, Lo-Fi header is expected only if LoFi Slow
// Connection Flag is enabled or session is part of Lo-Fi enabled field
// trial. For both cases, an additional condition is that network must be
// prohibitively slow.
bool expect_lofi_header = (tests[i].slow_connections_flag_enabled &&
tests[i].network_prohibitively_slow) ||
(!tests[i].slow_connections_flag_enabled &&
tests[i].auto_lofi_enabled_group &&
tests[i].network_prohibitively_slow);
std::string expected_header;
if (tests[i].slow_connections_flag_enabled) {
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kDataReductionProxyLoFi,
switches::kDataReductionProxyLoFiValueSlowConnectionsOnly);
}
base::FieldTrialList field_trial_list(nullptr);
if (tests[i].auto_lofi_enabled_group) {
base::FieldTrialList::CreateFieldTrial(params::GetLoFiFieldTrialName(),
"Enabled");
}
test_context_->config()->SetNetworkProhibitivelySlow(
tests[i].network_prohibitively_slow);
scoped_ptr<net::URLRequest> request =
CreateRequest(tests[i].network_prohibitively_slow);
net::HttpRequestHeaders headers;
NotifyBeforeSendProxyHeaders(&headers, request.get());
VerifyLoFiHeader(expect_lofi_header, headers);
}
}
} // namespace data_reduction_roxy
......@@ -174,12 +174,11 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendProxyHeadersInternal(
net::HttpRequestHeaders* headers) {
DCHECK(data_reduction_proxy_config_);
bool is_using_lofi_mode = false;
if (data_reduction_proxy_io_data_ &&
data_reduction_proxy_io_data_->lofi_decider() && request) {
LoFiDecider* lofi_decider = data_reduction_proxy_io_data_->lofi_decider();
is_using_lofi_mode = lofi_decider->IsUsingLoFiMode(*request);
bool is_using_lofi_mode =
lofi_decider->MaybeAddLoFiDirectiveToHeaders(*request, headers);
if ((request->load_flags() & net::LOAD_MAIN_FRAME) != 0) {
// TODO(megjablon): Need to switch to per page.
......@@ -190,7 +189,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendProxyHeadersInternal(
if (data_reduction_proxy_request_options_) {
data_reduction_proxy_request_options_->MaybeAddRequestHeader(
request, proxy_info.proxy_server(), headers, is_using_lofi_mode);
request, proxy_info.proxy_server(), headers);
}
}
......
......@@ -83,6 +83,27 @@ class TestLoFiDecider : public LoFiDecider {
should_request_lofi_resource_ = should_request_lofi_resource;
}
bool MaybeAddLoFiDirectiveToHeaders(
const net::URLRequest& request,
net::HttpRequestHeaders* headers) const override {
if (should_request_lofi_resource_) {
const char kChromeProxyHeader[] = "Chrome-Proxy";
std::string header_value;
if (headers->HasHeader(kChromeProxyHeader)) {
headers->GetHeader(kChromeProxyHeader, &header_value);
headers->RemoveHeader(kChromeProxyHeader);
header_value += ", ";
}
header_value += "q=low";
headers->SetHeader(kChromeProxyHeader, header_value);
return true;
}
return false;
}
private:
bool should_request_lofi_resource_;
};
......
......@@ -48,9 +48,7 @@ const char kSecureSessionHeaderOption[] = "s";
const char kBuildNumberHeaderOption[] = "b";
const char kPatchNumberHeaderOption[] = "p";
const char kClientHeaderOption[] = "c";
const char kLoFiHeaderOption[] = "q";
const char kExperimentsOption[] = "exp";
const char kLoFiExperimentID[] = "lofi_active_control";
// The empty version for the authentication protocol. Currently used by
// Android webview.
......@@ -160,56 +158,6 @@ void DataReductionProxyRequestOptions::UpdateVersion() {
RegenerateRequestHeaderValue();
}
void DataReductionProxyRequestOptions::MayRegenerateHeaderBasedOnLoFi(
bool is_using_lofi_mode) {
DCHECK(thread_checker_.CalledOnValidThread());
// The Lo-Fi directive should not be added for users in the Lo-Fi field
// trial "Control" group. Check that the user is in a group that should
// get "q=low".
bool lofi_enabled_via_flag_or_field_trial =
params::IsLoFiOnViaFlags() || params::IsIncludedInLoFiEnabledFieldTrial();
// Lo-Fi was not enabled, but now is. Add the header option.
if (lofi_.empty() && is_using_lofi_mode &&
lofi_enabled_via_flag_or_field_trial) {
lofi_ = "low";
RegenerateRequestHeaderValue();
return;
}
// Lo-Fi was enabled, but no longer is. Remove the header option.
if (!lofi_.empty() &&
(!is_using_lofi_mode || !lofi_enabled_via_flag_or_field_trial)) {
lofi_ = std::string();
RegenerateRequestHeaderValue();
return;
}
// User was not part of Lo-Fi active control experiment, but now is.
if (std::find(experiments_.begin(), experiments_.end(),
std::string(kLoFiExperimentID)) == experiments_.end() &&
is_using_lofi_mode && params::IsIncludedInLoFiControlFieldTrial()) {
experiments_.push_back(kLoFiExperimentID);
RegenerateRequestHeaderValue();
DCHECK(std::find(experiments_.begin(), experiments_.end(),
kLoFiExperimentID) != experiments_.end());
return;
}
// User was part of Lo-Fi active control experiment, but now is not.
auto it = std::find(experiments_.begin(), experiments_.end(),
std::string(kLoFiExperimentID));
if (it != experiments_.end() &&
(!is_using_lofi_mode || !params::IsIncludedInLoFiControlFieldTrial())) {
experiments_.erase(it);
RegenerateRequestHeaderValue();
DCHECK(std::find(experiments_.begin(), experiments_.end(),
std::string(kLoFiExperimentID)) == experiments_.end());
return;
}
}
void DataReductionProxyRequestOptions::UpdateExperiments() {
std::string experiments =
base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
......@@ -249,34 +197,30 @@ void DataReductionProxyRequestOptions::RandBytes(void* output,
void DataReductionProxyRequestOptions::MaybeAddRequestHeader(
net::URLRequest* request,
const net::ProxyServer& proxy_server,
net::HttpRequestHeaders* request_headers,
bool is_using_lofi_mode) {
net::HttpRequestHeaders* request_headers) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!proxy_server.is_valid())
return;
if (proxy_server.is_direct())
return;
MaybeAddRequestHeaderImpl(request, proxy_server.host_port_pair(), false,
request_headers, is_using_lofi_mode);
request_headers);
}
void DataReductionProxyRequestOptions::MaybeAddProxyTunnelRequestHandler(
const net::HostPortPair& proxy_server,
net::HttpRequestHeaders* request_headers) {
DCHECK(thread_checker_.CalledOnValidThread());
MaybeAddRequestHeaderImpl(nullptr, proxy_server, true, request_headers,
false);
MaybeAddRequestHeaderImpl(nullptr, proxy_server, true, request_headers);
}
void DataReductionProxyRequestOptions::SetHeader(
const net::URLRequest* request,
net::HttpRequestHeaders* headers,
bool is_using_lofi_mode) {
net::HttpRequestHeaders* headers) {
base::Time now = Now();
// Authorization credentials must be regenerated if they are expired.
if (!use_assigned_credentials_ && (now > credentials_expiration_time_))
UpdateCredentials();
MayRegenerateHeaderBasedOnLoFi(is_using_lofi_mode);
const char kChromeProxyHeader[] = "Chrome-Proxy";
std::string header_value;
if (headers->HasHeader(kChromeProxyHeader)) {
......@@ -395,14 +339,13 @@ void DataReductionProxyRequestOptions::MaybeAddRequestHeaderImpl(
const net::URLRequest* request,
const net::HostPortPair& proxy_server,
bool expect_ssl,
net::HttpRequestHeaders* request_headers,
bool is_using_lofi_mode) {
net::HttpRequestHeaders* request_headers) {
if (proxy_server.IsEmpty())
return;
if (data_reduction_proxy_config_->IsDataReductionProxy(proxy_server, NULL) &&
data_reduction_proxy_config_->UsingHTTPTunnel(proxy_server) ==
expect_ssl) {
SetHeader(request, request_headers, is_using_lofi_mode);
SetHeader(request, request_headers);
}
}
......@@ -422,8 +365,6 @@ void DataReductionProxyRequestOptions::RegenerateRequestHeaderValue() {
headers.push_back(FormatOption(kBuildNumberHeaderOption, build_));
headers.push_back(FormatOption(kPatchNumberHeaderOption, patch_));
}
if (!lofi_.empty())
headers.push_back(FormatOption(kLoFiHeaderOption, lofi_));
for (const auto& experiment : experiments_)
headers.push_back(FormatOption(kExperimentsOption, experiment));
......
......@@ -29,9 +29,7 @@ extern const char kSecureSessionHeaderOption[];
extern const char kBuildNumberHeaderOption[];
extern const char kPatchNumberHeaderOption[];
extern const char kClientHeaderOption[];
extern const char kLoFiHeaderOption[];
extern const char kExperimentsOption[];
extern const char kLoFiExperimentID[];
#if defined(OS_ANDROID)
extern const char kAndroidWebViewProtocolVersion[];
......@@ -94,14 +92,12 @@ class DataReductionProxyRequestOptions {
void Init();
// Adds a 'Chrome-Proxy' header to |request_headers| with the data reduction
// proxy authentication credentials. If |is_using_lofi_mode| is true
// adds the "q=low" directive to the header. Only adds this header if the
// proxy authentication credentials. Only adds this header if the
// provided |proxy_server| is a data reduction proxy and not the data
// reduction proxy's CONNECT server.
void MaybeAddRequestHeader(net::URLRequest* request,
const net::ProxyServer& proxy_server,
net::HttpRequestHeaders* request_headers,
bool is_using_lofi_mode);
net::HttpRequestHeaders* request_headers);
// Adds a 'Chrome-Proxy' header to |request_headers| with the data reduction
// proxy authentication credentials. Only adds this header if the provided
......@@ -139,8 +135,7 @@ class DataReductionProxyRequestOptions {
protected:
void SetHeader(const net::URLRequest* request,
net::HttpRequestHeaders* headers,
bool is_using_lofi_mode);
net::HttpRequestHeaders* headers);
// Returns a UTF16 string that's the hash of the configured authentication
// |key| and |salt|. Returns an empty UTF16 string if no key is configured or
......@@ -175,9 +170,6 @@ class DataReductionProxyRequestOptions {
// Updates client type, build, and patch.
void UpdateVersion();
// May regenerate the Chrome Proxy header based on changes in Lo-Fi status.
void MayRegenerateHeaderBasedOnLoFi(bool is_using_lofi_mode);
// Update the value of the experiments to be run and regenerate the header if
// necessary.
void UpdateExperiments();
......@@ -194,13 +186,11 @@ class DataReductionProxyRequestOptions {
// Adds authentication headers only if |expects_ssl| is true and
// |proxy_server| is a data reduction proxy used for ssl tunneling via
// HTTP CONNECT, or |expect_ssl| is false and |proxy_server| is a data
// reduction proxy for HTTP traffic. If |is_using_lofi_mode| is true adds the
// "q=low" directive to the header.
// reduction proxy for HTTP traffic.
void MaybeAddRequestHeaderImpl(const net::URLRequest* request,
const net::HostPortPair& proxy_server,
bool expect_ssl,
net::HttpRequestHeaders* request_headers,
bool is_using_lofi_mode);
net::HttpRequestHeaders* request_headers);
// Regenerates the |header_value_| string which is concatenated to the
// Chrome-proxy header.
......@@ -220,7 +210,6 @@ class DataReductionProxyRequestOptions {
std::string secure_session_;
std::string build_;
std::string patch_;
std::string lofi_;
std::vector<std::string> experiments_;
// The time at which the session expires. Used to ensure that a session is
......
......@@ -26,6 +26,7 @@ const char kChromeProxyHeader[] = "chrome-proxy";
const char kActionValueDelimiter = '=';
const char kChromeProxyLoFiDirective[] = "q=low";
const char kChromeProxyLoFiExperimentDirective[] = "exp=lofi_active_control";
const char kChromeProxyActionBlockOnce[] = "block-once";
const char kChromeProxyActionBlock[] = "block";
......@@ -60,6 +61,10 @@ const char* chrome_proxy_lo_fi_directive() {
return kChromeProxyLoFiDirective;
}
const char* chrome_proxy_lo_fi_experiment_directive() {
return kChromeProxyLoFiExperimentDirective;
}
bool GetDataReductionProxyActionValue(
const net::HttpResponseHeaders* headers,
const std::string& action_prefix,
......
......@@ -71,6 +71,10 @@ const char* chrome_proxy_header();
// and responses.
const char* chrome_proxy_lo_fi_directive();
// Gets the Chrome-Proxy directive used by data reduction proxy Lo-Fi control
// experiment requests.
const char* chrome_proxy_lo_fi_experiment_directive();
// Returns true if the Chrome-Proxy header is present and contains a bypass
// delay. Sets |proxy_info->bypass_duration| to the specified delay if greater
// than 0, and to 0 otherwise to indicate that the default proxy delay
......
......@@ -8,6 +8,10 @@
#include "base/macros.h"
#include "net/url_request/url_request.h"
namespace net {
class HttpRequestHeaders;
}
namespace data_reduction_proxy {
// Interface to determine if a request should be made for a low fidelity version
......@@ -17,9 +21,16 @@ class LoFiDecider {
virtual ~LoFiDecider() {}
// Returns true when Lo-Fi mode is on for the given |request|. This means the
// Lo-Fi header should be added to the given request, unless the user is in
// in the Lo-Fi control group.
// Lo-Fi header should be added to the given request.
virtual bool IsUsingLoFiMode(const net::URLRequest& request) const = 0;
// Returns true when Lo-Fi mode is on for the given |request|. If the
// |request| is using Lo-Fi mode, adds the "q=low" directive to the |headers|.
// If the user is in the experiment control group and Lo-Fi is on, adds the
// experiment directive "exp=lofi_active_control".
virtual bool MaybeAddLoFiDirectiveToHeaders(
const net::URLRequest& request,
net::HttpRequestHeaders* headers) const = 0;
};
} // namespace data_reduction_proxy
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment