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 f1095a764614bb4d99d27543c830db8abc1aef14..bdebb364a930d79219f60287db30b23fdbb1e274 100644
--- a/chrome/browser/ui/cocoa/history_menu_bridge.mm
+++ b/chrome/browser/ui/cocoa/history_menu_bridge.mm
@@ -65,7 +65,8 @@ HistoryMenuBridge::HistoryMenuBridge(Profile* profile)
       history_service_(NULL),
       tab_restore_service_(NULL),
       create_in_progress_(false),
-      need_recreate_(false) {
+      need_recreate_(false),
+      history_service_observer_(this) {
   // If we don't have a profile, do not bother initializing our data sources.
   // This shouldn't happen except in unit tests.
   if (profile_) {
@@ -74,9 +75,11 @@ HistoryMenuBridge::HistoryMenuBridge(Profile* profile)
     // for a notification that tells us the HistoryService is ready.
     HistoryService* hs = HistoryServiceFactory::GetForProfile(
         profile_, Profile::EXPLICIT_ACCESS);
-    if (hs != NULL && hs->BackendLoaded()) {
+    if (hs && hs->BackendLoaded()) {

[Peter Kasting, 2014/11/17 18:45:31]

Nit: Longer, but seems a little clearer to me:

    if (hs) {
      if (hs->BackendLoaded()) {
        history_service_ = hs;
        Init();
      } else {
        history_service_observer_.Add(hs);
      }
    }

[nshaik, 2014/11/17 23:24:29]

Done.

       history_service_ = hs;
       Init();
+    } else if (hs) {
+      history_service_observer_.Add(hs);
     }
 
     tab_restore_service_ = TabRestoreServiceFactory::GetForProfile(profile_);
@@ -100,12 +103,6 @@ HistoryMenuBridge::HistoryMenuBridge(Profile* profile)
   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
@@ -117,10 +114,6 @@ HistoryMenuBridge::~HistoryMenuBridge() {
   if (history_service_) {
     registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_URLS_DELETED,
                       content::Source<Profile>(profile_));
-    history_service_->RemoveObserver(this);
-  } else {
-    registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_LOADED,
-                      content::Source<Profile>(profile_));
   }
 
   if (tab_restore_service_)
@@ -138,22 +131,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.

[Peter Kasting, 2014/11/17 18:45:32]

Nit: Please update the comment ("other" is now confusing).

[nshaik, 2014/11/17 23:24:29]

Done.

   OnHistoryChanged();
@@ -283,6 +260,14 @@ void HistoryMenuBridge::OnURLsModified(HistoryService* history_service,
   OnHistoryChanged();
 }
 
+void HistoryMenuBridge::OnHistoryServiceLoaded(
+    HistoryService* history_service) {
+  DCHECK(history_service);
+  DCHECK(history_service->BackendLoaded());

[Peter Kasting, 2014/11/17 18:45:31]

Nit: I dunno how valuable these DCHECKs are (doesn't the function name sort of
imply them?)

[nshaik, 2014/11/17 23:24:29]

Done.

+  history_service_ = history_service;
+  Init();
+}
+
 HistoryMenuBridge::HistoryItem* HistoryMenuBridge::HistoryItemForMenuItem(
     NSMenuItem* item) {
   std::map<NSMenuItem*, HistoryItem*>::iterator it = menu_item_map_.find(item);