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

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.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 "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"
 #include "base/task.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
@@ skipping 77 matching lines @@
 
   bool UpdateState(chromeos::NetworkLibrary* cros);
 
   base::hash_set<std::string> observers_;
   std::string active_network_;
   std::string network_name_;
   State state_;
   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())
      return;
 
     io_thread_->globals()->dnsrr_resolver.get()->OnIPAddressChanged();
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback_);
   }
 
  private:
-  Task* callback_;
+  base::Closure callback_;
   IOThread* io_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(ClearDnsCacheTaskOnIOThread);
 };
+#endif
 
 // NetworkStateInformer implementation -----------------------------------------
 
 NetworkStateInformer::NetworkStateInformer(WebUI* web_ui) : web_ui_(web_ui) {
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   UpdateState(cros);
   cros->AddNetworkManagerObserver(this);
 }
 
 NetworkStateInformer::~NetworkStateInformer() {
@@ skipping 53 matching lines @@
     : delegate_(WebUILoginDisplay::GetInstance()),
       show_on_init_(false),
       oobe_ui_(false),
       dns_cleared_(false),
       dns_clear_task_running_(false),
       cookies_cleared_(false),
       extension_driven_(
           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);
 }
 
 void SigninScreenHandler::GetLocalizedStrings(
     DictionaryValue* localized_strings) {
   localized_strings->SetString("signinScreenTitle",
       l10n_util::GetStringUTF16(IDS_SIGNIN_SCREEN_TITLE));
   localized_strings->SetString("passwordHint",
       l10n_util::GetStringUTF16(IDS_LOGIN_POD_EMPTY_PASSWORD_TEXT));
@@ skipping 373 matching lines @@
     NOTREACHED();
     return;
   }
   network_state_informer_->RemoveObserver(callback);
 }
 
 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;
 }
 
 void SigninScreenHandler::StartClearingCookies() {
   cookies_cleared_ = false;
   if (cookie_remover_)
     cookie_remover_->RemoveObserver(this);
 
   cookie_remover_ = new BrowsingDataRemover(
       Profile::FromBrowserContext(web_ui_->tab_contents()->browser_context()),
       BrowsingDataRemover::EVERYTHING,
       base::Time());
   cookie_remover_->AddObserver(this);
   cookie_remover_->Remove(BrowsingDataRemover::REMOVE_SITE_DATA);
 }
 
 }  // namespace chromeos