Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Unified Diff: components/sync/engine/model_safe_worker.cc

Issue 2820623002: Revert of [Sync] Refactor ModelSafeWorker::DoWorkAndWaitUntilDone() to avoid code duplication. (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/sync/engine/model_safe_worker.h ('k') | components/sync/engine/model_safe_worker_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/sync/engine/model_safe_worker.cc
diff --git a/components/sync/engine/model_safe_worker.cc b/components/sync/engine/model_safe_worker.cc
index 773571675ba7806755af26c9d2ab6b1b7bec18f6..530f206599673cf1b91239e5d40aeff463b0406c 100644
--- a/components/sync/engine/model_safe_worker.cc
+++ b/components/sync/engine/model_safe_worker.cc
@@ -4,9 +4,6 @@
#include "components/sync/engine/model_safe_worker.h"
-#include <utility>
-
-#include "base/bind.h"
#include "base/json/json_writer.h"
#include "base/values.h"
@@ -72,86 +69,24 @@
}
}
-ModelSafeWorker::ModelSafeWorker()
- : work_done_or_abandoned_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
- base::WaitableEvent::InitialState::NOT_SIGNALED) {
-}
+ModelSafeWorker::ModelSafeWorker() {}
ModelSafeWorker::~ModelSafeWorker() {}
void ModelSafeWorker::RequestStop() {
- base::AutoLock auto_lock(lock_);
-
- // Set stop flag to prevent any *further* WorkCallback from starting to run
- // (note that one may alreay be running).
- stopped_ = true;
-
- // If no work is running, unblock DoWorkAndWaitUntilDone(). If work is
- // running, it is unsafe to return from DoWorkAndWaitUntilDone().
- // ScopedSignalWorkDoneOrAbandoned will take care of signaling the event when
- // the work is done.
- if (!is_work_running_)
- work_done_or_abandoned_.Signal();
+ // Set stop flag. This prevents any *further* tasks from being posted to
+ // worker threads (see DoWorkAndWaitUntilDone below), but note that one may
+ // already be posted.
+ stopped_.Set();
}
-SyncerError ModelSafeWorker::DoWorkAndWaitUntilDone(WorkCallback work) {
- {
- // It is important to check |stopped_| and reset |work_done_or_abandoned_|
- // atomically to prevent this race:
- //
- // Thread Action
- // Sync Sees that |stopped_| is false.
- // UI Calls RequestStop(). Signals |work_done_or_abandoned_|.
- // Sync Resets |work_done_or_abandoned_|.
- // Waits on |work_done_or_abandoned_| forever since the task may not
- // run after RequestStop() is called.
- base::AutoLock auto_lock(lock_);
- if (stopped_)
- return CANNOT_DO_WORK;
- DCHECK(!is_work_running_);
- work_done_or_abandoned_.Reset();
- }
-
- SyncerError error = UNSET;
- bool did_run = false;
- ScheduleWork(base::BindOnce(
- &ModelSafeWorker::DoWork, this, base::Passed(std::move(work)),
- base::Passed(base::ScopedClosureRunner(base::Bind(
- [](scoped_refptr<ModelSafeWorker> worker) {
- worker->work_done_or_abandoned_.Signal();
- },
- make_scoped_refptr(this)))),
- base::Unretained(&error), base::Unretained(&did_run)));
-
- // Unblocked when the task runs or is deleted or when RequestStop() is called
- // before the task starts running.
- work_done_or_abandoned_.Wait();
-
- return did_run ? error : CANNOT_DO_WORK;
+SyncerError ModelSafeWorker::DoWorkAndWaitUntilDone(const WorkCallback& work) {
+ if (stopped_.IsSet())
+ return CANNOT_DO_WORK;
+ return DoWorkAndWaitUntilDoneImpl(work);
}
-void ModelSafeWorker::DoWork(WorkCallback work,
- base::ScopedClosureRunner scoped_closure_runner,
- SyncerError* error,
- bool* did_run) {
- {
- base::AutoLock auto_lock(lock_);
- if (stopped_)
- return;
-
- // Set |is_work_running_| to make sure that DoWorkAndWaitUntilDone() doesn't
- // return while |work| is running.
- DCHECK(!is_work_running_);
- is_work_running_ = true;
- }
-
- *error = std::move(work).Run();
- *did_run = true;
-
- {
- base::AutoLock auto_lock(lock_);
- DCHECK(is_work_running_);
- is_work_running_ = false;
- }
+bool ModelSafeWorker::IsStopped() {
+ return stopped_.IsSet();
}
} // namespace syncer
« no previous file with comments | « components/sync/engine/model_safe_worker.h ('k') | components/sync/engine/model_safe_worker_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698