diff options
Diffstat (limited to 'net/base')
-rw-r--r-- | net/base/host_resolver.cc | 12 | ||||
-rw-r--r-- | net/base/host_resolver.h | 13 | ||||
-rw-r--r-- | net/base/host_resolver_unittest.cc | 107 |
3 files changed, 116 insertions, 16 deletions
diff --git a/net/base/host_resolver.cc b/net/base/host_resolver.cc index dd1ffcc..14ad212 100644 --- a/net/base/host_resolver.cc +++ b/net/base/host_resolver.cc @@ -184,7 +184,7 @@ class HostResolver::Request { addresses_(addresses) {} // Mark the request as cancelled. - void Cancel() { + void MarkAsCancelled() { job_ = NULL; callback_ = NULL; addresses_ = NULL; @@ -291,8 +291,12 @@ class HostResolver::Job : public base::RefCountedThreadSafe<HostResolver::Job> { void Cancel() { resolver_ = NULL; - AutoLock locked(origin_loop_lock_); - origin_loop_ = NULL; + // Mark the job as cancelled, so when worker thread completes it will + // not try to post completion to origin loop. + { + AutoLock locked(origin_loop_lock_); + origin_loop_ = NULL; + } } // Called from origin thread. @@ -483,7 +487,7 @@ void HostResolver::CancelRequest(Request* req) { DCHECK(req); DCHECK(req->job()); // NULL out the fields of req, to mark it as cancelled. - req->Cancel(); + req->MarkAsCancelled(); } void HostResolver::AddObserver(Observer* observer) { diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h index 34c803f..6e2add7 100644 --- a/net/base/host_resolver.h +++ b/net/base/host_resolver.h @@ -106,13 +106,18 @@ class HostResolver { // Called at the start of HostResolver::Resolve(). |id| is a unique number // given to the request, so it can be matched up with a corresponding call - // to OnFinishResolutionWithStatus(). + // to OnFinishResolutionWithStatus() or OnCancelResolution(). virtual void OnStartResolution(int id, const RequestInfo& info) = 0; // Called on completion of request |id|. Note that if the request was - // cancelled, OnFinishResolutionWithStatus() will not be called. + // cancelled, OnCancelResolution() will be called instead. virtual void OnFinishResolutionWithStatus(int id, bool was_resolved, const RequestInfo& info) = 0; + + // Called when request |id| has been cancelled. A request is "cancelled" + // if either the HostResolver is destroyed while a resolution is in + // progress, or HostResolver::CancelRequest() is called. + virtual void OnCancelResolution(int id, const RequestInfo& info) = 0; }; // Creates a HostResolver that caches up to |max_cache_entries| for @@ -175,11 +180,11 @@ class HostResolver { // Callback for when |job| has completed with |error| and |addrlist|. void OnJobComplete(Job* job, int error, const AddressList& addrlist); - // Notify all obsevers of the start of a resolve request. + // Notify all observers of the start of a resolve request. void NotifyObserversStartRequest(int request_id, const RequestInfo& info); - // Notify all obsevers of the completion of a resolve request. + // Notify all observers of the completion of a resolve request. void NotifyObserversFinishRequest(int request_id, const RequestInfo& info, int error); diff --git a/net/base/host_resolver_unittest.cc b/net/base/host_resolver_unittest.cc index 195b2fb..62281f0 100644 --- a/net/base/host_resolver_unittest.cc +++ b/net/base/host_resolver_unittest.cc @@ -702,7 +702,7 @@ class CapturingObserver : public net::HostResolver::Observer { // DnsResolutionObserver methods: virtual void OnStartResolution(int id, const net::HostResolver::RequestInfo& info) { - start_log.push_back(StartEntry(id, info)); + start_log.push_back(StartOrCancelEntry(id, info)); } virtual void OnFinishResolutionWithStatus( @@ -712,12 +712,17 @@ class CapturingObserver : public net::HostResolver::Observer { finish_log.push_back(FinishEntry(id, was_resolved, info)); } + virtual void OnCancelResolution(int id, + const net::HostResolver::RequestInfo& info) { + cancel_log.push_back(StartOrCancelEntry(id, info)); + } + // Tuple (id, info). - struct StartEntry { - StartEntry(int id, const net::HostResolver::RequestInfo& info) + struct StartOrCancelEntry { + StartOrCancelEntry(int id, const net::HostResolver::RequestInfo& info) : id(id), info(info) {} - bool operator==(const StartEntry& other) const { + bool operator==(const StartOrCancelEntry& other) const { return id == other.id && info == other.info; } @@ -742,11 +747,14 @@ class CapturingObserver : public net::HostResolver::Observer { net::HostResolver::RequestInfo info; }; - std::vector<StartEntry> start_log; + std::vector<StartOrCancelEntry> start_log; std::vector<FinishEntry> finish_log; + std::vector<StartOrCancelEntry> cancel_log; }; // Test that registering, unregistering, and notifying of observers works. +// Does not test the cancellation notification since all resolves are +// synchronous. TEST_F(HostResolverTest, Observers) { net::HostResolver host_resolver; @@ -763,8 +771,9 @@ TEST_F(HostResolverTest, Observers) { EXPECT_EQ(1U, observer.start_log.size()); EXPECT_EQ(1U, observer.finish_log.size()); + EXPECT_EQ(0U, observer.cancel_log.size()); EXPECT_TRUE(observer.start_log[0] == - CapturingObserver::StartEntry(0, info1)); + CapturingObserver::StartOrCancelEntry(0, info1)); EXPECT_TRUE(observer.finish_log[0] == CapturingObserver::FinishEntry(0, true, info1)); @@ -776,8 +785,9 @@ TEST_F(HostResolverTest, Observers) { EXPECT_EQ(2U, observer.start_log.size()); EXPECT_EQ(2U, observer.finish_log.size()); + EXPECT_EQ(0U, observer.cancel_log.size()); EXPECT_TRUE(observer.start_log[1] == - CapturingObserver::StartEntry(1, info1)); + CapturingObserver::StartOrCancelEntry(1, info1)); EXPECT_TRUE(observer.finish_log[1] == CapturingObserver::FinishEntry(1, true, info1)); @@ -789,7 +799,9 @@ TEST_F(HostResolverTest, Observers) { EXPECT_EQ(3U, observer.start_log.size()); EXPECT_EQ(3U, observer.finish_log.size()); - EXPECT_TRUE(observer.start_log[2] == CapturingObserver::StartEntry(2, info2)); + EXPECT_EQ(0U, observer.cancel_log.size()); + EXPECT_TRUE(observer.start_log[2] == + CapturingObserver::StartOrCancelEntry(2, info2)); EXPECT_TRUE(observer.finish_log[2] == CapturingObserver::FinishEntry(2, true, info2)); @@ -803,6 +815,85 @@ TEST_F(HostResolverTest, Observers) { // No effect this time, since observer was removed. EXPECT_EQ(3U, observer.start_log.size()); EXPECT_EQ(3U, observer.finish_log.size()); + EXPECT_EQ(0U, observer.cancel_log.size()); +} + +// Tests that observers are sent OnCancelResolution() whenever a request is +// cancelled. There are two ways to cancel a request: +// (1) Delete the HostResolver while job is outstanding. +// (2) Call HostResolver::CancelRequest() while a request is outstanding. +TEST_F(HostResolverTest, DISABLED_CancellationObserver) { + // Set a blocking mapper so we control when the resolver thread finishes. + scoped_refptr<WaitingHostMapper> mapper = new WaitingHostMapper(); + ScopedHostMapper scoped_mapper(mapper.get()); + + CapturingObserver observer; + { + // Create a host resolver and attach an observer. + net::HostResolver host_resolver; + host_resolver.AddObserver(&observer); + + TestCompletionCallback callback; + + EXPECT_EQ(0U, observer.start_log.size()); + EXPECT_EQ(0U, observer.finish_log.size()); + EXPECT_EQ(0U, observer.cancel_log.size()); + + // Start an async resolve for (host1:70). + net::HostResolver::RequestInfo info1("host1", 70); + net::HostResolver::Request* req = NULL; + net::AddressList addrlist; + int rv = host_resolver.Resolve(info1, &addrlist, &callback, &req); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + EXPECT_TRUE(NULL != req); + + EXPECT_EQ(1U, observer.start_log.size()); + EXPECT_EQ(0U, observer.finish_log.size()); + EXPECT_EQ(0U, observer.cancel_log.size()); + + EXPECT_TRUE(observer.start_log[0] == + CapturingObserver::StartOrCancelEntry(0, info1)); + + // Cancel the request (host mapper is blocked so it cant be finished yet). + host_resolver.CancelRequest(req); + + EXPECT_EQ(1U, observer.start_log.size()); + EXPECT_EQ(0U, observer.finish_log.size()); + EXPECT_EQ(1U, observer.cancel_log.size()); + + EXPECT_TRUE(observer.cancel_log[0] == + CapturingObserver::StartOrCancelEntry(0, info1)); + + // Start an async request for (host2:60) + net::HostResolver::RequestInfo info2("host2", 60); + rv = host_resolver.Resolve(info2, &addrlist, &callback, NULL); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + EXPECT_TRUE(NULL != req); + + EXPECT_EQ(2U, observer.start_log.size()); + EXPECT_EQ(0U, observer.finish_log.size()); + EXPECT_EQ(1U, observer.cancel_log.size()); + + EXPECT_TRUE(observer.start_log[1] == + CapturingObserver::StartOrCancelEntry(1, info2)); + + // Upon exiting this scope, HostResolver is destroyed, so all requests are + // implicitly cancelled. + } + + // Check that destroying the HostResolver sent a notification for + // cancellation of host2:60 request. + + EXPECT_EQ(2U, observer.start_log.size()); + EXPECT_EQ(0U, observer.finish_log.size()); + EXPECT_EQ(2U, observer.cancel_log.size()); + + net::HostResolver::RequestInfo info("host2", 60); + EXPECT_TRUE(observer.cancel_log[1] == + CapturingObserver::StartOrCancelEntry(1, info)); + + // Unblock the host mapper to let the worker threads finish. + mapper->Signal(); } } // namespace |