sync/glue: Convert BookmarkDataTypeController to BaseBookmarkModelObserver.

chrome/browser/sync/glue/bookmark_data_type_controller.cc

Index: chrome/browser/sync/glue/bookmark_data_type_controller.cc
diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller.cc b/chrome/browser/sync/glue/bookmark_data_type_controller.cc
index 14116abc9a1b3549ec55cacfa2ff920ec191c7c7..5c2ef357454232e9976bbaadd7e36a6d222917ab 100644
--- a/chrome/browser/sync/glue/bookmark_data_type_controller.cc
+++ b/chrome/browser/sync/glue/bookmark_data_type_controller.cc
@@ -34,36 +34,23 @@ syncer::ModelType BookmarkDataTypeController::type() const {
   return syncer::BOOKMARKS;
 }
 
-void BookmarkDataTypeController::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(state_, MODEL_STARTING);
-  if (type != chrome::NOTIFICATION_BOOKMARK_MODEL_LOADED &&
-      type != chrome::NOTIFICATION_HISTORY_LOADED) {
-    return;
-  }
-  if (!DependentsLoaded())
-    return;
-  registrar_.RemoveAll();
-  OnModelLoaded();
-}
-
 BookmarkDataTypeController::~BookmarkDataTypeController() {}
 
 bool BookmarkDataTypeController::StartModels() {
-  if (!DependentsLoaded()) {
-    registrar_.Add(this, chrome::NOTIFICATION_BOOKMARK_MODEL_LOADED,
-                   content::Source<Profile>(sync_service_->profile()));
+  BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile_);
+  if (!model || !model->IsLoaded()) {
+    model->AddObserver(this);
+    return false;
+  }
+  if (!HasHistoryServiceLoaded()) {
     registrar_.Add(this, chrome::NOTIFICATION_HISTORY_LOADED,
                    content::Source<Profile>(sync_service_->profile()));
     return false;
   }
+
   return true;
 }
 
-// Cleanup for our extra registrar usage.
 void BookmarkDataTypeController::CleanUpState() {
   registrar_.RemoveAll();
 }
@@ -76,20 +63,36 @@ void BookmarkDataTypeController::CreateSyncComponents() {
   set_change_processor(sync_components.change_processor);
 }
 
-// Check that both the bookmark model and the history service (for favicons)
-// are loaded.
-bool BookmarkDataTypeController::DependentsLoaded() {
-  BookmarkModel* bookmark_model =
-      BookmarkModelFactory::GetForProfile(profile_);
-  if (!bookmark_model || !bookmark_model->IsLoaded())
-    return false;
+void BookmarkDataTypeController::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(state_, MODEL_STARTING);
+  if (type != chrome::NOTIFICATION_HISTORY_LOADED)
+    return;
+
+  if (!HasHistoryServiceLoaded())
+    return;
+  registrar_.RemoveAll();
+  OnModelLoaded();
+}
+
+void BookmarkDataTypeController::BookmarkModelChanged() {
+}
+
+void BookmarkDataTypeController::Loaded(BookmarkModel* model,
+                                        bool ids_reassigned) {
+  model->RemoveObserver(this);
+  OnModelLoaded();

[Nicolas Zea, 2013/02/01 23:46:27]

This isn't quite right. I think you should keep the DependentsLoaded method and
call it here. The problem is that it's possible the HistoryService isn't loaded
at this point, so we don't want to call OnModelLoaded yet.

[tfarina, 2013/02/02 00:08:39]

Done.

+}
 
+bool BookmarkDataTypeController::HasHistoryServiceLoaded() const {
   HistoryService* history = HistoryServiceFactory::GetForProfile(
       profile_, Profile::EXPLICIT_ACCESS);
   if (!history || !history->BackendLoaded())
     return false;
 
-  // All necessary services are loaded.
   return true;
 }