Skip to content

Commit

Permalink
Add Finch experiment for selectively bypassing proxies.
Browse files Browse the repository at this point in the history
Add option to bypass the data compression proxy if the request resource
type (as inferred by the renderer process) is not an image.

For background, see this design doc:

https://docs.google.com/a/google.com/document/d/1Kz92Fmw3lv_R-2aNvLp8jW9lkfKOZciTZtni2qQ_Adc/edit

BUG=391836

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@281951 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rcs@chromium.org committed Jul 9, 2014
1 parent 3075912 commit a702da7
Show file tree
Hide file tree
Showing 26 changed files with 648 additions and 182 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "dbus/bus.h"
#include "dbus/message.h"
#include "dbus/exported_object.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/proxy/proxy_service.h"
#include "net/url_request/url_request_context.h"
Expand Down Expand Up @@ -134,8 +135,8 @@ class ProxyResolverImpl : public ProxyResolverInterface {
VLOG(1) << "Starting network proxy resolution for "
<< request->source_url_;
const int result = proxy_service->ResolveProxy(
GURL(request->source_url_), &request->proxy_info_,
request->callback_, NULL, net::BoundNetLog());
GURL(request->source_url_), net::LOAD_NORMAL, &request->proxy_info_,
request->callback_, NULL, NULL, net::BoundNetLog());
if (result != net::ERR_IO_PENDING) {
VLOG(1) << "Network proxy resolution completed synchronously.";
request->OnCompletion(result);
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/io_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
#if defined(OS_ANDROID) || defined(OS_IOS)
#include "components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h"
#include "components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
#include "components/data_reduction_proxy/browser/data_reduction_proxy_protocol.h"
#include "components/data_reduction_proxy/browser/data_reduction_proxy_settings.h"
#endif

Expand Down Expand Up @@ -638,6 +639,9 @@ void IOThread::InitAsync() {
globals_->data_reduction_proxy_params.reset(proxy_params);
globals_->data_reduction_proxy_auth_request_handler.reset(
new DataReductionProxyAuthRequestHandler(proxy_params));
globals_->on_resolve_proxy_handler =
ChromeNetworkDelegate::OnResolveProxyHandler(
base::Bind(data_reduction_proxy::OnResolveProxyHandler));
DataReductionProxyUsageStats* proxy_usage_stats =
new DataReductionProxyUsageStats(proxy_params,
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
Expand All @@ -647,6 +651,8 @@ void IOThread::InitAsync() {
network_delegate->set_data_reduction_proxy_usage_stats(proxy_usage_stats);
network_delegate->set_data_reduction_proxy_auth_request_handler(
globals_->data_reduction_proxy_auth_request_handler.get());
network_delegate->set_on_resolve_proxy_handler(
globals_->on_resolve_proxy_handler);
#endif // defined(SPDY_PROXY_AUTH_ORIGIN)
#endif // defined(OS_ANDROID) || defined(OS_IOS)
globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/io_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/memory/weak_ptr.h"
#include "base/prefs/pref_member.h"
#include "base/time/time.h"
#include "chrome/browser/net/chrome_network_delegate.h"
#include "chrome/browser/net/ssl_config_service_manager.h"
#include "components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h"
#include "components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
Expand Down Expand Up @@ -196,6 +197,7 @@ class IOThread : public content::BrowserThreadDelegate {
data_reduction_proxy_usage_stats;
scoped_ptr<data_reduction_proxy::DataReductionProxyAuthRequestHandler>
data_reduction_proxy_auth_request_handler;
ChromeNetworkDelegate::OnResolveProxyHandler on_resolve_proxy_handler;
};

// |net_log| must either outlive the IOThread or be NULL.
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/net/chrome_network_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,14 @@ int ChromeNetworkDelegate::OnBeforeURLRequest(
return rv;
}

void ChromeNetworkDelegate::OnResolveProxy(
const GURL& url, int load_flags, net::ProxyInfo* result) {
if (!on_resolve_proxy_handler_.is_null()) {
on_resolve_proxy_handler_.Run(url, load_flags,
data_reduction_proxy_params_, result);
}
}

int ChromeNetworkDelegate::OnBeforeSendHeaders(
net::URLRequest* request,
const net::CompletionCallback& callback,
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/net/chrome_network_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ class PrerenderTracker;
// add hooks into the network stack.
class ChromeNetworkDelegate : public net::NetworkDelegate {
public:
// Provides an opportunity to interpose on proxy resolution. Called before
// ProxyService.ResolveProxy() returns. |proxy_info| contains information
// about the proxy being used, and may be modified by this callback.
typedef base::Callback<void(
const GURL& url,
int load_flags,
const data_reduction_proxy::DataReductionProxyParams* params,
net::ProxyInfo* result)> OnResolveProxyHandler;

// |enable_referrers| (and all of the other optional PrefMembers) should be
// initialized on the UI thread (see below) beforehand. This object's owner is
// responsible for cleaning them up at shutdown.
Expand Down Expand Up @@ -142,6 +151,10 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
data_reduction_proxy_auth_request_handler_ = handler;
}

void set_on_resolve_proxy_handler(OnResolveProxyHandler handler) {
on_resolve_proxy_handler_ = handler;
}

// Adds the Client Hints header to HTTP requests.
void SetEnableClientHints();

Expand Down Expand Up @@ -178,6 +191,8 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
virtual int OnBeforeURLRequest(net::URLRequest* request,
const net::CompletionCallback& callback,
GURL* new_url) OVERRIDE;
virtual void OnResolveProxy(
const GURL& url, int load_flags, net::ProxyInfo* result) OVERRIDE;
virtual int OnBeforeSendHeaders(net::URLRequest* request,
const net::CompletionCallback& callback,
net::HttpRequestHeaders* headers) OVERRIDE;
Expand Down Expand Up @@ -280,6 +295,8 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
data_reduction_proxy::DataReductionProxyAuthRequestHandler*
data_reduction_proxy_auth_request_handler_;

OnResolveProxyHandler on_resolve_proxy_handler_;

DISALLOW_COPY_AND_ASSIGN(ChromeNetworkDelegate);
};

Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/net/network_stats.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/time/time.h"
#include "chrome/common/chrome_version_info.h"
#include "content/public/browser/browser_thread.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/base/network_change_notifier.h"
#include "net/base/test_completion_callback.h"
Expand Down Expand Up @@ -820,10 +821,12 @@ void ProxyDetector::StartResolveProxy() {
DCHECK(proxy_service_);
int rv = proxy_service_->ResolveProxy(
gurl,
net::LOAD_NORMAL,
&proxy_info_,
base::Bind(&ProxyDetector::OnResolveProxyComplete,
base::Unretained(this)),
NULL,
NULL,
net::BoundNetLog());
if (rv != net::ERR_IO_PENDING)
OnResolveProxyComplete(rv);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/profiles/profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,8 @@ void ProfileIOData::Init(
io_thread_globals->data_reduction_proxy_usage_stats.get());
network_delegate->set_data_reduction_proxy_auth_request_handler(
io_thread_globals->data_reduction_proxy_auth_request_handler.get());
network_delegate->set_on_resolve_proxy_handler(
io_thread_globals->on_resolve_proxy_handler);
if (command_line.HasSwitch(switches::kEnableClientHints))
network_delegate->SetEnableClientHints();
network_delegate->set_extension_info_map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ bool DataReductionProxyParams::IsIncludedInPreconnectHintingFieldTrial() {
"DataCompressionProxyPreconnectHints") == kEnabled;
}

// static
bool DataReductionProxyParams::IsIncludedInCriticalPathBypassFieldTrial() {
return IsIncludedInFieldTrial() &&
FieldTrialList::FindFullName(
"DataCompressionProxyCriticalBypass") == kEnabled;
}

DataReductionProxyParams::DataReductionProxyParams(int flags)
: allowed_((flags & kAllowed) == kAllowed),
fallback_allowed_((flags & kFallbackAllowed) == kFallbackAllowed),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class DataReductionProxyParams {
// hinting.
static bool IsIncludedInPreconnectHintingFieldTrial();

// Returns true if this client is part of a field trial that bypasses the
// proxy if the request resource type is on the critical path (e.g. HTML).
static bool IsIncludedInCriticalPathBypassFieldTrial();

// Constructs configuration parameters. If |kAllowed|, then the standard
// data reduction proxy configuration is allowed to be used. If
// |kfallbackAllowed| a fallback proxy can be used if the primary proxy is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "url/gurl.h"

namespace {

bool SetProxyServerFromGURL(const GURL& gurl,
net::ProxyServer* proxy_server) {
DCHECK(proxy_server);
Expand All @@ -30,6 +31,7 @@ bool SetProxyServerFromGURL(const GURL& gurl,
net::HostPortPair::FromURL(gurl));
return true;
}

} // namespace

namespace data_reduction_proxy {
Expand Down Expand Up @@ -88,7 +90,20 @@ bool MaybeBypassProxyAndPrepareToRetry(
return true;
}


void OnResolveProxyHandler(const GURL& url,
int load_flags,
const DataReductionProxyParams* params,
net::ProxyInfo* result) {
if ((load_flags & net::LOAD_BYPASS_DATA_REDUCTION_PROXY) &&
DataReductionProxyParams::IsIncludedInCriticalPathBypassFieldTrial() &&
!result->is_empty() &&
!result->is_direct() &&
params &&
params->IsDataReductionProxy(
result->proxy_server().host_port_pair(), NULL)) {
result->UseDirect();
}
}

bool IsRequestIdempotent(const net::URLRequest* request) {
DCHECK(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class TimeDelta;

namespace net {
class HttpResponseHeaders;
class ProxyInfo;
class ProxyServer;
class URLRequest;
}
Expand All @@ -33,6 +34,17 @@ bool MaybeBypassProxyAndPrepareToRetry(
const net::HttpResponseHeaders* original_response_headers,
scoped_refptr<net::HttpResponseHeaders>* override_response_headers);

// Configure |result| to proceed directly to the origin if |result|'s current
// proxy is the data reduction proxy, the
// |net::LOAD_BYPASS_DATA_REDUCTION_PROXY| |load_flag| is set, and the
// DataCompressionProxyCriticalBypass Finch trial is set.
// This handler is intended to be invoked only by
// |ChromeNetworkDelegate.NotifyResolveProxy|.
void OnResolveProxyHandler(const GURL& url,
int load_flags,
const DataReductionProxyParams* params,
net::ProxyInfo* result);

// Returns true if the request method is idempotent. Only idempotent requests
// are retried on a bypass. Visible as part of the public API for testing.
bool IsRequestIdempotent(const net::URLRequest* request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@

#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/metrics/field_trial.h"
#include "base/run_loop.h"
#include "base/strings/stringprintf.h"
#include "components/data_reduction_proxy/browser/data_reduction_proxy_params_test_utils.h"
#include "net/base/completion_callback.h"
#include "net/base/host_port_pair.h"
#include "net/base/load_flags.h"
#include "net/base/network_delegate.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_transaction_test_util.h"
#include "net/proxy/proxy_service.h"
#include "net/socket/socket_test_util.h"
#include "net/url_request/static_http_user_agent_settings.h"
#include "net/url_request/url_request.h"
Expand Down Expand Up @@ -611,4 +614,80 @@ TEST_F(DataReductionProxyProtocolTest,
TestBadProxies(0, -1, "", "");
}

class BadEntropyProvider : public base::FieldTrial::EntropyProvider {
public:
virtual ~BadEntropyProvider() {}

virtual double GetEntropyForTrial(const std::string& trial_name,
uint32 randomization_seed) const OVERRIDE {
return 0.5;
}
};

TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
int load_flags = net::LOAD_NORMAL;
GURL url("http://www.google.com/");

TestDataReductionProxyParams test_params(
DataReductionProxyParams::kAllowed |
DataReductionProxyParams::kFallbackAllowed |
DataReductionProxyParams::kPromoAllowed,
TestDataReductionProxyParams::HAS_EVERYTHING &
~TestDataReductionProxyParams::HAS_DEV_ORIGIN);

// Data reduction proxy
net::ProxyInfo info1;
std::string data_reduction_proxy;
base::TrimString(test_params.DefaultOrigin(), "/", &data_reduction_proxy);
info1.UseNamedProxy(data_reduction_proxy);
EXPECT_FALSE(info1.is_empty());

// Other proxy
net::ProxyInfo info2;
info2.UseNamedProxy("proxy.com");
EXPECT_FALSE(info2.is_empty());

// Without DataCompressionProxyCriticalBypass Finch trial set, should never
// bypass.
OnResolveProxyHandler(url, load_flags, &test_params, &info1);
EXPECT_FALSE(info1.is_direct());

OnResolveProxyHandler(url, load_flags, &test_params,&info2);
EXPECT_FALSE(info2.is_direct());

load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;

OnResolveProxyHandler(url, load_flags, &test_params, &info1);
EXPECT_FALSE(info1.is_direct());

OnResolveProxyHandler(url, load_flags, &test_params, &info2);
EXPECT_FALSE(info2.is_direct());

// With Finch trial set, should only bypass if LOAD flag is set and the
// effective proxy is the data compression proxy.
base::FieldTrialList field_trial_list(new BadEntropyProvider());
base::FieldTrialList::CreateFieldTrial("DataCompressionProxyRollout",
"Enabled");
base::FieldTrialList::CreateFieldTrial("DataCompressionProxyCriticalBypass",
"Enabled");
EXPECT_TRUE(
DataReductionProxyParams::IsIncludedInCriticalPathBypassFieldTrial());

load_flags = net::LOAD_NORMAL;

OnResolveProxyHandler(url, load_flags, &test_params, &info1);
EXPECT_FALSE(info1.is_direct());

OnResolveProxyHandler(url, load_flags, &test_params, &info2);
EXPECT_FALSE(info2.is_direct());

load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;

OnResolveProxyHandler(url, load_flags, &test_params, &info2);
EXPECT_FALSE(info2.is_direct());

OnResolveProxyHandler(url, load_flags, &test_params, &info1);
EXPECT_TRUE(info1.is_direct());
}

} // namespace data_reduction_proxy
5 changes: 5 additions & 0 deletions content/browser/loader/resource_dispatcher_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,11 @@ int ResourceDispatcherHostImpl::BuildLoadFlagsForRequest(
load_flags &= ~net::LOAD_REPORT_RAW_HEADERS;
}

// Add a flag to selectively bypass the data reduction proxy if the resource
// type is not an image.
if (request_data.resource_type != ResourceType::IMAGE)
load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;

return load_flags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/common/socket_permission_request.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/proxy/proxy_info.h"
#include "net/url_request/url_request_context.h"
Expand Down Expand Up @@ -145,9 +146,11 @@ void PepperNetworkProxyHost::TryToSendUnsentRequests() {
request.reply_context,
base::Owned(proxy_info));
int result = proxy_service_->ResolveProxy(request.url,
net::LOAD_NORMAL,
proxy_info,
callback,
&pending_request,
NULL,
net::BoundNetLog());
pending_requests_.push(pending_request);
// If it was handled synchronously, we must run the callback now;
Expand Down
5 changes: 3 additions & 2 deletions content/browser/resolve_proxy_msg_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/bind_helpers.h"
#include "base/compiler_specific.h"
#include "content/common/view_messages.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
Expand Down Expand Up @@ -91,10 +92,10 @@ void ResolveProxyMsgHelper::StartPendingRequest() {

// Start the request.
int result = proxy_service_->ResolveProxy(
req.url, &proxy_info_,
req.url, net::LOAD_NORMAL, &proxy_info_,
base::Bind(&ResolveProxyMsgHelper::OnResolveProxyCompleted,
base::Unretained(this)),
&req.pac_req, net::BoundNetLog());
&req.pac_req, NULL, net::BoundNetLog());

// Completed synchronously.
if (result != net::ERR_IO_PENDING)
Expand Down
Loading

0 comments on commit a702da7

Please sign in to comment.