Eliminate NOTIFICATION_HISTORY_LOADED notification

chrome/browser/ui/cocoa/history_menu_bridge.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/history_menu_bridge.h"
 
 #include "base/bind.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ skipping 59 matching lines @@
   // This shouldn't happen except in unit tests.
   if (profile_) {
     // Check to see if the history service is ready. Because it loads async, it
     // may not be ready when the Bridge is created. If this happens, register
     // for a notification that tells us the HistoryService is ready.
     HistoryService* hs = HistoryServiceFactory::GetForProfile(
         profile_, Profile::EXPLICIT_ACCESS);
     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_);
     if (tab_restore_service_) {
       tab_restore_service_->AddObserver(this);
       // If the tab entries are already loaded, invoke the observer method to
       // build the "Recently Closed" section. Otherwise it will be when the
       // backend loads.
       if (!tab_restore_service_->IsLoaded())
         tab_restore_service_->LoadTabsFromLastSession();
       else
         TabRestoreServiceChanged(tab_restore_service_);
     }
   }
 
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   default_favicon_.reset(
       rb.GetNativeImageNamed(IDR_DEFAULT_FAVICON).CopyNSImage());
 
   // 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
 // service will be automatically cancelled by their respective Consumers, so
 // task cancellation is not done manually here in the dtor.
 HistoryMenuBridge::~HistoryMenuBridge() {
   // Unregister ourselves as observers and notifications.
   DCHECK(profile_);
   if (history_service_) {
     registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
                       content::Source<Profile>(profile_));
     registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_URL_VISITED,
                       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_)
     tab_restore_service_->RemoveObserver(this);
 
   // Since the map owns the HistoryItems, delete anything that still exists.
   std::map<NSMenuItem*, HistoryItem*>::iterator it = menu_item_map_.begin();
   while (it != menu_item_map_.end()) {
     HistoryItem* item  = it->second;
     menu_item_map_.erase(it++);
     delete item;
   }
 }
 
 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;
   CreateMenu();
 }
 
 void HistoryMenuBridge::TabRestoreServiceChanged(TabRestoreService* service) {
   const TabRestoreService::Entries& entries = service->entries();
 
   // Clear the history menu before rebuilding.
@@ skipping 295 matching lines @@
 }
 
 void HistoryMenuBridge::CancelFaviconRequest(HistoryItem* item) {
   DCHECK(item);
   if (item->icon_requested) {
     cancelable_task_tracker_.TryCancel(item->icon_task_id);
     item->icon_requested = false;
     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);
+  }
+}