Adds support for the DHCP portion of the WPAD (proxy auto-discovery) protocol.

net/proxy/dhcp_proxy_script_adapter_fetcher_win.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_DHCP_SCRIPT_ADAPTER_FETCHER_WIN_H_
+#define NET_PROXY_DHCP_SCRIPT_ADAPTER_FETCHER_WIN_H_
+#pragma once
+
+#include "base/memory/ref_counted.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/timer.h"
+#include "net/proxy/proxy_script_fetcher_impl.h"

[eroman, 2011/04/21 05:22:48]

Is this header necessary? Could we just include proxy_script_fetcher.h (the
non-impl version), or forward-declare ProxyScriptFetcher

[Jói, 2011/05/03 21:20:59]

Moved to .cc file.

+
+namespace base {
+class MessageLoopProxy;
+}
+
+namespace net {
+
+// For a given adapter, this class takes care of first doing a DHCP lookup
+// to get the PAC URL, then if there is one, trying to fetch it.
+//
+// Non thread-safe for clients, but does some internal multi-threading.
+class DhcpProxyScriptAdapterFetcher
+    : public base::RefCountedThreadSafe<DhcpProxyScriptAdapterFetcher>,
+      public base::NonThreadSafe {

[eroman, 2011/04/21 05:22:48]

Feels a bit weird to call this nonthreadsafe, since it calls virtuals on the
worker thread too.

[Jói, 2011/04/21 15:57:38]

The public interface is non-threadsafe, so I think it makes sense.  There is a
comment right above explaining this.

+ public:
+  // |url_request_context| must outlive DhcpProxyScriptAdapterFetcher.
+  explicit DhcpProxyScriptAdapterFetcher(
+      URLRequestContext* url_request_context);
+  ~DhcpProxyScriptAdapterFetcher();

[eroman, 2011/04/21 05:22:48]

I always recommend making destructors virtual any time you have virtual methods.
(otherwise it is easy to accidentally end up leaking stuff in your derived class
when deleting via a base pointer).

That said, see my meta-comment about avoiding the virtuals in the first place.

[Jói, 2011/05/03 21:20:59]

Now virtual, thanks for catching that.

+
+  // Starts a fetch.  DhcpProxyScriptAdapterFetcher takes ownership of
+  // |adapter_name|. On completion (but not cancellation), |callback|

[eroman, 2011/04/21 05:22:48]

what does it mean to take ownership of |adapter_name|? (It is a |const string&|)

[Jói, 2011/04/21 15:57:38]

Sorry, will fix the comment, that is a holdover from when I was putting string*
instead of string into the set returned when enumerating adapters, to avoid a
string copy when passing the string to the worker thread (which happens within
Fetch().  I fixed that when I realized I needed a string conversion anyway.

+  // will be invoked with the network error indicating success or failure
+  // of fetching a DHCP-configured PAC file on this adapter.
+  //
+  // You may only call Fetch() once on a given instance of
+  // DhcpProxyScriptAdapterFetcher.
+  virtual void Fetch(const std::string& adapter_name,
+                     CompletionCallback* callback);
+
+  // Cancels the fetch on this adapter.
+  virtual void Cancel();
+
+  // Returns true if in the FINISH state (not CANCEL).
+  virtual bool DidFinish() const;
+
+  // Returns the network error indicating the result of the fetch. Will
+  // return IO_PENDING until the fetch is complete or cancelled.
+  virtual int result() const;
+
+  // Returns the contents of the PAC file retrieved.  Only valid if
+  // |IsComplete()| is true.  Returns the empty string if |GetResult()|
+  // returns anything other than OK.
+  virtual string16 pac_script() const;

[eroman, 2011/04/21 05:22:48]

Don't use hacker_notation() for virtuals. (I know some parts of chrome disobey
this... but they are wrong IMO)

[Jói, 2011/05/03 21:20:59]

Done.

+
+  // Returns the PAC URL retrieved from DHCP.  Only guaranteed to be
+  // valid if |IsComplete()| is true.  Returns an empty URL if no URL was
+  // configured in DHCP.  May return a valid URL even if |result()| does
+  // not return OK (this would indicate that we found a URL configured in
+  // DHCP but failed to download it).
+  virtual GURL pac_url() const;

[eroman, 2011/04/21 05:22:48]

ditto.

[Jói, 2011/05/03 21:20:59]

Done.

+
+  // Returns the PAC URL configured in DHCP for the given |adapter_name|, or
+  // the empty string if none is configured.
+  //
+  // This function executes synchronously due to limitations of the Windows
+  // DHCP client API.
+  static std::string GetPacURLFromDhcp(const std::string& adapter_name);
+
+ protected:
+  // This queries the synchronous Win32 API we need to use on the worker
+  // thread.
+  void QueryDhcpOnWorkerThread(const std::string& adapter_name);
+
+  // Event/state transition handlers
+  void OnQueryDhcpDone(std::string url);
+  void OnTimeout();
+  void OnFetcherDone(int result);
+  void TransitionToFinish();
+
+  // Virtual methods introduced to allow unit testing.
+  virtual std::string ImplGetPacURLFromDhcp(const std::string& adapter_name);
+  virtual ProxyScriptFetcher* ImplCreateScriptFetcher(const GURL& url);
+  virtual int ImplGetTimeoutMs() const;
+
+  // This is the state machine for fetching from a given adapter.
+  //
+  // The state machine goes from START->WAIT_DHCP when it starts
+  // a worker thread to fetch the PAC URL from DHCP.
+  //
+  // In state WAIT_DHCP, if the DHCP query finishes and has no URL, it
+  // moves to state FINISH.  If there is a URL, it starts a
+  // ProxyScriptFetcher to fetch it and moves to state WAIT_URL.
+  //
+  // It goes from WAIT_URL->FINISH when the ProxyScriptFetcher completes.
+  //
+  // In state FINISH, completion is indicated to the outer class, with
+  // the results of the fetch if a PAC script was successfully fetched.
+  //
+  // In any state except FINISH, a call to Cancel() will move to state
+  // CANCEL and cause all outstanding work to be cancelled or its
+  // results ignored when available.
+  enum State {
+    STATE_START,
+    STATE_WAIT_DHCP,
+    STATE_WAIT_URL,
+    STATE_FINISH,
+    STATE_CANCEL,

[eroman, 2011/04/21 05:22:48]

nit: how about STATE_CANCELLED

[Jói, 2011/05/03 21:20:59]

I considered that but then the others should change to STARTED, WAITING,
FINISHED and that feels wrong to me.

+  };
+
+  // Current state of this state machine.
+  State state_;
+
+  // A network error indicating result of operation.
+  int result_;
+
+  // Empty string or the PAC script downloaded.
+  string16 pac_script_;
+
+  // Empty URL or the PAC URL configured in DHCP.
+  GURL pac_url_;
+
+  // Callback to let our client know we're done. Invalid in states
+  // START, FINISH and CANCEL.
+  CompletionCallback* callback_;
+
+  // Fetcher to retrieve PAC files once URL is known.
+  scoped_ptr<ProxyScriptFetcher> script_fetcher_;
+
+  // Callback from the script fetcher.
+  CompletionCallbackImpl<DhcpProxyScriptAdapterFetcher>
+      script_fetcher_callback_;
+
+  // Implements a timeout on the call to the Win32 DHCP API.
+  base::OneShotTimer<DhcpProxyScriptAdapterFetcher> wait_timer_;
+
+  // Used by worker thread to post a message back to the original
+  // thread.  Fine to use a proxy since in the case where the original
+  // thread has gone away, that would mean this object is in state
+  // CANCEL and no further processing should be done.
+  scoped_refptr<base::MessageLoopProxy> origin_loop_;
+
+  URLRequestContext* url_request_context_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(DhcpProxyScriptAdapterFetcher);
+};
+
+}  // namespace net
+
+#endif  // NET_PROXY_DHCP_SCRIPT_ADAPTER_FETCHER_WIN_H_