Index: components/history/core/browser/history_model_worker.cc |
diff --git a/components/history/core/browser/history_model_worker.cc b/components/history/core/browser/history_model_worker.cc |
index 72b843dc9ffc765938f73116d1f5a49c1f27f4cd..94fc4aa4cd9fadd01b9bd1ddf2fa3e84a892dd8f 100644 |
--- a/components/history/core/browser/history_model_worker.cc |
+++ b/components/history/core/browser/history_model_worker.cc |
@@ -6,29 +6,41 @@ |
#include <utility> |
-#include "base/synchronization/waitable_event.h" |
+#include "base/memory/ptr_util.h" |
#include "components/history/core/browser/history_db_task.h" |
#include "components/history/core/browser/history_service.h" |
#include "components/sync/base/scoped_event_signal.h" |
namespace browser_sync { |
-class WorkerTask : public history::HistoryDBTask { |
+class HistoryModelWorker::WorkerTask : public history::HistoryDBTask { |
public: |
WorkerTask(const syncer::WorkCallback& work, |
syncer::ScopedEventSignal scoped_event_signal, |
+ scoped_refptr<HistoryModelWorker> history_model_worker, |
syncer::SyncerError* error) |
: work_(work), |
scoped_event_signal_(std::move(scoped_event_signal)), |
+ history_model_worker_(std::move(history_model_worker)), |
error_(error) {} |
+ ~WorkerTask() override { |
+ // The event in |scoped_event_signal_| is signaled at the end of this |
+ // scope if this is destroyed before RunOnDBThread runs. |
+ } |
+ |
bool RunOnDBThread(history::HistoryBackend* backend, |
history::HistoryDatabase* db) override { |
// Signal the completion event at the end of this scope. |
auto scoped_event_signal = std::move(scoped_event_signal_); |
- // Run the task. |
- *error_ = work_.Run(); |
+ // Run the task if RequestStop() hasn't been called. |
+ if (history_model_worker_->WillRunWorkCallback()) { |
+ *error_ = work_.Run(); |
+ history_model_worker_->DidRunWorkCallback(); |
+ } else { |
+ *error_ = syncer::CANNOT_DO_WORK; |
+ } |
return true; |
} |
@@ -38,88 +50,84 @@ class WorkerTask : public history::HistoryDBTask { |
void DoneRunOnMainThread() override {} |
protected: |
- ~WorkerTask() override { |
- // The event in |scoped_event_signal_| is signaled at the end of this |
- // scope if this is destroyed before RunOnDBThread runs. |
- } |
- |
syncer::WorkCallback work_; |
syncer::ScopedEventSignal scoped_event_signal_; |
+ const scoped_refptr<HistoryModelWorker> history_model_worker_; |
syncer::SyncerError* error_; |
}; |
-class AddDBThreadObserverTask : public history::HistoryDBTask { |
- public: |
- explicit AddDBThreadObserverTask(base::Closure register_callback) |
- : register_callback_(register_callback) {} |
- |
- bool RunOnDBThread(history::HistoryBackend* backend, |
- history::HistoryDatabase* db) override { |
- register_callback_.Run(); |
- return true; |
- } |
- |
- void DoneRunOnMainThread() override {} |
- |
- private: |
- ~AddDBThreadObserverTask() override {} |
- |
- base::Closure register_callback_; |
-}; |
- |
-namespace { |
- |
-// Post the work task on |history_service|'s DB thread from the UI |
-// thread. |
-void PostWorkerTask( |
- const base::WeakPtr<history::HistoryService>& history_service, |
- const syncer::WorkCallback& work, |
- syncer::ScopedEventSignal scoped_event_signal, |
- base::CancelableTaskTracker* cancelable_tracker, |
- syncer::SyncerError* error) { |
- if (history_service.get()) { |
- std::unique_ptr<history::HistoryDBTask> task( |
- new WorkerTask(work, std::move(scoped_event_signal), error)); |
- history_service->ScheduleDBTask(std::move(task), cancelable_tracker); |
- } else { |
- *error = syncer::CANNOT_DO_WORK; |
- // The event in |scoped_event_signal| is signaled at the end of this |
- // scope. |
- } |
-} |
- |
-} // namespace |
- |
HistoryModelWorker::HistoryModelWorker( |
const base::WeakPtr<history::HistoryService>& history_service, |
const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread) |
- : history_service_(history_service), ui_thread_(ui_thread) { |
+ : history_service_(history_service), |
+ ui_thread_(ui_thread), |
+ work_done_or_abandoned_(base::WaitableEvent::ResetPolicy::MANUAL, |
+ base::WaitableEvent::InitialState::NOT_SIGNALED), |
+ stop_requested_(base::WaitableEvent::ResetPolicy::MANUAL, |
+ base::WaitableEvent::InitialState::NOT_SIGNALED) { |
CHECK(history_service.get()); |
DCHECK(ui_thread_->BelongsToCurrentThread()); |
+ sequence_checker_.DetachFromSequence(); |
cancelable_tracker_.reset(new base::CancelableTaskTracker); |
} |
syncer::SyncerError HistoryModelWorker::DoWorkAndWaitUntilDoneImpl( |
const syncer::WorkCallback& work) { |
+ DCHECK(sequence_checker_.CalledOnValidSequence()); |
syncer::SyncerError error = syncer::UNSET; |
- |
- // Signaled after the task runs or when it is abandoned. |
- base::WaitableEvent work_done_or_abandoned( |
- base::WaitableEvent::ResetPolicy::AUTOMATIC, |
- base::WaitableEvent::InitialState::NOT_SIGNALED); |
- |
- if (ui_thread_->PostTask(FROM_HERE, |
- base::Bind(&PostWorkerTask, history_service_, work, |
- base::Passed(syncer::ScopedEventSignal( |
- &work_done_or_abandoned)), |
- cancelable_tracker_.get(), &error))) { |
- work_done_or_abandoned.Wait(); |
- } else { |
+ work_done_or_abandoned_.Reset(); |
+ |
+ // Post a task to the UI thread that itself posts |work| to the history DB |
+ // thread. |
+ if (!ui_thread_->PostTask( |
+ FROM_HERE, |
+ base::Bind( |
+ [](const base::WeakPtr<history::HistoryService>& history_service, |
maxbogue
2017/03/20 16:52:37
Optional: I love lambdas as much as anyone, but wi
fdoray
2017/03/20 18:54:54
Done.
|
+ std::unique_ptr<history::HistoryDBTask> worker_task, |
+ base::CancelableTaskTracker* cancelable_tracker, |
+ syncer::SyncerError* error) { |
+ if (history_service.get()) { |
maxbogue
2017/03/20 16:52:37
nit: do you need .get() here? WeakPtr implements t
fdoray
2017/03/20 18:54:54
Done.
|
+ history_service->ScheduleDBTask(std::move(worker_task), |
+ cancelable_tracker); |
+ } else { |
+ *error = syncer::CANNOT_DO_WORK; |
+ } |
+ }, |
+ history_service_, |
+ base::Passed(base::MakeUnique<WorkerTask>( |
+ work, syncer::ScopedEventSignal(&work_done_or_abandoned_), |
+ this, &error)), |
+ base::Unretained(cancelable_tracker_.get()), |
+ base::Unretained(&error)))) { |
error = syncer::CANNOT_DO_WORK; |
+ return error; |
} |
+ |
+ base::WaitableEvent* events[] = {&work_done_or_abandoned_, &stop_requested_}; |
+ base::WaitableEvent::WaitMany(events, arraysize(events)); |
+ |
return error; |
} |
+void HistoryModelWorker::RequestStop() { |
+ DCHECK(ui_thread_->BelongsToCurrentThread()); |
+ ModelSafeWorker::RequestStop(); |
+ |
+ base::AutoLock auto_lock(lock_); |
+ |
+ // If a WorkCallback is running on the history DB thread, do nothing. |
+ // DoWorkAndWaitUntilDoneImpl() will return when the WorkCallback completes. |
+ // |
+ // Note: Returning from DoWorkAndWaitUntilDoneImpl() when a WorkCallback is |
+ // running could result in invalid memory accesses. |
+ if (work_callback_running_) |
+ return; |
+ |
+ // Else, signal |stop_requested_| to unblock DoWorkAndWaitUntilDoneImpl() and |
+ // prevent any WorkCallback from being scheduled in the future. |
+ stop_requested_.Signal(); |
maxbogue
2017/03/20 16:52:37
"and prevent any WorkCallback from being scheduled
fdoray
2017/03/20 18:54:54
Done.
|
+} |
+ |
syncer::ModelSafeGroup HistoryModelWorker::GetModelSafeGroup() { |
return syncer::GROUP_HISTORY; |
} |
@@ -138,4 +146,20 @@ HistoryModelWorker::~HistoryModelWorker() { |
ui_thread_->DeleteSoon(FROM_HERE, cancelable_tracker_.release()); |
} |
+bool HistoryModelWorker::WillRunWorkCallback() { |
+ base::AutoLock auto_lock(lock_); |
+ |
+ if (stop_requested_.IsSignaled()) |
+ return false; |
+ |
+ work_callback_running_ = true; |
+ return true; |
+} |
+ |
+void HistoryModelWorker::DidRunWorkCallback() { |
+ base::AutoLock auto_lock(lock_); |
+ DCHECK(work_callback_running_); |
+ work_callback_running_ = false; |
+} |
+ |
} // namespace browser_sync |