Skip to content

Commit

Permalink
Use base::OnTaskRunnerDeleter in HistoryModelWorker.
Browse files Browse the repository at this point in the history
base::OnTaskRunnerDeleter is the preferred way of ensuring that
a member is destroyed on a specific sequence.

Change-Id: I86dde3b343f03201afdde3caed1e84762e55c1cb
Reviewed-on: https://chromium-review.googlesource.com/977888
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546552}
  • Loading branch information
fdoray authored and Commit Bot committed Mar 28, 2018
1 parent 32094aa commit 53f376e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
19 changes: 10 additions & 9 deletions components/history/core/browser/history_model_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,17 @@ void PostWorkerTask(

HistoryModelWorker::HistoryModelWorker(
const base::WeakPtr<history::HistoryService>& history_service,
const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread)
: history_service_(history_service), ui_thread_(ui_thread) {
scoped_refptr<base::SingleThreadTaskRunner> ui_thread)
: history_service_(history_service),
ui_thread_(std::move(ui_thread)),
// base::OnTaskRunnerDeleter ensures that |cancelable_tracker_| is
// destroyed on the UI thread, even if |this| is deleted on a different
// thread. It is a requirement of base::CancelableTaskTracker to be
// constructed and deleted on the same sequence.
cancelable_tracker_(new base::CancelableTaskTracker,
base::OnTaskRunnerDeleter(ui_thread_)) {
CHECK(history_service.get());
DCHECK(ui_thread_->BelongsToCurrentThread());
cancelable_tracker_.reset(new base::CancelableTaskTracker);
}

syncer::ModelSafeGroup HistoryModelWorker::GetModelSafeGroup() {
Expand All @@ -71,12 +77,7 @@ bool HistoryModelWorker::IsOnModelSequence() {
return true;
}

HistoryModelWorker::~HistoryModelWorker() {
// The base::CancelableTaskTracker class is not thread-safe and must only be
// used from a single thread but the current object may not be destroyed from
// the UI thread, so delete it from the UI thread.
ui_thread_->DeleteSoon(FROM_HERE, cancelable_tracker_.release());
}
HistoryModelWorker::~HistoryModelWorker() = default;

void HistoryModelWorker::ScheduleWork(base::OnceClosure work) {
ui_thread_->PostTask(
Expand Down
5 changes: 3 additions & 2 deletions components/history/core/browser/history_model_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class HistoryModelWorker : public syncer::ModelSafeWorker {
public:
explicit HistoryModelWorker(
const base::WeakPtr<history::HistoryService>& history_service,
const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread);
scoped_refptr<base::SingleThreadTaskRunner> ui_thread);

// syncer::ModelSafeWorker implementation.
syncer::ModelSafeGroup GetModelSafeGroup() override;
Expand All @@ -44,7 +44,8 @@ class HistoryModelWorker : public syncer::ModelSafeWorker {

// Helper object to make sure we don't leave tasks running on the history
// thread.
std::unique_ptr<base::CancelableTaskTracker> cancelable_tracker_;
const std::unique_ptr<base::CancelableTaskTracker, base::OnTaskRunnerDeleter>
cancelable_tracker_;

DISALLOW_COPY_AND_ASSIGN(HistoryModelWorker);
};
Expand Down

0 comments on commit 53f376e

Please sign in to comment.