Revert 18520.

The original code review for this change was:
<http://codereview.chromium.org/125171>

It is being reverted because it seems to have caused a valgrind on linux regression:
<http://crbug.com14218>

BUG=14218,14138,14188

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

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

3 files changed, 27 insertions, 173 deletions

diff --git a/net/base/host_resolver.cc b/net/base/host_resolver.cc
index ca742da..5a357eb 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 MarkAsCancelled() {
+  void Cancel() {
     job_ = NULL;
     callback_ = NULL;
     addresses_ = NULL;
@@ -289,26 +289,10 @@ 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
-    // not try to post completion to origin loop.
-    {
-      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);
-    }
+    AutoLock locked(origin_loop_lock_);
+    origin_loop_ = NULL;
   }
 
   // Called from origin thread.
@@ -406,7 +390,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 also be cancelled.
+  // requests, which will now never be completed.
   for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ++it)
     it->second->Cancel();
 
@@ -433,12 +417,7 @@ int HostResolver::Resolve(const RequestInfo& info,
         info.hostname(), base::TimeTicks::Now());
     if (cache_entry) {
       addresses->SetFrom(cache_entry->addrlist, info.port());
-      int error = OK;
-
-      // Notify registered observers.
-      NotifyObserversFinishRequest(request_id, info, error);
-
-      return error;
+      return OK;
     }
   }
 
@@ -499,8 +478,7 @@ void HostResolver::CancelRequest(Request* req) {
   DCHECK(req);
   DCHECK(req->job());
   // NULL out the fields of req, to mark it as cancelled.
-  req->MarkAsCancelled();
-  NotifyObserversCancelRequest(req->id(), req->info());
+  req->Cancel();
 }
 
 void HostResolver::AddObserver(Observer* observer) {
@@ -591,14 +569,6 @@ void HostResolver::NotifyObserversFinishRequest(int request_id,
   }
 }
 
-void HostResolver::NotifyObserversCancelRequest(int request_id,
-                                                const RequestInfo& info) {
-  for (ObserversList::iterator it = observers_.begin();
-       it != observers_.end(); ++it) {
-    (*it)->OnCancelResolution(request_id, info);
-  }
-}
-
 //-----------------------------------------------------------------------------
 
 SingleRequestHostResolver::SingleRequestHostResolver(HostResolver* resolver)
diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h
index 0b5c3fc..34c803f 100644
--- a/net/base/host_resolver.h
+++ b/net/base/host_resolver.h
@@ -106,18 +106,13 @@ 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() or OnCancelResolution().
+    // to OnFinishResolutionWithStatus().
     virtual void OnStartResolution(int id, const RequestInfo& info) = 0;
 
     // Called on completion of request |id|. Note that if the request was
-    // cancelled, OnCancelResolution() will be called instead.
+    // cancelled, OnFinishResolutionWithStatus() will not be called.
     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
@@ -180,19 +175,15 @@ class HostResolver {
   // Callback for when |job| has completed with |error| and |addrlist|.
   void OnJobComplete(Job* job, int error, const AddressList& addrlist);
 
-  // Notify all observers of the start of a resolve request.
+  // Notify all obsevers of the start of a resolve request.
   void NotifyObserversStartRequest(int request_id,
                                    const RequestInfo& info);
 
-  // Notify all observers of the completion of a resolve request.
+  // Notify all obsevers of the completion of a resolve request.
   void NotifyObserversFinishRequest(int request_id,
                                     const RequestInfo& info,
                                     int error);
 
-  // Notify all observers of the cancellation of a resolve request.
-  void NotifyObserversCancelRequest(int request_id,
-                                    const RequestInfo& info);
-
   // Cache of host resolution results.
   HostCache cache_;
 
diff --git a/net/base/host_resolver_unittest.cc b/net/base/host_resolver_unittest.cc
index ce2c495..9c07c29 100644
--- a/net/base/host_resolver_unittest.cc
+++ b/net/base/host_resolver_unittest.cc
@@ -20,7 +20,6 @@
 #include "net/base/completion_callback.h"
 #include "net/base/host_resolver_unittest.h"
 #include "net/base/net_errors.h"
-#include "net/base/test_completion_callback.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using net::RuleBasedHostMapper;
@@ -702,7 +701,7 @@ class CapturingObserver : public net::HostResolver::Observer {
   // DnsResolutionObserver methods:
   virtual void OnStartResolution(int id,
                                  const net::HostResolver::RequestInfo& info) {
-    start_log.push_back(StartOrCancelEntry(id, info));
+    start_log.push_back(StartEntry(id, info));
   }
 
   virtual void OnFinishResolutionWithStatus(
@@ -712,17 +711,12 @@ 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 StartOrCancelEntry {
-    StartOrCancelEntry(int id, const net::HostResolver::RequestInfo& info)
+  struct StartEntry {
+    StartEntry(int id, const net::HostResolver::RequestInfo& info)
         : id(id), info(info) {}
 
-    bool operator==(const StartOrCancelEntry& other) const {
+    bool operator==(const StartEntry& other) const {
       return id == other.id && info == other.info;
     }
 
@@ -747,14 +741,11 @@ class CapturingObserver : public net::HostResolver::Observer {
     net::HostResolver::RequestInfo info;
   };
 
-  std::vector<StartOrCancelEntry> start_log;
+  std::vector<StartEntry> 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;
 
@@ -766,44 +757,25 @@ TEST_F(HostResolverTest, Observers) {
 
   // Resolve "host1".
   net::HostResolver::RequestInfo info1("host1", 70);
-  int rv = host_resolver.Resolve(info1, &addrlist, NULL, NULL);
-  EXPECT_EQ(net::OK, rv);
+  host_resolver.Resolve(info1, &addrlist, NULL, NULL);
 
   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::StartOrCancelEntry(0, info1));
-  EXPECT_TRUE(observer.finish_log[0] ==
-              CapturingObserver::FinishEntry(0, true, info1));
-
-  // Resolve "host1" again -- this time it  will be served from cache, but it
-  // should still notify of completion.
-  TestCompletionCallback callback;
-  rv = host_resolver.Resolve(info1, &addrlist, &callback, NULL);
-  ASSERT_EQ(net::OK, rv);  // Should complete synchronously.
-
-  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::StartOrCancelEntry(1, info1));
-  EXPECT_TRUE(observer.finish_log[1] ==
-              CapturingObserver::FinishEntry(1, true, info1));
+  EXPECT_TRUE(
+      observer.start_log[0] == CapturingObserver::StartEntry(0, info1));
+  EXPECT_TRUE(
+      observer.finish_log[0] == CapturingObserver::FinishEntry(0, true, info1));
 
   // Resolve "host2", setting referrer to "http://foobar.com"
   net::HostResolver::RequestInfo info2("host2", 70);
   info2.set_referrer(GURL("http://foobar.com"));
-  rv = host_resolver.Resolve(info2, &addrlist, NULL, NULL);
-  EXPECT_EQ(net::OK, rv);
+  host_resolver.Resolve(info2, &addrlist, NULL, NULL);
 
-  EXPECT_EQ(3U, observer.start_log.size());
-  EXPECT_EQ(3U, observer.finish_log.size());
-  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));
+  EXPECT_EQ(2U, observer.start_log.size());
+  EXPECT_EQ(2U, observer.finish_log.size());
+  EXPECT_TRUE(observer.start_log[1] == CapturingObserver::StartEntry(1, info2));
+  EXPECT_TRUE(observer.finish_log[1] == CapturingObserver::FinishEntry(
+      1, true, info2));
 
   // Unregister the observer.
   host_resolver.RemoveObserver(&observer);
@@ -813,87 +785,8 @@ TEST_F(HostResolverTest, Observers) {
   host_resolver.Resolve(info3, &addrlist, NULL, NULL);
 
   // 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, 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();
+  EXPECT_EQ(2U, observer.finish_log.size());
 }
 
 }  // namespace