Refactor: Extract "InitProxyResolver" to "ProxyScriptDecider".

net/proxy/proxy_script_decider.h

Index: net/proxy/proxy_script_decider.h
===================================================================
--- net/proxy/proxy_script_decider.h	(revision 113525)
+++ net/proxy/proxy_script_decider.h	(working copy)
@@ -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,57 +16,72 @@
 #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 {
+    DCHECK_EQ(STATE_NONE, next_state_);
+    return effective_config_;
+  }
+
+  // TODO(eroman): Return a const-pointer.

[willchan no longer on Chromium, 2011/12/12 23:53:47]

Is there a reason you didn't do this now? Is this nontrivial?

[eroman, 2011/12/13 00:10:26]

Unfortunately it is non-trivial since one of the interfaces which consumes this
data, ProxyResolver::SetPacScript() is expecting a non-const pointer (for no
good reason). Before I can fix this, I need to massage the interface and its
consumers.

+  ProxyResolverScriptData* script_data() const {
+    DCHECK_EQ(STATE_NONE, next_state_);
+    return script_data_.get();
+  }
+
  private:
   // Represents the sources from which we can get PAC files; two types of
   // auto-detect or a custom URL.
@@ -92,8 +107,7 @@
     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,  // (Synchronous step).
   };
 
   // Returns ordered list of PAC urls to try for |config|.
@@ -109,8 +123,7 @@
   int DoFetchPacScript();
   int DoFetchPacScriptComplete(int result);
 
-  int DoSetPacScript();
-  int DoSetPacScriptComplete(int result);
+  int DoVerifyPacScript();
 
   // Tries restarting using the next fallback PAC URL:
   // |pac_sources_[++current_pac_source_index]|.
@@ -129,14 +142,14 @@
   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 +166,19 @@
 
   BoundNetLog net_log_;
 
+  bool fetch_pac_bytes_;
+
   base::TimeDelta wait_delay_;
-  base::OneShotTimer<InitProxyResolver> wait_timer_;
+  base::OneShotTimer<ProxyScriptDecider> wait_timer_;
 
-  ProxyConfig* effective_config_;
+  // Results.
+  ProxyConfig effective_config_;
+  scoped_refptr<ProxyResolverScriptData> script_data_;
 
-  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