diff options
| -rw-r--r-- | chrome/browser/net/resolve_proxy_msg_helper_unittest.cc | 38 | ||||
| -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 |
17 files changed, 1108 insertions, 325 deletions
diff --git a/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc b/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc index d33a03b..84bb1d5 100644 --- a/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc +++ b/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc @@ -8,6 +8,7 @@ #include "net/base/net_errors.h" #include "net/proxy/proxy_config_service.h" #include "net/proxy/proxy_resolver.h" +#include "net/proxy/single_threaded_proxy_resolver.h" #include "testing/gtest/include/gtest/gtest.h" // This ProxyConfigService always returns "http://pac" as the PAC url to use. @@ -22,21 +23,29 @@ class MockProxyConfigService: public net::ProxyConfigService { // This PAC resolver always returns the hostname of the query URL as the // proxy to use. The Block() method will make GetProxyForURL() hang until // Unblock() is called. -class MockProxyResolver : public net::ProxyResolver { +class SyncMockProxyResolver : public net::ProxyResolver { public: - MockProxyResolver() : ProxyResolver(true), event_(false, false), - is_blocked_(false) { + SyncMockProxyResolver() : ProxyResolver(false /*expects_pac_bytes*/), + event_(false, false), + is_blocked_(false) { } virtual int GetProxyForURL(const GURL& query_url, - const GURL& /*pac_url*/, - net::ProxyInfo* results) { + net::ProxyInfo* results, + net::CompletionCallback* callback, + RequestHandle* request) { if (is_blocked_) event_.Wait(); results->UseNamedProxy(query_url.host()); return net::OK; } + virtual void CancelRequest(RequestHandle request) { + NOTREACHED(); + } + + virtual void SetPacScriptByUrlInternal(const GURL& pac_url) {} + void Block() { is_blocked_ = true; event_.Reset(); @@ -52,6 +61,17 @@ class MockProxyResolver : public net::ProxyResolver { bool is_blocked_; }; +class MockProxyResolver : public net::SingleThreadedProxyResolver { + public: + MockProxyResolver() + : net::SingleThreadedProxyResolver(new SyncMockProxyResolver) { + x = reinterpret_cast<SyncMockProxyResolver*>(resolver()); + } + + // TODO(eroman): cleanup. + SyncMockProxyResolver* x; +}; + // This struct holds the values that were passed to // Delegate::OnResolveProxyCompleted(). The caller should use WaitUntilDone() // to block until the result has been populated. @@ -271,7 +291,7 @@ TEST(ResolveProxyMsgHelperTest, QueueRequests) { scoped_ptr<IPC::Message> msg3(new IPC::Message()); // Make the proxy resolver hang on the next request. - runner.proxy_resolver()->Block(); + runner.proxy_resolver()->x->Block(); // Start three requests. Since the proxy resolver is hung, the second two // will be pending. @@ -285,7 +305,7 @@ TEST(ResolveProxyMsgHelperTest, QueueRequests) { result3->WaitUntilStarted(); // Unblock the proxy service so requests 1-3 can complete. - runner.proxy_resolver()->Unblock(); + runner.proxy_resolver()->x->Unblock(); // Wait for all the requests to finish (they run in FIFO order). result3->WaitUntilDone(); @@ -320,7 +340,7 @@ TEST(ResolveProxyMsgHelperTest, CancelPendingRequests) { IPC::Message* msg3 = new IPC::Message(); // Make the next request block. - runner.proxy_resolver()->Block(); + runner.proxy_resolver()->x->Block(); // Start three requests; since the first one blocked, the other two should // be pending. @@ -338,7 +358,7 @@ TEST(ResolveProxyMsgHelperTest, CancelPendingRequests) { // Unblocking the proxy resolver means the three requests can complete -- // however they should not try to notify the delegate since we have already // deleted the helper. - runner.proxy_resolver()->Unblock(); + runner.proxy_resolver()->x->Unblock(); // Check that none of the requests were sent to the delegate. EXPECT_FALSE( 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 |