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

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

Issue 10808040: [Sync] Avoid ObserverListThreadSafe in ChromeSyncNotificationBridge (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix test errors Created 8 years, 5 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/chrome_sync_notification_bridge.cc
diff --git a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
index 9936fa08f020e93b0eff6fe86a1dcbecc3f81db8..78212d21b3f69bce073b10f088aaf66194effaaa 100644
--- a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
+++ b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
@@ -4,6 +4,9 @@
#include "chrome/browser/sync/glue/chrome_sync_notification_bridge.h"
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/observer_list.h"
#include "chrome/common/chrome_notification_types.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_service.h"
@@ -13,10 +16,73 @@ using content::BrowserThread;
namespace browser_sync {
+class ChromeSyncNotificationBridge::Core
+ : public base::RefCountedThreadSafe<Core> {
+ public:
+ // Created on UI thread.
+ explicit Core(
+ const scoped_refptr<base::SequencedTaskRunner>& sync_task_runner);
+
+ // All member functions below must be called on the sync task runner.
+
+ void AddObserver(syncer::SyncNotifierObserver* observer);
+ void RemoveObserver(syncer::SyncNotifierObserver* observer);
+
+ void EmitNotification(
+ syncer::ModelTypePayloadMap payload_map,
+ syncer::IncomingNotificationSource notification_source);
+
+ private:
+ friend class base::RefCountedThreadSafe<Core>;
+
+ // Destroyed on the UI thread or on |sync_task_runner_|.
+ ~Core();
+
+ const scoped_refptr<base::SequencedTaskRunner> sync_task_runner_;
+
+ // Used only on |sync_task_runner_|.
+ syncer::ModelTypeSet enabled_types_;
+ ObserverList<syncer::SyncNotifierObserver> observers_;
+};
+
+ChromeSyncNotificationBridge::Core::Core(
+ const scoped_refptr<base::SequencedTaskRunner>& sync_task_runner)
+ : sync_task_runner_(sync_task_runner) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(sync_task_runner_.get());
+}
+
+ChromeSyncNotificationBridge::Core::~Core() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
+ sync_task_runner_->RunsTasksOnCurrentThread());
+}
+
+void ChromeSyncNotificationBridge::Core::AddObserver(
+ syncer::SyncNotifierObserver* observer) {
+ DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+ observers_.AddObserver(observer);
+}
+
+void ChromeSyncNotificationBridge::Core::RemoveObserver(
+ syncer::SyncNotifierObserver* observer) {
+ DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+ observers_.RemoveObserver(observer);
+}
+
+void ChromeSyncNotificationBridge::Core::EmitNotification(
+ syncer::ModelTypePayloadMap payload_map,
+ syncer::IncomingNotificationSource notification_source) {
+ DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+ FOR_EACH_OBSERVER(
+ syncer::SyncNotifierObserver, observers_,
+ OnIncomingNotification(payload_map, notification_source));
+}
+
ChromeSyncNotificationBridge::ChromeSyncNotificationBridge(
- const Profile* profile)
- : observers_(
- new ObserverListThreadSafe<syncer::SyncNotifierObserver>()) {
+ const Profile* profile,
+ const scoped_refptr<base::SequencedTaskRunner>& sync_task_runner)
+ : sync_task_runner_(sync_task_runner),
+ core_(new Core(sync_task_runner_)) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(profile);
registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH_LOCAL,
@@ -35,12 +101,14 @@ void ChromeSyncNotificationBridge::UpdateEnabledTypes(
void ChromeSyncNotificationBridge::AddObserver(
syncer::SyncNotifierObserver* observer) {
- observers_->AddObserver(observer);
+ DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+ core_->AddObserver(observer);
}
void ChromeSyncNotificationBridge::RemoveObserver(
syncer::SyncNotifierObserver* observer) {
- observers_->RemoveObserver(observer);
+ DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+ core_->RemoveObserver(observer);
}
void ChromeSyncNotificationBridge::Observe(
@@ -55,7 +123,7 @@ void ChromeSyncNotificationBridge::Observe(
} else if (type == chrome::NOTIFICATION_SYNC_REFRESH_REMOTE) {
notification_source = syncer::REMOTE_NOTIFICATION;
} else {
- NOTREACHED() << "Unexpected notification type:" << type;
+ NOTREACHED() << "Unexpected notification type: " << type;
return;
}
@@ -69,9 +137,10 @@ void ChromeSyncNotificationBridge::Observe(
syncer::ModelTypePayloadMapFromEnumSet(enabled_types_, std::string());
}
- observers_->Notify(
- &syncer::SyncNotifierObserver::OnIncomingNotification,
- payload_map, notification_source);
+ sync_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(&Core::EmitNotification,
+ core_, payload_map, notification_source));
}
} // namespace browser_sync

Powered by Google App Engine
This is Rietveld 408576698