diff options
author | ericroman@google.com <ericroman@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-26 21:12:20 +0000 |
---|---|---|
committer | ericroman@google.com <ericroman@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-26 21:12:20 +0000 |
commit | 775fd9e3215360868781cb7bc615b0b4d782f0a9 (patch) | |
tree | 1436421924de756654737312b2dad652315981d1 /net | |
parent | 44d178bbf8cf937dc1180da5ccfd27ce9abe3f51 (diff) | |
download | chromium_src-775fd9e3215360868781cb7bc615b0b4d782f0a9.zip chromium_src-775fd9e3215360868781cb7bc615b0b4d782f0a9.tar.gz chromium_src-775fd9e3215360868781cb7bc615b0b4d782f0a9.tar.bz2 |
Remove the concept of threading from ProxyService, and move it into the ProxyResolver dependency.
ProxyResolver may now complete requests asynchronously, and is defined to handle multiple requests.
The code from ProxyService that queued requests onto the single PAC thread has moved into SingleThreadedProxyResolver.
This refactor lays the groundwork for:
(1) http://crbug.com/11746 -- Run PAC proxy resolving out of process.
(Can inject an IPC bridge implementation of ProxyResolver)
(2) http://crbug.com/11079 -- Run PAC proxy resolving on multiple threads.
(Can implement a MultithreadedProxyResolver type class; still complications around v8 threadsafety though).
BUG=http://crbug.com/11746, http://crbug.com/11079
TEST=existing unit-tests.
Review URL: http://codereview.chromium.org/149525
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@21631 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/net.gyp | 3 | ||||
-rw-r--r-- | net/proxy/proxy_resolver.h | 75 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_mac.cc | 7 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_mac.h | 23 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_perftest.cc | 17 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_v8.cc | 26 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_v8.h | 17 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_v8_unittest.cc | 40 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_winhttp.cc | 18 | ||||
-rw-r--r-- | net/proxy/proxy_resolver_winhttp.h | 22 | ||||
-rw-r--r-- | net/proxy/proxy_service.cc | 318 | ||||
-rw-r--r-- | net/proxy/proxy_service.h | 44 | ||||
-rw-r--r-- | net/proxy/proxy_service_unittest.cc | 153 | ||||
-rw-r--r-- | net/proxy/single_threaded_proxy_resolver.cc | 231 | ||||
-rw-r--r-- | net/proxy/single_threaded_proxy_resolver.h | 88 | ||||
-rw-r--r-- | net/proxy/single_threaded_proxy_resolver_unittest.cc | 313 |
16 files changed, 1079 insertions, 316 deletions
diff --git a/net/net.gyp b/net/net.gyp index 4397430..1b8fc96 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -274,6 +274,8 @@ 'proxy/proxy_server.h', 'proxy/proxy_service.cc', 'proxy/proxy_service.h', + 'proxy/single_threaded_proxy_resolver.cc', + 'proxy/single_threaded_proxy_resolver.h', 'socket/client_socket.cc', 'socket/client_socket.h', 'socket/client_socket_factory.cc', @@ -494,6 +496,7 @@ 'proxy/proxy_script_fetcher_unittest.cc', 'proxy/proxy_server_unittest.cc', 'proxy/proxy_service_unittest.cc', + 'proxy/single_threaded_proxy_resolver_unittest.cc', 'socket/client_socket_pool_base_unittest.cc', 'socket/socks5_client_socket_unittest.cc', 'socket/socks_client_socket_unittest.cc', diff --git a/net/proxy/proxy_resolver.h b/net/proxy/proxy_resolver.h index fac8fa0..ce0e81d 100644 --- a/net/proxy/proxy_resolver.h +++ b/net/proxy/proxy_resolver.h @@ -8,6 +8,7 @@ #include <string> #include "base/logging.h" +#include "net/base/completion_callback.h" class GURL; @@ -15,37 +16,69 @@ namespace net { class ProxyInfo; -// Synchronously resolve the proxy for a URL, using a PAC script. Called on the -// PAC Thread. +// Interface for "proxy resolvers". A ProxyResolver fills in a list of proxies +// to use for a particular URL. Generally the backend for a ProxyResolver is +// a PAC script, but it doesn't need to be. ProxyResolver can service multiple +// requests at a time. class ProxyResolver { public: + // Opaque pointer type, to return a handle to cancel outstanding requests. + typedef void* RequestHandle; - // If a subclass sets |does_fetch| to false, then the owning ProxyResolver - // will download PAC scripts on our behalf, and notify changes with - // SetPacScript(). Otherwise the subclass is expected to fetch the - // PAC script internally, and SetPacScript() will go unused. - ProxyResolver(bool does_fetch) : does_fetch_(does_fetch) {} + // See |expects_pac_bytes()| for the meaning of |expects_pac_bytes|. + explicit ProxyResolver(bool expects_pac_bytes) + : expects_pac_bytes_(expects_pac_bytes) {} virtual ~ProxyResolver() {} - // Query the proxy auto-config file (specified by |pac_url|) for the proxy to - // use to load the given |query_url|. Returns OK if successful or an error - // code otherwise. - virtual int GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - ProxyInfo* results) = 0; - - // Called whenever the PAC script has changed, with the contents of the - // PAC script. |bytes| may be empty string if there was a fetch error. - virtual void SetPacScript(const std::string& bytes) { - // Must override SetPacScript() if |does_fetch_ = true|. + // Gets a list of proxy servers to use for |url|. If the request will + // complete asynchronously returns ERR_IO_PENDING and notifies the result + // by running |callback|. If the result code is OK then + // the request was successful and |results| contains the proxy + // resolution information. In the case of asynchronous completion + // |*request| is written to, and can be passed to CancelRequest(). + virtual int GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request) = 0; + + // Cancels |request|. + virtual void CancelRequest(RequestHandle request) = 0; + + // The PAC script backend can be specified to the ProxyResolver either via + // URL, or via the javascript text itself. If |expects_pac_bytes| is true, + // then PAC scripts should be specified using SetPacScriptByData(). Otherwise + // they should be specified using SetPacScriptByUrl(). + bool expects_pac_bytes() const { return expects_pac_bytes_; } + + // Sets the PAC script backend to use for this proxy resolver (by URL). + void SetPacScriptByUrl(const GURL& pac_url) { + DCHECK(!expects_pac_bytes()); + SetPacScriptByUrlInternal(pac_url); + } + + // Sets the PAC script backend to use for this proxy resolver (by contents). + void SetPacScriptByData(const std::string& bytes) { + DCHECK(expects_pac_bytes()); + SetPacScriptByDataInternal(bytes); + } + + private: + // Called to set the PAC script backend to use. If |pac_url| is invalid, + // this is a request to use WPAD (auto detect). + virtual void SetPacScriptByUrlInternal(const GURL& pac_url) { + NOTREACHED(); + } + + // Called to set the PAC script backend to use. |bytes| may be empty if the + // fetch failed, or if the fetch returned no content. + virtual void SetPacScriptByDataInternal(const std::string& bytes) { NOTREACHED(); } - bool does_fetch() const { return does_fetch_; } + const bool expects_pac_bytes_; - protected: - bool does_fetch_; + DISALLOW_COPY_AND_ASSIGN(ProxyResolver); }; } // namespace net diff --git a/net/proxy/proxy_resolver_mac.cc b/net/proxy/proxy_resolver_mac.cc index f8906bb..b41e402 100644 --- a/net/proxy/proxy_resolver_mac.cc +++ b/net/proxy/proxy_resolver_mac.cc @@ -243,8 +243,9 @@ int ProxyConfigServiceMac::GetProxyConfig(ProxyConfig* config) { // Gets the proxy information for a query URL from a PAC. Implementation // inspired by http://developer.apple.com/samplecode/CFProxySupportTool/ int ProxyResolverMac::GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - ProxyInfo* results) { + ProxyInfo* results, + CompletionCallback* /*callback*/, + RequestHandle* /*request*/) { scoped_cftyperef<CFStringRef> query_ref( base::SysUTF8ToCFStringRef(query_url.spec())); scoped_cftyperef<CFURLRef> query_url_ref( @@ -254,7 +255,7 @@ int ProxyResolverMac::GetProxyForURL(const GURL& query_url, if (!query_url_ref.get()) return ERR_FAILED; scoped_cftyperef<CFStringRef> pac_ref( - base::SysUTF8ToCFStringRef(pac_url.spec())); + base::SysUTF8ToCFStringRef(pac_url_.spec())); scoped_cftyperef<CFURLRef> pac_url_ref( CFURLCreateWithString(kCFAllocatorDefault, pac_ref.get(), diff --git a/net/proxy/proxy_resolver_mac.h b/net/proxy/proxy_resolver_mac.h index 8413624..58f6096 100644 --- a/net/proxy/proxy_resolver_mac.h +++ b/net/proxy/proxy_resolver_mac.h @@ -5,6 +5,9 @@ #ifndef NET_PROXY_PROXY_RESOLVER_MAC_H_ #define NET_PROXY_PROXY_RESOLVER_MAC_H_ +#include <string> + +#include "googleurl/src/gurl.h" #include "net/proxy/proxy_config_service.h" #include "net/proxy/proxy_resolver.h" @@ -14,12 +17,24 @@ namespace net { // proxies. class ProxyResolverMac : public ProxyResolver { public: - ProxyResolverMac() : ProxyResolver(true) {} + ProxyResolverMac() : ProxyResolver(false /*expects_pac_bytes*/) {} // ProxyResolver methods: - virtual int GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - ProxyInfo* results); + virtual int GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request); + + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + + private: + virtual void SetPacScriptByUrlInternal(const GURL& pac_url) { + pac_url_ = pac_url; + } + + GURL pac_url_; }; class ProxyConfigServiceMac : public ProxyConfigService { diff --git a/net/proxy/proxy_resolver_perftest.cc b/net/proxy/proxy_resolver_perftest.cc index 7c74e72..45d1a7b 100644 --- a/net/proxy/proxy_resolver_perftest.cc +++ b/net/proxy/proxy_resolver_perftest.cc @@ -92,11 +92,11 @@ class PacPerfSuiteRunner { void RunTest(const std::string& script_name, const PacQuery* queries, int queries_len) { - GURL pac_url; - - if (resolver_->does_fetch()) { + if (!resolver_->expects_pac_bytes()) { InitHttpServer(); - pac_url = server_->TestServerPage(std::string("files/") + script_name); + GURL pac_url = + server_->TestServerPage(std::string("files/") + script_name); + resolver_->SetPacScriptByUrl(pac_url); } else { LoadPacScriptIntoResolver(script_name); } @@ -107,7 +107,7 @@ class PacPerfSuiteRunner { { net::ProxyInfo proxy_info; int result = resolver_->GetProxyForURL( - GURL("http://www.warmup.com"), pac_url, &proxy_info); + GURL("http://www.warmup.com"), &proxy_info, NULL, NULL); ASSERT_EQ(net::OK, result); } @@ -122,8 +122,7 @@ class PacPerfSuiteRunner { // Resolve. net::ProxyInfo proxy_info; int result = resolver_->GetProxyForURL(GURL(query.query_url), - pac_url, - &proxy_info); + &proxy_info, NULL, NULL); // Check that the result was correct. Note that ToPacString() and // ASSERT_EQ() are fast, so they won't skew the results. @@ -137,7 +136,7 @@ class PacPerfSuiteRunner { // Lazily startup an HTTP server (to serve the PAC script). void InitHttpServer() { - DCHECK(resolver_->does_fetch()); + DCHECK(!resolver_->expects_pac_bytes()); if (!server_) { server_ = HTTPTestServer::CreateServer( L"net/data/proxy_resolver_perftest", NULL); @@ -163,7 +162,7 @@ class PacPerfSuiteRunner { ASSERT_TRUE(ok); // Load the PAC script into the ProxyResolver. - resolver_->SetPacScript(file_contents); + resolver_->SetPacScriptByData(file_contents); } net::ProxyResolver* resolver_; diff --git a/net/proxy/proxy_resolver_v8.cc b/net/proxy/proxy_resolver_v8.cc index 6bbffec..b35a8ad 100644 --- a/net/proxy/proxy_resolver_v8.cc +++ b/net/proxy/proxy_resolver_v8.cc @@ -344,16 +344,19 @@ class ProxyResolverV8::Context { ProxyResolverV8::ProxyResolverV8( ProxyResolverV8::JSBindings* custom_js_bindings) - : ProxyResolver(false), js_bindings_(custom_js_bindings) { + : ProxyResolver(true /*expects_pac_bytes*/), + js_bindings_(custom_js_bindings) { } ProxyResolverV8::~ProxyResolverV8() {} int ProxyResolverV8::GetProxyForURL(const GURL& query_url, - const GURL& /*pac_url*/, - ProxyInfo* results) { - // If the V8 instance has not been initialized (either because SetPacScript() - // wasn't called yet, or because it was called with empty string). + ProxyInfo* results, + CompletionCallback* /*callback*/, + RequestHandle* /*request*/) { + // If the V8 instance has not been initialized (either because + // SetPacScriptByData() wasn't called yet, or because it was called with + // empty string). if (!context_.get()) return ERR_FAILED; @@ -361,10 +364,9 @@ int ProxyResolverV8::GetProxyForURL(const GURL& query_url, return context_->ResolveProxy(query_url, results); } -void ProxyResolverV8::SetPacScript(const std::string& data) { - context_.reset(); - if (!data.empty()) - context_.reset(new Context(js_bindings_.get(), data)); +void ProxyResolverV8::CancelRequest(RequestHandle request) { + // This is a synchronous ProxyResolver; no possibility for async requests. + NOTREACHED(); } // static @@ -373,4 +375,10 @@ ProxyResolverV8::JSBindings* ProxyResolverV8::CreateDefaultBindings( return new DefaultJSBindings(host_resolver, host_resolver_loop); } +void ProxyResolverV8::SetPacScriptByDataInternal(const std::string& data) { + context_.reset(); + if (!data.empty()) + context_.reset(new Context(js_bindings_.get(), data)); +} + } // namespace net diff --git a/net/proxy/proxy_resolver_v8.h b/net/proxy/proxy_resolver_v8.h index 6fbc968..4c364e5 100644 --- a/net/proxy/proxy_resolver_v8.h +++ b/net/proxy/proxy_resolver_v8.h @@ -43,13 +43,14 @@ class ProxyResolverV8 : public ProxyResolver { // is destroyed. explicit ProxyResolverV8(JSBindings* custom_js_bindings); - ~ProxyResolverV8(); + virtual ~ProxyResolverV8(); // ProxyResolver implementation: - virtual int GetProxyForURL(const GURL& query_url, - const GURL& /*pac_url*/, - ProxyInfo* results); - virtual void SetPacScript(const std::string& bytes); + virtual int GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* /*callback*/, + RequestHandle* /*request*/); + virtual void CancelRequest(RequestHandle request); JSBindings* js_bindings() const { return js_bindings_.get(); } @@ -71,8 +72,12 @@ class ProxyResolverV8 : public ProxyResolver { private: // Context holds the Javascript state for the most recently loaded PAC // script. It corresponds with the data from the last call to - // SetPacScript(). + // SetPacScriptByDataInternal(). class Context; + + // ProxyResolver implementation: + virtual void SetPacScriptByDataInternal(const std::string& bytes); + scoped_ptr<Context> context_; scoped_ptr<JSBindings> js_bindings_; diff --git a/net/proxy/proxy_resolver_v8_unittest.cc b/net/proxy/proxy_resolver_v8_unittest.cc index 86544d5..db359a4 100644 --- a/net/proxy/proxy_resolver_v8_unittest.cc +++ b/net/proxy/proxy_resolver_v8_unittest.cc @@ -85,7 +85,7 @@ class ProxyResolverV8WithMockBindings : public net::ProxyResolverV8 { ASSERT_TRUE(ok); // Load the PAC script into the ProxyResolver. - SetPacScript(file_contents); + SetPacScriptByData(file_contents); } }; @@ -100,7 +100,7 @@ TEST(ProxyResolverV8Test, Direct) { resolver.SetPacScriptFromDisk("direct.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_TRUE(proxy_info.is_direct()); @@ -114,7 +114,7 @@ TEST(ProxyResolverV8Test, ReturnEmptyString) { resolver.SetPacScriptFromDisk("return_empty_string.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_TRUE(proxy_info.is_direct()); @@ -133,7 +133,7 @@ TEST(ProxyResolverV8Test, Basic) { { net::ProxyInfo proxy_info; int result = resolver.GetProxyForURL(GURL("http://query.com/path"), - kPacUrl, &proxy_info); + &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_EQ("http.query.com.path.query.com:80", proxy_info.proxy_server().ToURI()); @@ -141,7 +141,7 @@ TEST(ProxyResolverV8Test, Basic) { { net::ProxyInfo proxy_info; int result = resolver.GetProxyForURL(GURL("ftp://query.com:90/path"), - kPacUrl, &proxy_info); + &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); // Note that FindProxyForURL(url, host) does not expect |host| to contain // the port number. @@ -171,7 +171,7 @@ TEST(ProxyResolverV8Test, BadReturnType) { resolver.SetPacScriptFromDisk(filenames[i]); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::ERR_PAC_SCRIPT_FAILED, result); @@ -190,7 +190,7 @@ TEST(ProxyResolverV8Test, NoEntryPoint) { resolver.SetPacScriptFromDisk("no_entrypoint.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::ERR_PAC_SCRIPT_FAILED, result); @@ -207,7 +207,7 @@ TEST(ProxyResolverV8Test, ParseError) { resolver.SetPacScriptFromDisk("missing_close_brace.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::ERR_PAC_SCRIPT_FAILED, result); @@ -234,7 +234,7 @@ TEST(ProxyResolverV8Test, SideEffects) { // The PAC script increments a counter each time we invoke it. for (int i = 0; i < 3; ++i) { net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_EQ(StringPrintf("sideffect_%d:80", i), proxy_info.proxy_server().ToURI()); @@ -246,7 +246,7 @@ TEST(ProxyResolverV8Test, SideEffects) { for (int i = 0; i < 3; ++i) { net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_EQ(StringPrintf("sideffect_%d:80", i), proxy_info.proxy_server().ToURI()); @@ -259,7 +259,7 @@ TEST(ProxyResolverV8Test, UnhandledException) { resolver.SetPacScriptFromDisk("unhandled_exception.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::ERR_PAC_SCRIPT_FAILED, result); @@ -278,7 +278,7 @@ TEST(ProxyResolverV8Test, DISABLED_ReturnUnicode) { resolver.SetPacScriptFromDisk("return_unicode.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); // The result from this resolve was unparseable, because it // wasn't ascii. @@ -291,7 +291,7 @@ TEST(ProxyResolverV8Test, JavascriptLibrary) { resolver.SetPacScriptFromDisk("pac_library_unittest.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); // If the javascript side of this unit-test fails, it will throw a javascript // exception. Otherwise it will return "PROXY success:80". @@ -302,33 +302,33 @@ TEST(ProxyResolverV8Test, JavascriptLibrary) { EXPECT_EQ(0U, resolver.mock_js_bindings()->errors.size()); } -// Try resolving when SetPacScript() has not been called. +// Try resolving when SetPacScriptByData() has not been called. TEST(ProxyResolverV8Test, NoSetPacScript) { ProxyResolverV8WithMockBindings resolver; net::ProxyInfo proxy_info; // Resolve should fail, as we are not yet initialized with a script. - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::ERR_FAILED, result); // Initialize it. resolver.SetPacScriptFromDisk("direct.js"); // Resolve should now succeed. - result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); // Clear it, by initializing with an empty string. - resolver.SetPacScript(std::string()); + resolver.SetPacScriptByData(std::string()); // Resolve should fail again now. - result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::ERR_FAILED, result); // Load a good script once more. resolver.SetPacScriptFromDisk("direct.js"); - result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_EQ(0U, resolver.mock_js_bindings()->alerts.size()); @@ -341,7 +341,7 @@ TEST(ProxyResolverV8Test, V8Bindings) { resolver.SetPacScriptFromDisk("bindings.js"); net::ProxyInfo proxy_info; - int result = resolver.GetProxyForURL(kQueryUrl, kPacUrl, &proxy_info); + int result = resolver.GetProxyForURL(kQueryUrl, &proxy_info, NULL, NULL); EXPECT_EQ(net::OK, result); EXPECT_TRUE(proxy_info.is_direct()); diff --git a/net/proxy/proxy_resolver_winhttp.cc b/net/proxy/proxy_resolver_winhttp.cc index a3ed9fe..d4bca9c5 100644 --- a/net/proxy/proxy_resolver_winhttp.cc +++ b/net/proxy/proxy_resolver_winhttp.cc @@ -45,7 +45,7 @@ static void FreeInfo(WINHTTP_PROXY_INFO* info) { } ProxyResolverWinHttp::ProxyResolverWinHttp() - : ProxyResolver(true), session_handle_(NULL) { + : ProxyResolver(false /*expects_pac_bytes*/), session_handle_(NULL) { } ProxyResolverWinHttp::~ProxyResolverWinHttp() { @@ -53,8 +53,9 @@ ProxyResolverWinHttp::~ProxyResolverWinHttp() { } int ProxyResolverWinHttp::GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - ProxyInfo* results) { + ProxyInfo* results, + CompletionCallback* /*callback*/, + RequestHandle* /*request*/) { // If we don't have a WinHTTP session, then create a new one. if (!session_handle_ && !OpenWinHttpSession()) return ERR_FAILED; @@ -68,7 +69,7 @@ int ProxyResolverWinHttp::GetProxyForURL(const GURL& query_url, WINHTTP_AUTOPROXY_OPTIONS options = {0}; options.fAutoLogonIfChallenged = FALSE; options.dwFlags = WINHTTP_AUTOPROXY_CONFIG_URL; - std::wstring pac_url_wide = ASCIIToWide(pac_url.spec()); + std::wstring pac_url_wide = ASCIIToWide(pac_url_.spec()); options.lpszAutoConfigUrl = pac_url_wide.empty() ? L"http://wpad/wpad.dat" : pac_url_wide.c_str(); @@ -134,6 +135,15 @@ int ProxyResolverWinHttp::GetProxyForURL(const GURL& query_url, return rv; } +void ProxyResolverWinHttp::CancelRequest(RequestHandle request) { + // This is a synchronous ProxyResolver; no possibility for async requests. + NOTREACHED(); +} + +void ProxyResolverWinHttp::SetPacScriptByUrlInternal(const GURL& pac_url) { + pac_url_ = pac_url; +} + bool ProxyResolverWinHttp::OpenWinHttpSession() { DCHECK(!session_handle_); session_handle_ = WinHttpOpen(NULL, diff --git a/net/proxy/proxy_resolver_winhttp.h b/net/proxy/proxy_resolver_winhttp.h index 066fa6f..4932ba6 100644 --- a/net/proxy/proxy_resolver_winhttp.h +++ b/net/proxy/proxy_resolver_winhttp.h @@ -5,6 +5,9 @@ #ifndef NET_PROXY_PROXY_RESOLVER_WINHTTP_H_ #define NET_PROXY_PROXY_RESOLVER_WINHTTP_H_ +#include <string> + +#include "googleurl/src/gurl.h" #include "net/proxy/proxy_resolver.h" typedef void* HINTERNET; // From winhttp.h @@ -16,20 +19,27 @@ namespace net { class ProxyResolverWinHttp : public ProxyResolver { public: ProxyResolverWinHttp(); - ~ProxyResolverWinHttp(); + virtual ~ProxyResolverWinHttp(); // ProxyResolver implementation: - virtual int GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - ProxyInfo* results); + virtual int GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* /*callback*/, + RequestHandle* /*request*/); + virtual void CancelRequest(RequestHandle request); private: - bool OpenWinHttpSession(); - void CloseWinHttpSession(); + // ProxyResolver implementation: + virtual void SetPacScriptByUrlInternal(const GURL& pac_url); + + bool OpenWinHttpSession(); + void CloseWinHttpSession(); // Proxy configuration is cached on the session handle. HINTERNET session_handle_; + GURL pac_url_; + DISALLOW_COPY_AND_ASSIGN(ProxyResolverWinHttp); }; diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc index e59c296..453625a 100644 --- a/net/proxy/proxy_service.cc +++ b/net/proxy/proxy_service.cc @@ -8,6 +8,7 @@ #include "base/compiler_specific.h" #include "base/logging.h" +#include "base/message_loop.h" #include "base/string_util.h" #include "googleurl/src/gurl.h" #include "net/base/net_errors.h" @@ -23,6 +24,7 @@ #endif #include "net/proxy/proxy_resolver.h" #include "net/proxy/proxy_resolver_v8.h" +#include "net/proxy/single_threaded_proxy_resolver.h" #include "net/url_request/url_request_context.h" using base::TimeDelta; @@ -42,14 +44,22 @@ class ProxyConfigServiceNull : public ProxyConfigService { // Proxy resolver that fails every time. class ProxyResolverNull : public ProxyResolver { public: - ProxyResolverNull() : ProxyResolver(true /*does_fetch*/) {} + ProxyResolverNull() : ProxyResolver(false /*expects_pac_bytes*/) {} // ProxyResolver implementation: - virtual int GetProxyForURL(const GURL& /*query_url*/, - const GURL& /*pac_url*/, - ProxyInfo* /*results*/) { + virtual int GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request) { return ERR_NOT_IMPLEMENTED; } + + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + + private: + virtual void SetPacScriptByUrlInternal(const GURL& pac_url) {} }; // Strip away any reference fragments and the username/password, as they @@ -64,132 +74,109 @@ static GURL SanitizeURLForProxyResolver(const GURL& url) { return url.ReplaceComponents(replacements); } -// Runs on the PAC thread to notify the proxy resolver of the fetched PAC -// script contents. This task shouldn't outlive ProxyService, since -// |resolver| is owned by ProxyService. -class NotifyFetchCompletionTask : public Task { - public: - NotifyFetchCompletionTask(ProxyResolver* resolver, const std::string& bytes) - : resolver_(resolver), bytes_(bytes) {} - - virtual void Run() { - resolver_->SetPacScript(bytes_); - } - - private: - ProxyResolver* resolver_; - std::string bytes_; -}; - // ProxyService::PacRequest --------------------------------------------------- -// We rely on the fact that the origin thread (and its message loop) will not -// be destroyed until after the PAC thread is destroyed. - class ProxyService::PacRequest - : public base::RefCountedThreadSafe<ProxyService::PacRequest> { + : public base::RefCounted<ProxyService::PacRequest> { public: - // |service| -- the ProxyService that owns this request. - // |url| -- the url of the query. - // |results| -- the structure to fill with proxy resolve results. PacRequest(ProxyService* service, const GURL& url, ProxyInfo* results, - CompletionCallback* callback) + CompletionCallback* user_callback) : service_(service), - callback_(callback), + user_callback_(user_callback), + ALLOW_THIS_IN_INITIALIZER_LIST(io_callback_( + this, &PacRequest::QueryComplete)), results_(results), url_(url), - is_started_(false), - origin_loop_(MessageLoop::current()) { - DCHECK(callback); + resolve_job_(NULL), + config_id_(ProxyConfig::INVALID_ID) { + DCHECK(user_callback); } - // Start the resolve proxy request on the PAC thread. - void Query() { - is_started_ = true; - AddRef(); // balanced in QueryComplete + // Starts the resolve proxy request. + int Start() { + DCHECK(!was_cancelled()); + DCHECK(!is_started()); - GURL query_url = SanitizeURLForProxyResolver(url_); - const GURL& pac_url = service_->config_.pac_url; - results_->config_id_ = service_->config_.id(); + config_id_ = service_->config_.id(); - service_->pac_thread()->message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(this, &ProxyService::PacRequest::DoQuery, - service_->resolver(), query_url, pac_url)); + return resolver()->GetProxyForURL( + url_, results_, &io_callback_, &resolve_job_); } - // Run the request's callback on the current message loop. - void PostCallback(int result_code) { - AddRef(); // balanced in DoCallback - MessageLoop::current()->PostTask(FROM_HERE, - NewRunnableMethod(this, &ProxyService::PacRequest::DoCallback, - result_code)); + bool is_started() const { + // Note that !! casts to bool. (VS gives a warning otherwise). + return !!resolve_job_; + } + + void StartAndComplete() { + int rv = Start(); + if (rv != ERR_IO_PENDING) + QueryComplete(rv); } void Cancel() { - // Clear these to inform QueryComplete that it should not try to - // access them. + // The request may already be running in the resolver. + if (is_started()) { + resolver()->CancelRequest(resolve_job_); + resolve_job_ = NULL; + } + + // Mark as cancelled, to prevent accessing this again later. service_ = NULL; - callback_ = NULL; + user_callback_ = NULL; results_ = NULL; } // Returns true if Cancel() has been called. - bool was_cancelled() const { return callback_ == NULL; } + bool was_cancelled() const { return user_callback_ == NULL; } - private: - friend class ProxyService; - - // Runs on the PAC thread. - void DoQuery(ProxyResolver* resolver, - const GURL& query_url, - const GURL& pac_url) { - int rv = resolver->GetProxyForURL(query_url, pac_url, &results_buf_); - origin_loop_->PostTask(FROM_HERE, - NewRunnableMethod(this, &PacRequest::QueryComplete, rv)); - } + // Helper to call after ProxyResolver completion (both synchronous and + // asynchronous). Fixes up the result that is to be returned to user. + int QueryDidComplete(int result_code) { + DCHECK(!was_cancelled()); - // Runs the completion callback on the origin thread. - void QueryComplete(int result_code) { - // The PacRequest may have been cancelled after it was started. - if (!was_cancelled()) { - service_->DidCompletePacRequest(results_->config_id_, result_code); + // Make a note in the results which configuration was in use at the + // time of the resolve. + results_->config_id_ = config_id_; - if (result_code == OK) { - results_->Use(results_buf_); - results_->RemoveBadProxies(service_->proxy_retry_info_); - } - callback_->Run(result_code); + // Reset the state associated with in-progress-resolve. + resolve_job_ = NULL; + config_id_ = ProxyConfig::INVALID_ID; - // We check for cancellation once again, in case the callback deleted - // the owning ProxyService (whose destructor will in turn cancel us). - if (!was_cancelled()) - service_->RemoveFrontOfRequestQueue(this); - } + // Notify the service of the completion. + service_->DidCompletePacRequest(results_->config_id_, result_code); + + // Clean up the results list. + if (result_code == OK) + results_->RemoveBadProxies(service_->proxy_retry_info_); - Release(); // balances the AddRef in Query. we may get deleted after - // we return. + return result_code; } - // Runs the completion callback on the origin thread. - void DoCallback(int result_code) { - if (!was_cancelled()) { - callback_->Run(result_code); - } - Release(); // balances the AddRef in PostCallback. + private: + // Callback for when the ProxyResolver request has completed. + void QueryComplete(int result_code) { + result_code = QueryDidComplete(result_code); + + // Remove this completed PacRequest from the service's pending list. + /// (which will probably cause deletion of |this|). + CompletionCallback* callback = user_callback_; + service_->RemovePendingRequest(this); + + callback->Run(result_code); } - // Must only be used on the "origin" thread. + ProxyResolver* resolver() const { return service_->resolver_.get(); } + ProxyService* service_; - CompletionCallback* callback_; + CompletionCallback* user_callback_; + CompletionCallbackImpl<PacRequest> io_callback_; ProxyInfo* results_; GURL url_; - bool is_started_; - - // Usable from within DoQuery on the PAC thread. - ProxyInfo results_buf_; - MessageLoop* origin_loop_; + ProxyResolver::RequestHandle resolve_job_; + ProxyConfig::ID config_id_; // The config id when the resolve was started. }; // ProxyService --------------------------------------------------------------- @@ -231,10 +218,14 @@ ProxyService* ProxyService::Create( proxy_resolver = CreateNonV8ProxyResolver(); } + // Wrap the (synchronous) ProxyResolver implementation in a single-threaded + // runner. This will dispatch requests to a threadpool of size 1. + proxy_resolver = new SingleThreadedProxyResolver(proxy_resolver); + ProxyService* proxy_service = new ProxyService( proxy_config_service, proxy_resolver); - if (!proxy_resolver->does_fetch()) { + if (proxy_resolver->expects_pac_bytes()) { // Configure PAC script downloads to be issued using |url_request_context|. DCHECK(url_request_context); proxy_service->SetProxyScriptFetcher( @@ -255,11 +246,13 @@ ProxyService* ProxyService::CreateNull() { return new ProxyService(new ProxyConfigServiceNull, new ProxyResolverNull); } -int ProxyService::ResolveProxy(const GURL& url, ProxyInfo* result, +int ProxyService::ResolveProxy(const GURL& raw_url, ProxyInfo* result, CompletionCallback* callback, PacRequest** pac_request) { DCHECK(callback); + GURL url = SanitizeURLForProxyResolver(raw_url); + // Check if the request can be completed right away. This is the case when // using a direct connection, or when the config is bad. UpdateConfigIfOld(); @@ -267,10 +260,20 @@ int ProxyService::ResolveProxy(const GURL& url, ProxyInfo* result, if (rv != ERR_IO_PENDING) return rv; - // Otherwise, push the request into the work queue. scoped_refptr<PacRequest> req = new PacRequest(this, url, result, callback); + + bool resolver_is_ready = PrepareResolverForRequests(); + + if (resolver_is_ready) { + // Start the resolve request. + rv = req->Start(); + if (rv != ERR_IO_PENDING) + return req->QueryDidComplete(rv); + } + + DCHECK_EQ(ERR_IO_PENDING, rv); + DCHECK(!ContainsPendingRequest(req)); pending_requests_.push_back(req); - ProcessPendingRequests(req.get()); // Completion will be notifed through |callback|, unless the caller cancels // the request using |pac_request|. @@ -350,50 +353,39 @@ void ProxyService::ApplyProxyRules(const GURL& url, } } -void ProxyService::InitPacThread() { - if (!pac_thread_.get()) { - pac_thread_.reset(new base::Thread("pac-thread")); - pac_thread_->Start(); - } -} - ProxyService::~ProxyService() { - // Cancel the inprogress request (if any), and free the rest. - for (PendingRequestsQueue::iterator it = pending_requests_.begin(); + // Cancel any inprogress requests. + for (PendingRequests::iterator it = pending_requests_.begin(); it != pending_requests_.end(); ++it) { (*it)->Cancel(); } } -void ProxyService::ProcessPendingRequests(PacRequest* recent_req) { - if (pending_requests_.empty()) - return; - - // While the PAC script is being downloaded, requests are blocked. - if (IsFetchingPacScript()) - return; - - // Get the next request to process (FIFO). - PacRequest* req = pending_requests_.front().get(); - if (req->is_started_) - return; +void ProxyService::ResumeAllPendingRequests() { + // Make a copy in case |this| is deleted during the synchronous completion + // of one of the requests. If |this| is deleted then all of the PacRequest + // instances will be Cancel()-ed. + PendingRequests pending_copy = pending_requests_; - // The configuration may have changed since |req| was added to the - // queue. It could be this request now completes synchronously. - if (req != recent_req) { - UpdateConfigIfOld(); - int rv = TryToCompleteSynchronously(req->url_, req->results_); - if (rv != ERR_IO_PENDING) { - req->PostCallback(rv); - RemoveFrontOfRequestQueue(req); - return; - } + for (PendingRequests::iterator it = pending_copy.begin(); + it != pending_copy.end(); + ++it) { + PacRequest* req = it->get(); + if (!req->is_started() && !req->was_cancelled()) + req->StartAndComplete(); } +} + +bool ProxyService::PrepareResolverForRequests() { + // While the PAC script is being downloaded, block requests. + if (IsFetchingPacScript()) + return false; // Check if a new PAC script needs to be downloaded. DCHECK(config_.id() != ProxyConfig::INVALID_ID); - if (!resolver_->does_fetch() && config_.id() != fetched_pac_config_id_) { + if (resolver_->expects_pac_bytes() && + config_.id() != fetched_pac_config_id_) { // For auto-detect we use the well known WPAD url. GURL pac_url = config_.auto_detect ? GURL("http://wpad/wpad.dat") : config_.pac_url; @@ -405,26 +397,16 @@ void ProxyService::ProcessPendingRequests(PacRequest* recent_req) { proxy_script_fetcher_->Fetch( pac_url, &in_progress_fetch_bytes_, &proxy_script_fetcher_callback_); - return; + return false; } - // The only choice left now is to actually run the ProxyResolver on - // the PAC thread. - InitPacThread(); - req->Query(); -} - -void ProxyService::RemoveFrontOfRequestQueue(PacRequest* expected_req) { - DCHECK(pending_requests_.front().get() == expected_req); - pending_requests_.pop_front(); - - // Start next work item. - ProcessPendingRequests(NULL); + // We are good to go. + return true; } void ProxyService::OnScriptFetchCompletion(int result) { DCHECK(IsFetchingPacScript()); - DCHECK(!resolver_->does_fetch()); + DCHECK(resolver_->expects_pac_bytes()); LOG(INFO) << "Completed PAC script fetch for config_id=" << in_progress_fetch_config_id_ @@ -434,18 +416,16 @@ void ProxyService::OnScriptFetchCompletion(int result) { // Notify the ProxyResolver of the new script data (will be empty string if // result != OK). - InitPacThread(); - pac_thread()->message_loop()->PostTask(FROM_HERE, - new NotifyFetchCompletionTask( - resolver_.get(), in_progress_fetch_bytes_)); + resolver_->SetPacScriptByData(in_progress_fetch_bytes_); fetched_pac_config_id_ = in_progress_fetch_config_id_; fetched_pac_error_ = result; in_progress_fetch_config_id_ = ProxyConfig::INVALID_ID; in_progress_fetch_bytes_.clear(); - // Start a pending request if any. - ProcessPendingRequests(NULL); + // Resume any requests which we had to defer until the PAC script was + // downloaded. + ResumeAllPendingRequests(); } int ProxyService::ReconsiderProxyAfterError(const GURL& url, @@ -498,30 +478,23 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url, return OK; } -// There are four states of the request we need to handle: -// (1) Not started (just sitting in the queue). -// (2) Executing PacRequest::DoQuery in the PAC thread. -// (3) Waiting for PacRequest::QueryComplete to be run on the origin thread. -// (4) Waiting for PacRequest::DoCallback to be run on the origin thread. void ProxyService::CancelPacRequest(PacRequest* req) { DCHECK(req); - - bool is_active_request = req->is_started_ && !pending_requests_.empty() && - pending_requests_.front().get() == req; - req->Cancel(); + RemovePendingRequest(req); +} - if (is_active_request) { - RemoveFrontOfRequestQueue(req); - return; - } +bool ProxyService::ContainsPendingRequest(PacRequest* req) { + PendingRequests::iterator it = std::find( + pending_requests_.begin(), pending_requests_.end(), req); + return pending_requests_.end() != it; +} - // Otherwise just delete the request from the queue. - PendingRequestsQueue::iterator it = std::find( +void ProxyService::RemovePendingRequest(PacRequest* req) { + DCHECK(ContainsPendingRequest(req)); + PendingRequests::iterator it = std::find( pending_requests_.begin(), pending_requests_.end(), req); - if (it != pending_requests_.end()) { - pending_requests_.erase(it); - } + pending_requests_.erase(it); } void ProxyService::SetProxyScriptFetcher( @@ -621,6 +594,15 @@ void ProxyService::SetConfig(const ProxyConfig& config) { // Reset state associated with latest config. config_is_bad_ = false; proxy_retry_info_.clear(); + + // Tell the resolver to use the new PAC URL (for resolvers that fetch the + // PAC script internally). + // TODO(eroman): this isn't quite right. See http://crbug.com/9985 + if ((config_.pac_url.is_valid() || config_.auto_detect) && + !resolver_->expects_pac_bytes()) { + const GURL pac_url = config_.auto_detect ? GURL() : config_.pac_url; + resolver_->SetPacScriptByUrl(pac_url); + } } void ProxyService::UpdateConfigIfOld() { diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h index e12892e..b76c830 100644 --- a/net/proxy/proxy_service.h +++ b/net/proxy/proxy_service.h @@ -5,11 +5,12 @@ #ifndef NET_PROXY_PROXY_SERVICE_H_ #define NET_PROXY_PROXY_SERVICE_H_ -#include <deque> #include <string> +#include <vector> #include "base/ref_counted.h" #include "base/scoped_ptr.h" +// TODO(eroman): remove this unused header; other callers are depending on it! #include "base/thread.h" #include "base/waitable_event.h" #include "net/base/completion_callback.h" @@ -17,6 +18,7 @@ #include "net/proxy/proxy_info.h" class GURL; +class MessageLoop; class URLRequestContext; namespace net { @@ -27,7 +29,7 @@ class ProxyResolver; // This class can be used to resolve the proxy server to use when loading a // HTTP(S) URL. It uses the given ProxyResolver to handle the actual proxy -// resolution. See ProxyResolverWinHttp for example. +// resolution. See ProxyResolverV8 for example. class ProxyService { public: // The instance takes ownership of |config_service| and |resolver|. @@ -37,6 +39,7 @@ class ProxyService { ~ProxyService(); // Used internally to handle PAC queries. + // TODO(eroman): consider naming this simply "Request". class PacRequest; // Returns ERR_IO_PENDING if the proxy information could not be provided @@ -57,6 +60,7 @@ class ProxyService { // 2. PAC URL // 3. WPAD auto-detection // + // TODO(eroman): see http://crbug.com/9985; the outline above is too simple. int ResolveProxy(const GURL& url, ProxyInfo* results, CompletionCallback* callback, @@ -126,6 +130,11 @@ class ProxyService { private: friend class PacRequest; + // TODO(eroman): change this to a std::set. Note that this requires updating + // some tests in proxy_service_unittest.cc such as: + // ProxyServiceTest.InitialPACScriptDownload + // which expects requests to finish in the order they were added. + typedef std::vector<scoped_refptr<PacRequest> > PendingRequests; // Creates a config service appropriate for this platform that fetches the // system proxy settings. @@ -136,9 +145,6 @@ class ProxyService { // on V8. static ProxyResolver* CreateNonV8ProxyResolver(); - ProxyResolver* resolver() { return resolver_.get(); } - base::Thread* pac_thread() { return pac_thread_.get(); } - // Identifies the proxy configuration. ProxyConfig::ID config_id() const { return config_.id(); } @@ -170,7 +176,6 @@ class ProxyService { // Returns ERR_IO_PENDING if the request cannot be completed synchronously. // Otherwise it fills |result| with the proxy information for |url|. // Completing synchronously means we don't need to query ProxyResolver. - // (ProxyResolver runs on PAC thread.) int TryToCompleteSynchronously(const GURL& url, ProxyInfo* result); // Set |result| with the proxy to use for |url|, based on |rules|. @@ -178,17 +183,20 @@ class ProxyService { const ProxyConfig::ProxyRules& rules, ProxyInfo* result); - // Starts the PAC thread if it isn't already running. - void InitPacThread(); + // Sends all the unstarted pending requests off to the resolver. + void ResumeAllPendingRequests(); + + // Returns true if |pending_requests_| contains |req|. + bool ContainsPendingRequest(PacRequest* req); - // Starts the next request from |pending_requests_| is possible. - // |recent_req| is the request that just got added, or NULL. - void ProcessPendingRequests(PacRequest* recent_req); + // Removes |req| from the list of pending requests. + void RemovePendingRequest(PacRequest* req); - // Removes the front entry of the requests queue. |expected_req| is our - // expectation of what the front of the request queue is; it is only used by - // DCHECK for verification purposes. - void RemoveFrontOfRequestQueue(PacRequest* expected_req); + // Returns true if the resolver is all set-up and ready to accept requests. + // Returns false if requests are blocked (because the PAC script is being + // downloaded). May have the side-effect of starting the PAC script + // download. + bool PrepareResolverForRequests(); // Called to indicate that a PacRequest completed. The |config_id| parameter // indicates the proxy configuration that was queried. |result_code| is OK @@ -204,7 +212,6 @@ class ProxyService { scoped_ptr<ProxyConfigService> config_service_; scoped_ptr<ProxyResolver> resolver_; - scoped_ptr<base::Thread> pac_thread_; // We store the proxy config and a counter (ID) that is incremented each time // the config changes. @@ -222,9 +229,8 @@ class ProxyService { // Map of the known bad proxies and the information about the retry time. ProxyRetryInfoMap proxy_retry_info_; - // FIFO queue of pending/inprogress requests. - typedef std::deque<scoped_refptr<PacRequest> > PendingRequestsQueue; - PendingRequestsQueue pending_requests_; + // Set of pending/inprogress requests. + PendingRequests pending_requests_; // The fetcher to use when downloading PAC scripts for the ProxyResolver. // This dependency can be NULL if our ProxyResolver has no need for diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc index 6774f2e..1e2e3a5 100644 --- a/net/proxy/proxy_service_unittest.cc +++ b/net/proxy/proxy_service_unittest.cc @@ -9,6 +9,7 @@ #include "net/proxy/proxy_resolver.h" #include "net/proxy/proxy_script_fetcher.h" #include "net/proxy/proxy_service.h" +#include "net/proxy/single_threaded_proxy_resolver.h" #include "testing/gtest/include/gtest/gtest.h" namespace { @@ -29,15 +30,17 @@ class MockProxyConfigService: public net::ProxyConfigService { net::ProxyConfig config; }; -class MockProxyResolver : public net::ProxyResolver { +class SyncMockProxyResolver : public net::ProxyResolver { public: - MockProxyResolver() : net::ProxyResolver(true), - fail_get_proxy_for_url(false) { + SyncMockProxyResolver() : net::ProxyResolver(false /*expects_pac_bytes*/), + fail_get_proxy_for_url(false) { } + // ProxyResolver implementation: virtual int GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - net::ProxyInfo* results) { + net::ProxyInfo* results, + net::CompletionCallback* callback, + RequestHandle* request) { if (fail_get_proxy_for_url) return net::ERR_FAILED; if (GURL(query_url).host() == info_predicate_query_host) { @@ -48,6 +51,13 @@ class MockProxyResolver : public net::ProxyResolver { return net::OK; } + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + + virtual void SetPacScriptByUrlInternal(const GURL& pac_url) {} + + net::ProxyInfo info; // info is only returned if query_url in GetProxyForURL matches this: @@ -58,6 +68,17 @@ class MockProxyResolver : public net::ProxyResolver { bool fail_get_proxy_for_url; }; +class MockProxyResolver : public net::SingleThreadedProxyResolver { + public: + MockProxyResolver() + : net::SingleThreadedProxyResolver(new SyncMockProxyResolver) { + x = reinterpret_cast<SyncMockProxyResolver*>(resolver()); + } + + // TODO(eroman): cleanup. + SyncMockProxyResolver* x; +}; + // ResultFuture is a handle to get at the result from // ProxyService::ResolveProxyForURL() that ran on another thread. class ResultFuture : public base::RefCountedThreadSafe<ResultFuture> { @@ -326,12 +347,13 @@ class SyncProxyService { }; // A ProxyResolver which can be set to block upon reaching GetProxyForURL. -class BlockableProxyResolver : public net::ProxyResolver { +class SyncBlockableProxyResolver : public net::ProxyResolver { public: - BlockableProxyResolver() : net::ProxyResolver(true), - should_block_(false), - unblocked_(true, true), - blocked_(true, false) { + SyncBlockableProxyResolver() + : net::ProxyResolver(false /*expects_pac_bytes*/), + should_block_(false), + unblocked_(true, true), + blocked_(true, false) { } void Block() { @@ -351,8 +373,9 @@ class BlockableProxyResolver : public net::ProxyResolver { // net::ProxyResolver implementation: virtual int GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - net::ProxyInfo* results) { + net::ProxyInfo* results, + net::CompletionCallback* callback, + RequestHandle* request) { if (should_block_) { blocked_.Signal(); unblocked_.Wait(); @@ -362,36 +385,72 @@ class BlockableProxyResolver : public net::ProxyResolver { return net::OK; } + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + + virtual void SetPacScriptByUrlInternal(const GURL& pac_url) {} + private: bool should_block_; base::WaitableEvent unblocked_; base::WaitableEvent blocked_; }; +class BlockableProxyResolver : public net::SingleThreadedProxyResolver { + public: + BlockableProxyResolver() + : net::SingleThreadedProxyResolver(new SyncBlockableProxyResolver) { + x = reinterpret_cast<SyncBlockableProxyResolver*>(resolver()); + } + + // TODO(eroman): cleanup. + SyncBlockableProxyResolver* x; +}; + // A mock ProxyResolverWithoutFetch which concatenates the query's host with // the last download PAC contents. This way the result describes what the last // downloaded PAC script's contents were, in addition to the query url itself. -class MockProxyResolverWithoutFetch : public net::ProxyResolver { +class SyncMockProxyResolverWithoutFetch : public net::ProxyResolver { public: - MockProxyResolverWithoutFetch() : net::ProxyResolver(false), - last_pac_contents_("NONE") {} + SyncMockProxyResolverWithoutFetch() + : net::ProxyResolver(true /*expects_pac_bytes*/), + last_pac_contents_("NONE") {} // net::ProxyResolver implementation: virtual int GetProxyForURL(const GURL& query_url, - const GURL& pac_url, - net::ProxyInfo* results) { + net::ProxyInfo* results, + net::CompletionCallback* callback, + RequestHandle* request) { results->UseNamedProxy(last_pac_contents_ + "." + query_url.host()); return net::OK; } - virtual void SetPacScript(const std::string& bytes) { + virtual void SetPacScriptByDataInternal(const std::string& bytes) { last_pac_contents_ = bytes; } + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + private: std::string last_pac_contents_; }; +class MockProxyResolverWithoutFetch : public net::SingleThreadedProxyResolver { + public: + MockProxyResolverWithoutFetch() + : net::SingleThreadedProxyResolver( + new SyncMockProxyResolverWithoutFetch) { + x = reinterpret_cast<SyncMockProxyResolverWithoutFetch*>(resolver()); + } + + // TODO(eroman): cleanup. + SyncMockProxyResolverWithoutFetch* x; +}; + + } // namespace // A mock ProxyScriptFetcher. No result will be returned to the fetch client @@ -465,8 +524,8 @@ TEST(ProxyServiceTest, PAC) { new MockProxyConfigService("http://foopy/proxy.pac"); MockProxyResolver* resolver = new MockProxyResolver; - resolver->info.UseNamedProxy("foopy"); - resolver->info_predicate_query_host = "www.google.com"; + resolver->x->info.UseNamedProxy("foopy"); + resolver->x->info_predicate_query_host = "www.google.com"; SyncProxyService service(config_service, resolver); @@ -484,8 +543,8 @@ TEST(ProxyServiceTest, PAC_FailoverToDirect) { new MockProxyConfigService("http://foopy/proxy.pac"); MockProxyResolver* resolver = new MockProxyResolver; - resolver->info.UseNamedProxy("foopy:8080"); - resolver->info_predicate_query_host = "www.google.com"; + resolver->x->info.UseNamedProxy("foopy:8080"); + resolver->x->info_predicate_query_host = "www.google.com"; SyncProxyService service(config_service, resolver); @@ -510,9 +569,9 @@ TEST(ProxyServiceTest, PAC_FailsToDownload) { new MockProxyConfigService("http://foopy/proxy.pac"); MockProxyResolver* resolver = new MockProxyResolver; - resolver->info.UseNamedProxy("foopy:8080"); - resolver->info_predicate_query_host = "www.google.com"; - resolver->fail_get_proxy_for_url = true; + resolver->x->info.UseNamedProxy("foopy:8080"); + resolver->x->info_predicate_query_host = "www.google.com"; + resolver->x->fail_get_proxy_for_url = true; SyncProxyService service(config_service, resolver); @@ -528,8 +587,8 @@ TEST(ProxyServiceTest, PAC_FailsToDownload) { EXPECT_EQ(rv, net::OK); EXPECT_TRUE(info.is_direct()); - resolver->fail_get_proxy_for_url = false; - resolver->info.UseNamedProxy("foopy_valid:8080"); + resolver->x->fail_get_proxy_for_url = false; + resolver->x->info.UseNamedProxy("foopy_valid:8080"); // But, if that fails, then we should give the proxy config another shot // since we have never tried it with this URL before. @@ -547,9 +606,9 @@ TEST(ProxyServiceTest, ProxyFallback) { new MockProxyConfigService("http://foopy/proxy.pac"); MockProxyResolver* resolver = new MockProxyResolver; - resolver->info.UseNamedProxy("foopy1:8080;foopy2:9090"); - resolver->info_predicate_query_host = "www.google.com"; - resolver->fail_get_proxy_for_url = false; + resolver->x->info.UseNamedProxy("foopy1:8080;foopy2:9090"); + resolver->x->info_predicate_query_host = "www.google.com"; + resolver->x->fail_get_proxy_for_url = false; SyncProxyService service(config_service, resolver); @@ -574,9 +633,9 @@ TEST(ProxyServiceTest, ProxyFallback) { // Create a new resolver that returns 3 proxies. The second one is already // known to be bad. config_service->config.pac_url = GURL("http://foopy/proxy.pac"); - resolver->info.UseNamedProxy("foopy3:7070;foopy1:8080;foopy2:9090"); - resolver->info_predicate_query_host = "www.google.com"; - resolver->fail_get_proxy_for_url = false; + resolver->x->info.UseNamedProxy("foopy3:7070;foopy1:8080;foopy2:9090"); + resolver->x->info_predicate_query_host = "www.google.com"; + resolver->x->fail_get_proxy_for_url = false; rv = service.ResolveProxy(url, &info); EXPECT_EQ(rv, net::OK); @@ -607,9 +666,9 @@ TEST(ProxyServiceTest, ProxyFallback_NewSettings) { new MockProxyConfigService("http://foopy/proxy.pac"); MockProxyResolver* resolver = new MockProxyResolver; - resolver->info.UseNamedProxy("foopy1:8080;foopy2:9090"); - resolver->info_predicate_query_host = "www.google.com"; - resolver->fail_get_proxy_for_url = false; + resolver->x->info.UseNamedProxy("foopy1:8080;foopy2:9090"); + resolver->x->info_predicate_query_host = "www.google.com"; + resolver->x->fail_get_proxy_for_url = false; SyncProxyService service(config_service, resolver); @@ -656,9 +715,9 @@ TEST(ProxyServiceTest, ProxyFallback_BadConfig) { new MockProxyConfigService("http://foopy/proxy.pac"); MockProxyResolver* resolver = new MockProxyResolver; - resolver->info.UseNamedProxy("foopy1:8080;foopy2:9090"); - resolver->info_predicate_query_host = "www.google.com"; - resolver->fail_get_proxy_for_url = false; + resolver->x->info.UseNamedProxy("foopy1:8080;foopy2:9090"); + resolver->x->info_predicate_query_host = "www.google.com"; + resolver->x->fail_get_proxy_for_url = false; SyncProxyService service(config_service, resolver); @@ -683,7 +742,7 @@ TEST(ProxyServiceTest, ProxyFallback_BadConfig) { // Fake a PAC failure. net::ProxyInfo info2; - resolver->fail_get_proxy_for_url = true; + resolver->x->fail_get_proxy_for_url = true; rv = service.ResolveProxy(url, &info2); EXPECT_EQ(rv, net::ERR_FAILED); @@ -692,7 +751,7 @@ TEST(ProxyServiceTest, ProxyFallback_BadConfig) { // The PAC is now fixed and will return a proxy server. // It should also clear the list of bad proxies. - resolver->fail_get_proxy_for_url = false; + resolver->x->fail_get_proxy_for_url = false; // Try to resolve, it will still return "direct" because we have no reason // to check the config since everything works. @@ -999,7 +1058,7 @@ TEST(ProxyServiceTest, CancelQueuedRequest) { ProxyServiceWithFutures service(config_service, resolver); // Cause requests to pile up, by having them block in the PAC thread. - resolver->Block(); + resolver->x->Block(); // Start 3 requests. scoped_refptr<ResultFuture> result1; @@ -1012,13 +1071,13 @@ TEST(ProxyServiceTest, CancelQueuedRequest) { service.ResolveProxy(&result3, GURL("http://request3")); // Wait until the first request has become blocked in the PAC thread. - resolver->WaitUntilBlocked(); + resolver->x->WaitUntilBlocked(); // Cancel the second request result2->Cancel(); // Unblock the PAC thread. - resolver->Unblock(); + resolver->x->Unblock(); // Wait for the final request to complete. result3->WaitUntilCompleted(); @@ -1046,7 +1105,7 @@ TEST(ProxyServiceTest, CancelInprogressRequest) { ProxyServiceWithFutures service(config_service, resolver); // Cause requests to pile up, by having them block in the PAC thread. - resolver->Block(); + resolver->x->Block(); // Start 3 requests. scoped_refptr<ResultFuture> result1; @@ -1059,13 +1118,13 @@ TEST(ProxyServiceTest, CancelInprogressRequest) { service.ResolveProxy(&result3, GURL("http://request3")); // Wait until the first request has become blocked in the PAC thread. - resolver->WaitUntilBlocked(); + resolver->x->WaitUntilBlocked(); // Cancel the first request result1->Cancel(); // Unblock the PAC thread. - resolver->Unblock(); + resolver->x->Unblock(); // Wait for the final request to complete. result3->WaitUntilCompleted(); diff --git a/net/proxy/single_threaded_proxy_resolver.cc b/net/proxy/single_threaded_proxy_resolver.cc new file mode 100644 index 0000000..e1e621de --- /dev/null +++ b/net/proxy/single_threaded_proxy_resolver.cc @@ -0,0 +1,231 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/proxy/single_threaded_proxy_resolver.h" + +#include "base/thread.h" +#include "net/base/net_errors.h" +#include "net/proxy/proxy_info.h" + +namespace net { + +// Runs on the worker thread to call ProxyResolver::SetPacScriptByUrl or +// ProxyResolver::SetPacScriptByData. +// TODO(eroman): the lifetime of this task is ill-defined; |resolver_| must +// be valid when the task eventually is run. Make this task cancellable. +class SetPacScriptTask : public Task { + public: + SetPacScriptTask(ProxyResolver* resolver, + const GURL& pac_url, + const std::string& bytes) + : resolver_(resolver), pac_url_(pac_url), bytes_(bytes) {} + + virtual void Run() { + if (resolver_->expects_pac_bytes()) + resolver_->SetPacScriptByData(bytes_); + else + resolver_->SetPacScriptByUrl(pac_url_); + } + + private: + ProxyResolver* resolver_; + GURL pac_url_; + std::string bytes_; +}; + +// SingleThreadedProxyResolver::Job ------------------------------------------- + +class SingleThreadedProxyResolver::Job + : public base::RefCountedThreadSafe<SingleThreadedProxyResolver::Job> { + public: + // |coordinator| -- the SingleThreadedProxyResolver that owns this job. + // |url| -- the URL of the query. + // |results| -- the structure to fill with proxy resolve results. + Job(SingleThreadedProxyResolver* coordinator, + const GURL& url, + ProxyInfo* results, + CompletionCallback* callback) + : coordinator_(coordinator), + callback_(callback), + results_(results), + url_(url), + is_started_(false), + origin_loop_(MessageLoop::current()) { + DCHECK(callback); + } + + // Start the resolve proxy request on the worker thread. + void Start() { + is_started_ = true; + AddRef(); // balanced in QueryComplete + + coordinator_->thread()->message_loop()->PostTask( + FROM_HERE, NewRunnableMethod(this, &Job::DoQuery, + coordinator_->resolver_.get())); + } + + bool is_started() const { return is_started_; } + + void Cancel() { + // Clear these to inform QueryComplete that it should not try to + // access them. + coordinator_ = NULL; + callback_ = NULL; + results_ = NULL; + } + + // Returns true if Cancel() has been called. + bool was_cancelled() const { return callback_ == NULL; } + + private: + // Runs on the worker thread. + void DoQuery(ProxyResolver* resolver) { + int rv = resolver->GetProxyForURL(url_, &results_buf_, NULL, NULL); + DCHECK_NE(rv, ERR_IO_PENDING); + origin_loop_->PostTask(FROM_HERE, + NewRunnableMethod(this, &Job::QueryComplete, rv)); + } + + // Runs the completion callback on the origin thread. + void QueryComplete(int result_code) { + // The Job may have been cancelled after it was started. + if (!was_cancelled()) { + if (result_code >= OK) { // Note: unit-tests use values > 0. + results_->Use(results_buf_); + } + callback_->Run(result_code); + + // We check for cancellation once again, in case the callback deleted + // the owning ProxyService (whose destructor will in turn cancel us). + if (!was_cancelled()) + coordinator_->RemoveFrontOfJobsQueueAndStartNext(this); + } + + Release(); // balances the AddRef in Query. we may get deleted after + // we return. + } + + // Must only be used on the "origin" thread. + SingleThreadedProxyResolver* coordinator_; + CompletionCallback* callback_; + ProxyInfo* results_; + GURL url_; + bool is_started_; + + // Usable from within DoQuery on the worker thread. + ProxyInfo results_buf_; + MessageLoop* origin_loop_; +}; + +// SingleThreadedProxyResolver ------------------------------------------------ + +SingleThreadedProxyResolver::SingleThreadedProxyResolver( + ProxyResolver* resolver) + : ProxyResolver(resolver->expects_pac_bytes()), + resolver_(resolver) { +} + +SingleThreadedProxyResolver::~SingleThreadedProxyResolver() { + // Cancel the inprogress job (if any), and free the rest. + for (PendingJobsQueue::iterator it = pending_jobs_.begin(); + it != pending_jobs_.end(); + ++it) { + (*it)->Cancel(); + } + + // Note that |thread_| is destroyed before |resolver_|. This is important + // since |resolver_| could be running on |thread_|. +} + +int SingleThreadedProxyResolver::GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request) { + DCHECK(callback); + + scoped_refptr<Job> job = new Job(this, url, results, callback); + pending_jobs_.push_back(job); + ProcessPendingJobs(); // Jobs can never finish synchronously. + + // Completion will be notified through |callback|, unless the caller cancels + // the request using |request|. + if (request) + *request = reinterpret_cast<RequestHandle>(job.get()); + + return ERR_IO_PENDING; +} + +// There are three states of the request we need to handle: +// (1) Not started (just sitting in the queue). +// (2) Executing Job::DoQuery in the worker thread. +// (3) Waiting for Job::QueryComplete to be run on the origin thread. +void SingleThreadedProxyResolver::CancelRequest(RequestHandle req) { + DCHECK(req); + + Job* job = reinterpret_cast<Job*>(req); + + bool is_active_job = job->is_started() && !pending_jobs_.empty() && + pending_jobs_.front().get() == job; + + job->Cancel(); + + if (is_active_job) { + RemoveFrontOfJobsQueueAndStartNext(job); + return; + } + + // Otherwise just delete the job from the queue. + PendingJobsQueue::iterator it = std::find( + pending_jobs_.begin(), pending_jobs_.end(), job); + DCHECK(it != pending_jobs_.end()); + pending_jobs_.erase(it); +} + +void SingleThreadedProxyResolver::SetPacScriptByUrlInternal( + const GURL& pac_url) { + SetPacScriptHelper(pac_url, std::string()); +} + +void SingleThreadedProxyResolver::SetPacScriptByDataInternal( + const std::string& bytes) { + SetPacScriptHelper(GURL(), bytes); +} + +void SingleThreadedProxyResolver::SetPacScriptHelper(const GURL& pac_url, + const std::string& bytes) { + EnsureThreadStarted(); + thread()->message_loop()->PostTask( + FROM_HERE, new SetPacScriptTask(resolver(), pac_url, bytes)); +} + +void SingleThreadedProxyResolver::EnsureThreadStarted() { + if (!thread_.get()) { + thread_.reset(new base::Thread("pac-thread")); + thread_->Start(); + } +} + +void SingleThreadedProxyResolver::ProcessPendingJobs() { + if (pending_jobs_.empty()) + return; + + // Get the next job to process (FIFO). + Job* job = pending_jobs_.front().get(); + if (job->is_started()) + return; + + EnsureThreadStarted(); + job->Start(); +} + +void SingleThreadedProxyResolver::RemoveFrontOfJobsQueueAndStartNext( + Job* expected_job) { + DCHECK_EQ(expected_job, pending_jobs_.front().get()); + pending_jobs_.pop_front(); + + // Start next work item. + ProcessPendingJobs(); +} + +} // namespace net diff --git a/net/proxy/single_threaded_proxy_resolver.h b/net/proxy/single_threaded_proxy_resolver.h new file mode 100644 index 0000000..010e526 --- /dev/null +++ b/net/proxy/single_threaded_proxy_resolver.h @@ -0,0 +1,88 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_PROXY_SINGLE_THREADED_PROXY_RESOLVER_H_ +#define NET_PROXY_SINGLE_THREADED_PROXY_RESOLVER_H_ + +#include <deque> +#include <string> + +#include "base/ref_counted.h" +#include "base/scoped_ptr.h" +#include "net/proxy/proxy_resolver.h" + +namespace base { +class Thread; +} // namespace base + +namespace net { + +// ProxyResolver implementation that wraps a synchronous ProxyResolver, and +// runs it on a single worker thread. If multiple requests accumulate, they +// are serviced in FIFO order. +class SingleThreadedProxyResolver : public ProxyResolver { + public: + + // |resolver| is a synchronous ProxyResolver implementation. It doesn't + // have to be thread-safe, since it is run on exactly one thread. The + // constructor takes ownership of |resolver|. + explicit SingleThreadedProxyResolver(ProxyResolver* resolver); + + virtual ~SingleThreadedProxyResolver(); + + // ProxyResolver implementation: + virtual int GetProxyForURL(const GURL& url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request); + virtual void CancelRequest(RequestHandle request); + + protected: + // The wrapped (synchronous) ProxyResolver. + ProxyResolver* resolver() { return resolver_.get(); } + + private: + // Refcounted helper class that bridges between origin thread and worker + // thread. + class Job; + friend class Job; + // FIFO queue that contains the in-progress job, and any pending jobs. + typedef std::deque<scoped_refptr<Job> > PendingJobsQueue; + + base::Thread* thread() { return thread_.get(); } + + // ProxyResolver implementation: + virtual void SetPacScriptByUrlInternal(const GURL& pac_url); + virtual void SetPacScriptByDataInternal(const std::string& bytes); + + // Helper for shared code between SetPacScriptByUrlInternal() and + // SetPacScriptByDataInternal() -- the unused parameter is set to empty. + void SetPacScriptHelper(const GURL& pac_url, const std::string& bytes); + + // Starts the worker thread if it isn't already running. + void EnsureThreadStarted(); + + // Starts the next job from |pending_jobs_| if possible. + void ProcessPendingJobs(); + + // Removes the front entry of the jobs queue. |expected_job| is our + // expectation of what the front of the job queue is; it is only used by + // DCHECK for verification purposes. + void RemoveFrontOfJobsQueueAndStartNext(Job* expected_job); + + // The synchronous resolver implementation. + scoped_ptr<ProxyResolver> resolver_; + + // The thread where |resolver_| is run on. + // Note that declaration ordering is important here. |thread_| needs to be + // destroyed *before* |resolver_|, in case |resolver_| is currently + // executing on |thread_|. + scoped_ptr<base::Thread> thread_; + + PendingJobsQueue pending_jobs_; +}; + +} // namespace net + +#endif // NET_PROXY_SINGLE_THREADED_PROXY_RESOLVER_H_ diff --git a/net/proxy/single_threaded_proxy_resolver_unittest.cc b/net/proxy/single_threaded_proxy_resolver_unittest.cc new file mode 100644 index 0000000..6fe7995 --- /dev/null +++ b/net/proxy/single_threaded_proxy_resolver_unittest.cc @@ -0,0 +1,313 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/waitable_event.h" +#include "googleurl/src/gurl.h" +#include "net/base/net_errors.h" +#include "net/base/test_completion_callback.h" +#include "net/proxy/proxy_info.h" +#include "net/proxy/single_threaded_proxy_resolver.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { +namespace { + +// A synchronous mock ProxyResolver implementation, which can be used in +// conjunction with SingleThreadedProxyResolver. +// - returns a single-item proxy list with the query's host. +class MockProxyResolver : public ProxyResolver { + public: + MockProxyResolver() + : ProxyResolver(true /*expects_pac_bytes*/), + wrong_loop_(MessageLoop::current()), + request_count_(0), + resolve_latency_ms_(0) {} + + // ProxyResolver implementation: + virtual int GetProxyForURL(const GURL& query_url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request) { + if (resolve_latency_ms_) + PlatformThread::Sleep(resolve_latency_ms_); + + CheckIsOnWorkerThread(); + + EXPECT_EQ(NULL, callback); + EXPECT_EQ(NULL, request); + + results->UseNamedProxy(query_url.host()); + + // Return a success code which represents the request's order. + return request_count_++; + } + + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + + virtual void SetPacScriptByDataInternal(const std::string& bytes) { + CheckIsOnWorkerThread(); + last_pac_bytes_ = bytes; + } + + const std::string& last_pac_bytes() const { return last_pac_bytes_; } + + void SetResolveLatency(int latency_ms) { + resolve_latency_ms_ = latency_ms; + } + + private: + void CheckIsOnWorkerThread() { + // We should be running on the worker thread -- while we don't know the + // message loop of SingleThreadedProxyResolver's worker thread, we do + // know that it is going to be distinct from the loop running the + // test, so at least make sure it isn't the main loop. + EXPECT_NE(MessageLoop::current(), wrong_loop_); + } + + MessageLoop* wrong_loop_; + int request_count_; + std::string last_pac_bytes_; + int resolve_latency_ms_; +}; + + +// A mock synchronous ProxyResolver which can be set to block upon reaching +// GetProxyForURL(). +class BlockableProxyResolver : public MockProxyResolver { + public: + BlockableProxyResolver() + : should_block_(false), + unblocked_(true, true), + blocked_(true, false) { + } + + void Block() { + should_block_ = true; + unblocked_.Reset(); + } + + void Unblock() { + should_block_ = false; + blocked_.Reset(); + unblocked_.Signal(); + } + + void WaitUntilBlocked() { + blocked_.Wait(); + } + + virtual int GetProxyForURL(const GURL& query_url, + ProxyInfo* results, + CompletionCallback* callback, + RequestHandle* request) { + if (should_block_) { + blocked_.Signal(); + unblocked_.Wait(); + } + + return MockProxyResolver::GetProxyForURL( + query_url, results, callback, request); + } + + private: + bool should_block_; + base::WaitableEvent unblocked_; + base::WaitableEvent blocked_; +}; + +TEST(SingleThreadedProxyResolverTest, Basic) { + MockProxyResolver* mock = new MockProxyResolver; + scoped_ptr<SingleThreadedProxyResolver> resolver( + new SingleThreadedProxyResolver(mock)); + + int rv; + + EXPECT_TRUE(resolver->expects_pac_bytes()); + + // Call SetPacScriptByData() -- we will make sure it reaches the sync resolver + // later on. + resolver->SetPacScriptByData("pac script bytes"); + + // Start request 0. + TestCompletionCallback callback0; + ProxyInfo results0; + rv = resolver->GetProxyForURL( + GURL("http://request0"), &results0, &callback0, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Wait for request 0 to finish. + rv = callback0.WaitForResult(); + EXPECT_EQ(0, rv); + EXPECT_EQ("PROXY request0:80", results0.ToPacString()); + + // Verify that the data from SetPacScriptByData() reached the resolver. + // (Since we waited for the first request to complete, we are guaranteed + // that the earlier post completed). + EXPECT_EQ("pac script bytes", mock->last_pac_bytes()); + + // Start 3 more requests (request1 to request3). + + TestCompletionCallback callback1; + ProxyInfo results1; + rv = resolver->GetProxyForURL( + GURL("http://request1"), &results1, &callback1, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + TestCompletionCallback callback2; + ProxyInfo results2; + rv = resolver->GetProxyForURL( + GURL("http://request2"), &results2, &callback2, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + TestCompletionCallback callback3; + ProxyInfo results3; + rv = resolver->GetProxyForURL( + GURL("http://request3"), &results3, &callback3, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Wait for the requests to finish (they must finish in the order they were + // started, which is what we check for from their magic return value) + + rv = callback1.WaitForResult(); + EXPECT_EQ(1, rv); + EXPECT_EQ("PROXY request1:80", results1.ToPacString()); + + rv = callback2.WaitForResult(); + EXPECT_EQ(2, rv); + EXPECT_EQ("PROXY request2:80", results2.ToPacString()); + + rv = callback3.WaitForResult(); + EXPECT_EQ(3, rv); + EXPECT_EQ("PROXY request3:80", results3.ToPacString()); +} + +// Cancel a request which is in progress, and then cancel a request which +// is pending. +TEST(SingleThreadedProxyResolverTest, CancelRequest) { + BlockableProxyResolver* mock = new BlockableProxyResolver; + scoped_ptr<SingleThreadedProxyResolver> resolver( + new SingleThreadedProxyResolver(mock)); + + int rv; + + // Block the proxy resolver, so no request can complete. + mock->Block(); + + // Start request 0. + ProxyResolver::RequestHandle request0; + TestCompletionCallback callback0; + ProxyInfo results0; + rv = resolver->GetProxyForURL( + GURL("http://request0"), &results0, &callback0, &request0); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Wait until requests 0 reaches the worker thread. + mock->WaitUntilBlocked(); + + // Start 3 more requests (request1 : request3). + + TestCompletionCallback callback1; + ProxyInfo results1; + rv = resolver->GetProxyForURL( + GURL("http://request1"), &results1, &callback1, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + ProxyResolver::RequestHandle request2; + TestCompletionCallback callback2; + ProxyInfo results2; + rv = resolver->GetProxyForURL( + GURL("http://request2"), &results2, &callback2, &request2); + EXPECT_EQ(ERR_IO_PENDING, rv); + + TestCompletionCallback callback3; + ProxyInfo results3; + rv = resolver->GetProxyForURL( + GURL("http://request3"), &results3, &callback3, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Cancel request0 (inprogress) and request2 (pending). + resolver->CancelRequest(request0); + resolver->CancelRequest(request2); + + // Unblock the worker thread so the requests can continue running. + mock->Unblock(); + + // Wait for requests 1 and 3 to finish. + + rv = callback1.WaitForResult(); + EXPECT_EQ(1, rv); + EXPECT_EQ("PROXY request1:80", results1.ToPacString()); + + rv = callback3.WaitForResult(); + // Note that since request2 was cancelled before reaching the resolver, + // the request count is 2 and not 3 here. + EXPECT_EQ(2, rv); + EXPECT_EQ("PROXY request3:80", results3.ToPacString()); + + // Requests 0 and 2 which were cancelled, hence their completion callbacks + // were never summoned. + EXPECT_FALSE(callback0.have_result()); + EXPECT_FALSE(callback2.have_result()); +} + +// Test that deleting SingleThreadedProxyResolver while requests are +// outstanding cancels them (and doesn't leak anything). +TEST(SingleThreadedProxyResolverTest, CancelRequestByDeleting) { + BlockableProxyResolver* mock = new BlockableProxyResolver; + scoped_ptr<SingleThreadedProxyResolver> resolver( + new SingleThreadedProxyResolver(mock)); + + int rv; + + // Block the proxy resolver, so no request can complete. + mock->Block(); + + // Start 3 requests. + + TestCompletionCallback callback0; + ProxyInfo results0; + rv = resolver->GetProxyForURL( + GURL("http://request0"), &results0, &callback0, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + TestCompletionCallback callback1; + ProxyInfo results1; + rv = resolver->GetProxyForURL( + GURL("http://request1"), &results1, &callback1, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + TestCompletionCallback callback2; + ProxyInfo results2; + rv = resolver->GetProxyForURL( + GURL("http://request2"), &results2, &callback2, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Wait until request 0 reaches the worker thread. + mock->WaitUntilBlocked(); + + // Add some latency, to improve the chance that when + // SingleThreadedProxyResolver is deleted below we are still running inside + // of the worker thread. The test will pass regardless, so this race doesn't + // cause flakiness. However the destruction during execution is a more + // interesting case to test. + mock->SetResolveLatency(100); + + // Unblock the worker thread and delete the underlying + // SingleThreadedProxyResolver immediately. + mock->Unblock(); + resolver.reset(); + + // Give any posted tasks a chance to run (in case there is badness). + MessageLoop::current()->RunAllPending(); + + // Check that none of the outstanding requests were completed. + EXPECT_FALSE(callback0.have_result()); + EXPECT_FALSE(callback1.have_result()); + EXPECT_FALSE(callback2.have_result()); +} + +} // namespace +} // namespace net |