base::Bind: Final cleanup in history.[cc,h].

BUG=none
TEST=none

R=csilv@chromium.org

Review URL: http://codereview.chromium.org/8211001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@104828 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 51 insertions, 39 deletions

diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index d736f2a..d21938c 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -78,15 +78,19 @@ class HistoryService::BackendDelegate : public HistoryBackend::Delegate {
   virtual void NotifyProfileError(int backend_id,
                                   sql::InitStatus init_status) OVERRIDE {
     // Send to the history service on the main thread.
-    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
-        &HistoryService::NotifyProfileError, backend_id, init_status));
+    message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&HistoryService::NotifyProfileError, history_service_.get(),
+                   backend_id, init_status));
   }
 
   virtual void SetInMemoryBackend(int backend_id,
       history::InMemoryHistoryBackend* backend) OVERRIDE {
     // Send the backend to the history service on the main thread.
-    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
-        &HistoryService::SetInMemoryBackend, backend_id, backend));
+    message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&HistoryService::SetInMemoryBackend, history_service_.get(),
+                   backend_id, backend));
   }
 
   virtual void BroadcastNotifications(
@@ -100,18 +104,24 @@ class HistoryService::BackendDelegate : public HistoryBackend::Delegate {
                                              det);
     }
     // Send the notification to the history service on the main thread.
-    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
-        &HistoryService::BroadcastNotifications, type, details));
+    message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&HistoryService::BroadcastNotifications,
+                   history_service_.get(), type, details));
   }
 
   virtual void DBLoaded(int backend_id) OVERRIDE {
-    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
-        &HistoryService::OnDBLoaded, backend_id));
+    message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&HistoryService::OnDBLoaded, history_service_.get(),
+                   backend_id));
   }
 
   virtual void StartTopSitesMigration(int backend_id) OVERRIDE {
-    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
-        &HistoryService::StartTopSitesMigration, backend_id));
+    message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&HistoryService::StartTopSitesMigration,
+                   history_service_.get(), backend_id));
   }
 
  private:
@@ -185,14 +195,21 @@ void HistoryService::UnloadBackend() {
   // run before we release our backend refptr, the last reference will be held
   // by this thread and the destructor will be called from here.
   //
-  // Therefore, we create a task to run the Closing operation first. This holds
-  // a reference to the backend. Then we release our reference, then we schedule
-  // the task to run. After the task runs, it will delete its reference from
-  // the history thread, ensuring everything works properly.
-  Task* closing_task =
-      NewRunnableMethod(history_backend_.get(), &HistoryBackend::Closing);
-  history_backend_ = NULL;
+  // Therefore, we create a closure to run the Closing operation first. This
+  // holds a reference to the backend. Then we release our reference, then we
+  // schedule the task to run. After the task runs, it will delete its reference
+  // from the history thread, ensuring everything works properly.
+  //
+  // TODO(ajwong): Cleanup HistoryBackend lifetime issues.
+  //     See http://crbug.com/99767.
+  history_backend_->AddRef();
+  base::Closure closing_task =
+      base::Bind(&HistoryBackend::Closing, history_backend_.get());
   ScheduleTask(PRIORITY_NORMAL, closing_task);
+  closing_task.Reset();
+  HistoryBackend* raw_ptr = history_backend_.get();
+  history_backend_ = NULL;
+  thread_->message_loop()->ReleaseSoon(FROM_HERE, raw_ptr);
 }
 
 void HistoryService::Cleanup() {
@@ -682,8 +699,8 @@ void HistoryService::ScheduleAutocomplete(HistoryURLProvider* provider,
 }
 
 void HistoryService::ScheduleTask(SchedulePriority priority,
-                                  Task* task) {
-  // TODO(brettw): do prioritization.
+                                  const base::Closure& task) {
+  // TODO(brettw): Do prioritization.
   thread_->message_loop()->PostTask(FROM_HERE, task);
 }
 
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index 7b9d219..bd503af 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -690,7 +690,7 @@ class HistoryService : public CancelableRequestProvider,
 
   // Call to schedule a given task for running on the history thread with the
   // specified priority. The task will have ownership taken.
-  void ScheduleTask(SchedulePriority priority, Task* task);
+  void ScheduleTask(SchedulePriority priority, const base::Closure& task);
 
   // Schedule ------------------------------------------------------------------
   //
@@ -708,8 +708,8 @@ class HistoryService : public CancelableRequestProvider,
     if (consumer)
       AddRequest(request, consumer);
     ScheduleTask(priority,
-                 NewRunnableMethod(history_backend_.get(), func,
-                                   scoped_refptr<RequestType>(request)));
+                 base::Bind(func, history_backend_.get(),
+                            scoped_refptr<RequestType>(request)));
     return request->handle();
   }
 
@@ -724,9 +724,8 @@ class HistoryService : public CancelableRequestProvider,
     if (consumer)
       AddRequest(request, consumer);
     ScheduleTask(priority,
-                 NewRunnableMethod(history_backend_.get(), func,
-                                   scoped_refptr<RequestType>(request),
-                                   a));
+                 base::Bind(func, history_backend_.get(),
+                            scoped_refptr<RequestType>(request), a));
     return request->handle();
   }
 
@@ -745,9 +744,8 @@ class HistoryService : public CancelableRequestProvider,
     if (consumer)
       AddRequest(request, consumer);
     ScheduleTask(priority,
-                 NewRunnableMethod(history_backend_.get(), func,
-                                   scoped_refptr<RequestType>(request),
-                                   a, b));
+                 base::Bind(func, history_backend_.get(),
+                            scoped_refptr<RequestType>(request), a, b));
     return request->handle();
   }
 
@@ -768,9 +766,8 @@ class HistoryService : public CancelableRequestProvider,
     if (consumer)
       AddRequest(request, consumer);
     ScheduleTask(priority,
-                 NewRunnableMethod(history_backend_.get(), func,
-                                   scoped_refptr<RequestType>(request),
-                                   a, b, c));
+                 base::Bind(func, history_backend_.get(),
+                            scoped_refptr<RequestType>(request), a, b, c));
     return request->handle();
   }
 
@@ -784,7 +781,7 @@ class HistoryService : public CancelableRequestProvider,
                          BackendFunc func) {  // Function to call on backend.
     DCHECK(thread_) << "History service being called after cleanup";
     LoadBackendIfNecessary();
-    ScheduleTask(priority, NewRunnableMethod(history_backend_.get(), func));
+    ScheduleTask(priority, base::Bind(func, history_backend_.get()));
   }
 
   template<typename BackendFunc, typename ArgA>
@@ -793,7 +790,7 @@ class HistoryService : public CancelableRequestProvider,
                          const ArgA& a) {
     DCHECK(thread_) << "History service being called after cleanup";
     LoadBackendIfNecessary();
-    ScheduleTask(priority, NewRunnableMethod(history_backend_.get(), func, a));
+    ScheduleTask(priority, base::Bind(func, history_backend_.get(), a));
   }
 
   template<typename BackendFunc, typename ArgA, typename ArgB>
@@ -803,8 +800,7 @@ class HistoryService : public CancelableRequestProvider,
                          const ArgB& b) {
     DCHECK(thread_) << "History service being called after cleanup";
     LoadBackendIfNecessary();
-    ScheduleTask(priority, NewRunnableMethod(history_backend_.get(), func,
-                                             a, b));
+    ScheduleTask(priority, base::Bind(func, history_backend_.get(), a, b));
   }
 
   template<typename BackendFunc, typename ArgA, typename ArgB, typename ArgC>
@@ -815,8 +811,7 @@ class HistoryService : public CancelableRequestProvider,
                          const ArgC& c) {
     DCHECK(thread_) << "History service being called after cleanup";
     LoadBackendIfNecessary();
-    ScheduleTask(priority, NewRunnableMethod(history_backend_.get(), func,
-                                             a, b, c));
+    ScheduleTask(priority, base::Bind(func, history_backend_.get(), a, b, c));
   }
 
   template<typename BackendFunc,
@@ -832,8 +827,8 @@ class HistoryService : public CancelableRequestProvider,
                          const ArgD& d) {
     DCHECK(thread_) << "History service being called after cleanup";
     LoadBackendIfNecessary();
-    ScheduleTask(priority, NewRunnableMethod(history_backend_.get(), func,
-                                             a, b, c, d));
+    ScheduleTask(priority, base::Bind(func, history_backend_.get(),
+                                      a, b, c, d));
   }
 
   NotificationRegistrar registrar_;