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

components/bookmarks/browser/bookmark_model.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 "components/bookmarks/browser/bookmark_model.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/bind.h"
@@ skipping 428 matching lines @@
   DCHECK(client_->CanSyncNode(node));
 
   if (sync_transaction_version == node->sync_transaction_version())
     return;
 
   AsMutable(node)->set_sync_transaction_version(sync_transaction_version);
   if (store_.get())
     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);
+    mutable_node->InvalidateFavicon();
+    CancelPendingFaviconLoadRequests(mutable_node);
+    FOR_EACH_OBSERVER(BookmarkModelObserver,
+                      observers_,
+                      BookmarkNodeFaviconChanged(this, mutable_node));
   }
 }
 
 void BookmarkModel::SetDateAdded(const BookmarkNode* node,
                                  Time date_added) {
   if (!node) {
     NOTREACHED();
     return;
   }
 
@@ skipping 20 matching lines @@
                                   std::vector<const BookmarkNode*>* nodes) {
   base::AutoLock url_lock(url_lock_);
   BookmarkNode tmp_node(url);
   NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(&tmp_node);
   while (i != nodes_ordered_by_url_set_.end() && (*i)->url() == url) {
     nodes->push_back(*i);
     ++i;
   }
 }
 
+void BookmarkModel::GetNodesByIconURL(const GURL& icon_url,
+                                      std::vector<const BookmarkNode*>* nodes) {
+  // TODO: Do something more efficient if this gets called a lot

[sky, 2015/05/22 03:09:06]

Any idea how often this will be called? Might a sync trigger this being called a
lot?

[pkotwicz, 2015/05/22 16:00:39]

Unfortunately, this is called more often than I expected. If a user has two
different devices which sync with each other then this can be called quite a
bit.
The favicon PNG data which is stored in the database is different when it is
encoded on iOS, Mac, Linux and Android (Android picks the largest bitmap of an
.ico and scales it down to 16x16)

GetNodesByIconURL() may be called for several icon URLs each time that a user
switches the device that they use

I am unsure whether this necessitates improving GetNodesByIconURL()

Unfortunately, fixing the Android behavior is hard (I hoped that the work that
rogerm@ was doing would fix it but he has moved on to another project)

+  base::AutoLock url_lock(url_lock_);
+  for (const BookmarkNode* node : nodes_ordered_by_url_set_) {
+    if (node->icon_url() == icon_url)
+      nodes->push_back(node);
+  }
+}
+
 const BookmarkNode* BookmarkModel::GetMostRecentlyAddedUserNodeForURL(
     const GURL& url) {
   std::vector<const BookmarkNode*> nodes;
   GetNodesByURL(url, &nodes);
   std::sort(nodes.begin(), nodes.end(), &MoreRecentlyAdded);
 
   // Look for the first node that the user can edit.
   for (size_t i = 0; i < nodes.size(); ++i) {
     if (client_->CanBeEditedByUser(nodes[i]))
       return nodes[i];
@@ skipping 532 matching lines @@
   return scoped_ptr<BookmarkLoadDetails>(new BookmarkLoadDetails(
       bb_node,
       other_node,
       mobile_node,
       client_->GetLoadExtraNodesCallback(),
       new BookmarkIndex(client_, accept_languages),
       next_node_id_));
 }
 
 }  // namespace bookmarks