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

chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.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 "base/rand_util.h"
 #include "base/run_loop.h"
 #include "base/values.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/test/integration/bookmarks_helper.h"
@@ skipping 211 matching lines @@
              bookmarks_helper::FROM_UI);
   ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0)));
   ASSERT_TRUE(AllModelsMatchVerifier());
 
   SetFavicon(0, bookmark0, icon_url2, CreateFavicon(SK_ColorGREEN),
              bookmarks_helper::FROM_UI);
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(AllModelsMatchVerifier());
 }
 
+// When two bookmarks use the same icon URL, the favicon for both bookmarks is
+// stored in the same row in the favicons database. Test that when the favicon

[Roger McFarlane (Chromium), 2015/05/13 18:28:12]

Assuming my understanding is correct, how about:

  When two bookmarks use the same icon URL, a single shared favicon
  row is stored in the favicons database. Test that...

[pkotwicz, 2015/05/14 14:18:54]

I modified the first sentence to be less wordy

+// is updated for one bookmark it is also updated for the other bookmark. This
+// ensures that sync has the most up to date data and prevents sync from
+// reverting the newly updated bookmark favicon back to the old favicon.
+// crbug.com/485657
+IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest,
+                       SC_SetFaviconTwoBookmarksSameIconURL) {
+  const GURL page_url1("http://www.google.com/a");
+  const GURL page_url2("http://www.google.com/b");
+  const GURL icon_url("http://www.google.com/favicon.ico");
+
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  const BookmarkNode* bookmark01 = AddURL(0, kGenericURLTitle, page_url1);
+  ASSERT_TRUE(bookmark01 != nullptr);
+  const BookmarkNode* bookmark02 = AddURL(0, kGenericURLTitle, page_url2);
+  ASSERT_TRUE(bookmark02 != nullptr);
+
+  SetFavicon(0, bookmark01, icon_url, CreateFavicon(SK_ColorWHITE),
+             bookmarks_helper::FROM_UI);
+  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  // Set |page_url2| to use a blue favicon and the same icon URL as
+  // |page_url1|. The favicon for |page_url1| should also become blue.
+  SetFavicon(0, bookmark02, icon_url, CreateFavicon(SK_ColorBLUE),
+             bookmarks_helper::FROM_UI);
+  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  // Set the title for |page_url1|. This should not revert either of the
+  // bookmark favicons back to white.
+  const char kNewTitle[] = "New Title";
+  ASSERT_STRNE(kGenericURLTitle, kNewTitle);
+  const BookmarkNode* bookmark11 = GetUniqueNodeByURL(1, page_url1);
+  SetTitle(1, bookmark11, std::string(kNewTitle));
+  ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0)));
+  ASSERT_TRUE(AllModelsMatchVerifier());
+}
+
 // Test Scribe ID - 370560.
 IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, SC_AddNonHTTPBMs) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   ASSERT_TRUE(AllModelsMatchVerifier());
 
   ASSERT_TRUE(AddURL(
       0, "FTP UR", GURL("ftp://user:password@host:1234/path")) != NULL);
   ASSERT_TRUE(AddURL(0, "File UR", GURL("file://host/path")) != NULL);
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(AllModelsMatchVerifier());
@@ skipping 1883 matching lines @@
 
   // Verify that the managed bookmark exists in the local model of the first
   // Profile, and has a child node.
   ASSERT_EQ(1, managed_node0->child_count());
   ASSERT_TRUE(managed_node0->IsVisible());
   EXPECT_EQ(GURL("http://youtube.com/"), managed_node0->GetChild(0)->url());
 
   // Verify that the second Profile didn't get this node.
   ASSERT_EQ(0, managed_node1->child_count());
 }