Fast-fail WPAD detection.

net/proxy/proxy_script_decider.cc

Index: net/proxy/proxy_script_decider.cc
diff --git a/net/proxy/proxy_script_decider.cc b/net/proxy/proxy_script_decider.cc
index 38bf751cd4da3e3c89ccdacbe079854911e1c058..8798646a80e66e3b3137a829a348c6db34bcebbd 100644
--- a/net/proxy/proxy_script_decider.cc
+++ b/net/proxy/proxy_script_decider.cc
@@ -9,6 +9,7 @@
 #include "base/compiler_specific.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ -47,6 +48,8 @@ bool LooksLikePacScript(const base::string16& script) {
 // http://code.google.com/p/chromium/issues/detail?id=18575#c20
 static const char kWpadUrl[] = "http://wpad/wpad.dat";
 
+static const int kQuickCheckDelayMs = 1000;
+
 base::Value* ProxyScriptDecider::PacSource::NetLogCallback(
     const GURL* effective_pac_url,
     NetLog::LogLevel /* log_level */) const {
@@ -81,7 +84,10 @@ ProxyScriptDecider::ProxyScriptDecider(
       next_state_(STATE_NONE),
       net_log_(BoundNetLog::Make(
           net_log, NetLog::SOURCE_PROXY_SCRIPT_DECIDER)),
-      fetch_pac_bytes_(false) {
+      fetch_pac_bytes_(false),
+      underlying_host_resolver_(
+          HostResolver::CreateDefaultResolver(net_log)),
+      host_resolver_(underlying_host_resolver_.get()) {
 }
 
 ProxyScriptDecider::~ProxyScriptDecider() {
@@ -110,7 +116,7 @@ int ProxyScriptDecider::Start(
   pac_sources_ = BuildPacSourcesFallbackList(config);
   DCHECK(!pac_sources_.empty());
 
-  next_state_ = STATE_WAIT;
+  next_state_ = STATE_QUICK_CHECK;
 
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
@@ -165,6 +171,9 @@ int ProxyScriptDecider::DoLoop(int result) {
     State state = next_state_;
     next_state_ = STATE_NONE;
     switch (state) {
+      case STATE_QUICK_CHECK:
+        rv = DoQuickCheck();
+        break;

[szym, 2013/08/22 19:06:41]

To be compatible with the DoLoop state machine you should:
case STATE_QUICK_CHECK_COMPLETE:
  rv = DoQuickCheckComplete(rv)

       case STATE_WAIT:
         DCHECK_EQ(OK, rv);
         rv = DoWait();
@@ -186,6 +195,8 @@ int ProxyScriptDecider::DoLoop(int result) {
       case STATE_VERIFY_PAC_SCRIPT_COMPLETE:
         rv = DoVerifyPacScriptComplete(rv);
         break;
+      case STATE_FAILED:
+        break;
       default:
         NOTREACHED() << "bad state";
         rv = ERR_UNEXPECTED;
@@ -201,6 +212,57 @@ void ProxyScriptDecider::DoCallback(int result) {
   callback_.Run(result);
 }
 
+int ProxyScriptDecider::DoQuickCheck() {
+
+  HostResolver::RequestInfo reqinfo(HostPortPair("wpad", 80));
+

[szym, 2013/08/22 16:38:24]

If you do
CompletionCallback callback = base::Bind(&ProxyScript, ...Now());

then...

+  int rv = host_resolver_.Resolve(reqinfo, HIGHEST,  // we block startup...
+                                   &wpad_addrs_,
+                                   base::Bind(
+                                      &ProxyScriptDecider::OnQuickCheckComplete,

[szym, 2013/08/22 19:06:41]

To be compatible with the DoLoop state machine you should just set next_state_ =
STATE_QUICK_CHECK_COMPLETE and pass OnIOComplete here.

This means that your original approach of storing the quick_check_start_time_
was more DoLoop-compatible.

+                                      base::Unretained(this),
+                                      base::Time::Now()),
+                                   net_log_);
+
+  // we can't get an error response - the name is known to be valid, and we
+  // don't cache negative dns responses.
+  CHECK(rv == OK || rv == ERR_IO_PENDING);
+
+  if (rv == OK) {
+    // already in cache or something and valid, we're golden
+    next_state_ = STATE_WAIT;
+    return OK;
+  } else if (rv != ERR_IO_PENDING) {
+    // already in cache and invalid, bail now
+    next_state_ = STATE_FAILED;
+    return rv;
+  }
+
+  quick_check_timer_.Start(FROM_HERE,
+                           base::TimeDelta::FromMilliseconds(
+                              kQuickCheckDelayMs),
+                           base::Bind(
+                              &ProxyScriptDecider::OnQuickCheckComplete,
+                              base::Unretained(this),
+                              base::Time::Now(),

[szym, 2013/08/22 16:38:24]

... you can just pass: base::Bind(callback, ERR_NAME_NOT_RESOLVED)

+                              ERR_NAME_NOT_RESOLVED));
+
+  next_state_ = STATE_QUICK_CHECK;
+  return ERR_IO_PENDING;
+}
+
+void ProxyScriptDecider::OnQuickCheckComplete(base::Time started, int result) {
+  base::TimeDelta delta = base::Time::Now() - started;
+  UMA_HISTOGRAM_TIMES("Net.WpadQuickCheck", delta);
+  host_resolver_.Cancel();
+  quick_check_timer_.Stop();
+  if (result == OK)
+    next_state_ = STATE_WAIT;
+  else
+    next_state_ = STATE_FAILED;
+  OnIOCompletion(result);
+}
+
 int ProxyScriptDecider::DoWait() {
   next_state_ = STATE_WAIT_COMPLETE;
 
@@ -399,6 +461,8 @@ void ProxyScriptDecider::Cancel() {
     case STATE_FETCH_PAC_SCRIPT_COMPLETE:
       proxy_script_fetcher_->Cancel();
       break;
+    case STATE_FAILED:
+      break;
     default:
       NOTREACHED();
       break;