Eliminate NOTIFICATION_HISTORY_LOADED notification

chrome/browser/predictors/resource_prefetch_predictor.cc

Index: chrome/browser/predictors/resource_prefetch_predictor.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.cc b/chrome/browser/predictors/resource_prefetch_predictor.cc
index 878113d3faa552f34d35eb178acd072970c99778..21f8129bc427cd0851f91404d2a8a5d16142bd30 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor.cc
@@ -305,9 +305,10 @@ ResourcePrefetchPredictor::ResourcePrefetchPredictor(
     : profile_(profile),
       config_(config),
       initialization_state_(NOT_INITIALIZED),
-      tables_(PredictorDatabaseFactory::GetForProfile(
-          profile)->resource_prefetch_tables()),
-      results_map_deleter_(&results_map_) {
+      tables_(PredictorDatabaseFactory::GetForProfile(profile)
+                  ->resource_prefetch_tables()),
+      results_map_deleter_(&results_map_),
+      history_service_observer_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Some form of learning has to be enabled.
@@ -398,15 +399,6 @@ void ResourcePrefetchPredictor::Observe(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   switch (type) {
-    case chrome::NOTIFICATION_HISTORY_LOADED: {
-      DCHECK_EQ(initialization_state_, INITIALIZING);
-      notification_registrar_.Remove(this,
-                                     chrome::NOTIFICATION_HISTORY_LOADED,
-                                     content::Source<Profile>(profile_));
-      OnHistoryAndCacheLoaded();
-      break;
-    }
-
     case chrome::NOTIFICATION_HISTORY_URLS_DELETED: {
       DCHECK_EQ(initialization_state_, INITIALIZED);
       const content::Details<const history::URLsDeletedDetails>
@@ -694,15 +686,7 @@ void ResourcePrefetchPredictor::CreateCaches(
   UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.HostTableHostCount",
                        host_table_cache_->size());
 
-  // Add notifications for history loading if it is not ready.
-  HistoryService* history_service = HistoryServiceFactory::GetForProfile(
-    profile_, Profile::EXPLICIT_ACCESS);
-  if (!history_service) {
-    notification_registrar_.Add(this, chrome::NOTIFICATION_HISTORY_LOADED,
-                                content::Source<Profile>(profile_));
-  } else {
-    OnHistoryAndCacheLoaded();
-  }
+  CheckForHistoryService();
 }
 
 void ResourcePrefetchPredictor::OnHistoryAndCacheLoaded() {
@@ -719,6 +703,7 @@ void ResourcePrefetchPredictor::OnHistoryAndCacheLoaded() {
         this, config_, profile_->GetRequestContext());
   }
 
+  history_service_observer_.RemoveAll();
   initialization_state_ = INITIALIZED;
 }
 
@@ -1221,4 +1206,25 @@ void ResourcePrefetchPredictor::ReportPredictedAccuracyStatsHelper(
 #undef RPP_PREDICTED_HISTOGRAM_COUNTS
 }
 
+bool ResourcePrefetchPredictor::CheckForHistoryService() {
+  // Register for HistoryServiceLoading if it is not ready.
+  HistoryService* history_service =
+      HistoryServiceFactory::GetForProfile(profile_, Profile::EXPLICIT_ACCESS);

[sdefresne, 2014/11/03 19:14:13]

nit: it would be simpler to test whether history_service is not NULL here,
something like

if (!history_service)
  return false;
if (history_service->BackendLoaded()) {
  OnHistoryAndCacheLoaded();
  return true;
}
DCHECK(!history_service_observer_.IsObserving(history_service));
history_service_observer_.Add(history_service);
return false;

[nshaik, 2014/11/04 00:13:05]

Done.

+  if (history_service && history_service->BackendLoaded()) {
+    // HistoryService is already loaded. Continue with Initialization.
+    OnHistoryAndCacheLoaded();
+    return true;
+  } else if (history_service &&

[sdefresne, 2014/11/03 19:14:13]

not: no need for else here since the if branch always return

[nshaik, 2014/11/04 00:13:05]

Done.

+             !history_service_observer_.IsObserving(history_service)) {

[sdefresne, 2014/11/03 19:14:13]

nit: ResourcePrefetchPredictor::CreateCaches asserts that it is only called
once, so ResourcePrefetchPredictor::CheckForHistoryService can only be called
once, and this test will always be true, so use DCHECK instead.

[nshaik, 2014/11/04 00:13:05]

Done.

+    history_service_observer_.Add(history_service);
+  }
+  return false;
+}
+
+void ResourcePrefetchPredictor::OnHistoryServiceLoaded(
+    HistoryService* history_service) {
+  OnHistoryAndCacheLoaded();
+  history_service_observer_.Remove(history_service);
+}
+
 }  // namespace predictors