base::Bind: More converts, mostly in WebUI.

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

Index: chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
index 057e29917db26dfe7a988afcf5d181064a7b592f..997f418664f97b0db2b107af7e07288636e1f90e 100644
--- a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/command_line.h"
 #include "base/hash_tables.h"
 #include "base/stringprintf.h"
@@ -101,15 +103,17 @@ class NetworkStateInformer
   WebUI* web_ui_;
 };
 
+#if 0

[awong, 2011/10/04 01:48:09]

Should this just be removed?

[James Hawkins, 2011/10/04 02:09:20]

Done.

 // Clears DNS cache on IO thread.
-class ClearDnsCacheTaskOnIOThread : public Task {
+class ClearDnsCacheTaskOnIOThread : public base::Callback<void(void)> {
  public:
-  ClearDnsCacheTaskOnIOThread(Task* callback, IOThread* io_thread)
-      : callback_(callback), io_thread_(io_thread) {
-  }
+  explicit ClearDnsCacheTaskOnIOThread() : io_thread_(NULL) {}
   virtual ~ClearDnsCacheTaskOnIOThread() {}
 
-  // Task overrides.
+  void set_callback(const base::Closure& callback) { callback_ = callback; }
+  void set_io_thread(IOThread* io_thread) { io_thread_ = io_thread; }
+
+  // base::Closure implementation.
   virtual void Run() OVERRIDE {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     if (browser_shutdown::IsTryingToQuit())
@@ -120,10 +124,12 @@ class ClearDnsCacheTaskOnIOThread : public Task {
   }
 
  private:
-  Task* callback_;
+  base::Closure callback_;
   IOThread* io_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(ClearDnsCacheTaskOnIOThread);
 };
+#endif
 
 // NetworkStateInformer implementation -----------------------------------------
 
@@ -197,12 +203,12 @@ SigninScreenHandler::SigninScreenHandler()
           CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kWebUILogin)),
       cookie_remover_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   delegate_->SetWebUIHandler(this);
 }
 
 SigninScreenHandler::~SigninScreenHandler() {
-  method_factory_.RevokeAll();
+  weak_factory_.InvalidateWeakPtrs();
   if (cookie_remover_)
     cookie_remover_->RemoveObserver(this);
 }
@@ -596,15 +602,28 @@ void SigninScreenHandler::HandleCreateAccount(const base::ListValue* args) {
   delegate_->CreateAccount();
 }
 
+void SigninScreenHandler::ClearDnsCache(
+    const base::Closure& callback, IOThread* io_thread) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (browser_shutdown::IsTryingToQuit())
+   return;
+
+  io_thread->globals()->dnsrr_resolver.get()->OnIPAddressChanged();
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
 void SigninScreenHandler::StartClearingDnsCache() {
   if (dns_clear_task_running_ || !g_browser_process->io_thread())
     return;
 
   dns_cleared_ = false;
-  ClearDnsCacheTaskOnIOThread* clear_dns_task = new ClearDnsCacheTaskOnIOThread(
-      method_factory_.NewRunnableMethod(&SigninScreenHandler::OnDnsCleared),
-      g_browser_process->io_thread());
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, clear_dns_task);
+  base::Closure callback =
+      base::Bind(&SigninScreenHandler::ClearDnsCache,
+                 weak_factory_.GetWeakPtr(),
+                 base::Bind(&SigninScreenHandler::OnDnsCleared,
+                            weak_factory_.GetWeakPtr()),
+                 g_browser_process->io_thread());
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, callback);

[awong, 2011/10/04 01:48:09]

I don't think is quite right....

StartClearingDnsCache() is invoked on the UI thread right?  Then
weak_factory_.GetWeakPtr() is going to generate a WeakPtr that can only be
deferenced on the UI thread.  Thus, callback.Run() can only be called on the UI
thread, yet it is being posted to the IO thread.  I think this will DCHECK.

This is different from the previous behavior where ClearDnsCacheTaskOnIOThread
was a disassociated Task that always runs regardless of whether or not the
originating SigninScreenHandler object is still alive. The liveliness of
SigninScreenHandler was only tested the NRM() for OnDnsCleaered was posted back
to the UI thread.

To simulate the old behavior you'd want to make ClearDnsCache either a static
method, or just a file-static function.

However, looking at this more, I think this is a really good candidate for
PostTaskAndReply.  Try this:

namespace {
void ClearDnsCache(IOThread* io_thread) {
  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
  if (browser_shutdown::IsTryingToQuit())
    return;

  io_thread->globals()->dnsrr_resolver.get()->OnIPAddressChanged();
}
}  // namespace

Then in StartClearingDnsCache()

BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE
   base::Bind(&ClearDnsCache, g_browser_process->io_thread()),
   base::Bind(&SigninScreenHandler::OnDnsCleared, weak_factory_.GetWeakPtr());

See message_loop_proxy.h's file comment for PostTaskAndReply() for details, but
here's the summary.

The first Closure, ClearDnsCache, is the "task."  It is run on the target
thread.

The second Closure, OnDnsCleared, is the "reply."  It is run on the current
thread.

Both closures and all arguments are deleted on the current thread.

This ensures you can safely use a WeakPtr<> to make the reply no-op itself if
the receiving object has died while task is running.

Let me know if that makes sense.

[James Hawkins, 2011/10/04 02:09:20]

Done.

   dns_clear_task_running_ = true;
 }