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

Unified Diff: chrome/browser/sync/glue/non_frontend_data_type_controller.cc

Issue 10391080: No longer keep a lingering reference to TypedUrlDataTypeController after PSS exits (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Allow DTC deletion on non-UI threads. Created 8 years, 7 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
Index: chrome/browser/sync/glue/non_frontend_data_type_controller.cc
diff --git a/chrome/browser/sync/glue/non_frontend_data_type_controller.cc b/chrome/browser/sync/glue/non_frontend_data_type_controller.cc
index 4b1cd92089bb24c47609ca0acaf4ce504c96dfe2..155dc6161212570f218774071153f812df50f95c 100644
--- a/chrome/browser/sync/glue/non_frontend_data_type_controller.cc
+++ b/chrome/browser/sync/glue/non_frontend_data_type_controller.cc
@@ -33,7 +33,6 @@ NonFrontendDataTypeController::NonFrontendDataTypeController(
state_(NOT_RUNNING),
abort_association_(false),
abort_association_complete_(false, false),
- datatype_stopped_(false, false),
start_association_called_(true, false),
start_models_failed_(false) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -111,6 +110,14 @@ void NonFrontendDataTypeController::StopWhileAssociating() {
StartDoneImpl(ABORTED, STOPPING, SyncError());
}
+namespace {
+// Helper function that signals the UI thread once the StopAssociation task
+// has finished completing (this task is queued after the StopAssociation task).
+void SignalCallback(base::WaitableEvent* wait_event) {
+ wait_event->Signal();
+}
+} // namespace
+
void NonFrontendDataTypeController::Stop() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK_NE(state_, NOT_RUNNING);
@@ -208,9 +215,7 @@ void NonFrontendDataTypeController::Stop() {
// for any more changes or process them from server.
profile_sync_service_->DeactivateDataType(type());
- if (StopAssociationAsync()) {
- datatype_stopped_.Wait();
- } else {
+ if (!StopAssociationAsync()) {
// We do DFATAL here because this will eventually lead to a failed CHECK
// when the change processor gets destroyed on the wrong thread.
LOG(DFATAL) << "Failed to destroy datatype " << name();
@@ -258,7 +263,6 @@ NonFrontendDataTypeController::NonFrontendDataTypeController()
state_(NOT_RUNNING),
abort_association_(false),
abort_association_complete_(false, false),
- datatype_stopped_(false, false),
start_association_called_(true, false) {
}
@@ -466,13 +470,31 @@ void NonFrontendDataTypeController::StartAssociation() {
bool NonFrontendDataTypeController::StopAssociationAsync() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK_EQ(state(), STOPPING);
- return PostTaskOnBackendThread(
- FROM_HERE,
- base::Bind(
- &NonFrontendDataTypeController::StopAssociation, this));
+ if (PostTaskOnBackendThread(
+ FROM_HERE,
+ base::Bind(
+ &NonFrontendDataTypeController::StopAssociation, this))) {
+ // The remote thread will hold on to a reference to this object until
+ // the StopAssociation task finishes running. We want to make sure that we
+ // do not return from this routine until there are no more references to
+ // this object on the remote thread, so we queue up the SignalCallback
+ // task below - this task does not maintain a reference to the DTC, so
+ // when it signals this thread, we know that the previous task has executed
+ // and there are no more lingering remote references to the DTC.
+ // This fixes the race described in http://crbug.com/127706.
+ base::WaitableEvent datatype_stopped(false, false);
+ if (PostTaskOnBackendThread(
+ FROM_HERE,
+ base::Bind(&SignalCallback, &datatype_stopped))) {
+ datatype_stopped.Wait();
+ return true;
+ }
+ }
+ return false;
}
void NonFrontendDataTypeController::StopAssociation() {
+ DCHECK(!HasOneRef());
DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
if (model_associator_.get()) {
SyncError error; // Not used.
@@ -480,7 +502,6 @@ void NonFrontendDataTypeController::StopAssociation() {
}
model_associator_.reset();
change_processor_.reset();
- datatype_stopped_.Signal();
}
} // namespace browser_sync

Powered by Google App Engine
This is Rietveld 408576698