Add WorkerPool::PostTaskAndReply and use in DHCP code.

net/proxy/dhcp_proxy_script_fetcher_win.cc

Index: net/proxy/dhcp_proxy_script_fetcher_win.cc
diff --git a/net/proxy/dhcp_proxy_script_fetcher_win.cc b/net/proxy/dhcp_proxy_script_fetcher_win.cc
index dced471f8f89948ff9ecf9a6de115784984f0847..ebbe86e7377a16f6b018477498940a82d212bdd6 100644
--- a/net/proxy/dhcp_proxy_script_fetcher_win.cc
+++ b/net/proxy/dhcp_proxy_script_fetcher_win.cc
@@ -4,6 +4,7 @@
 
 #include "net/proxy/dhcp_proxy_script_fetcher_win.h"
 
+#include "base/bind.h"
 #include "base/metrics/histogram.h"
 #include "base/perftimer.h"
 #include "base/threading/worker_pool.h"
@@ -67,8 +68,17 @@ int DhcpProxyScriptFetcherWin::Fetch(string16* utf16_text,
   client_callback_ = callback;
   destination_string_ = utf16_text;
 
-  worker_thread_ = ImplCreateWorkerThread(AsWeakPtr());
-  worker_thread_->Start();
+  last_query_ = ImplCreateAdapterQuery();
+  base::WorkerPool::PostTaskAndReply(

[awong, 2011/10/07 17:53:22]

Not sure this would be cleaner, but figured I'd point it out.

You could kill off the AdapterQuery class completely and do

set<string>* adapter_names = new set<string>();
PostTaskAndReply(
    FROM_HERE,
    base::Bind(
        &ImplGetCandidateAdapterNames
        adapter_names,
        last_query_.get()),
    base::Bind(
        &DhcpProxyScriptFetcherWin::OnGetCandidateAdapterNamesDone,
        AsWeakPtr(),
        adapter_names,
        last_query_.get());

And then delete adapter_names in the reply.

Next week, I expect to add another wrapper that makes it so you could pass 
base::Owned(adapter_names) to the OnGetCandidateAdapterNamesDone bind, and not
even bother with the delete.

Again, this would mess with your unittest error injection.

Just a thought. Feel free to ignore.

+      FROM_HERE,
+      base::Bind(
+          &DhcpProxyScriptFetcherWin::AdapterQuery::GetCandidateAdapterNames,
+          last_query_.get()),
+      base::Bind(
+          &DhcpProxyScriptFetcherWin::OnGetCandidateAdapterNamesDone,
+          AsWeakPtr(),
+          last_query_),
+      true);
 

[awong, 2011/10/07 17:53:22]

The old code used to DCHECK the success of PostTaskAndReply.  Did we want to
replicate that here?

[Jói, 2011/10/11 19:51:02]

Done.

   return ERR_IO_PENDING;
 }
@@ -105,15 +115,27 @@ void DhcpProxyScriptFetcherWin::CancelImpl() {
 }
 
 void DhcpProxyScriptFetcherWin::OnGetCandidateAdapterNamesDone(
-    const std::set<std::string>& adapter_names) {
+    scoped_refptr<AdapterQuery> query) {
   DCHECK(CalledOnValidThread());
 
+  // This can happen if this object is reused for multiple queries,
+  // and a previous query was cancelled before it completed.
+  if (query.get() != last_query_.get())
+    return;
+  last_query_.release();
+
+  // Enable unit tests to wait for this to happen; in production this function
+  // call is a no-op.
+  ImplOnGetCandidateAdapterNamesDone();
+
   // We may have been cancelled.
   if (state_ != STATE_WAIT_ADAPTERS)
     return;
 
   state_ = STATE_NO_RESULTS;
 
+  const std::set<std::string>& adapter_names = query->adapter_names();
+
   if (adapter_names.empty()) {
     TransitionToDone();
     return;
@@ -254,10 +276,9 @@ DhcpProxyScriptAdapterFetcher*
   return new DhcpProxyScriptAdapterFetcher(url_request_context_);
 }
 
-DhcpProxyScriptFetcherWin::WorkerThread*
-    DhcpProxyScriptFetcherWin::ImplCreateWorkerThread(
-        const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner) {
-  return new WorkerThread(owner);
+DhcpProxyScriptFetcherWin::AdapterQuery*
+    DhcpProxyScriptFetcherWin::ImplCreateAdapterQuery() {
+  return new AdapterQuery();
 }
 
 int DhcpProxyScriptFetcherWin::ImplGetMaxWaitMs() {
@@ -330,51 +351,22 @@ bool DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(
   return true;
 }
 
-DhcpProxyScriptFetcherWin::WorkerThread::WorkerThread(
-    const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner) {
-  Init(owner);
-}
-
-DhcpProxyScriptFetcherWin::WorkerThread::~WorkerThread() {
+DhcpProxyScriptFetcherWin::AdapterQuery::AdapterQuery() {
 }
 
-void DhcpProxyScriptFetcherWin::WorkerThread::Start() {
-  bool succeeded = base::WorkerPool::PostTask(
-      FROM_HERE,
-      NewRunnableMethod(
-          this,
-          &DhcpProxyScriptFetcherWin::WorkerThread::ThreadFunc),
-      true);
-  DCHECK(succeeded);
+DhcpProxyScriptFetcherWin::AdapterQuery::~AdapterQuery() {
 }
 
-void DhcpProxyScriptFetcherWin::WorkerThread::ThreadFunc() {
+void DhcpProxyScriptFetcherWin::AdapterQuery::GetCandidateAdapterNames() {
   ImplGetCandidateAdapterNames(&adapter_names_);
-
-  bool succeeded = origin_loop_->PostTask(
-      FROM_HERE,
-      NewRunnableMethod(
-          this,
-          &DhcpProxyScriptFetcherWin::WorkerThread::OnThreadDone));
-  DCHECK(succeeded);
-}
-
-void DhcpProxyScriptFetcherWin::WorkerThread::OnThreadDone() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (owner_)
-    owner_->OnGetCandidateAdapterNamesDone(adapter_names_);
-}
-
-DhcpProxyScriptFetcherWin::WorkerThread::WorkerThread() {
 }
 
-void DhcpProxyScriptFetcherWin::WorkerThread::Init(
-    const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner) {
-  owner_ = owner;
-  origin_loop_ = base::MessageLoopProxy::current();
+const std::set<std::string>&
+    DhcpProxyScriptFetcherWin::AdapterQuery::adapter_names() const {
+  return adapter_names_;
 }
 
-bool DhcpProxyScriptFetcherWin::WorkerThread::ImplGetCandidateAdapterNames(
+bool DhcpProxyScriptFetcherWin::AdapterQuery::ImplGetCandidateAdapterNames(
     std::set<std::string>* adapter_names) {
   return DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(adapter_names);
 }