Eliminate NOTIFICATION_HISTORY_LOADED notification

chrome/browser/safe_browsing/incident_reporting/last_download_finder.cc

 // Copyright 2014 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/safe_browsing/incident_reporting/last_download_finder.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 
@@ skipping 94 matching lines @@
 scoped_ptr<LastDownloadFinder> LastDownloadFinder::Create(
     const LastDownloadCallback& callback) {
   scoped_ptr<LastDownloadFinder> finder(make_scoped_ptr(new LastDownloadFinder(
       g_browser_process->profile_manager()->GetLoadedProfiles(), callback)));
   // Return NULL if there is no work to do.
   if (finder->profiles_.empty())
     return scoped_ptr<LastDownloadFinder>();
   return finder.Pass();
 }
 
-LastDownloadFinder::LastDownloadFinder() : weak_ptr_factory_(this) {
+LastDownloadFinder::LastDownloadFinder()
+    : weak_ptr_factory_(this),
+    history_service_observer_(this) {
 }
 
 LastDownloadFinder::LastDownloadFinder(const std::vector<Profile*>& profiles,
                                        const LastDownloadCallback& callback)
-    : callback_(callback), weak_ptr_factory_(this) {
+    : callback_(callback),
+    weak_ptr_factory_(this),
+    history_service_observer_(this) {
   // Observe profile lifecycle events so that the finder can begin or abandon
   // the search in profiles while it is running.
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_PROFILE_ADDED,
                               content::NotificationService::AllSources());
   notification_registrar_.Add(this,
-                              chrome::NOTIFICATION_HISTORY_LOADED,
-                              content::NotificationService::AllSources());
-  notification_registrar_.Add(this,
                               chrome::NOTIFICATION_PROFILE_DESTROYED,
                               content::NotificationService::AllSources());
 
   // Begin the seach for all given profiles.
   std::for_each(
       profiles.begin(),
       profiles.end(),
       std::bind1st(std::mem_fun(&LastDownloadFinder::SearchInProfile), this));
 }
 
@@ skipping 18 matching lines @@
   // No history service is returned for profiles that do not save history.
   if (!history_service)
     return;
 
   profiles_.push_back(profile);
   if (history_service->BackendLoaded()) {
     history_service->QueryDownloads(
         base::Bind(&LastDownloadFinder::OnDownloadQuery,
                    weak_ptr_factory_.GetWeakPtr(),
                    profile));
-  }  // else wait until history is loaded.
+  }  else {
+    // else wait until history is loaded.
+    history_service_observer_.Add(history_service);
+  }
 }
 
 void LastDownloadFinder::OnProfileHistoryLoaded(
     Profile* profile,
     HistoryService* history_service) {
   if (std::find(profiles_.begin(), profiles_.end(), profile) !=
       profiles_.end()) {
     history_service->QueryDownloads(
         base::Bind(&LastDownloadFinder::OnDownloadQuery,
                    weak_ptr_factory_.GetWeakPtr(),
@@ skipping 56 matching lines @@
   }
 }
 
 void LastDownloadFinder::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_PROFILE_ADDED:
       SearchInProfile(content::Source<Profile>(source).ptr());
       break;
-    case chrome::NOTIFICATION_HISTORY_LOADED:
-      OnProfileHistoryLoaded(content::Source<Profile>(source).ptr(),
-                             content::Details<HistoryService>(details).ptr());
-      break;
     case chrome::NOTIFICATION_PROFILE_DESTROYED:
       AbandonSearchInProfile(content::Source<Profile>(source).ptr());

[sdefresne, 2014/10/30 17:37:04]

You need to stop observing the HistoryService associated to the profile here,
since the HistoryService lifetime is bound to the Profile lifetime, hence it
will become invalid after this event has been processed. If LastDownloadFinder
outlives Profile, then the destructor of ScopedObserver will have pointers to
dangling HistoryService and will cause undefined behaviour.

[nshaik, 2014/10/30 21:48:36]

Fixed it. 

       break;
     default:
       break;
   }
 }
 
+void LastDownloadFinder::OnHistoryServiceLoaded(
+    HistoryService* history_service) {
+  OnProfileHistoryLoaded(history_service->profile(), history_service);
+  history_service_observer_.Remove(history_service);
+}
+
 }  // namespace safe_browsing