Refactor: Extract "InitProxyResolver" to "ProxyScriptDecider".

net/proxy/proxy_script_decider.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // 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>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
 #include "base/time.h"
 #include "base/timer.h"
 #include "googleurl/src/gurl.h"
 #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.
   struct PacSource {
     enum Type {
       WPAD_DHCP,
       WPAD_DNS,
       CUSTOM
     };
 
     PacSource(Type type, const GURL& url)
         : type(type), url(url) {}
 
     Type type;
     GURL url;  // Empty unless |type == PAC_SOURCE_CUSTOM|.
   };
 
   typedef std::vector<PacSource> PacSourceList;
 
   enum State {
     STATE_NONE,
     STATE_WAIT,
     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|.
   PacSourceList BuildPacSourcesFallbackList(const ProxyConfig& config) const;
 
   void OnIOCompletion(int result);
   int DoLoop(int result);
   void DoCallback(int result);
 
   int DoWait();
   int DoWaitComplete(int result);
 
   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]|.
   // Returns OK and rewinds the state machine when there
   // is something to try, otherwise returns |error|.
   int TryToFallbackPacSource(int error);
 
   // Gets the initial state (we skip fetching when the
   // ProxyResolver doesn't |expect_pac_bytes()|.
   State GetStartState() const;
 
   NetLogStringParameter* CreateNetLogParameterAndDetermineURL(
       const PacSource& pac_source, GURL* effective_pac_url);
 
   // Returns the current PAC URL we are fetching/testing.
   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_;
 
   // Filled when the PAC script fetch completes.
   string16 pac_script_;
 
   // Flag indicating whether the caller requested a mandatory pac script
   // (i.e. fallback to direct connections are prohibited).
   bool pac_mandatory_;
 
   PacSourceList pac_sources_;
   State next_state_;
 
   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