Add WorkerPool::PostTaskAndReply and use in DHCP code.

net/proxy/dhcp_proxy_script_fetcher_win.cc

 // 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.
 
 #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"
 #include "net/base/net_errors.h"
 #include "net/proxy/dhcp_proxy_script_adapter_fetcher_win.h"
 
 #include <winsock2.h>
 #include <iphlpapi.h>
 #pragma comment(lib, "iphlpapi.lib")
 
@@ skipping 43 matching lines @@
     NOTREACHED();
     return ERR_UNEXPECTED;
   }
 
   fetch_start_time_ = base::TimeTicks::Now();
 
   state_ = STATE_WAIT_ADAPTERS;
   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;
 }
 
 void DhcpProxyScriptFetcherWin::Cancel() {
   DCHECK(CalledOnValidThread());
 
   if (state_ != STATE_DONE) {
     // We only count this stat if the cancel was explicitly initiated by
     // our client, and if we weren't already in STATE_DONE.
     UMA_HISTOGRAM_TIMES("Net.DhcpWpadCancelTime",
@@ skipping 15 matching lines @@
          it != fetchers_.end();
          ++it) {
       (*it)->Cancel();
     }
 
     fetchers_.reset();
   }
 }
 
 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;
   }
 
   for (std::set<std::string>::const_iterator it = adapter_names.begin();
        it != adapter_names.end();
        ++it) {
     DhcpProxyScriptAdapterFetcher* fetcher(ImplCreateAdapterFetcher());
     fetcher->Fetch(*it, &fetcher_callback_);
@@ skipping 120 matching lines @@
 
 URLRequestContext* DhcpProxyScriptFetcherWin::url_request_context() const {
   return url_request_context_;
 }
 
 DhcpProxyScriptAdapterFetcher*
     DhcpProxyScriptFetcherWin::ImplCreateAdapterFetcher() {
   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() {
   return kMaxWaitAfterFirstResultMs;
 }
 
 bool DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(
     std::set<std::string>* adapter_names) {
   DCHECK(adapter_names);
   adapter_names->clear();
@@ skipping 52 matching lines @@
     if ((adapter->Flags & IP_ADAPTER_DHCP_ENABLED) == 0)
       continue;
 
     DCHECK(adapter->AdapterName);
     adapter_names->insert(adapter->AdapterName);
   }
 
   return true;
 }
 
-DhcpProxyScriptFetcherWin::WorkerThread::WorkerThread(
-    const base::WeakPtr<DhcpProxyScriptFetcherWin>& owner) {
-  Init(owner);
+DhcpProxyScriptFetcherWin::AdapterQuery::AdapterQuery() {
 }
 
-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);
+void DhcpProxyScriptFetcherWin::AdapterQuery::GetCandidateAdapterNames() {
+  ImplGetCandidateAdapterNames(&adapter_names_);
 }
 
-void DhcpProxyScriptFetcherWin::WorkerThread::ThreadFunc() {
-  ImplGetCandidateAdapterNames(&adapter_names_);
-
-  bool succeeded = origin_loop_->PostTask(
-      FROM_HERE,
-      NewRunnableMethod(
-          this,
-          &DhcpProxyScriptFetcherWin::WorkerThread::OnThreadDone));
-  DCHECK(succeeded);
+const std::set<std::string>&
+    DhcpProxyScriptFetcherWin::AdapterQuery::adapter_names() const {
+  return adapter_names_;
 }
 
-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();
-}
-
-bool DhcpProxyScriptFetcherWin::WorkerThread::ImplGetCandidateAdapterNames(
+bool DhcpProxyScriptFetcherWin::AdapterQuery::ImplGetCandidateAdapterNames(
     std::set<std::string>* adapter_names) {
   return DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(adapter_names);
 }
 
 }  // namespace net