From 514748c7a884a623bd496a77d2fe648c6ca4baa9 Mon Sep 17 00:00:00 2001 From: sammc Date: Thu, 30 Apr 2015 23:15:04 -0700 Subject: Use ProxyResolverFactory in MultiThreadedProxyResolver. This CL changes MultiThreadedProxyResolver to take a ProxyResolverFactory instead of a LegacyProxyResolverFactory and adds MultiThreadedProxyResolverFactory, which implements ProxyResolverFactory. This also fixes a bug introduced in https://crrev.com/323412 where ProxyResolverFactoryForSystem would run a ProxyResolver on the IO thread when a maximum of 1 proxy resolver thread was requested. BUG=467403 Review URL: https://codereview.chromium.org/1095973004 Cr-Commit-Position: refs/heads/master@{#327885} --- net/proxy/multi_threaded_proxy_resolver.cc | 365 +++++++++------ net/proxy/multi_threaded_proxy_resolver.h | 96 ++-- .../multi_threaded_proxy_resolver_unittest.cc | 511 +++++++++++---------- net/proxy/proxy_resolver_mac.cc | 49 +- net/proxy/proxy_resolver_mac.h | 34 +- net/proxy/proxy_resolver_perftest.cc | 67 ++- net/proxy/proxy_resolver_winhttp.cc | 67 ++- net/proxy/proxy_resolver_winhttp.h | 42 +- net/proxy/proxy_service.cc | 21 +- 9 files changed, 721 insertions(+), 531 deletions(-) diff --git a/net/proxy/multi_threaded_proxy_resolver.cc b/net/proxy/multi_threaded_proxy_resolver.cc index b8fea44..a579e79 100644 --- a/net/proxy/multi_threaded_proxy_resolver.cc +++ b/net/proxy/multi_threaded_proxy_resolver.cc @@ -4,38 +4,45 @@ #include "net/proxy/multi_threaded_proxy_resolver.h" +#include +#include + #include "base/bind.h" #include "base/bind_helpers.h" #include "base/message_loop/message_loop_proxy.h" +#include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" +#include "base/threading/non_thread_safe.h" #include "base/threading/thread.h" #include "base/threading/thread_restrictions.h" #include "net/base/net_errors.h" #include "net/log/net_log.h" #include "net/proxy/proxy_info.h" -#include "net/proxy/proxy_resolver_factory.h" - -// TODO(eroman): Have the MultiThreadedProxyResolver clear its PAC script -// data when SetPacScript fails. That will reclaim memory when -// testing bogus scripts. +#include "net/proxy/proxy_resolver.h" namespace net { +namespace { +class Job; // An "executor" is a job-runner for PAC requests. It encapsulates a worker // thread and a synchronous ProxyResolver (which will be operated on said // thread.) -class MultiThreadedProxyResolver::Executor - : public base::RefCountedThreadSafe { +class Executor : public base::RefCountedThreadSafe { public: + class Coordinator { + public: + virtual void OnExecutorReady(Executor* executor) = 0; + + protected: + virtual ~Coordinator() = default; + }; + // |coordinator| must remain valid throughout our lifetime. It is used to // signal when the executor is ready to receive work by calling // |coordinator->OnExecutorReady()|. - // The constructor takes ownership of |resolver|. // |thread_number| is an identifier used when naming the worker thread. - Executor(MultiThreadedProxyResolver* coordinator, - scoped_ptr resolver, - int thread_number); + Executor(Coordinator* coordinator, int thread_number); // Submit a job to this executor. void StartJob(Job* job); @@ -54,14 +61,24 @@ class MultiThreadedProxyResolver::Executor int thread_number() const { return thread_number_; } + void set_resolver(scoped_ptr resolver) { + resolver_ = resolver.Pass(); + } + + void set_coordinator(Coordinator* coordinator) { + DCHECK(coordinator); + DCHECK(coordinator_); + coordinator_ = coordinator; + } + private: friend class base::RefCountedThreadSafe; ~Executor(); - MultiThreadedProxyResolver* coordinator_; + Coordinator* coordinator_; const int thread_number_; - // The currently active job for this executor (either a SetPacScript or + // The currently active job for this executor (either a CreateProxyResolver or // GetProxyForURL task). scoped_refptr outstanding_job_; @@ -75,16 +92,68 @@ class MultiThreadedProxyResolver::Executor scoped_ptr thread_; }; -// MultiThreadedProxyResolver::Job --------------------------------------------- +class MultiThreadedProxyResolver : public ProxyResolver, + public Executor::Coordinator, + public base::NonThreadSafe { + public: + // Creates an asynchronous ProxyResolver that runs requests on up to + // |max_num_threads|. + // + // For each thread that is created, an accompanying synchronous ProxyResolver + // will be provisioned using |resolver_factory|. All methods on these + // ProxyResolvers will be called on the one thread. + MultiThreadedProxyResolver( + scoped_ptr resolver_factory, + size_t max_num_threads, + const scoped_refptr& script_data, + scoped_refptr executor); + + ~MultiThreadedProxyResolver() override; + + // ProxyResolver implementation: + int GetProxyForURL(const GURL& url, + ProxyInfo* results, + const CompletionCallback& callback, + RequestHandle* request, + const BoundNetLog& net_log) override; + void CancelRequest(RequestHandle request) override; + LoadState GetLoadState(RequestHandle request) const override; + void CancelSetPacScript() override; + int SetPacScript(const scoped_refptr& script_data, + const CompletionCallback& callback) override; -class MultiThreadedProxyResolver::Job - : public base::RefCountedThreadSafe { + private: + class GetProxyForURLJob; + // FIFO queue of pending jobs waiting to be started. + // TODO(eroman): Make this priority queue. + typedef std::deque> PendingJobsQueue; + typedef std::vector> ExecutorList; + + // Returns an idle worker thread which is ready to receive GetProxyForURL() + // requests. If all threads are occupied, returns NULL. + Executor* FindIdleExecutor(); + + // Creates a new worker thread, and appends it to |executors_|. + void AddNewExecutor(); + + // Starts the next job from |pending_jobs_| if possible. + void OnExecutorReady(Executor* executor) override; + + const scoped_ptr resolver_factory_; + const size_t max_num_threads_; + PendingJobsQueue pending_jobs_; + ExecutorList executors_; + scoped_refptr script_data_; +}; + +// Job --------------------------------------------- + +class Job : public base::RefCountedThreadSafe { public: // Identifies the subclass of Job (only being used for debugging purposes). enum Type { TYPE_GET_PROXY_FOR_URL, - TYPE_SET_PAC_SCRIPT, - TYPE_SET_PAC_SCRIPT_INTERNAL, + TYPE_CREATE_RESOLVER, }; Job(Type type, const CompletionCallback& callback) @@ -117,7 +186,7 @@ class MultiThreadedProxyResolver::Job // Returns true if this job still has a user callback. Some jobs // do not have a user callback, because they were helper jobs - // scheduled internally (for example TYPE_SET_PAC_SCRIPT_INTERNAL). + // scheduled internally (for example TYPE_CREATE_RESOLVER). // // Otherwise jobs that correspond with user-initiated work will // have a non-null callback up until the callback is run. @@ -150,7 +219,7 @@ class MultiThreadedProxyResolver::Job callback.Run(result); } - friend class base::RefCountedThreadSafe; + friend class base::RefCountedThreadSafe; virtual ~Job() {} @@ -161,51 +230,50 @@ class MultiThreadedProxyResolver::Job bool was_cancelled_; }; -// MultiThreadedProxyResolver::SetPacScriptJob --------------------------------- +// CreateResolverJob ----------------------------------------------------------- -// Runs on the worker thread to call ProxyResolver::SetPacScript. -class MultiThreadedProxyResolver::SetPacScriptJob - : public MultiThreadedProxyResolver::Job { +// Runs on the worker thread to call ProxyResolverFactory::CreateProxyResolver. +class CreateResolverJob : public Job { public: - SetPacScriptJob(const scoped_refptr& script_data, - const CompletionCallback& callback) - : Job(!callback.is_null() ? TYPE_SET_PAC_SCRIPT : - TYPE_SET_PAC_SCRIPT_INTERNAL, - callback), - script_data_(script_data) { - } + CreateResolverJob(const scoped_refptr& script_data, + ProxyResolverFactory* factory) + : Job(TYPE_CREATE_RESOLVER, CompletionCallback()), + script_data_(script_data), + factory_(factory) {} // Runs on the worker thread. void Run(scoped_refptr origin_loop) override { - ProxyResolver* resolver = executor()->resolver(); - int rv = resolver->SetPacScript(script_data_, CompletionCallback()); + scoped_ptr request; + int rv = factory_->CreateProxyResolver(script_data_, &resolver_, + CompletionCallback(), &request); DCHECK_NE(rv, ERR_IO_PENDING); origin_loop->PostTask( - FROM_HERE, - base::Bind(&SetPacScriptJob::RequestComplete, this, rv)); + FROM_HERE, base::Bind(&CreateResolverJob::RequestComplete, this, rv)); } protected: - ~SetPacScriptJob() override {} + ~CreateResolverJob() override {} private: // Runs the completion callback on the origin thread. void RequestComplete(int result_code) { // The task may have been cancelled after it was started. - if (!was_cancelled() && has_user_callback()) { - RunUserCallback(result_code); + if (!was_cancelled()) { + DCHECK(executor()); + executor()->set_resolver(resolver_.Pass()); } OnJobCompleted(); } const scoped_refptr script_data_; + ProxyResolverFactory* factory_; + scoped_ptr resolver_; }; // MultiThreadedProxyResolver::GetProxyForURLJob ------------------------------ -class MultiThreadedProxyResolver::GetProxyForURLJob - : public MultiThreadedProxyResolver::Job { +class MultiThreadedProxyResolver::GetProxyForURLJob : public Job { public: // |url| -- the URL of the query. // |results| -- the structure to fill with proxy resolve results. @@ -243,6 +311,7 @@ class MultiThreadedProxyResolver::GetProxyForURLJob // Runs on the worker thread. void Run(scoped_refptr origin_loop) override { ProxyResolver* resolver = executor()->resolver(); + DCHECK(resolver); int rv = resolver->GetProxyForURL( url_, &results_buf_, CompletionCallback(), NULL, net_log_); DCHECK_NE(rv, ERR_IO_PENDING); @@ -281,24 +350,18 @@ class MultiThreadedProxyResolver::GetProxyForURLJob bool was_waiting_for_thread_; }; -// MultiThreadedProxyResolver::Executor ---------------------------------------- +// Executor ---------------------------------------- -MultiThreadedProxyResolver::Executor::Executor( - MultiThreadedProxyResolver* coordinator, - scoped_ptr resolver, - int thread_number) - : coordinator_(coordinator), - thread_number_(thread_number), - resolver_(resolver.Pass()) { +Executor::Executor(Executor::Coordinator* coordinator, int thread_number) + : coordinator_(coordinator), thread_number_(thread_number) { DCHECK(coordinator); - DCHECK(resolver_); // Start up the thread. thread_.reset(new base::Thread(base::StringPrintf("PAC thread #%d", thread_number))); CHECK(thread_->Start()); } -void MultiThreadedProxyResolver::Executor::StartJob(Job* job) { +void Executor::StartJob(Job* job) { DCHECK(!outstanding_job_.get()); outstanding_job_ = job; @@ -311,13 +374,13 @@ void MultiThreadedProxyResolver::Executor::StartJob(Job* job) { base::Bind(&Job::Run, job, base::MessageLoopProxy::current())); } -void MultiThreadedProxyResolver::Executor::OnJobCompleted(Job* job) { +void Executor::OnJobCompleted(Job* job) { DCHECK_EQ(job, outstanding_job_.get()); outstanding_job_ = NULL; coordinator_->OnExecutorReady(this); } -void MultiThreadedProxyResolver::Executor::Destroy() { +void Executor::Destroy() { DCHECK(coordinator_); { @@ -344,7 +407,7 @@ void MultiThreadedProxyResolver::Executor::Destroy() { outstanding_job_ = NULL; } -MultiThreadedProxyResolver::Executor::~Executor() { +Executor::~Executor() { // The important cleanup happens as part of Destroy(), which should always be // called first. DCHECK(!coordinator_) << "Destroy() was not called"; @@ -356,18 +419,27 @@ MultiThreadedProxyResolver::Executor::~Executor() { // MultiThreadedProxyResolver -------------------------------------------------- MultiThreadedProxyResolver::MultiThreadedProxyResolver( - LegacyProxyResolverFactory* resolver_factory, - size_t max_num_threads) + scoped_ptr resolver_factory, + size_t max_num_threads, + const scoped_refptr& script_data, + scoped_refptr executor) : ProxyResolver(resolver_factory->expects_pac_bytes()), - resolver_factory_(resolver_factory), - max_num_threads_(max_num_threads) { - DCHECK_GE(max_num_threads, 1u); + resolver_factory_(resolver_factory.Pass()), + max_num_threads_(max_num_threads), + script_data_(script_data) { + DCHECK(script_data_); + executor->set_coordinator(this); + executors_.push_back(executor); } MultiThreadedProxyResolver::~MultiThreadedProxyResolver() { + DCHECK(CalledOnValidThread()); // We will cancel all outstanding requests. pending_jobs_.clear(); - ReleaseAllExecutors(); + + for (auto& executor : executors_) { + executor->Destroy(); + } } int MultiThreadedProxyResolver::GetProxyForURL( @@ -375,8 +447,6 @@ int MultiThreadedProxyResolver::GetProxyForURL( RequestHandle* request, const BoundNetLog& net_log) { DCHECK(CalledOnValidThread()); DCHECK(!callback.is_null()); - DCHECK(current_script_data_.get()) - << "Resolver is un-initialized. Must call SetPacScript() first!"; scoped_refptr job( new GetProxyForURLJob(url, results, callback, net_log)); @@ -401,11 +471,8 @@ int MultiThreadedProxyResolver::GetProxyForURL( // If we haven't already reached the thread limit, provision a new thread to // drain the requests more quickly. - if (executors_.size() < max_num_threads_) { - executor = AddNewExecutor(); - executor->StartJob( - new SetPacScriptJob(current_script_data_, CompletionCallback())); - } + if (executors_.size() < max_num_threads_) + AddNewExecutor(); return ERR_IO_PENDING; } @@ -437,72 +504,17 @@ LoadState MultiThreadedProxyResolver::GetLoadState(RequestHandle req) const { } void MultiThreadedProxyResolver::CancelSetPacScript() { - DCHECK(CalledOnValidThread()); - DCHECK_EQ(0u, pending_jobs_.size()); - DCHECK_EQ(1u, executors_.size()); - DCHECK_EQ(Job::TYPE_SET_PAC_SCRIPT, - executors_[0]->outstanding_job()->type()); - - // Defensively clear some data which shouldn't be getting used - // anymore. - current_script_data_ = NULL; - - ReleaseAllExecutors(); + NOTREACHED(); } int MultiThreadedProxyResolver::SetPacScript( const scoped_refptr& script_data, const CompletionCallback&callback) { - DCHECK(CalledOnValidThread()); - DCHECK(!callback.is_null()); - - // Save the script details, so we can provision new executors later. - current_script_data_ = script_data; - - // The user should not have any outstanding requests when they call - // SetPacScript(). - CheckNoOutstandingUserRequests(); - - // Destroy all of the current threads and their proxy resolvers. - ReleaseAllExecutors(); - - // Provision a new executor, and run the SetPacScript request. On completion - // notification will be sent through |callback|. - Executor* executor = AddNewExecutor(); - executor->StartJob(new SetPacScriptJob(script_data, callback)); - return ERR_IO_PENDING; + NOTREACHED(); + return ERR_NOT_IMPLEMENTED; } -void MultiThreadedProxyResolver::CheckNoOutstandingUserRequests() const { - DCHECK(CalledOnValidThread()); - CHECK_EQ(0u, pending_jobs_.size()); - - for (ExecutorList::const_iterator it = executors_.begin(); - it != executors_.end(); ++it) { - const Executor* executor = it->get(); - Job* job = executor->outstanding_job(); - // The "has_user_callback()" is to exclude jobs for which the callback - // has already been invoked, or was not user-initiated (as in the case of - // lazy thread provisions). User-initiated jobs may !has_user_callback() - // when the callback has already been run. (Since we only clear the - // outstanding job AFTER the callback has been invoked, it is possible - // for a new request to be started from within the callback). - CHECK(!job || job->was_cancelled() || !job->has_user_callback()); - } -} - -void MultiThreadedProxyResolver::ReleaseAllExecutors() { - DCHECK(CalledOnValidThread()); - for (ExecutorList::iterator it = executors_.begin(); - it != executors_.end(); ++it) { - Executor* executor = it->get(); - executor->Destroy(); - } - executors_.clear(); -} - -MultiThreadedProxyResolver::Executor* -MultiThreadedProxyResolver::FindIdleExecutor() { +Executor* MultiThreadedProxyResolver::FindIdleExecutor() { DCHECK(CalledOnValidThread()); for (ExecutorList::iterator it = executors_.begin(); it != executors_.end(); ++it) { @@ -513,16 +525,15 @@ MultiThreadedProxyResolver::FindIdleExecutor() { return NULL; } -MultiThreadedProxyResolver::Executor* -MultiThreadedProxyResolver::AddNewExecutor() { +void MultiThreadedProxyResolver::AddNewExecutor() { DCHECK(CalledOnValidThread()); DCHECK_LT(executors_.size(), max_num_threads_); // The "thread number" is used to give the thread a unique name. int thread_number = executors_.size(); - scoped_ptr resolver = resolver_factory_->CreateProxyResolver(); - Executor* executor = new Executor(this, resolver.Pass(), thread_number); + Executor* executor = new Executor(this, thread_number); + executor->StartJob( + new CreateResolverJob(script_data_, resolver_factory_.get())); executors_.push_back(make_scoped_refptr(executor)); - return executor; } void MultiThreadedProxyResolver::OnExecutorReady(Executor* executor) { @@ -537,4 +548,98 @@ void MultiThreadedProxyResolver::OnExecutorReady(Executor* executor) { executor->StartJob(job.get()); } +} // namespace + +class MultiThreadedProxyResolverFactory::Job + : public ProxyResolverFactory::Request, + public Executor::Coordinator { + public: + Job(MultiThreadedProxyResolverFactory* factory, + const scoped_refptr& script_data, + scoped_ptr* resolver, + scoped_ptr resolver_factory, + size_t max_num_threads, + const CompletionCallback& callback) + : factory_(factory), + resolver_out_(resolver), + resolver_factory_(resolver_factory.Pass()), + max_num_threads_(max_num_threads), + script_data_(script_data), + executor_(new Executor(this, 0)), + callback_(callback) { + executor_->StartJob( + new CreateResolverJob(script_data_, resolver_factory_.get())); + } + + ~Job() override { + if (factory_) { + executor_->Destroy(); + factory_->RemoveJob(this); + } + } + + void FactoryDestroyed() { + executor_->Destroy(); + executor_ = nullptr; + factory_ = nullptr; + } + + private: + void OnExecutorReady(Executor* executor) override { + int error = OK; + if (executor->resolver()) { + resolver_out_->reset(new MultiThreadedProxyResolver( + resolver_factory_.Pass(), max_num_threads_, script_data_.Pass(), + executor_)); + } else { + error = ERR_PAC_SCRIPT_FAILED; + executor_->Destroy(); + } + factory_->RemoveJob(this); + factory_ = nullptr; + callback_.Run(error); + } + + MultiThreadedProxyResolverFactory* factory_; + scoped_ptr* const resolver_out_; + scoped_ptr resolver_factory_; + const size_t max_num_threads_; + scoped_refptr script_data_; + scoped_refptr executor_; + const CompletionCallback callback_; +}; + +MultiThreadedProxyResolverFactory::MultiThreadedProxyResolverFactory( + size_t max_num_threads, + bool factory_expects_bytes) + : ProxyResolverFactory(factory_expects_bytes), + max_num_threads_(max_num_threads) { + DCHECK_GE(max_num_threads, 1u); +} + +MultiThreadedProxyResolverFactory::~MultiThreadedProxyResolverFactory() { + for (auto job : jobs_) { + job->FactoryDestroyed(); + } +} + +int MultiThreadedProxyResolverFactory::CreateProxyResolver( + const scoped_refptr& pac_script, + scoped_ptr* resolver, + const CompletionCallback& callback, + scoped_ptr* request) { + scoped_ptr job(new Job(this, pac_script, resolver, + CreateProxyResolverFactory(), max_num_threads_, + callback)); + jobs_.insert(job.get()); + *request = job.Pass(); + return ERR_IO_PENDING; +} + +void MultiThreadedProxyResolverFactory::RemoveJob( + MultiThreadedProxyResolverFactory::Job* job) { + size_t erased = jobs_.erase(job); + DCHECK_EQ(1u, erased); +} + } // namespace net diff --git a/net/proxy/multi_threaded_proxy_resolver.h b/net/proxy/multi_threaded_proxy_resolver.h index a83c273..30a7b1b 100644 --- a/net/proxy/multi_threaded_proxy_resolver.h +++ b/net/proxy/multi_threaded_proxy_resolver.h @@ -5,36 +5,32 @@ #ifndef NET_PROXY_MULTI_THREADED_PROXY_RESOLVER_H_ #define NET_PROXY_MULTI_THREADED_PROXY_RESOLVER_H_ -#include -#include +#include #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/threading/non_thread_safe.h" #include "net/base/net_export.h" -#include "net/proxy/proxy_resolver.h" - -namespace base { -class Thread; -} // namespace base +#include "net/proxy/proxy_resolver_factory.h" namespace net { -class LegacyProxyResolverFactory; +class ProxyResolver; -// MultiThreadedProxyResolver is a ProxyResolver implementation that runs -// synchronous ProxyResolver implementations on worker threads. +// MultiThreadedProxyResolverFactory creates instances of a ProxyResolver +// implementation that runs synchronous ProxyResolver implementations on worker +// threads. // // Threads are created lazily on demand, up to a maximum total. The advantage // of having a pool of threads, is faster performance. In particular, being // able to keep servicing PAC requests even if one blocks its execution. // -// During initialization (SetPacScript), a single thread is spun up to test -// the script. If this succeeds, we cache the input script, and will re-use +// During initialization (CreateProxyResolver), a single thread is spun up to +// test the script. If this succeeds, we cache the input script, and will re-use // this to lazily provision any new threads as needed. // -// For each new thread that we spawn, a corresponding new ProxyResolver is -// created using ProxyResolverFactory. +// For each new thread that we spawn in a particular MultiThreadedProxyResolver +// instance, a corresponding new ProxyResolver is created using the +// ProxyResolverFactory returned by CreateProxyResolverFactory(). // // Because we are creating multiple ProxyResolver instances, this means we // are duplicating script contexts for what is ordinarily seen as being a @@ -51,69 +47,31 @@ class LegacyProxyResolverFactory; // a global counter and using that to make a decision. In the // multi-threaded model, each thread may have a different value for this // counter, so it won't globally be seen as monotonically increasing! -class NET_EXPORT_PRIVATE MultiThreadedProxyResolver - : public ProxyResolver, - NON_EXPORTED_BASE(public base::NonThreadSafe) { +class NET_EXPORT_PRIVATE MultiThreadedProxyResolverFactory + : public ProxyResolverFactory { public: - // Creates an asynchronous ProxyResolver that runs requests on up to - // |max_num_threads|. - // - // For each thread that is created, an accompanying synchronous ProxyResolver - // will be provisioned using |resolver_factory|. All methods on these - // ProxyResolvers will be called on the one thread, with the exception of - // ProxyResolver::Shutdown() which will be called from the origin thread - // prior to destruction. - // - // The constructor takes ownership of |resolver_factory|. - MultiThreadedProxyResolver(LegacyProxyResolverFactory* resolver_factory, - size_t max_num_threads); - - ~MultiThreadedProxyResolver() override; + MultiThreadedProxyResolverFactory(size_t max_num_threads, + bool factory_expects_bytes); + ~MultiThreadedProxyResolverFactory() override; - // ProxyResolver implementation: - int GetProxyForURL(const GURL& url, - ProxyInfo* results, - const CompletionCallback& callback, - RequestHandle* request, - const BoundNetLog& net_log) override; - void CancelRequest(RequestHandle request) override; - LoadState GetLoadState(RequestHandle request) const override; - void CancelSetPacScript() override; - int SetPacScript(const scoped_refptr& script_data, - const CompletionCallback& callback) override; + int CreateProxyResolver( + const scoped_refptr& pac_script, + scoped_ptr* resolver, + const CompletionCallback& callback, + scoped_ptr* request) override; private: - class Executor; class Job; - class SetPacScriptJob; - class GetProxyForURLJob; - // FIFO queue of pending jobs waiting to be started. - // TODO(eroman): Make this priority queue. - typedef std::deque > PendingJobsQueue; - typedef std::vector > ExecutorList; - - // Asserts that there are no outstanding user-initiated jobs on any of the - // worker threads. - void CheckNoOutstandingUserRequests() const; - // Stops and deletes all of the worker threads. - void ReleaseAllExecutors(); + // Invoked to create a ProxyResolverFactory instance to pass to a + // MultiThreadedProxyResolver instance. + virtual scoped_ptr CreateProxyResolverFactory() = 0; - // Returns an idle worker thread which is ready to receive GetProxyForURL() - // requests. If all threads are occupied, returns NULL. - Executor* FindIdleExecutor(); + void RemoveJob(Job* job); - // Creates a new worker thread, and appends it to |executors_|. - Executor* AddNewExecutor(); - - // Starts the next job from |pending_jobs_| if possible. - void OnExecutorReady(Executor* executor); - - const scoped_ptr resolver_factory_; const size_t max_num_threads_; - PendingJobsQueue pending_jobs_; - ExecutorList executors_; - scoped_refptr current_script_data_; + + std::set jobs_; }; } // namespace net diff --git a/net/proxy/multi_threaded_proxy_resolver_unittest.cc b/net/proxy/multi_threaded_proxy_resolver_unittest.cc index 3231b88..9142029 100644 --- a/net/proxy/multi_threaded_proxy_resolver_unittest.cc +++ b/net/proxy/multi_threaded_proxy_resolver_unittest.cc @@ -4,11 +4,15 @@ #include "net/proxy/multi_threaded_proxy_resolver.h" +#include + #include "base/message_loop/message_loop.h" +#include "base/run_loop.h" #include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" +#include "base/synchronization/lock.h" #include "base/synchronization/waitable_event.h" #include "base/threading/platform_thread.h" #include "net/base/net_errors.h" @@ -36,7 +40,7 @@ class MockProxyResolver : public ProxyResolver { public: MockProxyResolver() : ProxyResolver(true /*expects_pac_bytes*/), - wrong_loop_(base::MessageLoop::current()), + worker_loop_(base::MessageLoop::current()), request_count_(0) {} // ProxyResolver implementation. @@ -70,36 +74,25 @@ class MockProxyResolver : public ProxyResolver { } void CancelSetPacScript() override { NOTREACHED(); } - int SetPacScript(const scoped_refptr& script_data, const CompletionCallback& callback) override { - CheckIsOnWorkerThread(); - last_script_data_ = script_data; - return OK; + NOTREACHED(); + return ERR_NOT_IMPLEMENTED; } int request_count() const { return request_count_; } - const ProxyResolverScriptData* last_script_data() const { - return last_script_data_.get(); - } - void SetResolveLatency(base::TimeDelta latency) { resolve_latency_ = latency; } private: void CheckIsOnWorkerThread() { - // We should be running on the worker thread -- while we don't know the - // message loop of MultiThreadedProxyResolver'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(base::MessageLoop::current(), wrong_loop_); + EXPECT_EQ(base::MessageLoop::current(), worker_loop_); } - base::MessageLoop* wrong_loop_; + base::MessageLoop* worker_loop_; int request_count_; - scoped_refptr last_script_data_; base::TimeDelta resolve_latency_; }; @@ -153,53 +146,110 @@ class BlockableProxyResolver : public MockProxyResolver { }; // This factory returns new instances of BlockableProxyResolver. -class BlockableProxyResolverFactory : public LegacyProxyResolverFactory { +class BlockableProxyResolverFactory : public ProxyResolverFactory { public: - BlockableProxyResolverFactory() : LegacyProxyResolverFactory(true) {} + BlockableProxyResolverFactory() : ProxyResolverFactory(false) {} - ~BlockableProxyResolverFactory() override { STLDeleteElements(&resolvers_); } + ~BlockableProxyResolverFactory() override {} - scoped_ptr CreateProxyResolver() override { + int CreateProxyResolver( + const scoped_refptr& script_data, + scoped_ptr* result, + const CompletionCallback& callback, + scoped_ptr* request) override { BlockableProxyResolver* resolver = new BlockableProxyResolver; + result->reset(resolver); + base::AutoLock l(lock_); resolvers_.push_back(resolver); - return make_scoped_ptr(new ForwardingProxyResolver(resolver)); + script_data_.push_back(script_data); + return OK; } std::vector resolvers() { + base::AutoLock l(lock_); return resolvers_; } + const std::vector> script_data() { + base::AutoLock l(lock_); + return script_data_; + } + private: std::vector resolvers_; + std::vector> script_data_; + base::Lock lock_; }; -TEST(MultiThreadedProxyResolverTest, SingleThread_Basic) { - const size_t kNumThreads = 1u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - MultiThreadedProxyResolver resolver(factory, kNumThreads); +class SingleShotMultiThreadedProxyResolverFactory + : public MultiThreadedProxyResolverFactory { + public: + SingleShotMultiThreadedProxyResolverFactory( + size_t max_num_threads, + scoped_ptr factory) + : MultiThreadedProxyResolverFactory(max_num_threads, false), + factory_(factory.Pass()) {} + + scoped_ptr CreateProxyResolverFactory() override { + DCHECK(factory_); + return factory_.Pass(); + } - int rv; + private: + scoped_ptr factory_; +}; - EXPECT_TRUE(resolver.expects_pac_bytes()); +class MultiThreadedProxyResolverTest : public testing::Test { + public: + void Init(size_t num_threads) { + scoped_ptr factory_owner( + new BlockableProxyResolverFactory); + factory_ = factory_owner.get(); + resolver_factory_.reset(new SingleShotMultiThreadedProxyResolverFactory( + num_threads, factory_owner.Pass())); + TestCompletionCallback ready_callback; + scoped_ptr request; + resolver_factory_->CreateProxyResolver( + ProxyResolverScriptData::FromUTF8("pac script bytes"), &resolver_, + ready_callback.callback(), &request); + EXPECT_TRUE(request); + ASSERT_EQ(OK, ready_callback.WaitForResult()); + + // Verify that the script data reaches the synchronous resolver factory. + ASSERT_EQ(1u, factory_->script_data().size()); + EXPECT_EQ(ASCIIToUTF16("pac script bytes"), + factory_->script_data()[0]->utf16()); + } - // Call SetPacScriptByData() -- verify that it reaches the synchronous - // resolver. - TestCompletionCallback set_script_callback; - rv = resolver.SetPacScript( - ProxyResolverScriptData::FromUTF8("pac script bytes"), - set_script_callback.callback()); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(OK, set_script_callback.WaitForResult()); - ASSERT_EQ(1u, factory->resolvers().size()); - EXPECT_EQ(ASCIIToUTF16("pac script bytes"), - factory->resolvers()[0]->last_script_data()->utf16()); + void ClearResolver() { resolver_.reset(); } + + BlockableProxyResolverFactory& factory() { + DCHECK(factory_); + return *factory_; + } + ProxyResolver& resolver() { + DCHECK(resolver_); + return *resolver_; + } + + private: + BlockableProxyResolverFactory* factory_ = nullptr; + scoped_ptr factory_owner_; + scoped_ptr resolver_factory_; + scoped_ptr resolver_; +}; + +TEST_F(MultiThreadedProxyResolverTest, SingleThread_Basic) { + const size_t kNumThreads = 1u; + ASSERT_NO_FATAL_FAILURE(Init(kNumThreads)); // Start request 0. + int rv; TestCompletionCallback callback0; BoundTestNetLog log0; ProxyInfo results0; - rv = resolver.GetProxyForURL(GURL("http://request0"), &results0, - callback0.callback(), NULL, log0.bound()); + rv = resolver().GetProxyForURL(GURL("http://request0"), &results0, + callback0.callback(), NULL, log0.bound()); EXPECT_EQ(ERR_IO_PENDING, rv); // Wait for request 0 to finish. @@ -221,20 +271,20 @@ TEST(MultiThreadedProxyResolverTest, SingleThread_Basic) { TestCompletionCallback callback1; ProxyInfo results1; - rv = resolver.GetProxyForURL(GURL("http://request1"), &results1, - callback1.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request1"), &results1, + callback1.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); TestCompletionCallback callback2; ProxyInfo results2; - rv = resolver.GetProxyForURL(GURL("http://request2"), &results2, - callback2.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request2"), &results2, + callback2.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); TestCompletionCallback callback3; ProxyInfo results3; - rv = resolver.GetProxyForURL(GURL("http://request3"), &results3, - callback3.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request3"), &results3, + callback3.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); // Wait for the requests to finish (they must finish in the order they were @@ -255,33 +305,23 @@ TEST(MultiThreadedProxyResolverTest, SingleThread_Basic) { // Tests that the NetLog is updated to include the time the request was waiting // to be scheduled to a thread. -TEST(MultiThreadedProxyResolverTest, - SingleThread_UpdatesNetLogWithThreadWait) { +TEST_F(MultiThreadedProxyResolverTest, + SingleThread_UpdatesNetLogWithThreadWait) { const size_t kNumThreads = 1u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - MultiThreadedProxyResolver resolver(factory, kNumThreads); + ASSERT_NO_FATAL_FAILURE(Init(kNumThreads)); int rv; - // Initialize the resolver. - TestCompletionCallback init_callback; - rv = resolver.SetPacScript(ProxyResolverScriptData::FromUTF8("foo"), - init_callback.callback()); - EXPECT_EQ(OK, init_callback.WaitForResult()); - - ASSERT_EQ(1u, factory->resolvers().size()); - BlockableProxyResolver* mock = factory->resolvers()[0]; - // Block the proxy resolver, so no request can complete. - mock->Block(); + factory().resolvers()[0]->Block(); // Start request 0. ProxyResolver::RequestHandle request0; TestCompletionCallback callback0; ProxyInfo results0; BoundTestNetLog log0; - rv = resolver.GetProxyForURL(GURL("http://request0"), &results0, - callback0.callback(), &request0, log0.bound()); + rv = resolver().GetProxyForURL(GURL("http://request0"), &results0, + callback0.callback(), &request0, log0.bound()); EXPECT_EQ(ERR_IO_PENDING, rv); // Start 2 more requests (request1 and request2). @@ -289,21 +329,21 @@ TEST(MultiThreadedProxyResolverTest, TestCompletionCallback callback1; ProxyInfo results1; BoundTestNetLog log1; - rv = resolver.GetProxyForURL(GURL("http://request1"), &results1, - callback1.callback(), NULL, log1.bound()); + rv = resolver().GetProxyForURL(GURL("http://request1"), &results1, + callback1.callback(), NULL, log1.bound()); EXPECT_EQ(ERR_IO_PENDING, rv); ProxyResolver::RequestHandle request2; TestCompletionCallback callback2; ProxyInfo results2; BoundTestNetLog log2; - rv = resolver.GetProxyForURL(GURL("http://request2"), &results2, - callback2.callback(), &request2, log2.bound()); + rv = resolver().GetProxyForURL(GURL("http://request2"), &results2, + callback2.callback(), &request2, log2.bound()); EXPECT_EQ(ERR_IO_PENDING, rv); // Unblock the worker thread so the requests can continue running. - mock->WaitUntilBlocked(); - mock->Unblock(); + factory().resolvers()[0]->WaitUntilBlocked(); + factory().resolvers()[0]->Unblock(); // Check that request 0 completed as expected. // The NetLog has 1 entry that came from the MultiThreadedProxyResolver, and @@ -351,63 +391,55 @@ TEST(MultiThreadedProxyResolverTest, // Cancel a request which is in progress, and then cancel a request which // is pending. -TEST(MultiThreadedProxyResolverTest, SingleThread_CancelRequest) { +TEST_F(MultiThreadedProxyResolverTest, SingleThread_CancelRequest) { const size_t kNumThreads = 1u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - MultiThreadedProxyResolver resolver(factory, kNumThreads); + ASSERT_NO_FATAL_FAILURE(Init(kNumThreads)); int rv; - // Initialize the resolver. - TestCompletionCallback init_callback; - rv = resolver.SetPacScript(ProxyResolverScriptData::FromUTF8("foo"), - init_callback.callback()); - EXPECT_EQ(OK, init_callback.WaitForResult()); - - ASSERT_EQ(1u, factory->resolvers().size()); - BlockableProxyResolver* mock = factory->resolvers()[0]; - // Block the proxy resolver, so no request can complete. - mock->Block(); + factory().resolvers()[0]->Block(); // Start request 0. ProxyResolver::RequestHandle request0; TestCompletionCallback callback0; ProxyInfo results0; - rv = resolver.GetProxyForURL(GURL("http://request0"), &results0, - callback0.callback(), &request0, BoundNetLog()); + rv = + resolver().GetProxyForURL(GURL("http://request0"), &results0, + callback0.callback(), &request0, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); // Wait until requests 0 reaches the worker thread. - mock->WaitUntilBlocked(); + factory().resolvers()[0]->WaitUntilBlocked(); // Start 3 more requests (request1 : request3). TestCompletionCallback callback1; ProxyInfo results1; - rv = resolver.GetProxyForURL(GURL("http://request1"), &results1, - callback1.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request1"), &results1, + callback1.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); ProxyResolver::RequestHandle request2; TestCompletionCallback callback2; ProxyInfo results2; - rv = resolver.GetProxyForURL(GURL("http://request2"), &results2, - callback2.callback(), &request2, BoundNetLog()); + rv = + resolver().GetProxyForURL(GURL("http://request2"), &results2, + callback2.callback(), &request2, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); TestCompletionCallback callback3; ProxyInfo results3; - rv = resolver.GetProxyForURL(GURL("http://request3"), &results3, - callback3.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request3"), &results3, + callback3.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); // Cancel request0 (inprogress) and request2 (pending). - resolver.CancelRequest(request0); - resolver.CancelRequest(request2); + resolver().CancelRequest(request0); + resolver().CancelRequest(request2); // Unblock the worker thread so the requests can continue running. - mock->Unblock(); + factory().resolvers()[0]->Unblock(); // Wait for requests 1 and 3 to finish. @@ -429,60 +461,51 @@ TEST(MultiThreadedProxyResolverTest, SingleThread_CancelRequest) { // Test that deleting MultiThreadedProxyResolver while requests are // outstanding cancels them (and doesn't leak anything). -TEST(MultiThreadedProxyResolverTest, SingleThread_CancelRequestByDeleting) { +TEST_F(MultiThreadedProxyResolverTest, SingleThread_CancelRequestByDeleting) { const size_t kNumThreads = 1u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - scoped_ptr resolver( - new MultiThreadedProxyResolver(factory, kNumThreads)); - - int rv; - - // Initialize the resolver. - TestCompletionCallback init_callback; - rv = resolver->SetPacScript(ProxyResolverScriptData::FromUTF8("foo"), - init_callback.callback()); - EXPECT_EQ(OK, init_callback.WaitForResult()); + ASSERT_NO_FATAL_FAILURE(Init(kNumThreads)); - ASSERT_EQ(1u, factory->resolvers().size()); - BlockableProxyResolver* mock = factory->resolvers()[0]; + ASSERT_EQ(1u, factory().resolvers().size()); // Block the proxy resolver, so no request can complete. - mock->Block(); + factory().resolvers()[0]->Block(); + int rv; // Start 3 requests. TestCompletionCallback callback0; ProxyInfo results0; - rv = resolver->GetProxyForURL(GURL("http://request0"), &results0, - callback0.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request0"), &results0, + callback0.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); TestCompletionCallback callback1; ProxyInfo results1; - rv = resolver->GetProxyForURL(GURL("http://request1"), &results1, - callback1.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request1"), &results1, + callback1.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); TestCompletionCallback callback2; ProxyInfo results2; - rv = resolver->GetProxyForURL(GURL("http://request2"), &results2, - callback2.callback(), NULL, BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request2"), &results2, + callback2.callback(), NULL, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); // Wait until request 0 reaches the worker thread. - mock->WaitUntilBlocked(); + factory().resolvers()[0]->WaitUntilBlocked(); // Add some latency, to improve the chance that when // MultiThreadedProxyResolver 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(base::TimeDelta::FromMilliseconds(100)); + factory().resolvers()[0]->SetResolveLatency( + base::TimeDelta::FromMilliseconds(100)); // Unblock the worker thread and delete the underlying // MultiThreadedProxyResolver immediately. - mock->Unblock(); - resolver.reset(); + factory().resolvers()[0]->Unblock(); + ClearResolver(); // Give any posted tasks a chance to run (in case there is badness). base::MessageLoop::current()->RunUntilIdle(); @@ -493,84 +516,35 @@ TEST(MultiThreadedProxyResolverTest, SingleThread_CancelRequestByDeleting) { EXPECT_FALSE(callback2.have_result()); } -// Cancel an outstanding call to SetPacScriptByData(). -TEST(MultiThreadedProxyResolverTest, SingleThread_CancelSetPacScript) { - const size_t kNumThreads = 1u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - MultiThreadedProxyResolver resolver(factory, kNumThreads); - - int rv; - - TestCompletionCallback set_pac_script_callback; - rv = resolver.SetPacScript(ProxyResolverScriptData::FromUTF8("data"), - set_pac_script_callback.callback()); - EXPECT_EQ(ERR_IO_PENDING, rv); - - EXPECT_EQ(1u, factory->resolvers().size()); - - // Cancel the SetPacScriptByData request. - resolver.CancelSetPacScript(); - - // Start another SetPacScript request - TestCompletionCallback set_pac_script_callback2; - rv = resolver.SetPacScript(ProxyResolverScriptData::FromUTF8("data2"), - set_pac_script_callback2.callback()); - EXPECT_EQ(ERR_IO_PENDING, rv); - - // Wait for the initialization to complete. - - rv = set_pac_script_callback2.WaitForResult(); - EXPECT_EQ(0, rv); - ASSERT_EQ(2u, factory->resolvers().size()); - EXPECT_EQ(ASCIIToUTF16("data2"), - factory->resolvers()[1]->last_script_data()->utf16()); - - // The first SetPacScript callback should never have been completed. - EXPECT_FALSE(set_pac_script_callback.have_result()); -} - // Tests setting the PAC script once, lazily creating new threads, and // cancelling requests. -TEST(MultiThreadedProxyResolverTest, ThreeThreads_Basic) { +TEST_F(MultiThreadedProxyResolverTest, ThreeThreads_Basic) { const size_t kNumThreads = 3u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - MultiThreadedProxyResolver resolver(factory, kNumThreads); - - int rv; + ASSERT_NO_FATAL_FAILURE(Init(kNumThreads)); - EXPECT_TRUE(resolver.expects_pac_bytes()); - - // Call SetPacScriptByData() -- verify that it reaches the synchronous - // resolver. - TestCompletionCallback set_script_callback; - rv = resolver.SetPacScript( - ProxyResolverScriptData::FromUTF8("pac script bytes"), - set_script_callback.callback()); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(OK, set_script_callback.WaitForResult()); + // Verify that it reaches the synchronous resolver. // One thread has been provisioned (i.e. one ProxyResolver was created). - ASSERT_EQ(1u, factory->resolvers().size()); - EXPECT_EQ(ASCIIToUTF16("pac script bytes"), - factory->resolvers()[0]->last_script_data()->utf16()); + ASSERT_EQ(1u, factory().resolvers().size()); const int kNumRequests = 9; + int rv; TestCompletionCallback callback[kNumRequests]; ProxyInfo results[kNumRequests]; ProxyResolver::RequestHandle request[kNumRequests]; // Start request 0 -- this should run on thread 0 as there is nothing else // going on right now. - rv = resolver.GetProxyForURL( - GURL("http://request0"), &results[0], callback[0].callback(), &request[0], - BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request0"), &results[0], + callback[0].callback(), &request[0], + BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); // Wait for request 0 to finish. rv = callback[0].WaitForResult(); EXPECT_EQ(0, rv); EXPECT_EQ("PROXY request0:80", results[0].ToPacString()); - ASSERT_EQ(1u, factory->resolvers().size()); - EXPECT_EQ(1, factory->resolvers()[0]->request_count()); + ASSERT_EQ(1u, factory().resolvers().size()); + EXPECT_EQ(1, factory().resolvers()[0]->request_count()); base::MessageLoop::current()->RunUntilIdle(); @@ -579,22 +553,16 @@ TEST(MultiThreadedProxyResolverTest, ThreeThreads_Basic) { // have). for (int i = 1; i < kNumRequests; ++i) { - rv = resolver.GetProxyForURL( + rv = resolver().GetProxyForURL( GURL(base::StringPrintf("http://request%d", i)), &results[i], callback[i].callback(), &request[i], BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); } - // We should now have a total of 3 threads, each with its own ProxyResolver - // that will get initialized with the same data. (We check this later since - // the assignment happens on the worker threads and may not have occurred - // yet.) - ASSERT_EQ(3u, factory->resolvers().size()); - // Cancel 3 of the 8 oustanding requests. - resolver.CancelRequest(request[1]); - resolver.CancelRequest(request[3]); - resolver.CancelRequest(request[6]); + resolver().CancelRequest(request[1]); + resolver().CancelRequest(request[3]); + resolver().CancelRequest(request[6]); // Wait for the remaining requests to complete. int kNonCancelledRequests[] = {2, 4, 5, 7, 8}; @@ -608,35 +576,26 @@ TEST(MultiThreadedProxyResolverTest, ThreeThreads_Basic) { EXPECT_FALSE(callback[3].have_result()); EXPECT_FALSE(callback[6].have_result()); - // We call SetPacScript again, solely to stop the current worker threads. - // (That way we can test to see the values observed by the synchronous - // resolvers in a non-racy manner). - TestCompletionCallback set_script_callback2; - rv = resolver.SetPacScript(ProxyResolverScriptData::FromUTF8("xyz"), - set_script_callback2.callback()); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(OK, set_script_callback2.WaitForResult()); - ASSERT_EQ(4u, factory->resolvers().size()); + // We should now have a total of 3 threads, each with its own ProxyResolver + // that will get initialized with the same data. + ASSERT_EQ(3u, factory().resolvers().size()); + ASSERT_EQ(3u, factory().script_data().size()); for (int i = 0; i < 3; ++i) { - EXPECT_EQ( - ASCIIToUTF16("pac script bytes"), - factory->resolvers()[i]->last_script_data()->utf16()) << "i=" << i; + EXPECT_EQ(ASCIIToUTF16("pac script bytes"), + factory().script_data()[i]->utf16()) + << "i=" << i; } - EXPECT_EQ(ASCIIToUTF16("xyz"), - factory->resolvers()[3]->last_script_data()->utf16()); - // We don't know the exact ordering that requests ran on threads with, // but we do know the total count that should have reached the threads. // 8 total were submitted, and three were cancelled. Of the three that // were cancelled, one of them (request 1) was cancelled after it had // already been posted to the worker thread. So the resolvers will // have seen 6 total (and 1 from the run prior). - ASSERT_EQ(4u, factory->resolvers().size()); int total_count = 0; for (int i = 0; i < 3; ++i) { - total_count += factory->resolvers()[i]->request_count(); + total_count += factory().resolvers()[i]->request_count(); } EXPECT_EQ(7, total_count); } @@ -644,26 +603,16 @@ TEST(MultiThreadedProxyResolverTest, ThreeThreads_Basic) { // Tests using two threads. The first request hangs the first thread. Checks // that other requests are able to complete while this first request remains // stalled. -TEST(MultiThreadedProxyResolverTest, OneThreadBlocked) { +TEST_F(MultiThreadedProxyResolverTest, OneThreadBlocked) { const size_t kNumThreads = 2u; - BlockableProxyResolverFactory* factory = new BlockableProxyResolverFactory; - MultiThreadedProxyResolver resolver(factory, kNumThreads); + ASSERT_NO_FATAL_FAILURE(Init(kNumThreads)); int rv; - EXPECT_TRUE(resolver.expects_pac_bytes()); - - // Initialize the resolver. - TestCompletionCallback set_script_callback; - rv = resolver.SetPacScript( - ProxyResolverScriptData::FromUTF8("pac script bytes"), - set_script_callback.callback()); - EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_EQ(OK, set_script_callback.WaitForResult()); // One thread has been provisioned (i.e. one ProxyResolver was created). - ASSERT_EQ(1u, factory->resolvers().size()); + ASSERT_EQ(1u, factory().resolvers().size()); EXPECT_EQ(ASCIIToUTF16("pac script bytes"), - factory->resolvers()[0]->last_script_data()->utf16()); + factory().script_data()[0]->utf16()); const int kNumRequests = 4; TestCompletionCallback callback[kNumRequests]; @@ -672,22 +621,22 @@ TEST(MultiThreadedProxyResolverTest, OneThreadBlocked) { // Start a request that will block the first thread. - factory->resolvers()[0]->Block(); + factory().resolvers()[0]->Block(); - rv = resolver.GetProxyForURL( - GURL("http://request0"), &results[0], callback[0].callback(), &request[0], - BoundNetLog()); + rv = resolver().GetProxyForURL(GURL("http://request0"), &results[0], + callback[0].callback(), &request[0], + BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); - factory->resolvers()[0]->WaitUntilBlocked(); + factory().resolvers()[0]->WaitUntilBlocked(); // Start 3 more requests -- they should all be serviced by thread #2 // since thread #1 is blocked. for (int i = 1; i < kNumRequests; ++i) { - rv = resolver.GetProxyForURL( - GURL(base::StringPrintf("http://request%d", i)), - &results[i], callback[i].callback(), &request[i], BoundNetLog()); + rv = resolver().GetProxyForURL( + GURL(base::StringPrintf("http://request%d", i)), &results[i], + callback[i].callback(), &request[i], BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, rv); } @@ -698,14 +647,116 @@ TEST(MultiThreadedProxyResolverTest, OneThreadBlocked) { } // Unblock the first thread. - factory->resolvers()[0]->Unblock(); + factory().resolvers()[0]->Unblock(); EXPECT_EQ(0, callback[0].WaitForResult()); // All in all, the first thread should have seen just 1 request. And the // second thread 3 requests. - ASSERT_EQ(2u, factory->resolvers().size()); - EXPECT_EQ(1, factory->resolvers()[0]->request_count()); - EXPECT_EQ(3, factory->resolvers()[1]->request_count()); + ASSERT_EQ(2u, factory().resolvers().size()); + EXPECT_EQ(1, factory().resolvers()[0]->request_count()); + EXPECT_EQ(3, factory().resolvers()[1]->request_count()); +} + +class FailingProxyResolverFactory : public ProxyResolverFactory { + public: + FailingProxyResolverFactory() : ProxyResolverFactory(false) {} + + // ProxyResolverFactory override. + int CreateProxyResolver( + const scoped_refptr& script_data, + scoped_ptr* result, + const CompletionCallback& callback, + scoped_ptr* request) override { + return ERR_PAC_SCRIPT_FAILED; + } +}; + +// Test that an error when creating the synchronous resolver causes the +// MultiThreadedProxyResolverFactory create request to fail with that error. +TEST_F(MultiThreadedProxyResolverTest, ProxyResolverFactoryError) { + const size_t kNumThreads = 1u; + SingleShotMultiThreadedProxyResolverFactory resolver_factory( + kNumThreads, make_scoped_ptr(new FailingProxyResolverFactory)); + TestCompletionCallback ready_callback; + scoped_ptr request; + scoped_ptr resolver; + EXPECT_EQ(ERR_IO_PENDING, + resolver_factory.CreateProxyResolver( + ProxyResolverScriptData::FromUTF8("pac script bytes"), + &resolver, ready_callback.callback(), &request)); + EXPECT_TRUE(request); + EXPECT_EQ(ERR_PAC_SCRIPT_FAILED, ready_callback.WaitForResult()); + EXPECT_FALSE(resolver); +} + +void Fail(int error) { + FAIL() << "Unexpected callback with error " << error; +} + +// Test that cancelling an in-progress create request works correctly. +TEST_F(MultiThreadedProxyResolverTest, CancelCreate) { + const size_t kNumThreads = 1u; + { + SingleShotMultiThreadedProxyResolverFactory resolver_factory( + kNumThreads, make_scoped_ptr(new BlockableProxyResolverFactory)); + scoped_ptr request; + scoped_ptr resolver; + EXPECT_EQ(ERR_IO_PENDING, + resolver_factory.CreateProxyResolver( + ProxyResolverScriptData::FromUTF8("pac script bytes"), + &resolver, base::Bind(&Fail), &request)); + EXPECT_TRUE(request); + request.reset(); + } + // The factory destructor will block until the worker thread stops, but it may + // post tasks to the origin message loop which are still pending. Run them + // now to ensure it works as expected. + base::RunLoop().RunUntilIdle(); +} + +void DeleteRequest(const CompletionCallback& callback, + scoped_ptr* request, + int result) { + callback.Run(result); + request->reset(); +} + +// Test that delete the Request during the factory callback works correctly. +TEST_F(MultiThreadedProxyResolverTest, DeleteRequestInFactoryCallback) { + const size_t kNumThreads = 1u; + SingleShotMultiThreadedProxyResolverFactory resolver_factory( + kNumThreads, make_scoped_ptr(new BlockableProxyResolverFactory)); + scoped_ptr request; + scoped_ptr resolver; + TestCompletionCallback callback; + EXPECT_EQ(ERR_IO_PENDING, + resolver_factory.CreateProxyResolver( + ProxyResolverScriptData::FromUTF8("pac script bytes"), + &resolver, base::Bind(&DeleteRequest, callback.callback(), + base::Unretained(&request)), + &request)); + EXPECT_TRUE(request); + EXPECT_EQ(OK, callback.WaitForResult()); +} + +// Test that deleting the factory with a request in-progress works correctly. +TEST_F(MultiThreadedProxyResolverTest, DestroyFactoryWithRequestsInProgress) { + const size_t kNumThreads = 1u; + scoped_ptr request; + scoped_ptr resolver; + { + SingleShotMultiThreadedProxyResolverFactory resolver_factory( + kNumThreads, make_scoped_ptr(new BlockableProxyResolverFactory)); + EXPECT_EQ(ERR_IO_PENDING, + resolver_factory.CreateProxyResolver( + ProxyResolverScriptData::FromUTF8("pac script bytes"), + &resolver, base::Bind(&Fail), &request)); + EXPECT_TRUE(request); + } + // The factory destructor will block until the worker thread stops, but it may + // post tasks to the origin message loop which are still pending. Run them + // now to ensure it works as expected. + base::RunLoop().RunUntilIdle(); } } // namespace diff --git a/net/proxy/proxy_resolver_mac.cc b/net/proxy/proxy_resolver_mac.cc index 6d69080..521bd6a 100644 --- a/net/proxy/proxy_resolver_mac.cc +++ b/net/proxy/proxy_resolver_mac.cc @@ -13,6 +13,7 @@ #include "base/strings/sys_string_conversions.h" #include "net/base/net_errors.h" #include "net/proxy/proxy_info.h" +#include "net/proxy/proxy_resolver.h" #include "net/proxy/proxy_server.h" #if defined(OS_IOS) @@ -62,10 +63,35 @@ void ResultCallback(void* client, CFArrayRef proxies, CFErrorRef error) { CFRunLoopStop(CFRunLoopGetCurrent()); } -} // namespace +class ProxyResolverMac : public ProxyResolver { + public: + explicit ProxyResolverMac( + const scoped_refptr& script_data); + ~ProxyResolverMac() override; + + // ProxyResolver methods: + int GetProxyForURL(const GURL& url, + ProxyInfo* results, + const CompletionCallback& callback, + RequestHandle* request, + const BoundNetLog& net_log) override; + + void CancelRequest(RequestHandle request) override; + + LoadState GetLoadState(RequestHandle request) const override; + + void CancelSetPacScript() override; + + int SetPacScript(const scoped_refptr& script_data, + const CompletionCallback& /*callback*/) override; + + private: + const scoped_refptr script_data_; +}; -ProxyResolverMac::ProxyResolverMac() - : ProxyResolver(false /*expects_pac_bytes*/) { +ProxyResolverMac::ProxyResolverMac( + const scoped_refptr& script_data) + : ProxyResolver(false /*expects_pac_bytes*/), script_data_(script_data) { } ProxyResolverMac::~ProxyResolverMac() {} @@ -196,7 +222,22 @@ void ProxyResolverMac::CancelSetPacScript() { int ProxyResolverMac::SetPacScript( const scoped_refptr& script_data, const CompletionCallback& /*callback*/) { - script_data_ = script_data; + NOTREACHED(); + return ERR_NOT_IMPLEMENTED; +} + +} // namespace + +ProxyResolverFactoryMac::ProxyResolverFactoryMac() + : ProxyResolverFactory(false /*expects_pac_bytes*/) { +} + +int ProxyResolverFactoryMac::CreateProxyResolver( + const scoped_refptr& pac_script, + scoped_ptr* resolver, + const CompletionCallback& callback, + scoped_ptr* request) { + resolver->reset(new ProxyResolverMac(pac_script)); return OK; } diff --git a/net/proxy/proxy_resolver_mac.h b/net/proxy/proxy_resolver_mac.h index 3db83f6..6d89ef5 100644 --- a/net/proxy/proxy_resolver_mac.h +++ b/net/proxy/proxy_resolver_mac.h @@ -6,38 +6,26 @@ #define NET_PROXY_PROXY_RESOLVER_MAC_H_ #include "base/compiler_specific.h" -#include "net/base/net_errors.h" #include "net/base/net_export.h" -#include "net/proxy/proxy_resolver.h" +#include "net/proxy/proxy_resolver_factory.h" #include "url/gurl.h" namespace net { -// Implementation of ProxyResolver that uses the Mac CFProxySupport to implement -// proxies. -class NET_EXPORT ProxyResolverMac : public ProxyResolver { +// Implementation of ProxyResolverFactory that uses the Mac CFProxySupport to +// implement proxies. +class NET_EXPORT ProxyResolverFactoryMac : public ProxyResolverFactory { public: - ProxyResolverMac(); - ~ProxyResolverMac() override; + ProxyResolverFactoryMac(); - // ProxyResolver methods: - int GetProxyForURL(const GURL& url, - ProxyInfo* results, - const CompletionCallback& callback, - RequestHandle* request, - const BoundNetLog& net_log) override; - - void CancelRequest(RequestHandle request) override; - - LoadState GetLoadState(RequestHandle request) const override; - - void CancelSetPacScript() override; - - int SetPacScript(const scoped_refptr& script_data, - const CompletionCallback& /*callback*/) override; + int CreateProxyResolver( + const scoped_refptr& pac_script, + scoped_ptr* resolver, + const CompletionCallback& callback, + scoped_ptr* request) override; private: - scoped_refptr script_data_; + DISALLOW_COPY_AND_ASSIGN(ProxyResolverFactoryMac); }; } // namespace net diff --git a/net/proxy/proxy_resolver_perftest.cc b/net/proxy/proxy_resolver_perftest.cc index 480908e..4307759 100644 --- a/net/proxy/proxy_resolver_perftest.cc +++ b/net/proxy/proxy_resolver_perftest.cc @@ -11,6 +11,7 @@ #include "net/base/net_errors.h" #include "net/dns/mock_host_resolver.h" #include "net/proxy/proxy_info.h" +#include "net/proxy/proxy_resolver_factory.h" #include "net/proxy/proxy_resolver_v8.h" #include "net/test/spawned_test_server/spawned_test_server.h" #include "testing/gtest/include/gtest/gtest.h" @@ -85,8 +86,9 @@ const int kNumIterations = 500; class PacPerfSuiteRunner { public: // |resolver_name| is the label used when logging the results. - PacPerfSuiteRunner(ProxyResolver* resolver, const std::string& resolver_name) - : resolver_(resolver), + PacPerfSuiteRunner(ProxyResolverFactory* factory, + const std::string& resolver_name) + : factory_(factory), resolver_name_(resolver_name), test_server_(SpawnedTestServer::TYPE_HTTP, SpawnedTestServer::kLocalhost, @@ -107,15 +109,18 @@ class PacPerfSuiteRunner { void RunTest(const std::string& script_name, const PacQuery* queries, int queries_len) { - if (!resolver_->expects_pac_bytes()) { + scoped_ptr resolver; + if (!factory_->expects_pac_bytes()) { GURL pac_url = test_server_.GetURL(std::string("files/") + script_name); - int rv = resolver_->SetPacScript( - ProxyResolverScriptData::FromURL(pac_url), CompletionCallback()); + int rv = factory_->CreateProxyResolver( + ProxyResolverScriptData::FromURL(pac_url), &resolver, + CompletionCallback(), nullptr); EXPECT_EQ(OK, rv); } else { - LoadPacScriptIntoResolver(script_name); + resolver = LoadPacScriptAndCreateResolver(script_name); } + ASSERT_TRUE(resolver); // Do a query to warm things up. In the case of internal-fetch proxy // resolvers, the first resolve will be slow since it has to download @@ -123,8 +128,8 @@ class PacPerfSuiteRunner { { ProxyInfo proxy_info; int result = - resolver_->GetProxyForURL(GURL("http://www.warmup.com"), &proxy_info, - CompletionCallback(), NULL, BoundNetLog()); + resolver->GetProxyForURL(GURL("http://www.warmup.com"), &proxy_info, + CompletionCallback(), NULL, BoundNetLog()); ASSERT_EQ(OK, result); } @@ -139,8 +144,8 @@ class PacPerfSuiteRunner { // Resolve. ProxyInfo proxy_info; int result = - resolver_->GetProxyForURL(GURL(query.query_url), &proxy_info, - CompletionCallback(), NULL, BoundNetLog()); + resolver->GetProxyForURL(GURL(query.query_url), &proxy_info, + CompletionCallback(), NULL, BoundNetLog()); // Check that the result was correct. Note that ToPacString() and // ASSERT_EQ() are fast, so they won't skew the results. @@ -153,7 +158,8 @@ class PacPerfSuiteRunner { } // Read the PAC script from disk and initialize the proxy resolver with it. - void LoadPacScriptIntoResolver(const std::string& script_name) { + scoped_ptr LoadPacScriptAndCreateResolver( + const std::string& script_name) { base::FilePath path; PathService::Get(base::DIR_SOURCE_ROOT, &path); path = path.AppendASCII("net"); @@ -167,29 +173,33 @@ class PacPerfSuiteRunner { // If we can't load the file from disk, something is misconfigured. LOG_IF(ERROR, !ok) << "Failed to read file: " << path.value(); - ASSERT_TRUE(ok); + if (!ok) + return nullptr; // Load the PAC script into the ProxyResolver. - int rv = resolver_->SetPacScript( - ProxyResolverScriptData::FromUTF8(file_contents), CompletionCallback()); + scoped_ptr resolver; + int rv = factory_->CreateProxyResolver( + ProxyResolverScriptData::FromUTF8(file_contents), &resolver, + CompletionCallback(), nullptr); EXPECT_EQ(OK, rv); + return resolver; } - ProxyResolver* resolver_; + ProxyResolverFactory* factory_; std::string resolver_name_; SpawnedTestServer test_server_; }; #if defined(OS_WIN) TEST(ProxyResolverPerfTest, ProxyResolverWinHttp) { - ProxyResolverWinHttp resolver; - PacPerfSuiteRunner runner(&resolver, "ProxyResolverWinHttp"); + ProxyResolverFactoryWinHttp factory; + PacPerfSuiteRunner runner(&factory, "ProxyResolverWinHttp"); runner.RunAllTests(); } #elif defined(OS_MACOSX) TEST(ProxyResolverPerfTest, ProxyResolverMac) { - ProxyResolverMac resolver; - PacPerfSuiteRunner runner(&resolver, "ProxyResolverMac"); + ProxyResolverFactoryMac factory; + PacPerfSuiteRunner runner(&factory, "ProxyResolverMac"); runner.RunAllTests(); } #endif @@ -213,11 +223,22 @@ class MockJSBindings : public ProxyResolverV8::JSBindings { } }; +class ProxyResolverV8Factory : public LegacyProxyResolverFactory { + public: + ProxyResolverV8Factory() : LegacyProxyResolverFactory(true) {} + scoped_ptr CreateProxyResolver() override { + scoped_ptr resolver(new ProxyResolverV8); + resolver->set_js_bindings(&js_bindings_); + return resolver.Pass(); + } + + private: + MockJSBindings js_bindings_; +}; + TEST(ProxyResolverPerfTest, ProxyResolverV8) { - MockJSBindings js_bindings; - ProxyResolverV8 resolver; - resolver.set_js_bindings(&js_bindings); - PacPerfSuiteRunner runner(&resolver, "ProxyResolverV8"); + ProxyResolverV8Factory factory; + PacPerfSuiteRunner runner(&factory, "ProxyResolverV8"); runner.RunAllTests(); } diff --git a/net/proxy/proxy_resolver_winhttp.cc b/net/proxy/proxy_resolver_winhttp.cc index 755a1bf..f7193d1 100644 --- a/net/proxy/proxy_resolver_winhttp.cc +++ b/net/proxy/proxy_resolver_winhttp.cc @@ -12,6 +12,7 @@ #include "base/strings/utf_string_conversions.h" #include "net/base/net_errors.h" #include "net/proxy/proxy_info.h" +#include "net/proxy/proxy_resolver.h" #include "url/gurl.h" #pragma comment(lib, "winhttp.lib") @@ -20,6 +21,7 @@ using base::TimeDelta; using base::TimeTicks; namespace net { +namespace { static void FreeInfo(WINHTTP_PROXY_INFO* info) { if (info->lpszProxy) @@ -28,8 +30,46 @@ static void FreeInfo(WINHTTP_PROXY_INFO* info) { GlobalFree(info->lpszProxyBypass); } -ProxyResolverWinHttp::ProxyResolverWinHttp() - : ProxyResolver(false /*expects_pac_bytes*/), session_handle_(NULL) { +class ProxyResolverWinHttp : public ProxyResolver { + public: + ProxyResolverWinHttp( + const scoped_refptr& script_data); + ~ProxyResolverWinHttp() override; + + // ProxyResolver implementation: + int GetProxyForURL(const GURL& url, + ProxyInfo* results, + const CompletionCallback& /*callback*/, + RequestHandle* /*request*/, + const BoundNetLog& /*net_log*/) override; + void CancelRequest(RequestHandle request) override; + + LoadState GetLoadState(RequestHandle request) const override; + + void CancelSetPacScript() override; + + int SetPacScript(const scoped_refptr& script_data, + const CompletionCallback& /*callback*/) override; + + private: + bool OpenWinHttpSession(); + void CloseWinHttpSession(); + + // Proxy configuration is cached on the session handle. + HINTERNET session_handle_; + + const GURL pac_url_; + + DISALLOW_COPY_AND_ASSIGN(ProxyResolverWinHttp); +}; + +ProxyResolverWinHttp::ProxyResolverWinHttp( + const scoped_refptr& script_data) + : ProxyResolver(false /*expects_pac_bytes*/), + session_handle_(NULL), + pac_url_(script_data->type() == ProxyResolverScriptData::TYPE_AUTO_DETECT + ? GURL("http://wpad/wpad.dat") + : script_data->url()) { } ProxyResolverWinHttp::~ProxyResolverWinHttp() { @@ -136,12 +176,8 @@ void ProxyResolverWinHttp::CancelSetPacScript() { int ProxyResolverWinHttp::SetPacScript( const scoped_refptr& script_data, const CompletionCallback& /*callback*/) { - if (script_data->type() == ProxyResolverScriptData::TYPE_AUTO_DETECT) { - pac_url_ = GURL("http://wpad/wpad.dat"); - } else { - pac_url_ = script_data->url(); - } - return OK; + NOTREACHED(); + return ERR_NOT_IMPLEMENTED; } bool ProxyResolverWinHttp::OpenWinHttpSession() { @@ -171,4 +207,19 @@ void ProxyResolverWinHttp::CloseWinHttpSession() { } } +} // namespace + +ProxyResolverFactoryWinHttp::ProxyResolverFactoryWinHttp() + : ProxyResolverFactory(false /*expects_pac_bytes*/) { +} + +int ProxyResolverFactoryWinHttp::CreateProxyResolver( + const scoped_refptr& pac_script, + scoped_ptr* resolver, + const CompletionCallback& callback, + scoped_ptr* request) { + resolver->reset(new ProxyResolverWinHttp(pac_script)); + return OK; +} + } // namespace net diff --git a/net/proxy/proxy_resolver_winhttp.h b/net/proxy/proxy_resolver_winhttp.h index 3c2004a..843711c 100644 --- a/net/proxy/proxy_resolver_winhttp.h +++ b/net/proxy/proxy_resolver_winhttp.h @@ -6,45 +6,27 @@ #define NET_PROXY_PROXY_RESOLVER_WINHTTP_H_ #include "base/compiler_specific.h" -#include "net/proxy/proxy_resolver.h" +#include "net/base/net_export.h" +#include "net/proxy/proxy_resolver_factory.h" #include "url/gurl.h" -typedef void* HINTERNET; // From winhttp.h - namespace net { -// An implementation of ProxyResolver that uses WinHTTP and the system +// An implementation of ProxyResolverFactory that uses WinHTTP and the system // proxy settings. -class NET_EXPORT_PRIVATE ProxyResolverWinHttp : public ProxyResolver { +class NET_EXPORT_PRIVATE ProxyResolverFactoryWinHttp + : public ProxyResolverFactory { public: - ProxyResolverWinHttp(); - ~ProxyResolverWinHttp() override; - - // ProxyResolver implementation: - int GetProxyForURL(const GURL& url, - ProxyInfo* results, - const CompletionCallback& /*callback*/, - RequestHandle* /*request*/, - const BoundNetLog& /*net_log*/) override; - void CancelRequest(RequestHandle request) override; - - LoadState GetLoadState(RequestHandle request) const override; - - void CancelSetPacScript() override; + ProxyResolverFactoryWinHttp(); - int SetPacScript(const scoped_refptr& script_data, - const CompletionCallback& /*callback*/) override; + int CreateProxyResolver( + const scoped_refptr& pac_script, + scoped_ptr* resolver, + const CompletionCallback& callback, + scoped_ptr* request) override; private: - bool OpenWinHttpSession(); - void CloseWinHttpSession(); - - // Proxy configuration is cached on the session handle. - HINTERNET session_handle_; - - GURL pac_url_; - - DISALLOW_COPY_AND_ASSIGN(ProxyResolverWinHttp); + DISALLOW_COPY_AND_ASSIGN(ProxyResolverFactoryWinHttp); }; } // namespace net diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc index b3d3be4..10019b8 100644 --- a/net/proxy/proxy_service.cc +++ b/net/proxy/proxy_service.cc @@ -236,22 +236,17 @@ class ProxyResolverFromPacString : public ProxyResolver { }; // Creates ProxyResolvers using a platform-specific implementation. -class ProxyResolverFactoryForSystem : public LegacyProxyResolverFactory { +class ProxyResolverFactoryForSystem : public MultiThreadedProxyResolverFactory { public: explicit ProxyResolverFactoryForSystem(size_t max_num_threads) - : LegacyProxyResolverFactory(false /*expects_pac_bytes*/), - max_num_threads_(max_num_threads) {} - - scoped_ptr CreateProxyResolver() override { - DCHECK(IsSupported()); - if (max_num_threads_ > 1) { - return make_scoped_ptr(new MultiThreadedProxyResolver( - new ProxyResolverFactoryForSystem(1), max_num_threads_)); - } + : MultiThreadedProxyResolverFactory(max_num_threads, + false /*expects_pac_bytes*/) {} + + scoped_ptr CreateProxyResolverFactory() override { #if defined(OS_WIN) - return make_scoped_ptr(new ProxyResolverWinHttp()); + return make_scoped_ptr(new ProxyResolverFactoryWinHttp()); #elif defined(OS_MACOSX) - return make_scoped_ptr(new ProxyResolverMac()); + return make_scoped_ptr(new ProxyResolverFactoryMac()); #else NOTREACHED(); return NULL; @@ -267,8 +262,6 @@ class ProxyResolverFactoryForSystem : public LegacyProxyResolverFactory { } private: - const size_t max_num_threads_; - DISALLOW_COPY_AND_ASSIGN(ProxyResolverFactoryForSystem); }; -- cgit v1.1