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

Unified Diff: chrome/browser/ui/cocoa/history_menu_bridge.mm

Issue 573553004: Eliminate NOTIFICATION_HISTORY_LOADED notification (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Move HistoryServiceObserver functions to private Created 6 years, 1 month 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/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:
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 con
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
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);

Powered by Google App Engine
This is Rietveld 408576698