Remove redundant FaviconHandler::handler_type_

BUG=542057

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

Cr-Commit-Position: refs/heads/master@{#355371}

4 files changed, 60 insertions, 61 deletions

diff --git a/components/favicon/core/favicon_driver_impl.cc b/components/favicon/core/favicon_driver_impl.cc
index 8896bd8..47bfced 100644
--- a/components/favicon/core/favicon_driver_impl.cc
+++ b/components/favicon/core/favicon_driver_impl.cc
@@ -48,14 +48,15 @@ FaviconDriverImpl::FaviconDriverImpl(FaviconService* favicon_service,
       history_service_(history_service),
       bookmark_model_(bookmark_model) {
   favicon_handler_.reset(new FaviconHandler(
-      favicon_service_, this, FaviconHandler::FAVICON, kEnableTouchIcon));
+      favicon_service_, this, kEnableTouchIcon ? FaviconHandler::LARGEST_FAVICON
+                                               : FaviconHandler::FAVICON));
   if (kEnableTouchIcon) {
-    touch_icon_handler_.reset(new FaviconHandler(favicon_service_, this,
-                                                 FaviconHandler::TOUCH, true));
+    touch_icon_handler_.reset(new FaviconHandler(
+        favicon_service_, this, FaviconHandler::LARGEST_TOUCH));
   }
   if (IsIconNTPEnabled()) {
-    large_icon_handler_.reset(new FaviconHandler(favicon_service_, this,
-                                                 FaviconHandler::LARGE, true));
+    large_icon_handler_.reset(new FaviconHandler(
+        favicon_service_, this, FaviconHandler::LARGEST_TOUCH));
   }
 }
 
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
index 9f45d83..11b2b13 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -212,13 +212,12 @@ FaviconHandler::FaviconCandidate::FaviconCandidate(
 
 FaviconHandler::FaviconHandler(FaviconService* service,
                                FaviconDriver* driver,
-                               Type handler_type,
-                               bool download_largest_icon)
+                               Type handler_type)
     : got_favicon_from_history_(false),
       favicon_expired_or_incomplete_(false),
-      handler_type_(handler_type),
       icon_types_(FaviconHandler::GetIconTypesFromHandlerType(handler_type)),
-      download_largest_icon_(download_largest_icon),
+      download_largest_icon_(handler_type == LARGEST_FAVICON ||
+                             handler_type == LARGEST_TOUCH),
       service_(service),
       driver_(driver),
       current_candidate_index_(0u) {
@@ -233,12 +232,10 @@ int FaviconHandler::GetIconTypesFromHandlerType(
     FaviconHandler::Type handler_type) {
   switch (handler_type) {
     case FAVICON:
+    case LARGEST_FAVICON:
       return favicon_base::FAVICON;
-    case TOUCH:  // Falls through.
-    case LARGE:
+    case LARGEST_TOUCH:
       return favicon_base::TOUCH_ICON | favicon_base::TOUCH_PRECOMPOSED_ICON;
-    default:
-      NOTREACHED();
   }
   return 0;
 }
@@ -340,8 +337,7 @@ void FaviconHandler::NotifyFaviconAvailable(const GURL& icon_url,
   gfx::Image image_with_adjusted_colorspace = image;
   favicon_base::SetFaviconColorSpace(&image_with_adjusted_colorspace);
 
-  bool is_active_favicon =
-      (handler_type_ == FAVICON && !download_largest_icon_);
+  bool is_active_favicon = !download_largest_icon_;
 
   driver_->OnFaviconAvailable(
       url_, icon_url, image_with_adjusted_colorspace, is_active_favicon);
@@ -527,7 +523,6 @@ void FaviconHandler::SetHistoryFavicons(const GURL& page_url,
                                         const GURL& icon_url,
                                         favicon_base::IconType icon_type,
                                         const gfx::Image& image) {
-  // TODO(huangs): Get the following to garbage collect if handler_type_ == ALL.
   if (service_) {
     service_->SetFavicons(page_url, icon_url, icon_type, image);
   }
@@ -569,7 +564,7 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
       preferred_icon_size(), favicon_bitmap_results);
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
 
-  if (has_results && handler_type_ == FAVICON && !download_largest_icon_ &&
+  if (has_results && !download_largest_icon_ &&
       (!current_candidate() ||
        DoUrlsAndIconsMatch(*current_candidate(), favicon_bitmap_results))) {
     if (has_valid_result) {
@@ -586,7 +581,7 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
     }
   }
 
-  if (has_valid_result && (handler_type_ != FAVICON || download_largest_icon_))
+  if (has_valid_result && download_largest_icon_)
     NotifyFaviconAvailable(favicon_bitmap_results);
 
   if (current_candidate())
diff --git a/components/favicon/core/favicon_handler.h b/components/favicon/core/favicon_handler.h
index 4ca7830..eecc111 100644
--- a/components/favicon/core/favicon_handler.h
+++ b/components/favicon/core/favicon_handler.h
@@ -78,12 +78,25 @@ class TestFaviconHandler;
 
 class FaviconHandler {
  public:
-  enum Type { FAVICON, TOUCH, LARGE };
+  enum Type {
+    // Selects the icon URL of type favicon_base::FAVICON with bitmaps whose
+    // edge sizes most closely match "16 * favicon_base::GetFaviconScales()".
+    // When favicon_base::GetFaviconScales() returns multiple scales, the ideal
+    // match is a .ico file.
+    FAVICON,
+
+    // Selects the icon URL of type favicon_base::FAVICON with the largest
+    // bitmap.
+    LARGEST_FAVICON,
+
+    // Selects the icon URL of type favicon_base::TOUCH_ICON or
+    // favicon_base::TOUCH_PRECOMPOSED_ICON with the largest bitmap.
+    LARGEST_TOUCH
+  };
 
   FaviconHandler(FaviconService* service,
                  FaviconDriver* driver,
-                 Type handler_type,
-                 bool download_largest_icon);
+                 Type handler_type);
   virtual ~FaviconHandler();
 
   // Returns the bit mask of favicon_base::IconType based on the handler's type.
@@ -239,9 +252,7 @@ class FaviconHandler {
   // Returns the preferred size of the image. 0 means no preference (any size
   // will do).
   int preferred_icon_size() const {
-    if (download_largest_icon_)
-      return 0;
-    return handler_type_ == FAVICON ? gfx::kFaviconSize : 0;
+    return download_largest_icon_ ? 0 : gfx::kFaviconSize;
   }
 
   // Used for FaviconService requests.
@@ -263,9 +274,6 @@ class FaviconHandler {
   typedef std::map<int, DownloadRequest> DownloadRequests;
   DownloadRequests download_requests_;
 
-  // The type of the current handler.
-  const Type handler_type_;
-
   // The combination of the supported icon types.
   const int icon_types_;
 
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index 2afd9bf..fce6eed 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -306,9 +306,8 @@ class TestFaviconHandler : public FaviconHandler {
 
   TestFaviconHandler(const GURL& page_url,
                      TestFaviconDriver* driver,
-                     Type type,
-                     bool download_largest_icon)
-      : FaviconHandler(nullptr, driver, type, download_largest_icon),
+                     Type type)
+      : FaviconHandler(nullptr, driver, type),
         download_id_(0) {
     driver->SetActiveURL(page_url);
     download_handler_.reset(new DownloadHandler(this));
@@ -529,7 +528,7 @@ TEST_F(FaviconHandlerTest, GetFaviconFromHistory) {
   const GURL icon_url("http://www.google.com/favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -568,7 +567,7 @@ TEST_F(FaviconHandlerTest, DownloadFavicon) {
   const GURL icon_url("http://www.google.com/favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -635,7 +634,7 @@ TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -714,7 +713,7 @@ TEST_F(FaviconHandlerTest, FaviconInHistoryInvalid) {
   const GURL icon_url("http://www.google.com/favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -784,7 +783,7 @@ TEST_F(FaviconHandlerTest, UpdateFavicon) {
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -844,7 +843,7 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::TOUCH, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::LARGEST_TOUCH);
   std::set<GURL> fail_downloads;
   fail_downloads.insert(icon_url);
   helper.download_handler()->FailDownloadForIconURLs(fail_downloads);
@@ -959,7 +958,7 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::TOUCH, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::LARGEST_TOUCH);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -1081,7 +1080,7 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLs) {
                                     std::vector<gfx::Size>()));
 
   TestFaviconDriver driver;
-  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON);
 
   // Initiate a request for favicon data for |page_url|. History does not know
   // about the page URL or the icon URLs.
@@ -1152,8 +1151,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   // 1) Test that if there are several single resolution favicons to choose from
   // that the largest exact match is chosen.
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
-                              false);
+  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON);
 
   const int kSizes1[] = { 16, 24, 32, 48, 256 };
   std::vector<FaviconURL> urls1(kSourceIconURLs,
@@ -1174,8 +1172,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   // 2) Test that if there are several single resolution favicons to choose
   // from, the exact match is preferred even if it results in upsampling.
   TestFaviconDriver driver2;
-  TestFaviconHandler handler2(kPageURL, &driver2, FaviconHandler::FAVICON,
-                              false);
+  TestFaviconHandler handler2(kPageURL, &driver2, FaviconHandler::FAVICON);
 
   const int kSizes2[] = { 16, 24, 48, 256 };
   std::vector<FaviconURL> urls2(kSourceIconURLs,
@@ -1191,8 +1188,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   // 3) Test that favicons which need to be upsampled a little or downsampled
   // a little are preferred over huge favicons.
   TestFaviconDriver driver3;
-  TestFaviconHandler handler3(kPageURL, &driver3, FaviconHandler::FAVICON,
-                              false);
+  TestFaviconHandler handler3(kPageURL, &driver3, FaviconHandler::FAVICON);
 
   const int kSizes3[] = { 256, 48 };
   std::vector<FaviconURL> urls3(kSourceIconURLs,
@@ -1206,8 +1202,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
             driver3.GetActiveFaviconURL());
 
   TestFaviconDriver driver4;
-  TestFaviconHandler handler4(kPageURL, &driver4, FaviconHandler::FAVICON,
-                              false);
+  TestFaviconHandler handler4(kPageURL, &driver4, FaviconHandler::FAVICON);
 
   const int kSizes4[] = { 17, 256 };
   std::vector<FaviconURL> urls4(kSourceIconURLs,
@@ -1241,7 +1236,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons404) {
   };
 
   TestFaviconDriver driver;
-  TestFaviconHandler handler(kPageURL, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler handler(kPageURL, &driver, FaviconHandler::FAVICON);
   DownloadHandler* download_handler = handler.download_handler();
 
   std::set<GURL> k404URLs;
@@ -1291,7 +1286,7 @@ TEST_F(FaviconHandlerTest, MultipleFaviconsAll404) {
   };
 
   TestFaviconDriver driver;
-  TestFaviconHandler handler(kPageURL, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler handler(kPageURL, &driver, FaviconHandler::FAVICON);
   DownloadHandler* download_handler = handler.download_handler();
 
   std::set<GURL> k404URLs;
@@ -1332,7 +1327,7 @@ TEST_F(FaviconHandlerTest, FaviconInvalidURL) {
                          std::vector<gfx::Size>());
 
   TestFaviconDriver driver;
-  TestFaviconHandler handler(kPageURL, &driver, FaviconHandler::FAVICON, false);
+  TestFaviconHandler handler(kPageURL, &driver, FaviconHandler::FAVICON);
   UpdateFaviconURL(&driver, &handler, kPageURL,
                    std::vector<FaviconURL>(1u, favicon_url));
   EXPECT_EQ(0u, handler.image_urls().size());
@@ -1364,8 +1359,8 @@ TEST_F(FaviconHandlerTest, TestSortFavicon) {
                  std::vector<gfx::Size>())};
 
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
-                              true);
+  TestFaviconHandler handler1(kPageURL, &driver1,
+                              FaviconHandler::LARGEST_FAVICON);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
@@ -1427,8 +1422,8 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
                  std::vector<gfx::Size>())};
 
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
-                              true);
+  TestFaviconHandler handler1(kPageURL, &driver1,
+                              FaviconHandler::LARGEST_FAVICON);
 
   std::set<GURL> fail_icon_urls;
   for (size_t i = 0; i < arraysize(kSourceIconURLs); ++i) {
@@ -1495,8 +1490,8 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
           GURL("http://www.google.com/c"), favicon_base::FAVICON, two_icons)};
 
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
-                              true);
+  TestFaviconHandler handler1(kPageURL, &driver1,
+                              FaviconHandler::LARGEST_FAVICON);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
@@ -1562,8 +1557,8 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
           GURL("http://www.google.com/c"), favicon_base::FAVICON, icon2)};
 
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
-                              true);
+  TestFaviconHandler handler1(kPageURL, &driver1,
+                              FaviconHandler::LARGEST_FAVICON);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
@@ -1623,8 +1618,8 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
                  std::vector<gfx::Size>())};
 
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(kPageURL, &driver1, FaviconHandler::FAVICON,
-                              true);
+  TestFaviconHandler handler1(kPageURL, &driver1,
+                              FaviconHandler::LARGEST_FAVICON);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
   UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
@@ -1702,8 +1697,8 @@ TEST_P(FaviconHandlerActiveFaviconValidityParamTest,
   const FaviconURL source_icon_urls[] = {
       FaviconURL(new_favicon_url, favicon_base::FAVICON, one_icon)};
   TestFaviconDriver driver1;
-  TestFaviconHandler handler1(page_url, &driver1, FaviconHandler::FAVICON,
-                              true);
+  TestFaviconHandler handler1(page_url, &driver1,
+                              FaviconHandler::LARGEST_FAVICON);
   std::vector<FaviconURL> urls1(source_icon_urls,
                                 source_icon_urls + arraysize(source_icon_urls));
   UpdateFaviconURL(&driver1, &handler1, page_url, urls1);