Eliminate NOTIFICATION_HISTORY_LOADED notification

chrome/browser/ui/cocoa/history_menu_bridge.mm

Index: chrome/browser/ui/cocoa/history_menu_bridge.mm
diff --git a/chrome/browser/ui/cocoa/history_menu_bridge.mm b/chrome/browser/ui/cocoa/history_menu_bridge.mm
index e4785f345b4979c20bc3295acd408558705f72ab..f4991a520b9ff38bea02c749dbd034091ecd5fbb 100644
--- a/chrome/browser/ui/cocoa/history_menu_bridge.mm
+++ b/chrome/browser/ui/cocoa/history_menu_bridge.mm
@@ -77,6 +77,9 @@ HistoryMenuBridge::HistoryMenuBridge(Profile* profile)
     if (hs != NULL && hs->BackendLoaded()) {
       history_service_ = hs;
       Init();
+    } else if (hs != NULL) {
+      // The service is not ready for use yet, so become notified when it does.
+      hs->AddHistoryServiceObserver(this);

[sdefresne, 2014/10/20 13:15:42]

Use ScopedObserver<>

     }
 
     tab_restore_service_ = TabRestoreServiceFactory::GetForProfile(profile_);
@@ -99,13 +102,6 @@ HistoryMenuBridge::HistoryMenuBridge(Profile* profile)
   // Set the static icons in the menu.
   NSMenuItem* item = [HistoryMenu() itemWithTag:IDC_SHOW_HISTORY];
   [item setImage:rb.GetNativeImageNamed(IDR_HISTORY_FAVICON).ToNSImage()];
-
-  // The service is not ready for use yet, so become notified when it does.
-  if (!history_service_) {
-    registrar_.Add(
-        this, chrome::NOTIFICATION_HISTORY_LOADED,
-        content::Source<Profile>(profile_));
-  }
 }
 
 // Note that all requests sent to either the history service or the favicon
@@ -121,9 +117,7 @@ HistoryMenuBridge::~HistoryMenuBridge() {
                       content::Source<Profile>(profile_));
     registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_URLS_DELETED,
                       content::Source<Profile>(profile_));
-  } else {
-    registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_LOADED,
-                      content::Source<Profile>(profile_));
+    history_service_->RemoveHistoryServiceObserver(this);
   }
 
   if (tab_restore_service_)
@@ -141,22 +135,6 @@ HistoryMenuBridge::~HistoryMenuBridge() {
 void HistoryMenuBridge::Observe(int type,
                                 const content::NotificationSource& source,
                                 const content::NotificationDetails& details) {
-  // A history service is now ready. Check to see if it's the one for the main
-  // profile. If so, perform final initialization.
-  if (type == chrome::NOTIFICATION_HISTORY_LOADED) {
-    HistoryService* hs = HistoryServiceFactory::GetForProfile(
-        profile_, Profile::EXPLICIT_ACCESS);
-    if (hs != NULL && hs->BackendLoaded()) {
-      history_service_ = hs;
-      Init();
-
-      // Found our HistoryService, so stop listening for this notification.
-      registrar_.Remove(this,
-                        chrome::NOTIFICATION_HISTORY_LOADED,
-                        content::Source<Profile>(profile_));
-    }
-  }
-
   // All other notification types that we observe indicate that the history has
   // changed and we need to rebuild.
   need_recreate_ = true;
@@ -472,3 +450,13 @@ void HistoryMenuBridge::CancelFaviconRequest(HistoryItem* item) {
     item->icon_task_id = base::CancelableTaskTracker::kBadTaskId;
   }
 }
+
+void HistoryMenuBridge::HistoryServiceLoaded(HistoryService* history_service) {
+  HistoryService* hs =
+      HistoryServiceFactory::GetForProfile(profile_, Profile::EXPLICIT_ACCESS);

[sdefresne, 2014/10/20 13:15:42]

No need to use HistoryServiceFactory to get the HistoryService, it is passed as
parameter to the function, just use it.

DCHECK(history_service);
DCHECK(history_service->BackendLoaded());
history_service->RemoveObserver(this);
history_service_ = history_service;
Init();

[nshaik, 2014/10/29 08:43:39]

Okay. The only reason why I did this was to ensure I received notification for
the right profile.
I was trying to write the code in line with the comment present before "A
history service is now ready. Check to see if it's the one for the main
profile."
Implemented it the way you suggested.

+  if (hs != NULL && hs->BackendLoaded()) {
+    history_service_ = hs;
+    Init();
+    history_service_->RemoveHistoryServiceObserver(this);
+  }
+}