Revert "DnsRRResolver: hoist inner class and callback on MessageLoop."

(Everything goes to hell when the try servers don't work.)

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

3 files changed, 49 insertions, 111 deletions

diff --git a/net/base/dnsrr_resolver.cc b/net/base/dnsrr_resolver.cc
index 79bb2da..8c041d9 100644
--- a/net/base/dnsrr_resolver.cc
+++ b/net/base/dnsrr_resolver.cc
@@ -8,7 +8,6 @@
 #include <resolv.h>
 #endif
 
-#include "base/message_loop.h"
 #include "base/string_piece.h"
 #include "base/task.h"
 #include "base/worker_pool.h"
@@ -20,63 +19,6 @@ namespace net {
 
 static const uint16 kClassIN = 1;
 
-namespace {
-
-class CompletionCallbackTask : public Task,
-                               public MessageLoop::DestructionObserver {
- public:
-  explicit CompletionCallbackTask(CompletionCallback* callback,
-                                  MessageLoop* ml)
-      : callback_(callback),
-        rv_(OK),
-        posted_(false),
-        message_loop_(ml) {
-    ml->AddDestructionObserver(this);
-  }
-
-  void set_rv(int rv) {
-    rv_ = rv;
-  }
-
-  void Post() {
-    AutoLock locked(lock_);
-    DCHECK(!posted_);
-
-    if (!message_loop_) {
-      // MessageLoop got deleted, nothing to do.
-      delete this;
-      return;
-    }
-    posted_ = true;
-    message_loop_->PostTask(FROM_HERE, this);
-  }
-
-  // Task interface
-  void Run() {
-    message_loop_->RemoveDestructionObserver(this);
-    callback_->Run(rv_);
-    // We will be deleted by the message loop.
-  }
-
-  // DestructionObserver interface
-  virtual void WillDestroyCurrentMessageLoop() {
-    AutoLock locked(lock_);
-    message_loop_ = NULL;
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(CompletionCallbackTask);
-
-  CompletionCallback* callback_;
-  int rv_;
-  bool posted_;
-
-  Lock lock_;  // covers |message_loop_|
-  MessageLoop* message_loop_;
-};
-
-}  // anonymous namespace
-
 #if defined(OS_POSIX)
 
 // A Buffer is used for walking over a DNS packet.
@@ -205,16 +147,14 @@ class Buffer {
   }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(Buffer);
-
   const uint8* p_;
   const uint8* const packet_;
   unsigned len_;
   const unsigned packet_len_;
 };
 
-bool RRResponse::ParseFromResponse(const uint8* p, unsigned len,
-                                   uint16 rrtype_requested) {
+bool DnsRRResolver::Response::ParseFromResponse(const uint8* p, unsigned len,
+                                                uint16 rrtype_requested) {
   name.clear();
   ttl = 0;
   dnssec = false;
@@ -300,40 +240,35 @@ class ResolveTask : public Task {
  public:
   ResolveTask(const std::string& name, uint16 rrtype,
               uint16 flags, CompletionCallback* callback,
-              RRResponse* response, MessageLoop* ml)
+              DnsRRResolver::Response* response)
       : name_(name),
         rrtype_(rrtype),
         flags_(flags),
-        subtask_(new CompletionCallbackTask(callback, ml)),
+        callback_(callback),
         response_(response) {
   }
 
   virtual void Run() {
     // Runs on a worker thread.
 
-    bool r = true;
     if ((_res.options & RES_INIT) == 0) {
       if (res_ninit(&_res) != 0)
-        r = false;
+        return Failure();
     }
 
-    if (r) {
-      unsigned long saved_options = _res.options;
-      r = Do();
+    unsigned long saved_options = _res.options;
+    bool r = Do();
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)
-      if (!r && DnsReloadTimerHasExpired()) {
-        res_nclose(&_res);
-        if (res_ninit(&_res) == 0)
-          r = Do();
-      }
-#endif
-      _res.options = saved_options;
+    if (!r && DnsReloadTimerHasExpired()) {
+      res_nclose(&_res);
+      if (res_ninit(&_res) == 0)
+        r = Do();
     }
+#endif
+    _res.options = saved_options;
     int error = r ? OK : ERR_NAME_NOT_RESOLVED;
-
-    subtask_->set_rv(error);
-    subtask_.release()->Post();
+    callback_->Run(error);
   }
 
   bool Do() {
@@ -367,11 +302,15 @@ class ResolveTask : public Task {
  private:
   DISALLOW_COPY_AND_ASSIGN(ResolveTask);
 
+  void Failure() {
+    callback_->Run(ERR_NAME_NOT_RESOLVED);
+  }
+
   const std::string name_;
   const uint16 rrtype_;
   const uint16 flags_;
-  scoped_ptr<CompletionCallbackTask> subtask_;
-  RRResponse* const response_;
+  CompletionCallback* const callback_;
+  DnsRRResolver::Response* const response_;
 };
 #else  // OS_POSIX
 // On non-Linux platforms we fail everything for now.
@@ -379,7 +318,7 @@ class ResolveTask : public Task {
  public:
   ResolveTask(const std::string& name, uint16 rrtype,
               uint16 flags, CompletionCallback* callback,
-              RRResponse* response)
+              DnsRRResolver::Response* response)
       : callback_(callback) {
   }
 
@@ -396,7 +335,7 @@ class ResolveTask : public Task {
 // static
 bool DnsRRResolver::Resolve(const std::string& name, uint16 rrtype,
                             uint16 flags, CompletionCallback* callback,
-                            RRResponse* response) {
+                            Response* response) {
   if (!callback || !response || name.empty())
     return false;
 
@@ -404,8 +343,7 @@ bool DnsRRResolver::Resolve(const std::string& name, uint16 rrtype,
   if (rrtype == kDNS_ANY)
     return false;
 
-  ResolveTask* task = new ResolveTask(name, rrtype, flags, callback, response,
-                                      MessageLoop::current());
+  ResolveTask* task = new ResolveTask(name, rrtype, flags, callback, response);
 
   return WorkerPool::PostTask(FROM_HERE, task, true /* task is slow */);
 }
diff --git a/net/base/dnsrr_resolver.h b/net/base/dnsrr_resolver.h
index 28bceaa..6dfea2f 100644
--- a/net/base/dnsrr_resolver.h
+++ b/net/base/dnsrr_resolver.h
@@ -15,24 +15,6 @@
 
 namespace net {
 
-// RRResponse contains the result of a successful request for a resource record.
-struct RRResponse {
-  // name contains the canonical name of the resulting domain. If the queried
-  // name was a CNAME then this can differ.
-  std::string name;
-  // ttl contains the TTL of the resource records.
-  uint32 ttl;
-  // dnssec is true if the response was DNSSEC validated.
-  bool dnssec;
-  std::vector<std::string> rrdatas;
-  // sigs contains the RRSIG records returned.
-  std::vector<std::string> signatures;
-
-  // For testing only
-  bool ParseFromResponse(const uint8* data, unsigned len,
-                         uint16 rrtype_requested);
-};
-
 // DnsRRResolver resolves arbitary DNS resource record types. It should not be
 // confused with HostResolver and should not be used to resolve A/AAAA records.
 //
@@ -43,19 +25,37 @@ struct RRResponse {
 // the name is a fully qualified DNS domain.
 class DnsRRResolver {
  public:
+  // Response contains the details of a successful request.
+  struct Response {
+    // name contains the canonical name of the resulting domain. If the queried
+    // name was a CNAME then this can differ.
+    std::string name;
+    // ttl contains the TTL of the resource records.
+    uint32 ttl;
+    // dnssec is true if the response was DNSSEC validated.
+    bool dnssec;
+    std::vector<std::string> rrdatas;
+    // sigs contains the RRSIG records returned.
+    std::vector<std::string> signatures;
+
+    // For testing only
+    bool ParseFromResponse(const uint8* data, unsigned len,
+                           uint16 rrtype_requested);
+  };
+
   enum {
     // Try harder to get a DNSSEC signed response. This doesn't mean that the
-    // RRResponse will always have the dnssec bit set.
+    // Response will always have the dnssec bit set.
     FLAG_WANT_DNSSEC = 1,
   };
 
   // Resolve starts the resolution process. When complete, |callback| is called
   // with a result. If the result is |OK| then |response| is filled with the
-  // result of the resolution. Note the |callback| is called on the current
-  // MessageLoop.
+  // result of the resolution. Note the |callback| is called from a random
+  // worker thread.
   static bool Resolve(const std::string& name, uint16 rrtype,
                       uint16 flags, CompletionCallback* callback,
-                      RRResponse* response);
+                      Response* response);
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DnsRRResolver);
diff --git a/net/base/dnsrr_resolver_unittest.cc b/net/base/dnsrr_resolver_unittest.cc
index 23a0b40..6e36126 100644
--- a/net/base/dnsrr_resolver_unittest.cc
+++ b/net/base/dnsrr_resolver_unittest.cc
@@ -52,7 +52,7 @@ class Rendezvous : public CallbackRunner<Tuple1<int> > {
 // This test is disabled because it depends on the external network to pass.
 // However, it may be useful when chaging the code.
 TEST_F(DnsRRResolverTest, DISABLED_NetworkResolve) {
-  RRResponse response;
+  DnsRRResolver::Response response;
   Rendezvous callback;
   ASSERT_TRUE(DnsRRResolver::Resolve(
         "agl._pka.imperialviolet.org", kDNS_TXT, 0, &callback, &response));
@@ -111,7 +111,7 @@ static const uint8 kExamplePacket[] = {
 };
 
 TEST_F(DnsRRResolverTest, ParseExample) {
-  RRResponse response;
+  DnsRRResolver::Response response;
   ASSERT_TRUE(response.ParseFromResponse(kExamplePacket,
               sizeof(kExamplePacket), kDNS_TXT));
   ASSERT_EQ(1u, response.rrdatas.size());
@@ -124,7 +124,7 @@ TEST_F(DnsRRResolverTest, ParseExample) {
 }
 
 TEST_F(DnsRRResolverTest, FuzzTruncation) {
-  RRResponse response;
+  DnsRRResolver::Response response;
 
   for (unsigned len = sizeof(kExamplePacket); len <= sizeof(kExamplePacket);
        len--) {
@@ -133,7 +133,7 @@ TEST_F(DnsRRResolverTest, FuzzTruncation) {
 }
 
 TEST_F(DnsRRResolverTest, FuzzCorruption) {
-  RRResponse response;
+  DnsRRResolver::Response response;
   uint8 copy[sizeof(kExamplePacket)];