Avoid doing concurrent DNS resolves of the same hostname

net/proxy/proxy_resolver_v8.cc

Index: net/proxy/proxy_resolver_v8.cc
===================================================================
--- net/proxy/proxy_resolver_v8.cc	(revision 18213)
+++ net/proxy/proxy_resolver_v8.cc	(working copy)
@@ -4,7 +4,10 @@
 
 #include "net/proxy/proxy_resolver_v8.h"
 
+#include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/waitable_event.h"
 #include "base/string_util.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/address_list.h"
@@ -50,9 +53,78 @@
   return true;
 }
 
+// Wrapper around HostResolver to give a sync API while running the resolve
+// in async mode on |host_resolver_loop|. If |host_resolver_loop| is NULL,
+// runs sync on the current thread (this mode is just used by testing).
+class SyncHostResolverBridge
+    : public base::RefCountedThreadSafe<SyncHostResolverBridge> {
+ public:
+  SyncHostResolverBridge(HostResolver* host_resolver,
+                         MessageLoop* host_resolver_loop)
+      : host_resolver_(host_resolver),
+        host_resolver_loop_(host_resolver_loop),
+        event_(false, false),
+        ALLOW_THIS_IN_INITIALIZER_LIST(
+            callback_(this, &SyncHostResolverBridge::OnResolveCompletion)) {
+  }
+
+  // Run the resolve on host_resolver_loop, and wait for result.
+  int Resolve(const std::string& hostname, net::AddressList* addresses) {
+    int kPort = 80;   // Doesn't matter.
+
+    // Hack for tests -- run synchronously on current thread.
+    if (!host_resolver_loop_)
+      return host_resolver_->Resolve(hostname, kPort, addresses, NULL, NULL);
+
+    // Otherwise start an async resolve on the resolver's thread.
+    host_resolver_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+        &SyncHostResolverBridge::StartResolve, hostname, kPort, addresses));
+
+    // Wait for the resolve to complete in the resolver's thread.
+    event_.Wait();
+    return err_;
+  }
+
+ private:
+  // Called on host_resolver_loop_.
+  void StartResolve(const std::string& hostname,
+                    int port,
+                    net::AddressList* addresses) {
+    DCHECK_EQ(host_resolver_loop_, MessageLoop::current());
+    int error = host_resolver_->Resolve(
+        hostname, port, addresses, &callback_, NULL);
+    if (error != ERR_IO_PENDING)
+      OnResolveCompletion(error);  // Completed synchronously.
+  }
+
+  // Called on host_resolver_loop_.
+  void OnResolveCompletion(int result) {
+    DCHECK_EQ(host_resolver_loop_, MessageLoop::current());
+    err_ = result;
+    event_.Signal();
+  }
+
+  HostResolver* host_resolver_;
+  MessageLoop* host_resolver_loop_;
+
+  // Event to notify completion of resolve request.
+  base::WaitableEvent event_;
+
+  // Callback for when the resolve completes on host_resolver_loop_.
+  net::CompletionCallbackImpl<SyncHostResolverBridge> callback_;
+
+  // The result from the result request (set by in host_resolver_loop_).
+  int err_;
+};
+
 // JSBIndings implementation.
 class DefaultJSBindings : public ProxyResolverV8::JSBindings {
  public:
+  DefaultJSBindings(HostResolver* host_resolver,
+                    MessageLoop* host_resolver_loop)
+      : host_resolver_(new SyncHostResolverBridge(
+          host_resolver, host_resolver_loop)) {}
+
   // Handler for "alert(message)".
   virtual void Alert(const std::string& message) {
     LOG(INFO) << "PAC-alert: " << message;
@@ -71,10 +143,9 @@
     if (host.empty())
       return std::string();
 
-    // Try to resolve synchronously.
+    // Do a sync resolve of the hostname.
     net::AddressList address_list;
-    const int kPort = 80;  // Doesn't matter what this is.
-    int result = host_resolver_.Resolve(host, kPort, &address_list, NULL);
+    int result = host_resolver_->Resolve(host, &address_list);
 
     if (result != OK)
       return std::string();  // Failed.
@@ -96,7 +167,7 @@
   }
 
  private:
-  HostResolver host_resolver_;
+  scoped_refptr<SyncHostResolverBridge> host_resolver_;
 };
 
 }  // namespace
@@ -106,7 +177,7 @@
 class ProxyResolverV8::Context {
  public:
   Context(JSBindings* js_bindings, const std::string& pac_data)
-       : js_bindings_(js_bindings) {
+      : js_bindings_(js_bindings) {
     DCHECK(js_bindings != NULL);
     InitV8(pac_data);
   }
@@ -266,20 +337,12 @@
   }
 
   JSBindings* js_bindings_;
-  HostResolver host_resolver_;
   v8::Persistent<v8::External> v8_this_;
   v8::Persistent<v8::Context> v8_context_;
 };
 
 // ProxyResolverV8 ------------------------------------------------------------
 
-// the |false| argument to ProxyResolver means the ProxyService will handle
-// downloading of the PAC script, and notify changes through SetPacScript().
-ProxyResolverV8::ProxyResolverV8()
-    : ProxyResolver(false /*does_fetch*/),
-      js_bindings_(new DefaultJSBindings()) {
-}
-
 ProxyResolverV8::ProxyResolverV8(
     ProxyResolverV8::JSBindings* custom_js_bindings)
     : ProxyResolver(false), js_bindings_(custom_js_bindings) {
@@ -305,4 +368,10 @@
     context_.reset(new Context(js_bindings_.get(), data));
 }
 
+// static
+ProxyResolverV8::JSBindings* ProxyResolverV8::CreateDefaultBindings(
+    HostResolver* host_resolver, MessageLoop* host_resolver_loop) {
+  return new DefaultJSBindings(host_resolver, host_resolver_loop);
+}
+
 }  // namespace net