Allow favicons for synced bookmarks to expire

chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc

Index: chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
index e574eec2bbd0d88a45cce7685c250eb464da3d8f..142a4a8abaa91b741dd38196db8b31b495ec0cd5 100644
--- a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
@@ -25,9 +25,11 @@ using bookmarks_helper::AddFolder;
 using bookmarks_helper::AddURL;
 using bookmarks_helper::AllModelsMatch;
 using bookmarks_helper::AllModelsMatchVerifier;
+using bookmarks_helper::CheckFaviconExpired;
 using bookmarks_helper::ContainsDuplicateBookmarks;
 using bookmarks_helper::CountBookmarksWithTitlesMatching;
 using bookmarks_helper::CreateFavicon;
+using bookmarks_helper::ExpireFavicon;
 using bookmarks_helper::GetBookmarkBarNode;
 using bookmarks_helper::GetManagedNode;
 using bookmarks_helper::GetOtherNode;
@@ -229,6 +231,51 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, SC_SetFaviconHiDPI) {
   ASSERT_TRUE(AllModelsMatchVerifier());
 }
 
+// Test that if sync does not modify a favicon bitmap's data that it does not
+// modify the favicon bitmap's "last updated time" either.
+// (http://crbug.com/481414)
+IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest,
+                       SC_UpdatingTitleDoesNotUpdateFaviconLastUpdatedTime) {
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+
+  std::vector<ui::ScaleFactor> supported_scale_factors;
+  supported_scale_factors.push_back(ui::SCALE_FACTOR_100P);
+  ui::SetSupportedScaleFactors(supported_scale_factors);
+
+  const GURL page_url(kGenericURL);
+  const GURL icon_url("http://www.google.com/favicon.ico");
+
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  const BookmarkNode* bookmark0 = AddURL(0, kGenericURLTitle, page_url);
+  ASSERT_TRUE(bookmark0 != nullptr);

[Nicolas Zea, 2015/05/14 04:08:23]

nit: ASSERT_NE?

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

Done.

+  gfx::Image favicon0 = CreateFavicon(SK_ColorBLUE);
+  ASSERT_FALSE(favicon0.IsEmpty());
+  SetFavicon(0, bookmark0, icon_url, favicon0, bookmarks_helper::FROM_UI);
+
+  // Expire the favicon (e.g. as a result of the user "clearing browsing

[Nicolas Zea, 2015/05/14 04:08:23]

Could you explain why we need to expire the favicon before doing the change?

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

SetFavicon() on line 255 adds an unexpired favicon to the database. In fact,
FaviconService::SetFavicons() always sets the "last updated time" to the current
time.

"Expiring the favicon" emulates either:
The favicon getting out of date naturally (i.e. as a result of time passing)
The favicon getting expired as a result of the user "clearing history from the
beginning of time" via chrome://history

The test that I would love to write but am unsure how to is:
1) expire the favicon
2) emulate sync startup (i.e. restarting Chrome)
3) check that the favicon is still expired

+  // history from the beginning of time")
+  ExpireFavicon(0, bookmark0);
+  CheckFaviconExpired(0, icon_url);
+
+  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  // Change the bookmark's title for profile 1. Changing the title will cause
+  // the bookmark's favicon data to be synced from profile 1 to profile 0 even
+  // though the favicon data did not change.
+  const std::string kNewTitle = "New Title";
+  ASSERT_NE(kNewTitle, kGenericURLTitle);
+  const BookmarkNode* bookmark1 = GetUniqueNodeByURL(1, page_url);
+  SetTitle(1, bookmark1, kNewTitle);
+  ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0)));
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  // The favicon for profile 0 should still be expired.
+  CheckFaviconExpired(0, icon_url);
+}
+
 // Test Scribe ID - 370560.
 IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, SC_AddNonHTTPBMs) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";