Ensures TopSitesImpl::MergeCachedForcedURLs doesn't keep forced URLs if they are part of the redirect list of an entry in non-forced URLs.

BUG=360894

Review URL: https://codereview.chromium.org/226543008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262866 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 31 insertions, 1 deletions

diff --git a/chrome/browser/history/top_sites_impl.cc b/chrome/browser/history/top_sites_impl.cc
index 298c16b..3d45f35 100644
--- a/chrome/browser/history/top_sites_impl.cc
+++ b/chrome/browser/history/top_sites_impl.cc
@@ -661,7 +661,9 @@ size_t TopSitesImpl::MergeCachedForcedURLs(MostVisitedURLList* new_list) {
   std::set<GURL> all_new_urls;
   size_t num_forced = 0;
   for (size_t i = 0; i < new_list->size(); ++i) {
-    all_new_urls.insert((*new_list)[i].url);
+    for (size_t j = 0; j < (*new_list)[i].redirects.size(); j++) {
+      all_new_urls.insert((*new_list)[i].redirects[j]);
+    }
     if (!(*new_list)[i].last_forced_time.is_null())
       ++num_forced;
   }
diff --git a/chrome/browser/history/top_sites_impl_unittest.cc b/chrome/browser/history/top_sites_impl_unittest.cc
index ae89a75..c432799 100644
--- a/chrome/browser/history/top_sites_impl_unittest.cc
+++ b/chrome/browser/history/top_sites_impl_unittest.cc
@@ -1444,6 +1444,34 @@ TEST_F(TopSitesImplTest, SetTopSitesIdentical) {
   ASSERT_NO_FATAL_FAILURE(ContainsPrepopulatePages(querier, 3));
 }
 
+TEST_F(TopSitesImplTest, SetTopSitesWithAlreadyExistingForcedURLs) {
+  // Set the initial list of URLs.
+  MostVisitedURLList old_url_list;
+  AppendForcedMostVisitedURL(&old_url_list, GURL("http://url/0/redir"), 1000);
+  AppendForcedMostVisitedURL(&old_url_list, GURL("http://url/1"), 2000);
+  SetTopSites(old_url_list);
+
+  // Setup a new URL list that will cause collisions.
+  MostVisitedURLList new_url_list;
+  AppendMostVisitedURLWithRedirect(&new_url_list, GURL("http://url/0/redir"),
+                                   GURL("http://url/0"));
+  AppendMostVisitedURL(&new_url_list, GURL("http://url/1"));
+  SetTopSites(new_url_list);
+
+  // Query all URLs.
+  TopSitesQuerier querier;
+  querier.QueryAllTopSites(top_sites(), false, true);
+
+  // Check URLs. When collision occurs, the non-forced one is always preferred.
+  ASSERT_EQ(2u + GetPrepopulatePages().size(), querier.urls().size());
+  EXPECT_EQ("http://url/0", querier.urls()[0].url.spec());
+  EXPECT_EQ("http://url/0/redir", querier.urls()[0].redirects[0].spec());
+  EXPECT_TRUE(querier.urls()[0].last_forced_time.is_null());
+  EXPECT_EQ("http://url/1", querier.urls()[1].url.spec());
+  EXPECT_TRUE(querier.urls()[1].last_forced_time.is_null());
+  ASSERT_NO_FATAL_FAILURE(ContainsPrepopulatePages(querier, 2));
+}
+
 TEST_F(TopSitesImplTest, AddForcedURL) {
   // Set the initial list of URLs.
   MostVisitedURLList url_list;