Refactor HistoryService to not send NOTIFICATION_FAVICON_CHANGED

chrome/browser/history/android/android_provider_backend.cc

 // 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/history/android/android_provider_backend.h"
 
 #include "base/i18n/case_conversion.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/favicon/favicon_changed_details.h"
 #include "chrome/browser/history/android/android_time.h"
 #include "chrome/browser/history/android/android_urls_sql_handler.h"
 #include "chrome/browser/history/android/bookmark_model_sql_handler.h"
 #include "chrome/browser/history/android/favicon_sql_handler.h"
 #include "chrome/browser/history/android/urls_sql_handler.h"
 #include "chrome/browser/history/android/visit_sql_handler.h"
 #include "chrome/browser/history/history_backend.h"
 #include "chrome/browser/history/history_database.h"
 #include "chrome/browser/history/thumbnail_database.h"
 #include "components/history/core/browser/history_client.h"
@@ skipping 88 matching lines @@
         return false;
 
       if (row.last_visit_time().ToInternalValue() -
           row.created().ToInternalValue() < row.visit_count())
         return false;
     }
   }
   return true;
 }
 
+void RunNotifyFaviconChanged(HistoryBackend::Delegate* delegate,
+                             scoped_ptr<std::set<GURL>> changed_favicons) {

[brettw, 2014/09/02 23:23:10]

Put space in >> (still no C++11 in codebase). Same below.

[sdefresne, 2014/09/03 10:00:01]

Done.

+  delegate->NotifyFaviconChanged(*changed_favicons);
+}
+
 }  // namespace
 
-
-// AndroidProviderBackend::HistoryNotifications -------------------------------
-
-AndroidProviderBackend::HistoryNotifications::HistoryNotifications() {
-}
-
-AndroidProviderBackend::HistoryNotifications::~HistoryNotifications() {
-}
-
-void AndroidProviderBackend::HistoryNotifications::PushBack(
-    int type,
-    scoped_ptr<HistoryDetails> detail) {
-  DCHECK_EQ(types_.size(), details_.size());
-  types_.push_back(type);
-  details_.push_back(detail.release());
-}
-
-int AndroidProviderBackend::HistoryNotifications::PopBackType() {
-  DCHECK(!empty());
-  int type = types_.back();
-  types_.pop_back();
-  return type;
-}
-
-scoped_ptr<HistoryDetails>
-    AndroidProviderBackend::HistoryNotifications::PopBackDetails() {
-  DCHECK(!details_.empty());
-  scoped_ptr<HistoryDetails> detail(details_.back());
-  details_.weak_erase(details_.end() - 1);
-  return detail.Pass();
-}
-
-
 // AndroidProviderBackend::ScopedTransaction ----------------------------------
 
 AndroidProviderBackend::ScopedTransaction::ScopedTransaction(
     HistoryDatabase* history_db,
     ThumbnailDatabase* thumbnail_db)
     : history_db_(history_db),
       thumbnail_db_(thumbnail_db),
       committed_(false),
       history_transaction_nesting_(history_db_->transaction_nesting()),
       thumbnail_transaction_nesting_(
@@ skipping 194 matching lines @@
   for (std::vector<SQLHandler*>::iterator i =
        sql_handlers_.begin(); i != sql_handlers_.end(); ++i) {
     if ((*i)->HasColumnIn(row)) {
       if (!(*i)->Update(row, ids_set))
         return false;
     }
   }
   *updated_count = ids_set.size();
 
   scoped_ptr<URLsModifiedDetails> modified(new URLsModifiedDetails);
-  scoped_ptr<FaviconChangedDetails> favicon(new FaviconChangedDetails);
+  scoped_ptr<std::set<GURL>> favicon(new std::set<GURL>);
 
   for (TableIDRows::const_iterator i = ids_set.begin(); i != ids_set.end();
        ++i) {
     if (row.is_value_set_explicitly(HistoryAndBookmarkRow::TITLE) ||
         row.is_value_set_explicitly(HistoryAndBookmarkRow::VISIT_COUNT) ||
         row.is_value_set_explicitly(HistoryAndBookmarkRow::LAST_VISIT_TIME)) {
       URLRow url_row;
       if (!history_db_->GetURLRow(i->url_id, &url_row))
         return false;
       modified->changed_urls.push_back(url_row);
     }
     if (thumbnail_db_ &&
         row.is_value_set_explicitly(HistoryAndBookmarkRow::FAVICON))
-      favicon->urls.insert(i->url);
+      favicon->insert(i->url);
   }
 
   if (!modified->changed_urls.empty()) {
-    notifications->PushBack(chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
-                            modified.PassAs<HistoryDetails>());
+    scoped_ptr<HistoryDetails> details = modified.PassAs<HistoryDetails>();
+    notifications->push_back(
+        base::Bind(&HistoryBackend::Delegate::BroadcastNotifications,
+                   base::Unretained(delegate_),
+                   chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
+                   base::Passed(&details)));
   }
 
-  if (!favicon->urls.empty()) {
-    notifications->PushBack(chrome::NOTIFICATION_FAVICON_CHANGED,
-                            favicon.PassAs<HistoryDetails>());
+  if (!favicon->empty()) {
+    notifications->push_back(base::Bind(&RunNotifyFaviconChanged,
+                                        base::Unretained(delegate_),
+                                        base::Passed(&favicon)));
   }
 
   return true;
 }
 
 AndroidURLID AndroidProviderBackend::InsertHistoryAndBookmark(
     const HistoryAndBookmarkRow& values,
     bool ensure_initialized_and_updated,
     HistoryNotifications* notifications) {
   if (!IsHistoryAndBookmarkRowValid(values))
     return false;
 
   if (ensure_initialized_and_updated && !EnsureInitializedAndUpdated())
     return 0;
 
   DCHECK(values.is_value_set_explicitly(HistoryAndBookmarkRow::URL));
   // Make a copy of values as we need change it during insert.
   HistoryAndBookmarkRow row = values;
   for (std::vector<SQLHandler*>::iterator i =
        sql_handlers_.begin(); i != sql_handlers_.end(); ++i) {
     if (!(*i)->Insert(&row))
       return 0;
   }
 
   URLRow url_row;
   if (!history_db_->GetURLRow(row.url_id(), &url_row))
     return false;
 
   scoped_ptr<URLsModifiedDetails> modified(new URLsModifiedDetails);
-  if (!modified.get())
-    return false;
   modified->changed_urls.push_back(url_row);
 
-  scoped_ptr<FaviconChangedDetails> favicon;
+  scoped_ptr<std::set<GURL>> favicon;
   // No favicon should be changed if the thumbnail_db_ is not available.
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::FAVICON) &&
       row.favicon_valid() && thumbnail_db_) {
-    favicon.reset(new FaviconChangedDetails);
-    if (!favicon.get())
-      return false;
-    favicon->urls.insert(url_row.url());
+    favicon.reset(new std::set<GURL>);
+    favicon->insert(url_row.url());
   }
 
-  notifications->PushBack(chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
-                          modified.PassAs<HistoryDetails>());
+  scoped_ptr<HistoryDetails> details = modified.PassAs<HistoryDetails>();
+  notifications->push_back(
+      base::Bind(&HistoryBackend::Delegate::BroadcastNotifications,
+                 base::Unretained(delegate_),
+                 chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
+                 base::Passed(&details)));
+
   if (favicon) {
-    notifications->PushBack(chrome::NOTIFICATION_FAVICON_CHANGED,
-                            favicon.PassAs<HistoryDetails>());
+    DCHECK(!favicon->empty());
+    notifications->push_back(base::Bind(&RunNotifyFaviconChanged,
+                                        base::Unretained(delegate_),
+                                        base::Passed(&favicon)));
   }
 
   return row.id();
 }
 
 bool AndroidProviderBackend::DeleteHistoryAndBookmarks(
     const std::string& selection,
     const std::vector<base::string16>& selection_args,
     int * deleted_count,
     HistoryNotifications* notifications) {
@@ skipping 553 matching lines @@
 
   HistoryAndBookmarkRow new_row;
   new_row.set_last_visit_time(FromDatabaseTime(
       statement->statement()->ColumnInt64(0)));
   new_row.set_created(FromDatabaseTime(
       statement->statement()->ColumnInt64(1)));
   new_row.set_visit_count(statement->statement()->ColumnInt(2));
   new_row.set_title(statement->statement()->ColumnString16(3));
 
   scoped_ptr<URLsDeletedDetails> deleted_details(new URLsDeletedDetails);
-  scoped_ptr<FaviconChangedDetails> favicon_details(new FaviconChangedDetails);
+  scoped_ptr<std::set<GURL>> favicons(new std::set<GURL>);
   scoped_ptr<URLsModifiedDetails> modified(new URLsModifiedDetails);
   URLRow old_url_row;
   if (!history_db_->GetURLRow(ids[0].url_id, &old_url_row))
     return false;
   deleted_details->rows.push_back(old_url_row);
 
   favicon_base::FaviconID favicon_id = statement->statement()->ColumnInt64(4);
   if (favicon_id) {
     std::vector<FaviconBitmap> favicon_bitmaps;
     if (!thumbnail_db_ ||
         !thumbnail_db_->GetFaviconBitmaps(favicon_id, &favicon_bitmaps))
       return false;
    scoped_refptr<base::RefCountedMemory> bitmap_data =
        favicon_bitmaps[0].bitmap_data;
    if (bitmap_data.get() && bitmap_data->size())
       new_row.set_favicon(bitmap_data);
-    favicon_details->urls.insert(old_url_row.url());
-    favicon_details->urls.insert(row.url());
+   favicons->insert(old_url_row.url());
+   favicons->insert(row.url());
   }
   new_row.set_is_bookmark(statement->statement()->ColumnBool(5));
 
   // The SQLHandler vector is not used here because the row in android_url
   // shouldn't be deleted, we need keep the AndroidUIID unchanged, so it
   // appears update to the client.
   if (!urls_handler_->Delete(ids))
     return false;
 
   if (!visit_handler_->Delete(ids))
@@ skipping 10 matching lines @@
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::LAST_VISIT_TIME))
     new_row.set_last_visit_time(row.last_visit_time());
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::CREATED))
     new_row.set_created(row.created());
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::VISIT_COUNT))
     new_row.set_visit_count(row.visit_count());
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::TITLE))
     new_row.set_title(row.title());
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::FAVICON)) {
     new_row.set_favicon(row.favicon());
-    favicon_details->urls.insert(new_row.url());
+    favicons->insert(new_row.url());
   }
   if (row.is_value_set_explicitly(HistoryAndBookmarkRow::BOOKMARK))
     new_row.set_is_bookmark(row.is_bookmark());
 
   if (!urls_handler_->Insert(&new_row))
     return false;
 
   if (!visit_handler_->Insert(&new_row))
     return false;
 
   // Update the current row instead of inserting a new row in android urls
   // table. We need keep the AndroidUIID unchanged, so it appears update
   // to the client.
   if (!android_urls_handler_->Update(new_row, ids))
     return false;
 
   if (favicon_handler_ && !favicon_handler_->Insert(&new_row))
     return false;
 
   if (!bookmark_model_handler_->Insert(&new_row))
     return false;
 
   URLRow new_url_row;
   if (!history_db_->GetURLRow(new_row.url_id(), &new_url_row))
     return false;
 
   modified->changed_urls.push_back(new_url_row);
 
-  notifications->PushBack(chrome::NOTIFICATION_HISTORY_URLS_DELETED,
-                          deleted_details.PassAs<HistoryDetails>());
-  if (favicon_details && !favicon_details->urls.empty()) {
-    notifications->PushBack(chrome::NOTIFICATION_FAVICON_CHANGED,
-                            favicon_details.PassAs<HistoryDetails>());
+  scoped_ptr<HistoryDetails> details = deleted_details.PassAs<HistoryDetails>();
+  notifications->push_back(
+      base::Bind(&HistoryBackend::Delegate::BroadcastNotifications,
+                 base::Unretained(delegate_),
+                 chrome::NOTIFICATION_HISTORY_URLS_DELETED,
+                 base::Passed(&details)));
+  if (favicons && !favicons->empty()) {
+    notifications->push_back(base::Bind(&RunNotifyFaviconChanged,
+                                        base::Unretained(delegate_),
+                                        base::Passed(&favicons)));
   }
-  notifications->PushBack(chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
-                          modified.PassAs<HistoryDetails>());
+  scoped_ptr<HistoryDetails> other_details = modified.PassAs<HistoryDetails>();
+  notifications->push_back(
+      base::Bind(&HistoryBackend::Delegate::BroadcastNotifications,
+                 base::Unretained(delegate_),
+                 chrome::NOTIFICATION_HISTORY_URLS_MODIFIED,
+                 base::Passed(&other_details)));
 
   return true;
 }
 
 AndroidStatement* AndroidProviderBackend::QueryHistoryAndBookmarksInternal(
     const std::vector<HistoryAndBookmarkRow::ColumnID>& projections,
     const std::string& selection,
     const std::vector<base::string16>& selection_args,
     const std::string& sort_order) {
   std::string sql;
@@ skipping 23 matching lines @@
   }
   sql::Statement* result = statement.release();
   return new AndroidStatement(result, replaced_index);
 }
 
 bool AndroidProviderBackend::DeleteHistoryInternal(
     const TableIDRows& urls,
     bool delete_bookmarks,
     HistoryNotifications* notifications) {
   scoped_ptr<URLsDeletedDetails> deleted_details(new URLsDeletedDetails);
-  scoped_ptr<FaviconChangedDetails> favicon(new FaviconChangedDetails);
+  scoped_ptr<std::set<GURL>> favicon(new std::set<GURL>);
   for (TableIDRows::const_iterator i = urls.begin(); i != urls.end(); ++i) {
     URLRow url_row;
     if (!history_db_->GetURLRow(i->url_id, &url_row))
       return false;
     deleted_details->rows.push_back(url_row);
     if (thumbnail_db_ &&
         thumbnail_db_->GetIconMappingsForPageURL(url_row.url(), NULL))
-      favicon->urls.insert(url_row.url());
+      favicon->insert(url_row.url());
   }
 
   // Only invoke Delete on the BookmarkModelHandler if we need
   // to delete bookmarks.
   for (std::vector<SQLHandler*>::iterator i =
        sql_handlers_.begin(); i != sql_handlers_.end(); ++i) {
     if ((*i) != bookmark_model_handler_.get() || delete_bookmarks)
       if (!(*i)->Delete(urls))
         return false;
   }
 
-  notifications->PushBack(chrome::NOTIFICATION_HISTORY_URLS_DELETED,
-                          deleted_details.PassAs<HistoryDetails>());
-  if (favicon && !favicon->urls.empty()) {
-    notifications->PushBack(chrome::NOTIFICATION_FAVICON_CHANGED,
-                            favicon.PassAs<HistoryDetails>());
+  scoped_ptr<HistoryDetails> details = deleted_details.PassAs<HistoryDetails>();
+  notifications->push_back(
+      base::Bind(&HistoryBackend::Delegate::BroadcastNotifications,
+                 base::Unretained(delegate_),
+                 chrome::NOTIFICATION_HISTORY_URLS_DELETED,
+                 base::Passed(&details)));
+  if (favicon && !favicon->empty()) {
+    notifications->push_back(base::Bind(&RunNotifyFaviconChanged,
+                                        base::Unretained(delegate_),
+                                        base::Passed(&favicon)));
   }
   return true;
 }
 
 void AndroidProviderBackend::BroadcastNotifications(
     HistoryNotifications* notifications) {
   while (!notifications->empty()) {
-    delegate_->BroadcastNotifications(notifications->PopBackType(),
-                                      notifications->PopBackDetails());
+    notifications->back().Run();
+    notifications->pop_back();
   }
 }
 
 bool AndroidProviderBackend::AddSearchTerm(const SearchRow& values) {
   DCHECK(values.is_value_set_explicitly(SearchRow::SEARCH_TERM));
   DCHECK(values.is_value_set_explicitly(SearchRow::KEYWORD_ID));
   DCHECK(values.is_value_set_explicitly(SearchRow::URL));
 
   URLRow url_row;
   HistoryAndBookmarkRow bookmark_row;
@@ skipping 35 matching lines @@
       return false;
 
     if (!history_db_->SetKeywordSearchTermsForURL(bookmark_row.url_id(),
                           values.keyword_id(), values.search_term()))
       return false;
   }
   return true;
 }
 
 }  // namespace history