Remove NOTIFICATION_HISTORY_URLS_DELETED

chrome/browser/autocomplete/shortcuts_backend.cc

Index: chrome/browser/autocomplete/shortcuts_backend.cc
diff --git a/chrome/browser/autocomplete/shortcuts_backend.cc b/chrome/browser/autocomplete/shortcuts_backend.cc
index 62ebaf53b15312c841f7830ff14f102245a261dd..0a228f86bd560d7bf9f86e3297c8915b137ff86f 100644
--- a/chrome/browser/autocomplete/shortcuts_backend.cc
+++ b/chrome/browser/autocomplete/shortcuts_backend.cc
@@ -13,9 +13,8 @@
 #include "base/guid.h"
 #include "base/i18n/case_conversion.h"
 #include "base/strings/string_util.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/history/history_notifications.h"
 #include "chrome/browser/history/history_service.h"
+#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/shortcuts_database.h"
 #include "chrome/browser/omnibox/omnibox_log.h"
 #include "chrome/browser/profiles/profile.h"
@@ -79,7 +78,8 @@ AutocompleteMatch::Type GetTypeForShortcut(AutocompleteMatch::Type type) {
 ShortcutsBackend::ShortcutsBackend(Profile* profile, bool suppress_db)
     : profile_(profile),
       current_state_(NOT_INITIALIZED),
-      no_db_access_(suppress_db) {
+      no_db_access_(suppress_db),
+      history_service_observer_(this) {
   if (!suppress_db) {
     db_ = new history::ShortcutsDatabase(
         profile->GetPath().Append(chrome::kShortcutsDatabaseName));
@@ -92,9 +92,10 @@ ShortcutsBackend::ShortcutsBackend(Profile* profile, bool suppress_db)
         extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED,
         content::Source<Profile>(profile));
 #endif
-    notification_registrar_.Add(
-        this, chrome::NOTIFICATION_HISTORY_URLS_DELETED,
-        content::Source<Profile>(profile));
+    HistoryService* hs =
+        HistoryServiceFactory::GetForProfile(profile, Profile::EXPLICIT_ACCESS);
+    if (hs)
+      history_service_observer_.Add(hs);
   }
 }
 
@@ -174,42 +175,23 @@ void ShortcutsBackend::ShutdownOnUIThread() {
   DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::UI));
   notification_registrar_.RemoveAll();
+  history_service_observer_.RemoveAll();
 }
 
 void ShortcutsBackend::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
+#if defined(ENABLE_EXTENSIONS)
+  DCHECK_EQ(type, extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED);

[Peter Kasting, 2014/12/11 19:59:34]

Nit: (expected, actual)

[nshaik, 2014/12/12 05:40:56]

Done.

   if (!initialized())
     return;
 
-#if defined(ENABLE_EXTENSIONS)
-  if (type == extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED) {
-    // When an extension is unloaded, we want to remove any Shortcuts associated
-    // with it.
-    DeleteShortcutsWithURL(content::Details<extensions::UnloadedExtensionInfo>(
-        details)->extension->url(), false);
-    return;
-  }
+  // When an extension is unloaded, we want to remove any Shortcuts associated
+  // with it.
+  DeleteShortcutsWithURL(content::Details<extensions::UnloadedExtensionInfo>(
+                             details)->extension->url(),
+                         false);

[Peter Kasting, 2014/12/11 19:59:35]

Nit: I think both these are legal, but somehow this looks less weird to me:

  DeleteShortcutsWithURL(
      content::Details<extensions::UnloadedExtensionInfo>(
          details)->extension->url(),
      false);

[nshaik, 2014/12/12 05:40:56]

Done.

 #endif
-
-  DCHECK_EQ(chrome::NOTIFICATION_HISTORY_URLS_DELETED, type);
-  const history::URLsDeletedDetails* deleted_details =
-      content::Details<const history::URLsDeletedDetails>(details).ptr();
-  if (deleted_details->all_history) {
-    DeleteAllShortcuts();
-    return;
-  }
-
-  const history::URLRows& rows(deleted_details->rows);
-  history::ShortcutsDatabase::ShortcutIDs shortcut_ids;
-  for (GuidMap::const_iterator it(guid_map_.begin()); it != guid_map_.end();
-        ++it) {
-    if (std::find_if(
-        rows.begin(), rows.end(), history::URLRow::URLRowHasURL(
-            it->second->second.match_core.destination_url)) != rows.end())
-      shortcut_ids.push_back(it->first);
-  }
-  DeleteShortcutsWithIDs(shortcut_ids);
 }
 
 void ShortcutsBackend::InitInternal() {
@@ -335,3 +317,28 @@ bool ShortcutsBackend::DeleteAllShortcuts() {
                          &history::ShortcutsDatabase::DeleteAllShortcuts),
                      db_.get()));
 }
+
+void ShortcutsBackend::OnURLsDeleted(HistoryService* history_service,
+                                     bool all_history,
+                                     bool expired,
+                                     const history::URLRows& deleted_rows,
+                                     const std::set<GURL>& favicon_urls) {
+  if (!initialized())
+    return;
+  if (all_history) {

[Peter Kasting, 2014/12/11 19:59:34]

Nit: I'd probably put a blank line above this

[nshaik, 2014/12/12 05:40:56]

Done.

+    DeleteAllShortcuts();
+    return;
+  }
+
+  history::ShortcutsDatabase::ShortcutIDs shortcut_ids;
+  for (const auto& guid_pair : guid_map_) {
+    if (std::find_if(
+            deleted_rows.begin(), deleted_rows.end(),
+            history::URLRow::URLRowHasURL(
+                guid_pair.second->second.match_core.destination_url)) !=
+        deleted_rows.end()) {

[Peter Kasting, 2014/12/11 19:59:34]

Nit: {} unnecessary

[nshaik, 2014/12/12 05:40:56]

True. But since the condition is complex, added the braces for better
readability.
Please let me know if you think its not needed.

[Peter Kasting, 2014/12/12 19:06:52]

I don't think it's necessary, but either way is allowed, so it's up to you.

+      shortcut_ids.push_back(guid_pair.first);
+    }
+  }
+  DeleteShortcutsWithIDs(shortcut_ids);
+}