Update all bookmarks which use an icon URL when a favicon's bitmap is updated

components/bookmarks/browser/bookmark_model.cc

Index: components/bookmarks/browser/bookmark_model.cc
diff --git a/components/bookmarks/browser/bookmark_model.cc b/components/bookmarks/browser/bookmark_model.cc
index d781fdbe8242beed9270e347d1384cdcdddec876..8439e35ef8bf274664f6fdb675af620a0917feed 100644
--- a/components/bookmarks/browser/bookmark_model.cc
+++ b/components/bookmarks/browser/bookmark_model.cc
@@ -42,6 +42,20 @@ BookmarkPermanentNode* AsMutable(const BookmarkPermanentNode* node) {
   return const_cast<BookmarkPermanentNode*>(node);
 }
 
+// Removes |node| from |set|. This method is useful when the multiset has a
+// custom comparator.
+template<class T> void RemoveFromMultiset(
+    std::multiset<BookmarkNode*, T>* set,
+    BookmarkNode* node) {
+  std::multiset<BookmarkNode*>::iterator it = set->find(node);
+  if (it == set->end())
+    return;
+
+  while (*it != node)
+    ++it;
+  set->erase(it);
+}
+
 // Comparator used when sorting permanent nodes. Nodes that are initially
 // visible are sorted before nodes that are initially hidden.
 class VisibilityComparator
@@ -346,8 +360,7 @@ void BookmarkModel::SetURL(const BookmarkNode* node, const GURL& url) {
     return;
 
   BookmarkNode* mutable_node = AsMutable(node);
-  mutable_node->InvalidateFavicon();
-  CancelPendingFaviconLoadRequests(mutable_node);
+  InvalidateFavicon(mutable_node);
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     OnWillChangeBookmarkNode(this, node));
@@ -435,24 +448,34 @@ void BookmarkModel::SetNodeSyncTransactionVersion(
     store_->ScheduleSave();
 }
 
-void BookmarkModel::OnFaviconChanged(const std::set<GURL>& urls) {
+void BookmarkModel::OnFaviconsChanged(const std::vector<GURL>& page_urls,
+                                      const std::vector<GURL>& icon_urls) {
   // Ignore events if |Load| has not been called yet.
   if (!store_)
     return;
 
-  // Prevent the observers from getting confused for multiple favicon loads.
-  for (std::set<GURL>::const_iterator i = urls.begin(); i != urls.end(); ++i) {
+  std::set<const BookmarkNode*> to_update;
+  for (const GURL& page_url : page_urls) {
     std::vector<const BookmarkNode*> nodes;
-    GetNodesByURL(*i, &nodes);
-    for (size_t i = 0; i < nodes.size(); ++i) {
-      // Got an updated favicon, for a URL, do a new request.
-      BookmarkNode* node = AsMutable(nodes[i]);
-      node->InvalidateFavicon();
-      CancelPendingFaviconLoadRequests(node);
-      FOR_EACH_OBSERVER(BookmarkModelObserver,
-                        observers_,
-                        BookmarkNodeFaviconChanged(this, node));
-    }
+    GetNodesByURL(page_url, &nodes);
+    for (const BookmarkNode* node : nodes)
+      to_update.insert(node);
+  }
+
+  for (const GURL& icon_url : icon_urls) {
+    std::vector<const BookmarkNode*> nodes;
+    GetNodesByIconURL(icon_url, &nodes);
+    for (const BookmarkNode* node : nodes)
+      to_update.insert(node);
+  }
+
+  for (const BookmarkNode* node : to_update) {
+    // Rerequest the favicon.
+    BookmarkNode* mutable_node = AsMutable(node);
+    InvalidateFavicon(mutable_node);
+    FOR_EACH_OBSERVER(BookmarkModelObserver,
+                      observers_,
+                      BookmarkNodeFaviconChanged(this, node));
   }
 }
 
@@ -484,6 +507,19 @@ void BookmarkModel::GetNodesByURL(const GURL& url,
   }
 }
 
+void BookmarkModel::GetNodesByIconURL(const GURL& icon_url,
+                                      std::vector<const BookmarkNode*>* nodes) {
+  BookmarkNode tmp_node = BookmarkNode(GURL());
+  tmp_node.set_icon_url(icon_url);
+  NodesOrderedByURLSet::iterator i =
+      nodes_ordered_by_favicon_url_set_.find(&tmp_node);
+  while (i != nodes_ordered_by_favicon_url_set_.end() &&
+         (*i)->icon_url() == icon_url) {
+    nodes->push_back(*i);
+    ++i;
+  }
+}
+
 const BookmarkNode* BookmarkModel::GetMostRecentlyAddedUserNodeForURL(
     const GURL& url) {
   std::vector<const BookmarkNode*> nodes;
@@ -850,16 +886,12 @@ void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) {
 
 void BookmarkModel::RemoveNodeFromInternalMaps(BookmarkNode* node) {
   index_->Remove(node);
+  if (!node->icon_url().is_empty())
+    RemoveFromMultiset(&nodes_ordered_by_favicon_url_set_, node);
   // NOTE: this is called in such a way that url_lock_ is already held. As
   // such, this doesn't explicitly grab the lock.
   url_lock_.AssertAcquired();
-  NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(node);
-  DCHECK(i != nodes_ordered_by_url_set_.end());
-  // i points to the first node with the URL, advance until we find the
-  // node we're removing.
-  while (*i != node)
-    ++i;
-  nodes_ordered_by_url_set_.erase(i);
+  RemoveFromMultiset(&nodes_ordered_by_url_set_, node);
 }
 
 void BookmarkModel::RemoveNodeAndGetRemovedUrls(BookmarkNode* node,
@@ -911,6 +943,8 @@ BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent,
 
 void BookmarkModel::AddNodeToInternalMaps(BookmarkNode* node) {
   index_->Add(node);
+  if (!node->icon_url().is_empty())
+    nodes_ordered_by_favicon_url_set_.insert(node);
   url_lock_.AssertAcquired();
   nodes_ordered_by_url_set_.insert(node);
 }
@@ -961,9 +995,10 @@ void BookmarkModel::OnFaviconDataAvailable(
   node->set_favicon_load_task_id(base::CancelableTaskTracker::kBadTaskId);
   node->set_favicon_state(BookmarkNode::LOADED_FAVICON);
   if (!image_result.image.IsEmpty()) {
+    SetFaviconURL(node, image_result.icon_url);
     node->set_favicon_type(icon_type);
     node->set_favicon(image_result.image);
-    node->set_icon_url(image_result.icon_url);
+
     FaviconLoaded(node);
   } else if (icon_type == favicon_base::TOUCH_ICON) {
     // Couldn't load the touch icon, fallback to the regular favicon.
@@ -998,6 +1033,22 @@ void BookmarkModel::FaviconLoaded(const BookmarkNode* node) {
                     BookmarkNodeFaviconChanged(this, node));
 }
 
+void BookmarkModel::SetFaviconURL(BookmarkNode* node, const GURL& icon_url) {
+  if (!node->icon_url().is_empty())
+    RemoveFromMultiset(&nodes_ordered_by_favicon_url_set_, node);
+  node->set_icon_url(icon_url);
+  if (!node->icon_url().is_empty())
+    nodes_ordered_by_favicon_url_set_.insert(node);
+}
+
+void BookmarkModel::InvalidateFavicon(BookmarkNode* node) {
+  SetFaviconURL(node, GURL());
+  node->set_favicon(gfx::Image());
+  node->set_favicon_type(favicon_base::INVALID_ICON);
+  node->set_favicon_state(BookmarkNode::INVALID_FAVICON);
+  CancelPendingFaviconLoadRequests(node);
+}
+
 void BookmarkModel::CancelPendingFaviconLoadRequests(BookmarkNode* node) {
   if (node->favicon_load_task_id() != base::CancelableTaskTracker::kBadTaskId) {
     cancelable_task_tracker_.TryCancel(node->favicon_load_task_id());