Ignore duplicate FaviconHandler::OnUpdateFaviconURL() calls

components/favicon/core/favicon_handler_unittest.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 "components/favicon/core/favicon_handler.h"
 
 #include<set>
 #include<vector>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 311 matching lines @@
 
   // This method will take the ownership of the given handler.
   void set_history_handler(HistoryRequestHandler* handler) {
     history_handler_.reset(handler);
   }
 
   DownloadHandler* download_handler() {
     return download_handler_.get();
   }
 
-  // Methods to access favicon internals.
-  const std::vector<FaviconURL>& urls() {
-    return image_urls_;
-  }
-
   FaviconURL* current_candidate() {
     return FaviconHandler::current_candidate();
   }
 
+  size_t current_candidate_index() const {
+    return current_candidate_index_;
+  }
+
   const FaviconCandidate& best_favicon_candidate() {
     return best_favicon_candidate_;
   }
 
  protected:
   void UpdateFaviconMappingAndFetch(
       const GURL& page_url,
       const GURL& icon_url,
       favicon_base::IconType icon_type,
       const favicon_base::FaviconResultsCallback& callback,
@@ skipping 197 matching lines @@
   EXPECT_TRUE(driver.GetActiveFaviconValidity());
   EXPECT_EQ(icon_url, driver.GetActiveFaviconURL());
 
   // Simulates update favicon url.
   std::vector<FaviconURL> urls;
   urls.push_back(
       FaviconURL(icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, urls);
 
   // Verify FaviconHandler status
-  EXPECT_EQ(1U, helper.urls().size());
+  EXPECT_EQ(1u, helper.image_urls().size());
   ASSERT_TRUE(helper.current_candidate());
   ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
 
   // Favicon shouldn't request to download icon.
   EXPECT_FALSE(helper.download_handler()->HasDownload());
 }
 
 TEST_F(FaviconHandlerTest, DownloadFavicon) {
   const GURL page_url("http://www.google.com");
@@ skipping 21 matching lines @@
   EXPECT_TRUE(driver.GetActiveFaviconValidity());
   EXPECT_EQ(icon_url, driver.GetActiveFaviconURL());
 
   // Simulates update favicon url.
   std::vector<FaviconURL> urls;
   urls.push_back(
       FaviconURL(icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, urls);
 
   // Verify FaviconHandler status
-  EXPECT_EQ(1U, helper.urls().size());
+  EXPECT_EQ(1u, helper.image_urls().size());
   ASSERT_TRUE(helper.current_candidate());
   ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
 
   // Favicon should request to download icon now.
   DownloadHandler* download_handler = helper.download_handler();
   EXPECT_TRUE(helper.download_handler()->HasDownload());
 
   // Verify the download request.
   EXPECT_EQ(icon_url, download_handler->GetImageUrl());
@@ skipping 48 matching lines @@
   // history.
   helper.set_history_handler(nullptr);
 
   // Simulates update with the different favicon url.
   std::vector<FaviconURL> urls;
   urls.push_back(FaviconURL(
       new_icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, urls);
 
   // Verify FaviconHandler status.
-  EXPECT_EQ(1U, helper.urls().size());
+  EXPECT_EQ(1u, helper.image_urls().size());
   ASSERT_TRUE(helper.current_candidate());
   ASSERT_EQ(new_icon_url, helper.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
   history_handler = helper.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(new_icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ skipping 127 matching lines @@
   // history.
   helper.set_history_handler(nullptr);
 
   // Simulates update with the different favicon url.
   std::vector<FaviconURL> urls;
   urls.push_back(FaviconURL(
       new_icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, urls);
 
   // Verify FaviconHandler status.
-  EXPECT_EQ(1U, helper.urls().size());
+  EXPECT_EQ(1u, helper.image_urls().size());
   ASSERT_TRUE(helper.current_candidate());
   ASSERT_EQ(new_icon_url, helper.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
   history_handler = helper.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(new_icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ skipping 48 matching lines @@
   urls.push_back(FaviconURL(icon_url,
                             favicon_base::TOUCH_PRECOMPOSED_ICON,
                             std::vector<gfx::Size>()));
   urls.push_back(FaviconURL(
       new_icon_url, favicon_base::TOUCH_ICON, std::vector<gfx::Size>()));
   urls.push_back(FaviconURL(
       new_icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, urls);
 
   // Verify FaviconHandler status.
-  EXPECT_EQ(2U, helper.urls().size());
+  EXPECT_EQ(2u, helper.image_urls().size());
+  EXPECT_EQ(0u, helper.current_candidate_index());
   ASSERT_TRUE(helper.current_candidate());
   ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
             helper.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
   history_handler = helper.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON, history_handler->icon_type_);
   EXPECT_EQ(page_url, history_handler->page_url_);
 
   // Simulate not find icon.
   history_handler->history_results_.clear();
   history_handler->InvokeCallback();
 
   // Should request download favicon.
   DownloadHandler* download_handler = helper.download_handler();
   EXPECT_TRUE(helper.download_handler()->HasDownload());
 
   // Verify the download request.
   EXPECT_EQ(icon_url, download_handler->GetImageUrl());
 
   // Reset the history_handler to verify whether favicon is request from
   // history.
   helper.set_history_handler(nullptr);
   download_handler->InvokeCallback();
 
   // Left 1 url.
-  EXPECT_EQ(1U, helper.urls().size());
+  EXPECT_EQ(1u, helper.current_candidate_index());
   ASSERT_TRUE(helper.current_candidate());
   EXPECT_EQ(new_icon_url, helper.current_candidate()->icon_url);
   EXPECT_EQ(favicon_base::TOUCH_ICON, helper.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
   history_handler = helper.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(new_icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::TOUCH_ICON, history_handler->icon_type_);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ skipping 60 matching lines @@
   urls.push_back(FaviconURL(icon_url,
                             favicon_base::TOUCH_PRECOMPOSED_ICON,
                             std::vector<gfx::Size>()));
   urls.push_back(FaviconURL(
       new_icon_url, favicon_base::TOUCH_ICON, std::vector<gfx::Size>()));
   urls.push_back(FaviconURL(
       new_icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, urls);
 
   // Verify FaviconHandler status.
-  EXPECT_EQ(2U, helper.urls().size());
-  ASSERT_TRUE(helper.current_candidate());
+  EXPECT_EQ(2u, helper.image_urls().size());
+  ASSERT_EQ(0u, helper.current_candidate_index());
   ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
             helper.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
   history_handler = helper.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON, history_handler->icon_type_);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ skipping 11 matching lines @@
 
   // Reset the history_handler to verify whether favicon is request from
   // history.
   helper.set_history_handler(nullptr);
   const GURL latest_icon_url("http://www.google.com/latest_favicon");
   std::vector<FaviconURL> latest_urls;
   latest_urls.push_back(FaviconURL(
       latest_icon_url, favicon_base::TOUCH_ICON, std::vector<gfx::Size>()));
   helper.OnUpdateFaviconURL(page_url, latest_urls);
 
-  EXPECT_EQ(1U, helper.urls().size());
+  EXPECT_EQ(1u, helper.image_urls().size());
+  EXPECT_EQ(0u, helper.current_candidate_index());
   EXPECT_EQ(latest_icon_url, helper.current_candidate()->icon_url);
   EXPECT_EQ(favicon_base::TOUCH_ICON, helper.current_candidate()->icon_type);
 
   // Whether new icon is requested from history
   history_handler = helper.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(latest_icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::TOUCH_ICON, history_handler->icon_type_);
   EXPECT_EQ(page_url, history_handler->page_url_);
 
@@ skipping 17 matching lines @@
   SetFaviconRawBitmapResult(latest_icon_url,
                             favicon_base::TOUCH_ICON,
                             false /* expired */,
                             &handler->history_results_);
   handler->InvokeCallback();
 
   // No download request.
   EXPECT_FALSE(download_handler->HasDownload());
 }
 
+// Test that sending an icon URL update identical to the previous icon URL
+// update is a no-op.
+TEST_F(FaviconHandlerTest, UpdateSameIconURLs) {
+  const GURL page_url("http://www.google.com");
+  const GURL icon_url1("http://www.google.com/favicon1");
+  const GURL icon_url2("http://www.google.com/favicon2");
+  std::vector<FaviconURL> favicon_urls;
+  favicon_urls.push_back(FaviconURL(GURL("http://www.google.com/favicon1"),
+                                    favicon_base::FAVICON,
+                                    std::vector<gfx::Size>()));
+  favicon_urls.push_back(FaviconURL(GURL("http://www.google.com/favicon2"),
+                                    favicon_base::FAVICON,
+                                    std::vector<gfx::Size>()));
+
+  TestFaviconDriver driver;
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+
+  // Initiate a request for favicon data for |page_url|. History does not know
+  // about the page URL or the icon URLs.
+  helper.FetchFavicon(page_url);
+  helper.history_handler()->InvokeCallback();
+  helper.set_history_handler(nullptr);
+
+  // Got icon URLs.
+  helper.OnUpdateFaviconURL(page_url, favicon_urls);
+
+  // There should be an ongoing history request for |icon_url1|.
+  ASSERT_EQ(2u, helper.image_urls().size());
+  ASSERT_EQ(0u, helper.current_candidate_index());
+  HistoryRequestHandler* history_handler = helper.history_handler();
+  ASSERT_TRUE(history_handler);
+
+  // Calling OnUpdateFaviconURL() with the same icon URLs should have no effect.
+  helper.OnUpdateFaviconURL(page_url, favicon_urls);
+  EXPECT_EQ(history_handler, helper.history_handler());
+
+  // Complete history request for |icon_url1| and do download.
+  helper.history_handler()->InvokeCallback();
+  helper.set_history_handler(nullptr);
+  helper.download_handler()->SetImageSizes(std::vector<int>(1u, 10));
+  helper.download_handler()->InvokeCallback();
+  helper.download_handler()->Reset();
+
+  // There should now be an ongoing history request for |icon_url2|.
+  ASSERT_EQ(1u, helper.current_candidate_index());
+  history_handler = helper.history_handler();
+  ASSERT_TRUE(history_handler);
+
+  // Calling OnUpdateFaviconURL() with the same icon URLs should have no effect.
+  helper.OnUpdateFaviconURL(page_url, favicon_urls);
+  EXPECT_EQ(history_handler, helper.history_handler());
+}
+
 // Test the favicon which is selected when the web page provides several
 // favicons and none of the favicons are cached in history.
 // The goal of this test is to be more of an integration test than
 // SelectFaviconFramesTest.*.
 TEST_F(FaviconHandlerTest, MultipleFavicons) {
   const GURL kPageURL("http://www.google.com");
   const FaviconURL kSourceIconURLs[] = {
       FaviconURL(GURL("http://www.google.com/a"),
                  favicon_base::FAVICON,
                  std::vector<gfx::Size>()),
@@ skipping 22 matching lines @@
   TestFaviconDriver driver1;
   TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
                               false);
 
   const int kSizes1[] = { 16, 24, 32, 48, 256 };
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes1));
   DownloadTillDoneIgnoringHistory(
       &driver1, &handler1, kPageURL, urls1, kSizes1);
 
-  EXPECT_EQ(0u, handler1.image_urls().size());
+  EXPECT_EQ(nullptr, handler1.current_candidate());
   EXPECT_TRUE(driver1.GetActiveFaviconValidity());
   EXPECT_FALSE(driver1.GetActiveFaviconImage().IsEmpty());
   EXPECT_EQ(gfx::kFaviconSize, driver1.GetActiveFaviconImage().Width());
 
   size_t expected_index = 2u;
   EXPECT_EQ(32, kSizes1[expected_index]);
   EXPECT_EQ(kSourceIconURLs[expected_index].icon_url,
             driver1.GetActiveFaviconURL());
 
   // 2) Test that if there are several single resolution favicons to choose
@@ skipping 82 matching lines @@
 
   // Do a fetch now that the initial download for |k404IconURL| has failed. The
   // behavior is different because OnDidDownloadFavicon() is invoked
   // synchronously from DownloadFavicon().
   const int kSizes2[] = { 10, 0, 16 };
   std::vector<FaviconURL> urls2(kFaviconURLs,
                                 kFaviconURLs + arraysize(kFaviconURLs));
   DownloadTillDoneIgnoringHistory(
       &driver, &handler, kPageURL, urls2, kSizes2);
 
-  EXPECT_EQ(0u, handler.image_urls().size());
+  EXPECT_EQ(nullptr, handler.current_candidate());
   EXPECT_TRUE(driver.GetActiveFaviconValidity());
   EXPECT_FALSE(driver.GetActiveFaviconImage().IsEmpty());
   int expected_index = 2u;
   EXPECT_EQ(16, kSizes2[expected_index]);
   EXPECT_EQ(kFaviconURLs[expected_index].icon_url,
             driver.GetActiveFaviconURL());
 }
 
 // Test that no favicon is selected when:
 // - The page provides several favicons.
@@ skipping 31 matching lines @@
   EXPECT_TRUE(download_handler->DidFailDownloadForIconURL(k404IconURL2));
 
   // Do a fetch now that the initial downloads for |kFaviconURLs| have failed.
   // The behavior is different because OnDidDownloadFavicon() is invoked
   // synchronously from DownloadFavicon().
   const int kSizes[] = { 0, 0 };
   std::vector<FaviconURL> urls(kFaviconURLs,
                                kFaviconURLs + arraysize(kFaviconURLs));
   DownloadTillDoneIgnoringHistory(&driver, &handler, kPageURL, urls, kSizes);
 
-  EXPECT_EQ(0u, handler.image_urls().size());
+  EXPECT_EQ(nullptr, handler.current_candidate());
   EXPECT_FALSE(driver.GetActiveFaviconValidity());
   EXPECT_TRUE(driver.GetActiveFaviconImage().IsEmpty());
 }
 
 // Test that no favicon is selected when the page's only icon uses an invalid
 // URL syntax.
 TEST_F(FaviconHandlerTest, FaviconInvalidURL) {
   const GURL kPageURL("http://www.google.com");
   const GURL kInvalidFormatURL("invalid");
   ASSERT_TRUE(kInvalidFormatURL.is_empty());
@@ skipping 106 matching lines @@
   }
   handler1.download_handler()->FailDownloadForIconURLs(fail_icon_urls);
 
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
   // Simulate the download failed, to check whether the icons were requested
   // to download according their size.
   struct ExpectedResult {
-    // The size of image_urls_.
-    size_t image_urls_size;
     // The favicon's index in kSourceIconURLs.
     size_t favicon_index;
     // Width of largest bitmap.
     int width;
   } results[] = {
-    {5, 0, 1024},
-    {4, 2, 512},
-    {3, 1, 15},
+    {0, 1024},
+    {2, 512},
+    {1, 15},
     // The rest of bitmaps come in order.
-    {2, 3, -1},
-    {1, 4, -1},
+    {3, -1},
+    {4, -1},
   };
 
   for (int i = 0; i < 5; ++i) {
-    ASSERT_EQ(results[i].image_urls_size, handler1.image_urls().size());
     EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
               handler1.current_candidate()->icon_url);
     if (results[i].width != -1) {
       EXPECT_EQ(results[i].width, handler1.current_candidate()->
                 icon_sizes[0].width());
     }
 
     // Simulate no favicon from history.
     handler1.history_handler()->history_results_.clear();
     handler1.history_handler()->InvokeCallback();
@@ skipping 24 matching lines @@
       FaviconURL(
           GURL("http://www.google.com/c"), favicon_base::FAVICON, two_icons)};
 
   TestFaviconDriver driver1;
   TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
                               true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
-  ASSERT_EQ(2u, handler1.urls().size());
+  ASSERT_EQ(2u, handler1.image_urls().size());
 
   // Index of largest favicon in kSourceIconURLs.
   size_t i = 1;
   // The largest bitmap's index in Favicon .
   int b = 1;
 
   // Verify the icon_bitmaps_ was initialized correctly.
   EXPECT_EQ(kSourceIconURLs[i].icon_url,
             handler1.current_candidate()->icon_url);
   EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b],
@@ skipping 46 matching lines @@
       FaviconURL(
           GURL("http://www.google.com/c"), favicon_base::FAVICON, icon2)};
 
   TestFaviconDriver driver1;
   TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
                               true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
-  ASSERT_EQ(2u, handler1.urls().size());
+  ASSERT_EQ(2u, handler1.image_urls().size());
 
   // Index of largest favicon in kSourceIconURLs.
   size_t i = 1;
   // The largest bitmap's index in Favicon .
   int b = 0;
 
   // Verify the icon_bitmaps_ was initialized correctly.
   EXPECT_EQ(kSourceIconURLs[i].icon_url,
             handler1.current_candidate()->icon_url);
   EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b],
@@ skipping 39 matching lines @@
       FaviconURL(GURL("http://www.google.com/d"),
                  favicon_base::FAVICON,
                  std::vector<gfx::Size>())};
 
   TestFaviconDriver driver1;
   TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
                               true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
-  ASSERT_EQ(3u, handler1.urls().size());
+  ASSERT_EQ(3u, handler1.image_urls().size());
 
   // Simulate no favicon from history.
   handler1.history_handler()->history_results_.clear();
   handler1.history_handler()->InvokeCallback();
 
   // Verify the first icon was request to download
   ASSERT_TRUE(handler1.download_handler()->HasDownload());
   EXPECT_EQ(kSourceIconURLs[0].icon_url,
             handler1.download_handler()->GetImageUrl());
 
@@ skipping 108 matching lines @@
   EXPECT_FALSE(driver1.update_active_favicon());
   EXPECT_EQ(3u, driver1.num_favicon_available());
 }
 
 INSTANTIATE_TEST_CASE_P(FaviconHandlerTestActiveFaviconValidityTrueOrFalse,
                         FaviconHandlerActiveFaviconValidityParamTest,
                         ::testing::Bool());
 
 }  // namespace
 }  // namespace favicon