Make HistoryDBTask not refcounted, and ensure it's destroyed on its origin thread.

chrome/browser/history/history_backend.cc

 // Copyright (c) 2012 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/history/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
@@ skipping 145 matching lines @@
   }
 
  private:
   friend class base::RefCounted<CommitLaterTask>;
 
   ~CommitLaterTask() {}
 
   scoped_refptr<HistoryBackend> history_backend_;
 };
 
-// QueuedHistoryDBTask ---------------------------------------------------------
 
 QueuedHistoryDBTask::QueuedHistoryDBTask(
-    scoped_refptr<HistoryDBTask> task,
+    scoped_ptr<HistoryDBTask> task,
     scoped_refptr<base::SingleThreadTaskRunner> origin_loop,
     const base::CancelableTaskTracker::IsCanceledCallback& is_canceled)
-    : task_(task), origin_loop_(origin_loop), is_canceled_(is_canceled) {
+    : task_(task.Pass()), origin_loop_(origin_loop), is_canceled_(is_canceled) {
   DCHECK(task_);
   DCHECK(origin_loop_);
   DCHECK(!is_canceled_.is_null());
 }
 
 QueuedHistoryDBTask::~QueuedHistoryDBTask() {
+  origin_loop_->PostTask(

[sky, 2014/07/14 20:27:44]

Document why this is done.

[Bernhard Bauer, 2014/07/15 10:17:24]

Done.

+      FROM_HERE,
+      base::Bind(&base::DeletePointer<HistoryDBTask>,
+                 base::Unretained(task_.release())));
 }
 
 bool QueuedHistoryDBTask::is_canceled() {
   return is_canceled_.Run();
 }
 
-bool QueuedHistoryDBTask::RunOnDBThread(HistoryBackend* backend,
+bool QueuedHistoryDBTask::Run(HistoryBackend* backend,
                                         HistoryDatabase* db) {
   return task_->RunOnDBThread(backend, db);
 }
 
-void QueuedHistoryDBTask::DoneRunOnMainThread() {
+void QueuedHistoryDBTask::DoneRun() {
   origin_loop_->PostTask(
       FROM_HERE,
       base::Bind(&RunUnlessCanceled,
-                 base::Bind(&HistoryDBTask::DoneRunOnMainThread, task_),
+                 base::Bind(&HistoryDBTask::DoneRunOnMainThread,
+                            base::Unretained(task_.get())),
                  is_canceled_));
 }
 
 // HistoryBackend --------------------------------------------------------------
 
 HistoryBackend::HistoryBackend(const base::FilePath& history_dir,
                                Delegate* delegate,
                                HistoryClient* history_client)
     : delegate_(delegate),
       history_dir_(history_dir),
       scheduled_kill_db_(false),
       expirer_(this, history_client),
       recent_redirects_(kMaxRedirectCount),
       backend_destroy_message_loop_(NULL),
       segment_queried_(false),
       history_client_(history_client) {
 }
 
 HistoryBackend::~HistoryBackend() {
   DCHECK(!scheduled_commit_.get()) << "Deleting without cleanup";
+  STLDeleteContainerPointers(queued_history_db_tasks_.begin(),

[sky, 2014/07/14 20:27:44]

Should we post to the main thread here?

[Bernhard Bauer, 2014/07/15 10:17:24]

No, QueuedHistoryTask lives fully on the DB thread. I've added some more
comments to explain that.

+                             queued_history_db_tasks_.end());
   queued_history_db_tasks_.clear();
 
 #if defined(OS_ANDROID)
   // Release AndroidProviderBackend before other objects.
   android_provider_backend_.reset();
 #endif
 
   // First close the databases before optionally running the "destroy" task.
   CloseAllDatabases();
 
@@ skipping 2095 matching lines @@
 void HistoryBackend::CancelScheduledCommit() {
   if (scheduled_commit_.get()) {
     scheduled_commit_->Cancel();
     scheduled_commit_ = NULL;
   }
 }
 
 void HistoryBackend::ProcessDBTaskImpl() {
   if (!db_) {
     // db went away, release all the refs.
+    STLDeleteContainerPointers(queued_history_db_tasks_.begin(),
+                               queued_history_db_tasks_.end());
     queued_history_db_tasks_.clear();
     return;
   }
 
   // Remove any canceled tasks.
   while (!queued_history_db_tasks_.empty()) {
-    QueuedHistoryDBTask& task = queued_history_db_tasks_.front();
-    if (!task.is_canceled()) {
+    QueuedHistoryDBTask* task = queued_history_db_tasks_.front();
+    if (!task->is_canceled())
       break;
-    }
+
+    delete task;
     queued_history_db_tasks_.pop_front();
   }
   if (queued_history_db_tasks_.empty())
     return;
 
   // Run the first task.
-  QueuedHistoryDBTask task = queued_history_db_tasks_.front();
+  scoped_ptr<QueuedHistoryDBTask> task(queued_history_db_tasks_.front());
   queued_history_db_tasks_.pop_front();
-  if (task.RunOnDBThread(this, db_.get())) {
+  if (task->Run(this, db_.get())) {
     // The task is done, notify the callback.
-    task.DoneRunOnMainThread();
+    task->DoneRun();
   } else {
     // The task wants to run some more. Schedule it at the end of the current
     // tasks, and process it after an invoke later.
-    queued_history_db_tasks_.insert(queued_history_db_tasks_.end(), task);
+    queued_history_db_tasks_.push_back(task.release());
     base::MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(&HistoryBackend::ProcessDBTaskImpl, this));
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // Generic operations
 //
 ////////////////////////////////////////////////////////////////////////////////
@@ skipping 165 matching lines @@
   // The expirer keeps tabs on the active databases. Tell it about the
   // databases which will be closed.
   expirer_.SetDatabases(NULL, NULL);
 
   // Reopen a new transaction for |db_| for the sake of CloseAllDatabases().
   db_->BeginTransaction();
   CloseAllDatabases();
 }
 
 void HistoryBackend::ProcessDBTask(
-    scoped_refptr<HistoryDBTask> task,
+    scoped_ptr<HistoryDBTask> task,
     scoped_refptr<base::SingleThreadTaskRunner> origin_loop,
     const base::CancelableTaskTracker::IsCanceledCallback& is_canceled) {
   bool scheduled = !queued_history_db_tasks_.empty();
-  queued_history_db_tasks_.insert(
-      queued_history_db_tasks_.end(),
-      QueuedHistoryDBTask(task, origin_loop, is_canceled));
+  queued_history_db_tasks_.push_back(
+      new QueuedHistoryDBTask(task.Pass(), origin_loop, is_canceled));
   if (!scheduled)
     ProcessDBTaskImpl();
 }
 
 void HistoryBackend::BroadcastNotifications(
     int type,
     scoped_ptr<HistoryDetails> details) {
   // |delegate_| may be NULL if |this| is in the process of closing (closed by
   // HistoryService -> HistoryBackend::Closing().
   if (delegate_)
@@ skipping 184 matching lines @@
   int rank = kPageVisitStatsMaxTopSites;
   std::map<GURL, int>::const_iterator it = most_visited_urls_map_.find(url);
   if (it != most_visited_urls_map_.end())
     rank = (*it).second;
   UMA_HISTOGRAM_ENUMERATION("History.TopSitesVisitsByRank",
                             rank, kPageVisitStatsMaxTopSites + 1);
 }
 #endif
 
 }  // namespace history