Improve performance of proxy resolver by tracing DNS dependencies.

net/proxy/proxy_resolver_v8.cc

Index: net/proxy/proxy_resolver_v8.cc
diff --git a/net/proxy/proxy_resolver_v8.cc b/net/proxy/proxy_resolver_v8.cc
index 51503170ae7b64e8ffa3a3fa57c921644d6c7054..6dabe0de141bcec123c56053bc1a7c0c4c37f8d1 100644
--- a/net/proxy/proxy_resolver_v8.cc
+++ b/net/proxy/proxy_resolver_v8.cc
@@ -16,13 +16,10 @@
 #include "base/utf_string_conversions.h"
 #include "googleurl/src/gurl.h"
 #include "googleurl/src/url_canon.h"
-#include "net/base/host_cache.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/proxy/proxy_info.h"
-#include "net/proxy/proxy_resolver_js_bindings.h"
-#include "net/proxy/proxy_resolver_request_context.h"
 #include "net/proxy/proxy_resolver_script.h"
 #include "v8/include/v8.h"
 
@@ -335,10 +332,8 @@ bool IsInNetEx(const std::string& ip_address, const std::string& ip_prefix) {
 
 class ProxyResolverV8::Context {
  public:
-  explicit Context(ProxyResolverJSBindings* js_bindings)
-      : is_resolving_host_(false),
-        js_bindings_(js_bindings) {
-    DCHECK(js_bindings != NULL);
+  explicit Context(ProxyResolverV8* parent)
+      : parent_(parent) {
   }
 
   ~Context() {
@@ -354,6 +349,10 @@ class ProxyResolverV8::Context {
     PurgeMemory();
   }
 
+  JSBindings* js_bindings() {
+    return parent_->js_bindings_;
+  }
+
   int ResolveProxy(const GURL& query_url, ProxyInfo* results) {
     v8::Locker locked;
     v8::HandleScope scope;
@@ -362,7 +361,7 @@ class ProxyResolverV8::Context {
 
     v8::Local<v8::Value> function;
     if (!GetFindProxyForURL(&function)) {
-      js_bindings_->OnError(
+      js_bindings()->OnError(
           -1, ASCIIToUTF16("FindProxyForURL() is undefined."));
       return ERR_PAC_SCRIPT_FAILED;
     }
@@ -382,7 +381,7 @@ class ProxyResolverV8::Context {
     }
 
     if (!ret->IsString()) {
-      js_bindings_->OnError(
+      js_bindings()->OnError(
           -1, ASCIIToUTF16("FindProxyForURL() did not return a string."));
       return ERR_PAC_SCRIPT_FAILED;
     }
@@ -397,7 +396,7 @@ class ProxyResolverV8::Context {
       string16 error_message =
           ASCIIToUTF16("FindProxyForURL() returned a non-ASCII string "
                        "(crbug.com/47234): ") + ret_str;
-      js_bindings_->OnError(-1, error_message);
+      js_bindings()->OnError(-1, error_message);
       return ERR_PAC_SCRIPT_FAILED;
     }
 
@@ -475,7 +474,7 @@ class ProxyResolverV8::Context {
     // to be a legitimiate PAC script.
     v8::Local<v8::Value> function;
     if (!GetFindProxyForURL(&function)) {
-      js_bindings_->OnError(
+      js_bindings()->OnError(
           -1, ASCIIToUTF16("FindProxyForURL() is undefined."));
       return ERR_PAC_SCRIPT_FAILED;
     }
@@ -483,49 +482,12 @@ class ProxyResolverV8::Context {
     return OK;
   }
 
-  void SetCurrentRequestContext(ProxyResolverRequestContext* context) {
-    js_bindings_->set_current_request_context(context);
-  }
-
   void PurgeMemory() {
     v8::Locker locked;
     v8::V8::LowMemoryNotification();
   }
 
-  bool is_resolving_host() const {
-    base::AutoLock auto_lock(lock_);
-    return is_resolving_host_;
-  }
-
  private:
-  class ScopedHostResolve {
-   public:
-    explicit ScopedHostResolve(Context* context)
-        : context_(context) {
-      context_->BeginHostResolve();
-    }
-
-    ~ScopedHostResolve() {
-      context_->EndHostResolve();
-    }
-
-   private:
-    Context* const context_;
-    DISALLOW_COPY_AND_ASSIGN(ScopedHostResolve);
-  };
-
-  void BeginHostResolve() {
-    base::AutoLock auto_lock(lock_);
-    DCHECK(!is_resolving_host_);
-    is_resolving_host_ = true;
-  }
-
-  void EndHostResolve() {
-    base::AutoLock auto_lock(lock_);
-    DCHECK(is_resolving_host_);
-    is_resolving_host_ = false;
-  }
-
   bool GetFindProxyForURL(v8::Local<v8::Value>* function) {
     *function = v8_context_->Global()->Get(
         ASCIILiteralToV8String("FindProxyForURL"));
@@ -541,7 +503,7 @@ class ProxyResolverV8::Context {
     int line_number = message->GetLineNumber();
     string16 error_message;
     V8ObjectToUTF16String(message->Get(), &error_message);
-    js_bindings_->OnError(line_number, error_message);
+    js_bindings()->OnError(line_number, error_message);
   }
 
   // Compiles and runs |script| in the current V8 context.
@@ -582,7 +544,7 @@ class ProxyResolverV8::Context {
         return v8::Undefined();  // toString() threw an exception.
     }
 
-    context->js_bindings_->Alert(message);
+    context->js_bindings()->Alert(message);
     return v8::Undefined();
   }
 
@@ -596,11 +558,11 @@ class ProxyResolverV8::Context {
 
     {
       v8::Unlocker unlocker;
-      ScopedHostResolve scoped_host_resolve(context);
 
       // We shouldn't be called with any arguments, but will not complain if
       // we are.
-      success = context->js_bindings_->MyIpAddress(&result);
+      success = context->js_bindings()->ResolveDns(
+          "", JSBindings::MY_IP_ADDRESS, &result);
     }
 
     if (!success)
@@ -619,11 +581,10 @@ class ProxyResolverV8::Context {
 
     {
       v8::Unlocker unlocker;
-      ScopedHostResolve scoped_host_resolve(context);
-
       // We shouldn't be called with any arguments, but will not complain if
       // we are.
-      success = context->js_bindings_->MyIpAddressEx(&ip_address_list);
+      success = context->js_bindings()->ResolveDns(
+          "", JSBindings::MY_IP_ADDRESS_EX, &ip_address_list);
     }
 
     if (!success)
@@ -646,8 +607,8 @@ class ProxyResolverV8::Context {
 
     {
       v8::Unlocker unlocker;
-      ScopedHostResolve scoped_host_resolve(context);
-      success = context->js_bindings_->DnsResolve(hostname, &ip_address);
+      success = context->js_bindings()->ResolveDns(
+          hostname, JSBindings::DNS_RESOLVE, &ip_address);
     }
 
     return success ? ASCIIStringToV8String(ip_address) : v8::Null();
@@ -668,8 +629,8 @@ class ProxyResolverV8::Context {
 
     {
       v8::Unlocker unlocker;
-      ScopedHostResolve scoped_host_resolve(context);
-      success = context->js_bindings_->DnsResolveEx(hostname, &ip_address_list);
+      success = context->js_bindings()->ResolveDns(
+          hostname, JSBindings::DNS_RESOLVE_EX, &ip_address_list);
     }
 
     if (!success)
@@ -712,18 +673,16 @@ class ProxyResolverV8::Context {
   }
 
   mutable base::Lock lock_;
-  bool is_resolving_host_;
-  ProxyResolverJSBindings* js_bindings_;
+  ProxyResolverV8* parent_;
   v8::Persistent<v8::External> v8_this_;
   v8::Persistent<v8::Context> v8_context_;
 };
 
 // ProxyResolverV8 ------------------------------------------------------------
 
-ProxyResolverV8::ProxyResolverV8(
-    ProxyResolverJSBindings* custom_js_bindings)
+ProxyResolverV8::ProxyResolverV8()
     : ProxyResolver(true /*expects_pac_bytes*/),
-      js_bindings_(custom_js_bindings) {
+      js_bindings_(NULL) {
 }
 
 ProxyResolverV8::~ProxyResolverV8() {}
@@ -738,21 +697,8 @@ int ProxyResolverV8::GetProxyForURL(
   if (!context_.get())
     return ERR_FAILED;
 

[mmenke, 2013/01/24 21:06:33]

DCHECK(js_bindings_);?

[eroman, 2013/01/25 03:02:01]

Done.

-  // Associate some short-lived context with this request. This context will be
-  // available to any of the javascript "bindings" that are subsequently invoked
-  // from the javascript.
-  //
-  // In particular, we create a HostCache to aggressively cache failed DNS
-  // resolves.
-  const unsigned kMaxCacheEntries = 50;
-  HostCache host_cache(kMaxCacheEntries);
-
-  ProxyResolverRequestContext request_context(&net_log, &host_cache);
-
   // Otherwise call into V8.
-  context_->SetCurrentRequestContext(&request_context);
   int rv = context_->ResolveProxy(query_url, results);
-  context_->SetCurrentRequestContext(NULL);
 
   return rv;
 }
@@ -767,12 +713,6 @@ LoadState ProxyResolverV8::GetLoadState(RequestHandle request) const {
   return LOAD_STATE_IDLE;
 }
 
-LoadState ProxyResolverV8::GetLoadStateThreadSafe(RequestHandle request) const {
-  if (context_->is_resolving_host())
-    return LOAD_STATE_RESOLVING_HOST_IN_PROXY_SCRIPT;
-  return LOAD_STATE_RESOLVING_PROXY_FOR_URL;
-}
-
 void ProxyResolverV8::CancelSetPacScript() {
   NOTREACHED();
 }
@@ -781,10 +721,6 @@ void ProxyResolverV8::PurgeMemory() {
   context_->PurgeMemory();
 }
 
-void ProxyResolverV8::Shutdown() {
-  js_bindings_->Shutdown();
-}
-
 int ProxyResolverV8::SetPacScript(
     const scoped_refptr<ProxyResolverScriptData>& script_data,
     const CompletionCallback& /*callback*/) {
@@ -794,7 +730,7 @@ int ProxyResolverV8::SetPacScript(
     return ERR_PAC_SCRIPT_FAILED;
 

[mmenke, 2013/01/24 21:06:33]

DCHECK(js_bindings);?

[eroman, 2013/01/25 03:02:01]

Done.

   // Try parsing the PAC script.
-  scoped_ptr<Context> context(new Context(js_bindings_.get()));
+  scoped_ptr<Context> context(new Context(this));
   int rv = context->InitV8(script_data);
   if (rv == OK)
     context_.reset(context.release());