Refactor: Extract "InitProxyResolver" to "ProxyScriptDecider".

This is primarily a rename, with the exception that the initialization of ProxyResolver (i.e. the javascript runtime environment for the PAC script) is no longer done inside of ProxyScriptDecider.

The motivation for doing this is to make it possible to poll our automatic proxy settings (PAC URLs or WPAD) in the background, _without_ needing to feed the scripts into the javascript parser for validation.

There likely won't be any user-visible consequence of this change, however the new mechanism is weaker than the original -- it is possible for the PAC selection to now choose a PAC script which fails to parse as javascript, even though there was a later fallback choice which does parse. This should be rare though (the bad response would need to contain the substring "FindProxyForURL").

BUG=TODO (bugtracker down right now)
Review URL: http://codereview.chromium.org/8896019

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

chrome/browser/net/passive_log_collector.cc

@@ -66,8 +66,8 @@ PassiveLogCollector::PassiveLogCollector()
   trackers_[net::NetLog::SOURCE_SOCKET_STREAM] = &socket_stream_tracker_;
   trackers_[net::NetLog::SOURCE_CONNECT_JOB] = &connect_job_tracker_;
   trackers_[net::NetLog::SOURCE_SOCKET] = &socket_tracker_;
-  trackers_[net::NetLog::SOURCE_INIT_PROXY_RESOLVER] =
-      &init_proxy_resolver_tracker_;
+  trackers_[net::NetLog::SOURCE_PROXY_SCRIPT_DECIDER] =
+      &proxy_script_decider_tracker_;
   trackers_[net::NetLog::SOURCE_SPDY_SESSION] = &spdy_session_tracker_;
   trackers_[net::NetLog::SOURCE_HOST_RESOLVER_IMPL_REQUEST] =
       &dns_request_tracker_;
@@ -488,22 +488,23 @@ PassiveLogCollector::RequestTracker::DoAddEntry(
 }
 
 //----------------------------------------------------------------------------
-// InitProxyResolverTracker
+// ProxyScriptDeciderTracker
 //----------------------------------------------------------------------------
 
-const size_t PassiveLogCollector::InitProxyResolverTracker::kMaxNumSources = 20;
-const size_t PassiveLogCollector::InitProxyResolverTracker::kMaxGraveyardSize =
-    3;
+const size_t PassiveLogCollector::ProxyScriptDeciderTracker::kMaxNumSources
+    = 20;
+const size_t PassiveLogCollector::ProxyScriptDeciderTracker::kMaxGraveyardSize
+    = 3;
 
-PassiveLogCollector::InitProxyResolverTracker::InitProxyResolverTracker()
+PassiveLogCollector::ProxyScriptDeciderTracker::ProxyScriptDeciderTracker()
     : SourceTracker(kMaxNumSources, kMaxGraveyardSize, NULL) {
 }
 
 PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::InitProxyResolverTracker::DoAddEntry(
+PassiveLogCollector::ProxyScriptDeciderTracker::DoAddEntry(
     const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
   AddEntryToSourceInfo(entry, out_info);
-  if (entry.type == net::NetLog::TYPE_INIT_PROXY_RESOLVER &&
+  if (entry.type == net::NetLog::TYPE_PROXY_SCRIPT_DECIDER &&
       entry.phase == net::NetLog::PHASE_END) {
     return ACTION_MOVE_TO_GRAVEYARD;
   }

chrome/browser/net/passive_log_collector.h

@@ -237,19 +237,19 @@ class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserverImpl {
   };
 
   // Specialization of SourceTracker for handling
-  // SOURCE_INIT_PROXY_RESOLVER.
-  class InitProxyResolverTracker : public SourceTracker {
+  // SOURCE_PROXY_SCRIPT_DECIDER.
+  class ProxyScriptDeciderTracker : public SourceTracker {
    public:
     static const size_t kMaxNumSources;
     static const size_t kMaxGraveyardSize;
 
-    InitProxyResolverTracker();
+    ProxyScriptDeciderTracker();
 
    private:
     virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
                               SourceInfo* out_info) OVERRIDE;
 
-    DISALLOW_COPY_AND_ASSIGN(InitProxyResolverTracker);
+    DISALLOW_COPY_AND_ASSIGN(ProxyScriptDeciderTracker);
   };
 
   // Tracks the log entries for the last seen SOURCE_SPDY_SESSION.
@@ -447,7 +447,7 @@ class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserverImpl {
   SocketTracker socket_tracker_;
   RequestTracker url_request_tracker_;
   RequestTracker socket_stream_tracker_;
-  InitProxyResolverTracker init_proxy_resolver_tracker_;
+  ProxyScriptDeciderTracker proxy_script_decider_tracker_;
   SpdySessionTracker spdy_session_tracker_;
   DNSRequestTracker dns_request_tracker_;
   DNSJobTracker dns_job_tracker_;

chrome/browser/resources/net_internals/events_view.css

@@ -92,7 +92,7 @@ found in the LICENSE file.
   color: #803030;
 }
 
-#events-view-source-list-tbody .source_INIT_PROXY_RESOLVER {
+#events-view-source-list-tbody .source_PROXY_SCRIPT_DECIDER {
   color: green;
 }
 

chrome/browser/resources/net_internals/proxy_view.html

@@ -20,7 +20,6 @@
   <td valign=top>
   <h3>Effective settings</h3>
   <pre id=proxy-view-effective-settings></pre>
-  </pre>
   </td>
 
   <td style='width: 30px'>&nbsp;</td>
@@ -35,7 +34,7 @@
   <h4>Proxy auto-config initialization</h4>
   <ul>
     <li>
-      <a href='#events&q=type:INIT_PROXY_RESOLVER'>View all events</a>
+      <a href='#events&q=type:PROXY_SCRIPT_DECIDER'>View all events</a>
     </li>
     <li>
       Latest proxy resolver event:

chrome/browser/resources/net_internals/proxy_view.js

@@ -7,7 +7,7 @@
  *
  *   - Shows the current proxy settings.
  *   - Has a button to reload these settings.
- *   - Shows the log entries for the most recent INIT_PROXY_RESOLVER source
+ *   - Shows the log entries for the most recent PROXY_SCRIPT_DECIDER source
  *   - Shows the list of proxy hostnames that are cached as "bad".
  *   - Has a button to clear the cached bad proxies.
  */
@@ -65,9 +65,9 @@ var ProxyView = (function() {
     },
 
     onLoadLogFinish: function(data, tabData) {
-      // It's possible that the last INIT_PROXY_RESOLVER source was deleted from
-      // the log, but earlier sources remain.  When that happens, clear the list
-      // of entries here, to avoid displaying misleading information.
+      // It's possible that the last PROXY_SCRIPT_DECIDER source was deleted
+      // from the log, but earlier sources remain.  When that happens, clear the
+      // list of entries here, to avoid displaying misleading information.
       if (tabData != this.latestProxySourceId_)
         this.clearLog_();
       return this.onProxySettingsChanged(data.proxySettings) &&
@@ -127,14 +127,14 @@ var ProxyView = (function() {
 
     /**
      * Called whenever SourceEntries are updated with new log entries.  Updates
-     * |proxyResolverLogPre_| with the log entries of the INIT_PROXY_RESOLVER
+     * |proxyResolverLogPre_| with the log entries of the PROXY_SCRIPT_DECIDER
      * SourceEntry with the greatest id.
      */
     onSourceEntriesUpdated: function(sourceEntries) {
       for (var i = sourceEntries.length - 1; i >= 0; --i) {
         var sourceEntry = sourceEntries[i];
 
-        if (sourceEntry.getSourceType() != LogSourceType.INIT_PROXY_RESOLVER ||
+        if (sourceEntry.getSourceType() != LogSourceType.PROXY_SCRIPT_DECIDER ||
             this.latestProxySourceId_ > sourceEntry.getSourceId()) {
           continue;
         }

chrome/test/data/extensions/api_test/proxy/events/parse_error.js

@@ -22,7 +22,7 @@ function test() {
   var config = {
     mode: "pac_script",
     pacScript: {
-      data: "trash!",
+      data: "trash!-FindProxyForURL",
       mandatory: false
     }
   };

net/base/net_log_event_type_list.h

@@ -145,12 +145,12 @@ EVENT_TYPE(HOST_RESOLVER_IMPL_JOB)
 // ------------------------------------------------------------------------
 
 // The start/end of auto-detect + custom PAC URL configuration.
-EVENT_TYPE(INIT_PROXY_RESOLVER)
+EVENT_TYPE(PROXY_SCRIPT_DECIDER)
 
 // The start/end of when proxy autoconfig was artificially paused following
 // a network change event. (We wait some amount of time after being told of
 // network changes to avoid hitting spurious errors during auto-detect).
-EVENT_TYPE(INIT_PROXY_RESOLVER_WAIT)
+EVENT_TYPE(PROXY_SCRIPT_DECIDER_WAIT)
 
 // The start/end of download of a PAC script. This could be the well-known
 // WPAD URL (if testing auto-detect), or a custom PAC URL.
@@ -164,24 +164,15 @@ EVENT_TYPE(INIT_PROXY_RESOLVER_WAIT)
 //   {
 //      "net_error": <Net error code integer>,
 //   }
-EVENT_TYPE(INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)
-
-// The start/end of the testing of a PAC script (trying to parse the fetched
-// file as javascript).
-//
-// If the parsing of the script failed, the END phase will have parameters:
-//   {
-//      "net_error": <Net error code integer>,
-//   }
-EVENT_TYPE(INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)
+EVENT_TYPE(PROXY_SCRIPT_DECIDER_FETCH_PAC_SCRIPT)
 
 // This event means that initialization failed because there was no
 // configured script fetcher. (This indicates a configuration error).
-EVENT_TYPE(INIT_PROXY_RESOLVER_HAS_NO_FETCHER)
+EVENT_TYPE(PROXY_SCRIPT_DECIDER_HAS_NO_FETCHER)
 
 // This event is emitted after deciding to fall-back to the next source
 // of PAC scripts in the list.
-EVENT_TYPE(INIT_PROXY_RESOLVER_FALLING_BACK_TO_NEXT_PAC_SOURCE)
+EVENT_TYPE(PROXY_SCRIPT_DECIDER_FALLING_BACK_TO_NEXT_PAC_SOURCE)
 
 // ------------------------------------------------------------------------
 // ProxyService

net/base/net_log_source_type_list.h

@@ -10,7 +10,7 @@ SOURCE_TYPE(NONE, 0)
 
 SOURCE_TYPE(URL_REQUEST, 1)
 SOURCE_TYPE(SOCKET_STREAM, 2)
-SOURCE_TYPE(INIT_PROXY_RESOLVER, 3)
+SOURCE_TYPE(PROXY_SCRIPT_DECIDER, 3)
 SOURCE_TYPE(CONNECT_JOB, 4)
 SOURCE_TYPE(SOCKET, 5)
 SOURCE_TYPE(SPDY_SESSION, 6)

net/net.gyp

@@ -503,8 +503,6 @@
         'proxy/dhcp_proxy_script_fetcher_win.h',
         'proxy/dhcpcsvc_init_win.cc',
         'proxy/dhcpcsvc_init_win.h',
-        'proxy/init_proxy_resolver.cc',
-        'proxy/init_proxy_resolver.h',
         'proxy/multi_threaded_proxy_resolver.cc',
         'proxy/multi_threaded_proxy_resolver.h',
         'proxy/network_delegate_error_observer.cc',
@@ -543,6 +541,8 @@
         'proxy/proxy_resolver_winhttp.cc',
         'proxy/proxy_resolver_winhttp.h',
         'proxy/proxy_retry_info.h',
+        'proxy/proxy_script_decider.cc',
+        'proxy/proxy_script_decider.h',
         'proxy/proxy_script_fetcher.h',
         'proxy/proxy_script_fetcher_impl.cc',
         'proxy/proxy_script_fetcher_impl.h',
@@ -1126,7 +1126,6 @@
         'proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc',
         'proxy/dhcp_proxy_script_fetcher_factory_unittest.cc',
         'proxy/dhcp_proxy_script_fetcher_win_unittest.cc',
-        'proxy/init_proxy_resolver_unittest.cc',
         'proxy/multi_threaded_proxy_resolver_unittest.cc',
         'proxy/network_delegate_error_observer_unittest.cc',
         'proxy/proxy_bypass_rules_unittest.cc',
@@ -1136,6 +1135,7 @@
         'proxy/proxy_list_unittest.cc',
         'proxy/proxy_resolver_js_bindings_unittest.cc',
         'proxy/proxy_resolver_v8_unittest.cc',
+        'proxy/proxy_script_decider_unittest.cc',
         'proxy/proxy_script_fetcher_impl_unittest.cc',
         'proxy/proxy_server_unittest.cc',
         'proxy/proxy_service_unittest.cc',

net/proxy/init_proxy_resolver.h → net/proxy/proxy_script_decider.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_PROXY_INIT_PROXY_RESOLVER_H_
-#define NET_PROXY_INIT_PROXY_RESOLVER_H_
+#ifndef NET_PROXY_SCRIPT_DECIDER_H
+#define NET_PROXY_SCRIPT_DECIDER_H
 #pragma once
 
 #include <vector>
@@ -16,56 +16,65 @@
 #include "net/base/completion_callback.h"
 #include "net/base/net_export.h"
 #include "net/base/net_log.h"
+#include "net/proxy/proxy_config.h"
+#include "net/proxy/proxy_resolver.h"
 
 namespace net {
 
 class DhcpProxyScriptFetcher;
 class NetLogParameter;
-class ProxyConfig;
 class ProxyResolver;
 class ProxyScriptFetcher;
 
-// InitProxyResolver is a helper class used by ProxyService to
-// initialize a ProxyResolver with the PAC script data specified
-// by a particular ProxyConfig.
+// ProxyScriptDecider is a helper class used by ProxyService to determine which
+// PAC script to use given our proxy configuration.
 //
 // This involves trying to use PAC scripts in this order:
 //
-//   (1) WPAD (DNS) if auto-detect is on.
-//   (2) Custom PAC script if a URL was given.
+//   (1) WPAD (DHCP) if auto-detect is on.
+//   (2) WPAD (DNS) if auto-detect is on.
+//   (3) Custom PAC script if a URL was given.
 //
-// If no PAC script was successfully downloaded + parsed, then it fails with
-// a network error. Otherwise the proxy resolver is left initialized with
-// the PAC script.
+// If no PAC script was successfully selected, then it fails with either a
+// network error, or PAC_SCRIPT_FAILED (indicating it did not pass our
+// validation).
 //
-// Deleting InitProxyResolver while Init() is in progress, will
+// On successful completion, the fetched PAC script data can be accessed using
+// script_data().
+//
+// Deleting ProxyScriptDecider while Init() is in progress, will
 // cancel the request.
 //
-class NET_EXPORT_PRIVATE InitProxyResolver {
+class NET_EXPORT_PRIVATE ProxyScriptDecider {
  public:
-  // |resolver|, |proxy_script_fetcher|, |dhcp_proxy_script_fetcher| and
-  // |net_log| must remain valid for the lifespan of InitProxyResolver.
-  InitProxyResolver(ProxyResolver* resolver,
-                    ProxyScriptFetcher* proxy_script_fetcher,
-                    DhcpProxyScriptFetcher* dhcp_proxy_script_fetcher,
-                    NetLog* net_log);
+  // |proxy_script_fetcher|, |dhcp_proxy_script_fetcher| and
+  // |net_log| must remain valid for the lifespan of ProxyScriptDecider.
+  ProxyScriptDecider(ProxyScriptFetcher* proxy_script_fetcher,
+                     DhcpProxyScriptFetcher* dhcp_proxy_script_fetcher,
+                     NetLog* net_log);
 
   // Aborts any in-progress request.
-  ~InitProxyResolver();
+  ~ProxyScriptDecider();
 
-  // Applies the PAC settings of |config| to |resolver_|.
+  // Evaluates the effective proxy settings for |config|, and downloads the
+  // associated PAC script.
   // If |wait_delay| is positive, the initialization will pause for this
   // amount of time before getting started.
-  // If |effective_config| is non-NULL, then on successful initialization of
-  // |resolver_| the "effective" proxy settings we ended up using will be
-  // written out to |*effective_config|. Note that this may differ from
-  // |config| since we will have stripped any manual settings, and decided
-  // whether to use auto-detect or the custom PAC URL. Finally, if auto-detect
-  // was used we may now have resolved that to a specific script URL.
-  int Init(const ProxyConfig& config,
-           const base::TimeDelta wait_delay,
-           ProxyConfig* effective_config,
-           OldCompletionCallback* callback);
+  // On successful completion, the "effective" proxy settings we ended up
+  // deciding on will be available vial the effective_settings() accessor.
+  // Note that this may differ from |config| since we will have stripped any
+  // manual settings, and decided whether to use auto-detect or the custom PAC
+  // URL. Finally, if auto-detect was used we may now have resolved that to a
+  // specific script URL.
+  int Start(const ProxyConfig& config,
+            const base::TimeDelta wait_delay,
+            bool fetch_pac_bytes,
+            OldCompletionCallback* callback);
+
+  const ProxyConfig& effective_config() const;
+
+  // TODO(eroman): Return a const-pointer.
+  ProxyResolverScriptData* script_data() const;
 
  private:
   // Represents the sources from which we can get PAC files; two types of
@@ -92,8 +101,8 @@ class NET_EXPORT_PRIVATE InitProxyResolver {
     STATE_WAIT_COMPLETE,
     STATE_FETCH_PAC_SCRIPT,
     STATE_FETCH_PAC_SCRIPT_COMPLETE,
-    STATE_SET_PAC_SCRIPT,
-    STATE_SET_PAC_SCRIPT_COMPLETE,
+    STATE_VERIFY_PAC_SCRIPT,
+    STATE_VERIFY_PAC_SCRIPT_COMPLETE,
   };
 
   // Returns ordered list of PAC urls to try for |config|.
@@ -109,8 +118,8 @@ class NET_EXPORT_PRIVATE InitProxyResolver {
   int DoFetchPacScript();
   int DoFetchPacScriptComplete(int result);
 
-  int DoSetPacScript();
-  int DoSetPacScriptComplete(int result);
+  int DoVerifyPacScript();
+  int DoVerifyPacScriptComplete(int result);
 
   // Tries restarting using the next fallback PAC URL:
   // |pac_sources_[++current_pac_source_index]|.
@@ -129,14 +138,14 @@ class NET_EXPORT_PRIVATE InitProxyResolver {
   const PacSource& current_pac_source() const;
 
   void OnWaitTimerFired();
-  void DidCompleteInit();
+  void DidComplete();
   void Cancel();
 
   ProxyResolver* resolver_;
   ProxyScriptFetcher* proxy_script_fetcher_;
   DhcpProxyScriptFetcher* dhcp_proxy_script_fetcher_;
 
-  OldCompletionCallbackImpl<InitProxyResolver> io_callback_;
+  OldCompletionCallbackImpl<ProxyScriptDecider> io_callback_;
   OldCompletionCallback* user_callback_;
 
   size_t current_pac_source_index_;
@@ -153,14 +162,19 @@ class NET_EXPORT_PRIVATE InitProxyResolver {
 
   BoundNetLog net_log_;
 
+  bool fetch_pac_bytes_;
+
   base::TimeDelta wait_delay_;
-  base::OneShotTimer<InitProxyResolver> wait_timer_;
+  base::OneShotTimer<ProxyScriptDecider> wait_timer_;
+
+  // Results.
+  ProxyConfig effective_config_;
+  scoped_refptr<ProxyResolverScriptData> script_data_;
 
-  ProxyConfig* effective_config_;
 
-  DISALLOW_COPY_AND_ASSIGN(InitProxyResolver);
+  DISALLOW_COPY_AND_ASSIGN(ProxyScriptDecider);
 };
 
 }  // namespace net
 
-#endif  // NET_PROXY_INIT_PROXY_RESOLVER_H_
+#endif  // NET_PROXY_SCRIPT_DECIDER_H

net/proxy/proxy_service.cc

@@ -18,13 +18,13 @@
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/proxy/dhcp_proxy_script_fetcher.h"
-#include "net/proxy/init_proxy_resolver.h"
 #include "net/proxy/multi_threaded_proxy_resolver.h"
 #include "net/proxy/network_delegate_error_observer.h"
 #include "net/proxy/proxy_config_service_fixed.h"
 #include "net/proxy/proxy_resolver.h"
 #include "net/proxy/proxy_resolver_js_bindings.h"
 #include "net/proxy/proxy_resolver_v8.h"
+#include "net/proxy/proxy_script_decider.h"
 #include "net/proxy/proxy_script_fetcher.h"
 #include "net/proxy/sync_host_resolver_bridge.h"
 #include "net/url_request/url_request_context.h"
@@ -314,6 +314,142 @@ class BadProxyListNetLogParam : public NetLog::EventParameters {
 
 }  // namespace
 
+// ProxyService::InitProxyResolver --------------------------------------------
+
+// This glues together two asynchronous steps:
+//   (1) ProxyScriptDecider -- try to fetch/validate a sequence of PAC scripts to
+//       figure out what we should configure against.
+//   (2) Feed the fetched PAC script into the ProxyResolver.
+//
+// TODO(eroman): This is something of a temporary shim while refactoring to keep
+// things similar. It will probably end up changing while solving bug 90581.
+
+class ProxyService::InitProxyResolver {
+ public:
+  InitProxyResolver(ProxyResolver* proxy_resolver,
+                    ProxyScriptFetcher* proxy_script_fetcher,
+                    DhcpProxyScriptFetcher* dhcp_proxy_script_fetcher,
+                    NetLog* net_log)
+      : decider_(proxy_script_fetcher, dhcp_proxy_script_fetcher, net_log),
+        effective_config_(NULL),
+        proxy_resolver_(proxy_resolver),
+        user_callback_(NULL),
+        ALLOW_THIS_IN_INITIALIZER_LIST(io_callback_(
+            this, &InitProxyResolver::OnIOCompletion)) {
+  }
+
+  ~InitProxyResolver() {
+    // Note that the destruction of ProxyScriptDecider will automatically cancel
+    // any outstanding work.
+    if (next_state_ == STATE_SET_PAC_SCRIPT_COMPLETE) {
+      proxy_resolver_->CancelSetPacScript();
+    }
+  }
+
+  int Init(const ProxyConfig& config,
+           base::TimeDelta wait_delay,
+           ProxyConfig* effective_config,
+           OldCompletionCallback* callback) {
+    config_ = config;
+    wait_delay_ = wait_delay;
+    effective_config_ = effective_config;
+    user_callback_ = callback;
+
+    next_state_ = STATE_DECIDE_PROXY_SCRIPT;
+    return DoLoop(OK);
+  }
+
+ private:
+  enum State {
+    STATE_NONE,
+    STATE_DECIDE_PROXY_SCRIPT,
+    STATE_DECIDE_PROXY_SCRIPT_COMPLETE,
+    STATE_SET_PAC_SCRIPT,
+    STATE_SET_PAC_SCRIPT_COMPLETE,
+  };
+
+  int DoLoop(int result) {
+    DCHECK_NE(next_state_, STATE_NONE);
+    int rv = result;
+    do {
+      State state = next_state_;
+      next_state_ = STATE_NONE;
+      switch (state) {
+        case STATE_DECIDE_PROXY_SCRIPT:
+          DCHECK_EQ(OK, rv);
+          rv = DoDecideProxyScript();
+          break;
+        case STATE_DECIDE_PROXY_SCRIPT_COMPLETE:
+          rv = DoDecideProxyScriptComplete(rv);
+          break;
+        case STATE_SET_PAC_SCRIPT:
+          DCHECK_EQ(OK, rv);
+          rv = DoSetPacScript();
+          break;
+        case STATE_SET_PAC_SCRIPT_COMPLETE:
+          rv = DoSetPacScriptComplete(rv);
+          break;
+        default:
+          NOTREACHED() << "bad state: " << state;
+          rv = ERR_UNEXPECTED;
+          break;
+      }
+    } while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);
+    return rv;
+  }
+
+  int DoDecideProxyScript() {
+    next_state_ = STATE_DECIDE_PROXY_SCRIPT_COMPLETE;
+
+    return decider_.Start(
+        config_, wait_delay_, proxy_resolver_->expects_pac_bytes(),
+        &io_callback_);
+  }
+
+  int DoDecideProxyScriptComplete(int result) {
+    if (result != OK)
+      return result;
+
+    *effective_config_ = decider_.effective_config();
+    next_state_ = STATE_SET_PAC_SCRIPT;
+    return OK;
+  }
+
+  int DoSetPacScript() {
+    DCHECK(decider_.script_data());
+    // TODO(eroman): Should log this latency to the NetLog.
+    next_state_ = STATE_SET_PAC_SCRIPT_COMPLETE;
+    return proxy_resolver_->SetPacScript(decider_.script_data(), &io_callback_);
+  }
+
+  int DoSetPacScriptComplete(int result) {
+    return result;
+  }
+
+  void OnIOCompletion(int result) {
+    DCHECK_NE(STATE_NONE, next_state_);
+    int rv = DoLoop(result);
+    if (rv != ERR_IO_PENDING)
+      DoCallback(rv);
+  }
+
+  void DoCallback(int result) {
+    DCHECK_NE(ERR_IO_PENDING, result);
+    user_callback_->Run(result);
+  }
+
+  ProxyConfig config_;
+  base::TimeDelta wait_delay_;
+  ProxyScriptDecider decider_;
+  ProxyConfig* effective_config_;
+  ProxyResolver* proxy_resolver_;
+  OldCompletionCallback* user_callback_;
+  OldCompletionCallbackImpl<InitProxyResolver> io_callback_;
+  State next_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(InitProxyResolver);
+};
+
 // ProxyService::PacRequest ---------------------------------------------------
 
 class ProxyService::PacRequest

net/proxy/proxy_service.h

@@ -30,9 +30,9 @@ namespace net {
 
 class DhcpProxyScriptFetcher;
 class HostResolver;
-class InitProxyResolver;
 class NetworkDelegate;
 class ProxyResolver;
+class ProxyScriptDecider;
 class ProxyScriptFetcher;
 
 // This class can be used to resolve the proxy server to use when loading a
@@ -67,7 +67,7 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   // the caller will not need to cancel the request.
   //
   // We use the three possible proxy access types in the following order,
-  // doing fallback if one doesn't work.  See "init_proxy_resolver.h"
+  // doing fallback if one doesn't work.  See "pac_script_decider.h"
   // for the specifics.
   //   1.  WPAD auto-detection
   //   2.  PAC URL
@@ -245,6 +245,7 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigAfterFailedAutodetect);
   FRIEND_TEST_ALL_PREFIXES(ProxyServiceTest, UpdateConfigFromPACToDirect);
   friend class PacRequest;
+  class InitProxyResolver;
 
   // TODO(eroman): change this to a std::set. Note that this requires updating
   // some tests in proxy_service_unittest.cc such as:

net/proxy/proxy_service_unittest.cc

@@ -28,6 +28,9 @@
 namespace net {
 namespace {
 
+const char kValidPacScript1[] = "pac-script-v1-FindProxyForURL";
+const char kValidPacScript2[] = "pac-script-v2-FindProxyForURL";
+
 class MockProxyConfigService: public ProxyConfigService {
  public:
   explicit MockProxyConfigService(const ProxyConfig& config)
@@ -423,17 +426,12 @@ TEST(ProxyServiceTest, ProxyResolverFailsParsingJavaScriptMandatoryPac) {
   EXPECT_EQ(GURL("http://foopy/proxy.pac"), fetcher->pending_request_url());
   fetcher->NotifyFetchCompletion(OK, "invalid-script-contents");
 
-  // Simulate a parse error.
-  EXPECT_EQ(ASCIIToUTF16("invalid-script-contents"),
-            resolver->pending_set_pac_script_request()->script_data()->utf16());
-  resolver->pending_set_pac_script_request()->CompleteNow(
-      ERR_PAC_SCRIPT_FAILED);
-
   EXPECT_FALSE(fetcher->has_pending_request());
   ASSERT_EQ(0u, resolver->pending_requests().size());
 
-  // As the proxy resolver failed the request and is configured for a mandatory
-  // PAC script, ProxyService must not implicitly fall-back to DIRECT.
+  // Since ProxyScriptDecider failed to identify a valid PAC and PAC was
+  // mandatory for this configuration, the ProxyService must not implicitly
+  // fall-back to DIRECT.
   EXPECT_EQ(ERR_MANDATORY_PROXY_CONFIGURATION_FAILED,
             callback.WaitForResult());
   EXPECT_FALSE(info.is_direct());
@@ -1195,11 +1193,11 @@ TEST(ProxyServiceTest, InitialPACScriptDownload) {
   // At this point the ProxyService should be waiting for the
   // ProxyScriptFetcher to invoke its completion callback, notifying it of
   // PAC script download completion.
-  fetcher->NotifyFetchCompletion(OK, "pac-v1");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
   // Now that the PAC script is downloaded, it will have been sent to the proxy
   // resolver.
-  EXPECT_EQ(ASCIIToUTF16("pac-v1"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1277,11 +1275,11 @@ TEST(ProxyServiceTest, ChangeScriptFetcherWhilePACDownloadInProgress) {
   // Nothing has been sent to the resolver yet.
   EXPECT_TRUE(resolver->pending_requests().empty());
 
-  fetcher->NotifyFetchCompletion(OK, "pac-v1");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
   // Now that the PAC script is downloaded, it will have been sent to the proxy
   // resolver.
-  EXPECT_EQ(ASCIIToUTF16("pac-v1"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1340,11 +1338,11 @@ TEST(ProxyServiceTest, CancelWhilePACFetching) {
   // At this point the ProxyService should be waiting for the
   // ProxyScriptFetcher to invoke its completion callback, notifying it of
   // PAC script download completion.
-  fetcher->NotifyFetchCompletion(OK, "pac-v1");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
   // Now that the PAC script is downloaded, it will have been sent to the
   // proxy resolver.
-  EXPECT_EQ(ASCIIToUTF16("pac-v1"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1423,9 +1421,9 @@ TEST(ProxyServiceTest, FallbackFromAutodetectToCustomPac) {
   // Next it should be trying the custom PAC url.
   EXPECT_TRUE(fetcher->has_pending_request());
   EXPECT_EQ(GURL("http://foopy/proxy.pac"), fetcher->pending_request_url());
-  fetcher->NotifyFetchCompletion(OK, "custom-pac-script");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
-  EXPECT_EQ(ASCIIToUTF16("custom-pac-script"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1490,18 +1488,15 @@ TEST(ProxyServiceTest, FallbackFromAutodetectToCustomPac2) {
   EXPECT_EQ(GURL("http://wpad/wpad.dat"), fetcher->pending_request_url());
   fetcher->NotifyFetchCompletion(OK, "invalid-script-contents");
 
-  // Simulate a parse error.
-  EXPECT_EQ(ASCIIToUTF16("invalid-script-contents"),
-            resolver->pending_set_pac_script_request()->script_data()->utf16());
-  resolver->pending_set_pac_script_request()->CompleteNow(
-      ERR_PAC_SCRIPT_FAILED);
+  // The script contents passed failed basic verification step (since didn't
+  // contain token FindProxyForURL), so it was never passed to the resolver.
 
   // Next it should be trying the custom PAC url.
   EXPECT_TRUE(fetcher->has_pending_request());
   EXPECT_EQ(GURL("http://foopy/proxy.pac"), fetcher->pending_request_url());
-  fetcher->NotifyFetchCompletion(OK, "custom-pac-script");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
-  EXPECT_EQ(ASCIIToUTF16("custom-pac-script"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1615,9 +1610,9 @@ TEST(ProxyServiceTest, BypassDoesntApplyToPac) {
   // It should be trying to auto-detect first -- succeed the download.
   EXPECT_TRUE(fetcher->has_pending_request());
   EXPECT_EQ(GURL("http://wpad/wpad.dat"), fetcher->pending_request_url());
-  fetcher->NotifyFetchCompletion(OK, "auto-detect");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
-  EXPECT_EQ(ASCIIToUTF16("auto-detect"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1823,11 +1818,11 @@ TEST(ProxyServiceTest, NetworkChangeTriggersPacRefetch) {
   // At this point the ProxyService should be waiting for the
   // ProxyScriptFetcher to invoke its completion callback, notifying it of
   // PAC script download completion.
-  fetcher->NotifyFetchCompletion(OK, "pac-v1");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript1);
 
   // Now that the PAC script is downloaded, the request will have been sent to
   // the proxy resolver.
-  EXPECT_EQ(ASCIIToUTF16("pac-v1"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript1),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1865,11 +1860,11 @@ TEST(ProxyServiceTest, NetworkChangeTriggersPacRefetch) {
 
   // Simulate the PAC script fetch as having completed (this time with
   // different data).
-  fetcher->NotifyFetchCompletion(OK, "pac-v2");
+  fetcher->NotifyFetchCompletion(OK, kValidPacScript2);
 
   // Now that the PAC script is downloaded, the second request will have been
   // sent to the proxy resolver.
-  EXPECT_EQ(ASCIIToUTF16("pac-v2"),
+  EXPECT_EQ(ASCIIToUTF16(kValidPacScript2),
             resolver->pending_set_pac_script_request()->script_data()->utf16());
   resolver->pending_set_pac_script_request()->CompleteNow(OK);
 
@@ -1893,7 +1888,7 @@ TEST(ProxyServiceTest, NetworkChangeTriggersPacRefetch) {
 
   EXPECT_TRUE(LogContainsEntryWithType(entries, 0,
                                        NetLog::TYPE_PROXY_CONFIG_CHANGED));
-  ASSERT_EQ(13u, entries.size());
+  ASSERT_EQ(9u, entries.size());
   for (size_t i = 1; i < entries.size(); ++i)
     EXPECT_NE(NetLog::TYPE_PROXY_CONFIG_CHANGED, entries[i].type);
 }