Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(71)

Unified Diff: net/proxy/dhcp_proxy_script_fetcher_win.h

Issue 7189016: Do GetAdaptersAddresses on a worker thread. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix handling of cancellation. Add regression test. Created 9 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/proxy/dhcp_proxy_script_adapter_fetcher_win.h ('k') | net/proxy/dhcp_proxy_script_fetcher_win.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/proxy/dhcp_proxy_script_fetcher_win.h
diff --git a/net/proxy/dhcp_proxy_script_fetcher_win.h b/net/proxy/dhcp_proxy_script_fetcher_win.h
index a220f56f06f42717965f42427c60f45f3ce0107c..ae9548aeae1529318c86dc9faa2c6fc2773370b7 100644
--- a/net/proxy/dhcp_proxy_script_fetcher_win.h
+++ b/net/proxy/dhcp_proxy_script_fetcher_win.h
@@ -11,6 +11,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
+#include "base/message_loop_proxy.h"
#include "base/threading/non_thread_safe.h"
#include "base/time.h"
#include "base/timer.h"
@@ -24,6 +25,7 @@ class URLRequestContext;
// Windows-specific implementation.
class NET_TEST DhcpProxyScriptFetcherWin
: public DhcpProxyScriptFetcher,
+ public base::SupportsWeakPtr<DhcpProxyScriptFetcherWin>,
NON_EXPORTED_BASE(public base::NonThreadSafe) {
public:
// Creates a DhcpProxyScriptFetcherWin that issues requests through
@@ -48,15 +50,71 @@ class NET_TEST DhcpProxyScriptFetcherWin
URLRequestContext* url_request_context() const;
+ // This inner class is used to encapsulate a worker thread that calls
+ // GetCandidateAdapterNames as it can take a couple of hundred
+ // milliseconds.
+ //
+ // TODO(joi): Replace with PostTaskAndReply once http://crbug.com/86301
+ // has been implemented.
+ class NET_TEST WorkerThread
+ : public base::RefCountedThreadSafe<WorkerThread> {
+ public:
+ // Creates and initializes (but does not start) the worker thread.
+ explicit WorkerThread(
+ const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner);
+ virtual ~WorkerThread();
+
+ // Starts the worker thread.
+ void Start();
+
+ protected:
+ WorkerThread(); // To override in unit tests only.
+ void Init(const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner);
+
+ // Virtual method introduced to allow unit testing.
+ virtual bool ImplGetCandidateAdapterNames(
+ std::set<std::string>* adapter_names);
+
+ // Callback for ThreadFunc; this executes back on the main thread,
+ // not the worker thread. May be overridden by unit tests.
+ virtual void OnThreadDone();
+
+ private:
+ // This is the method that runs on the worker thread.
+ void ThreadFunc();
+
+ // All work except ThreadFunc and (sometimes) destruction should occur
+ // on the thread that constructs the object.
+ base::ThreadChecker thread_checker_;
+
+ // May only be accessed on the thread that constructs the object.
+ base::WeakPtr<DhcpProxyScriptFetcherWin> owner_;
+
+ // 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 the |owner_| object is gone
+ // anyway, so there is nobody to receive the result.
+ scoped_refptr<base::MessageLoopProxy> origin_loop_;
+
+ // This is constructed on the originating thread, then used on the
+ // worker thread, then used again on the originating thread only when
+ // the task has completed on the worker thread. No locking required.
+ std::set<std::string> adapter_names_;
+
+ DISALLOW_COPY_AND_ASSIGN(WorkerThread);
+ };
+
// Virtual methods introduced to allow unit testing.
virtual DhcpProxyScriptAdapterFetcher* ImplCreateAdapterFetcher();
- virtual bool ImplGetCandidateAdapterNames(
- std::set<std::string>* adapter_names);
+ virtual WorkerThread* ImplCreateWorkerThread(
+ const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner);
virtual int ImplGetMaxWaitMs();
private:
// Event/state transition handlers
void CancelImpl();
+ void OnGetCandidateAdapterNamesDone(
+ const std::set<std::string>& adapter_names);
void OnFetcherDone(int result);
void OnWaitTimer();
void TransitionToDone();
@@ -73,7 +131,9 @@ class NET_TEST DhcpProxyScriptFetcherWin
// it has completed for the fastest adapter, return the PAC file
// available for the most preferred network adapter, if any.
//
- // The state machine goes from START->NO_RESULTS when it creates
+ // The state machine goes from START->WAIT_ADAPTERS when it starts a
+ // worker thread to get the list of adapters with DHCP enabled.
+ // It then goes from WAIT_ADAPTERS->NO_RESULTS when it creates
// and starts an DhcpProxyScriptAdapterFetcher for each adapter. It goes
// from NO_RESULTS->SOME_RESULTS when it gets the first result; at this
// point a wait timer is started. It goes from SOME_RESULTS->DONE in
@@ -88,6 +148,7 @@ class NET_TEST DhcpProxyScriptFetcherWin
// allowed to be outstanding at any given time.
enum State {
STATE_START,
+ STATE_WAIT_ADAPTERS,
STATE_NO_RESULTS,
STATE_SOME_RESULTS,
STATE_DONE,
@@ -122,6 +183,8 @@ class NET_TEST DhcpProxyScriptFetcherWin
scoped_refptr<URLRequestContext> url_request_context_;
+ scoped_refptr<WorkerThread> worker_thread_;
+
// Time |Fetch()| was last called, 0 if never.
base::TimeTicks fetch_start_time_;
« no previous file with comments | « net/proxy/dhcp_proxy_script_adapter_fetcher_win.h ('k') | net/proxy/dhcp_proxy_script_fetcher_win.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698