diff options
| -rw-r--r-- | chrome/browser/net/passive_log_collector.cc | 3 | ||||
| -rw-r--r-- | net/base/host_resolver_impl.cc | 40 | ||||
| -rw-r--r-- | net/base/host_resolver_impl.h | 14 | ||||
| -rw-r--r-- | net/base/host_resolver_impl_unittest.cc | 120 |
4 files changed, 152 insertions, 25 deletions
diff --git a/chrome/browser/net/passive_log_collector.cc b/chrome/browser/net/passive_log_collector.cc index 3dd8ccd..c62c3321 100644 --- a/chrome/browser/net/passive_log_collector.cc +++ b/chrome/browser/net/passive_log_collector.cc @@ -251,6 +251,8 @@ void PassiveLogCollector::SourceTracker::AddToDeletionQueue( DCHECK_GE(sources_.find(source_id)->second.reference_count, 0); DCHECK_LE(deletion_queue_.size(), max_graveyard_size_); + DCHECK(std::find(deletion_queue_.begin(), deletion_queue_.end(), + source_id) == deletion_queue_.end()); deletion_queue_.push_back(source_id); // After the deletion queue has reached its maximum size, start @@ -534,4 +536,3 @@ PassiveLogCollector::DNSJobTracker::DoAddEntry(const Entry& entry, return ACTION_NONE; } } - diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc index f4f7bbd..b93ae31 100644 --- a/net/base/host_resolver_impl.cc +++ b/net/base/host_resolver_impl.cc @@ -258,7 +258,9 @@ class HostResolverImpl::Request { void OnComplete(int error, const AddressList& addrlist) { if (error == OK) addresses_->SetFrom(addrlist, port()); - callback_->Run(error); + CompletionCallback* callback = callback_; + MarkAsCancelled(); + callback->Run(error); } int port() const { @@ -840,8 +842,7 @@ HostResolverImpl::~HostResolverImpl() { // requests, which will also be cancelled. DiscardIPv6ProbeJob(); - for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ++it) - it->second->Cancel(); + CancelAllJobs(); // In case we are being deleted during the processing of a callback. if (cur_completing_job_) @@ -1035,9 +1036,7 @@ void HostResolverImpl::Shutdown() { DCHECK(CalledOnValidThread()); // Cancel the outstanding jobs. - for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ++it) - it->second->Cancel(); - jobs_.clear(); + CancelAllJobs(); DiscardIPv6ProbeJob(); shutdown_ = true; @@ -1090,6 +1089,18 @@ void HostResolverImpl::OnJobComplete(Job* job, if (cache_.get()) cache_->Set(job->key(), net_error, addrlist, base::TimeTicks::Now()); + OnJobCompleteInternal(job, net_error, os_error, addrlist); +} + +void HostResolverImpl::AbortJob(Job* job) { + OnJobCompleteInternal(job, ERR_ABORTED, 0 /* no os_error */, AddressList()); +} + +void HostResolverImpl::OnJobCompleteInternal( + Job* job, + int net_error, + int os_error, + const AddressList& addrlist) { // Make a note that we are executing within OnJobComplete() in case the // HostResolver is deleted by a callback invocation. DCHECK(!cur_completing_job_); @@ -1199,6 +1210,7 @@ void HostResolverImpl::OnIPAddressChanged() { ipv6_probe_job_ = new IPv6ProbeJob(this); ipv6_probe_job_->Start(); } + AbortAllJobs(); #if defined(OS_LINUX) if (HaveOnlyLoopbackAddresses()) { additional_resolver_flags_ |= HOST_RESOLVER_LOOPBACK_ONLY; @@ -1317,4 +1329,20 @@ int HostResolverImpl::EnqueueRequest(JobPool* pool, Request* req) { return ERR_IO_PENDING; } +void HostResolverImpl::CancelAllJobs() { + JobMap jobs; + jobs.swap(jobs_); + for (JobMap::iterator it = jobs.begin(); it != jobs.end(); ++it) + it->second->Cancel(); +} + +void HostResolverImpl::AbortAllJobs() { + JobMap jobs; + jobs.swap(jobs_); + for (JobMap::iterator it = jobs.begin(); it != jobs.end(); ++it) { + AbortJob(it->second); + it->second->Cancel(); + } +} + } // namespace net diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h index a06a478..58d8598 100644 --- a/net/base/host_resolver_impl.h +++ b/net/base/host_resolver_impl.h @@ -156,6 +156,14 @@ class HostResolverImpl : public HostResolver, void OnJobComplete(Job* job, int net_error, int os_error, const AddressList& addrlist); + // Aborts |job|. Same as OnJobComplete() except does not remove |job| + // from |jobs_| and does not cache the result (ERR_ABORTED). + void AbortJob(Job* job); + + // Used by both OnJobComplete() and AbortJob(); + void OnJobCompleteInternal(Job* job, int net_error, int os_error, + const AddressList& addrlist); + // Called when a request has just been started. void OnStartRequest(const BoundNetLog& source_net_log, const BoundNetLog& request_net_log, @@ -211,6 +219,12 @@ class HostResolverImpl : public HostResolver, // Adds a pending request |req| to |pool|. int EnqueueRequest(JobPool* pool, Request* req); + // Cancels all jobs. + void CancelAllJobs(); + + // Aborts all jobs. + void AbortAllJobs(); + // Cache of host resolution results. scoped_ptr<HostCache> cache_; diff --git a/net/base/host_resolver_impl_unittest.cc b/net/base/host_resolver_impl_unittest.cc index a78d1c0..78b2087 100644 --- a/net/base/host_resolver_impl_unittest.cc +++ b/net/base/host_resolver_impl_unittest.cc @@ -255,7 +255,7 @@ class HostResolverImplTest : public testing::Test { }; TEST_F(HostResolverImplTest, SynchronousLookup) { - AddressList adrlist; + AddressList addrlist; const int kPortnum = 80; scoped_refptr<RuleBasedHostResolverProc> resolver_proc = @@ -267,7 +267,7 @@ TEST_F(HostResolverImplTest, SynchronousLookup) { HostResolver::RequestInfo info("just.testing", kPortnum); CapturingBoundNetLog log(CapturingNetLog::kUnbounded); - int err = host_resolver->Resolve(info, &adrlist, NULL, NULL, log.bound()); + int err = host_resolver->Resolve(info, &addrlist, NULL, NULL, log.bound()); EXPECT_EQ(OK, err); EXPECT_EQ(2u, log.entries().size()); @@ -276,7 +276,7 @@ TEST_F(HostResolverImplTest, SynchronousLookup) { EXPECT_TRUE(LogContainsEndEvent( log.entries(), 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); - const struct addrinfo* ainfo = adrlist.head(); + const struct addrinfo* ainfo = addrlist.head(); EXPECT_EQ(static_cast<addrinfo*>(NULL), ainfo->ai_next); EXPECT_EQ(sizeof(struct sockaddr_in), ainfo->ai_addrlen); @@ -287,7 +287,7 @@ TEST_F(HostResolverImplTest, SynchronousLookup) { } TEST_F(HostResolverImplTest, AsynchronousLookup) { - AddressList adrlist; + AddressList addrlist; const int kPortnum = 80; scoped_refptr<RuleBasedHostResolverProc> resolver_proc = @@ -299,7 +299,7 @@ TEST_F(HostResolverImplTest, AsynchronousLookup) { HostResolver::RequestInfo info("just.testing", kPortnum); CapturingBoundNetLog log(CapturingNetLog::kUnbounded); - int err = host_resolver->Resolve(info, &adrlist, &callback_, NULL, + int err = host_resolver->Resolve(info, &addrlist, &callback_, NULL, log.bound()); EXPECT_EQ(ERR_IO_PENDING, err); @@ -316,7 +316,7 @@ TEST_F(HostResolverImplTest, AsynchronousLookup) { EXPECT_TRUE(LogContainsEndEvent( log.entries(), 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); - const struct addrinfo* ainfo = adrlist.head(); + const struct addrinfo* ainfo = addrlist.head(); EXPECT_EQ(static_cast<addrinfo*>(NULL), ainfo->ai_next); EXPECT_EQ(sizeof(struct sockaddr_in), ainfo->ai_addrlen); @@ -338,11 +338,11 @@ TEST_F(HostResolverImplTest, CanceledAsynchronousLookup) { CreateDefaultCache(), kMaxJobs, &net_log)); - AddressList adrlist; + AddressList addrlist; const int kPortnum = 80; HostResolver::RequestInfo info("just.testing", kPortnum); - int err = host_resolver->Resolve(info, &adrlist, &callback_, NULL, + int err = host_resolver->Resolve(info, &addrlist, &callback_, NULL, log.bound()); EXPECT_EQ(ERR_IO_PENDING, err); @@ -395,13 +395,13 @@ TEST_F(HostResolverImplTest, NumericIPv4Address) { scoped_refptr<HostResolver> host_resolver( CreateHostResolverImpl(resolver_proc)); - AddressList adrlist; + AddressList addrlist; const int kPortnum = 5555; HostResolver::RequestInfo info("127.1.2.3", kPortnum); - int err = host_resolver->Resolve(info, &adrlist, NULL, NULL, BoundNetLog()); + int err = host_resolver->Resolve(info, &addrlist, NULL, NULL, BoundNetLog()); EXPECT_EQ(OK, err); - const struct addrinfo* ainfo = adrlist.head(); + const struct addrinfo* ainfo = addrlist.head(); EXPECT_EQ(static_cast<addrinfo*>(NULL), ainfo->ai_next); EXPECT_EQ(sizeof(struct sockaddr_in), ainfo->ai_addrlen); @@ -420,13 +420,13 @@ TEST_F(HostResolverImplTest, NumericIPv6Address) { // the caller should have removed them. scoped_refptr<HostResolver> host_resolver( CreateHostResolverImpl(resolver_proc)); - AddressList adrlist; + AddressList addrlist; const int kPortnum = 5555; HostResolver::RequestInfo info("2001:db8::1", kPortnum); - int err = host_resolver->Resolve(info, &adrlist, NULL, NULL, BoundNetLog()); + int err = host_resolver->Resolve(info, &addrlist, NULL, NULL, BoundNetLog()); EXPECT_EQ(OK, err); - const struct addrinfo* ainfo = adrlist.head(); + const struct addrinfo* ainfo = addrlist.head(); EXPECT_EQ(static_cast<addrinfo*>(NULL), ainfo->ai_next); EXPECT_EQ(sizeof(struct sockaddr_in6), ainfo->ai_addrlen); @@ -450,10 +450,10 @@ TEST_F(HostResolverImplTest, EmptyHost) { scoped_refptr<HostResolver> host_resolver( CreateHostResolverImpl(resolver_proc)); - AddressList adrlist; + AddressList addrlist; const int kPortnum = 5555; HostResolver::RequestInfo info("", kPortnum); - int err = host_resolver->Resolve(info, &adrlist, NULL, NULL, BoundNetLog()); + int err = host_resolver->Resolve(info, &addrlist, NULL, NULL, BoundNetLog()); EXPECT_EQ(ERR_NAME_NOT_RESOLVED, err); } @@ -464,11 +464,11 @@ TEST_F(HostResolverImplTest, LongHost) { scoped_refptr<HostResolver> host_resolver( CreateHostResolverImpl(resolver_proc)); - AddressList adrlist; + AddressList addrlist; const int kPortnum = 5555; std::string hostname(4097, 'a'); HostResolver::RequestInfo info(hostname, kPortnum); - int err = host_resolver->Resolve(info, &adrlist, NULL, NULL, BoundNetLog()); + int err = host_resolver->Resolve(info, &addrlist, NULL, NULL, BoundNetLog()); EXPECT_EQ(ERR_NAME_NOT_RESOLVED, err); } @@ -1103,6 +1103,90 @@ TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) { EXPECT_EQ(OK, callback.WaitForResult()); } +// Test that IP address changes send ERR_ABORTED to pending requests. +TEST_F(HostResolverImplTest, AbortOnIPAddressChanged) { + scoped_refptr<WaitingHostResolverProc> resolver_proc = + new WaitingHostResolverProc(NULL); + HostCache* cache = CreateDefaultCache(); + scoped_refptr<HostResolver> host_resolver( + new HostResolverImpl(resolver_proc, cache, kMaxJobs, NULL)); + + // Resolve "host1". + HostResolver::RequestInfo info("host1", 70); + TestCompletionCallback callback; + AddressList addrlist; + int rv = host_resolver->Resolve(info, &addrlist, &callback, NULL, + BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Triggering an IP address change. + NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests(); + MessageLoop::current()->RunAllPending(); // Notification happens async. + resolver_proc->Signal(); + + EXPECT_EQ(ERR_ABORTED, callback.WaitForResult()); + EXPECT_EQ(0u, cache->size()); +} + +class ResolveWithinCallback : public CallbackRunner< Tuple1<int> > { + public: + ResolveWithinCallback( + const scoped_refptr<MockHostResolver>& host_resolver, + const HostResolver::RequestInfo& info) + : host_resolver_(host_resolver), + info_(info) { + DCHECK(host_resolver.get()); + } + + virtual void RunWithParams(const Tuple1<int>& params) { + // Ditch the WaitingHostResolverProc so that the subsequent request + // succeeds. + host_resolver_->Reset(NULL); + callback_.RunWithParams(params); + EXPECT_EQ(ERR_IO_PENDING, + host_resolver_->Resolve(info_, &addrlist_, &nested_callback_, + NULL, BoundNetLog())); + } + + int WaitForResult() { + return callback_.WaitForResult(); + } + + int WaitForNestedResult() { + return nested_callback_.WaitForResult(); + } + + private: + const scoped_refptr<MockHostResolver> host_resolver_; + const HostResolver::RequestInfo info_; + AddressList addrlist_; + TestCompletionCallback callback_; + TestCompletionCallback nested_callback_; +}; + +TEST_F(HostResolverImplTest, OnlyAbortExistingRequestsOnIPAddressChange) { + scoped_refptr<WaitingHostResolverProc> resolver_proc = + new WaitingHostResolverProc(NULL); + scoped_refptr<MockHostResolver> host_resolver(new MockHostResolver()); + host_resolver->Reset(resolver_proc); + + // Resolve "host1". + HostResolver::RequestInfo info("host1", 70); + ResolveWithinCallback callback(host_resolver, info); + AddressList addrlist; + int rv = host_resolver->Resolve(info, &addrlist, &callback, NULL, + BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + // Triggering an IP address change. + NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests(); + MessageLoop::current()->RunAllPending(); // Notification happens async. + + EXPECT_EQ(ERR_ABORTED, callback.WaitForResult()); + resolver_proc->Signal(); + EXPECT_EQ(OK, callback.WaitForNestedResult()); +} + // Tests that when the maximum threads is set to 1, requests are dequeued // in order of priority. TEST_F(HostResolverImplTest, HigherPriorityRequestsStartedFirst) { |