Allow favicons for synced bookmarks to expire

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"
 #include "chrome/browser/sync/test/integration/passwords_helper.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
 #include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "components/policy/core/common/policy_map.h"
 #include "policy/policy_constants.h"
 #include "sync/internal_api/public/sessions/sync_session_snapshot.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/base/layout.h"
 
 using bookmarks::BookmarkNode;
 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;
 using bookmarks_helper::GetSyncedBookmarksNode;
 using bookmarks_helper::GetUniqueNodeByURL;
 using bookmarks_helper::HasNodeWithURL;
 using bookmarks_helper::IndexedFolderName;
 using bookmarks_helper::IndexedSubfolderName;
 using bookmarks_helper::IndexedSubsubfolderName;
 using bookmarks_helper::IndexedURL;
@@ skipping 181 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());
 }
 
+// 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.";
   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());
 }