Re-land final subset of r18520.

The original code review that this belongs to is: <http://codereview.chromium.org/125171>. (BUG=14138)

The original was backed out because of a valgrind linux error (BUG=14218), so I relanded it in smaller chunks to see at which point problem happens.

This differs from the original, in that I have changed the unit-test to no longer use ScopedMapper + WaitingMapper, which I believe was the source of a race.

Review URL: http://codereview.chromium.org/125232

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18587 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 14 insertions, 10 deletions

diff --git a/net/base/host_resolver.cc b/net/base/host_resolver.cc
index d834eb1..ca742da 100644
--- a/net/base/host_resolver.cc
+++ b/net/base/host_resolver.cc
@@ -289,6 +289,7 @@ class HostResolver::Job : public base::RefCountedThreadSafe<HostResolver::Job> {
 
   // Cancels the current job. Callable from origin thread.
   void Cancel() {
+    HostResolver* resolver = resolver_;
     resolver_ = NULL;
 
     // Mark the job as cancelled, so when worker thread completes it will
@@ -297,6 +298,17 @@ class HostResolver::Job : public base::RefCountedThreadSafe<HostResolver::Job> {
       AutoLock locked(origin_loop_lock_);
       origin_loop_ = NULL;
     }
+
+    // We don't have to do anything further to actually cancel the requests
+    // that were attached to this job (since they are unreachable now).
+    // But we will call HostResolver::CancelRequest(Request*) on each one
+    // in order to notify any observers.
+    for (RequestsList::const_iterator it = requests_.begin();
+         it != requests_.end(); ++it) {
+      HostResolver::Request* req = *it;
+      if (!req->was_cancelled())
+        resolver->CancelRequest(req);
+    }
   }
 
   // Called from origin thread.
@@ -394,7 +406,7 @@ HostResolver::HostResolver(int max_cache_entries, int cache_duration_ms)
 
 HostResolver::~HostResolver() {
   // Cancel the outstanding jobs. Those jobs may contain several attached
-  // requests, which will now never be completed.
+  // requests, which will also be cancelled.
   for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ++it)
     it->second->Cancel();
 
diff --git a/net/base/host_resolver_unittest.cc b/net/base/host_resolver_unittest.cc
index 41e7047..8fb0d16 100644
--- a/net/base/host_resolver_unittest.cc
+++ b/net/base/host_resolver_unittest.cc
@@ -823,10 +823,6 @@ TEST_F(HostResolverTest, Observers) {
 //  (1) Delete the HostResolver while job is outstanding.
 //  (2) Call HostResolver::CancelRequest() while a request is outstanding.
 TEST_F(HostResolverTest, 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.
@@ -863,8 +859,7 @@ TEST_F(HostResolverTest, CancellationObserver) {
 
     EXPECT_TRUE(observer.cancel_log[0] ==
                 CapturingObserver::StartOrCancelEntry(0, info1));
-  }
-#if 0
+
     // Start an async request for (host2:60)
     net::HostResolver::RequestInfo info2("host2", 60);
     rv = host_resolver.Resolve(info2, &addrlist, &callback, NULL);
@@ -892,9 +887,6 @@ TEST_F(HostResolverTest, CancellationObserver) {
   net::HostResolver::RequestInfo info("host2", 60);
   EXPECT_TRUE(observer.cancel_log[1] ==
               CapturingObserver::StartOrCancelEntry(1, info));
-#endif
-  // Unblock the host mapper to let the worker threads finish.
-  mapper->Signal();
 }
 
 }  // namespace