Always select best favicon bitmap

components/favicon/core/favicon_handler.h

Index: components/favicon/core/favicon_handler.h
diff --git a/components/favicon/core/favicon_handler.h b/components/favicon/core/favicon_handler.h
index f3d9d4b662c5d9c6f034cacffc7f5a8cfd67d9db..80e356ce9d578e2c95c007b6adedd7d909680d84 100644
--- a/components/favicon/core/favicon_handler.h
+++ b/components/favicon/core/favicon_handler.h
@@ -14,8 +14,10 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "components/favicon/core/favicon_driver_observer.h"
+#include "components/favicon/core/favicon_selector.h"
 #include "components/favicon/core/favicon_url.h"
 #include "components/favicon_base/favicon_callback.h"
 #include "ui/gfx/favicon_size.h"
@@ -68,11 +70,10 @@ class FaviconService;
 // favicon.
 //
 // When the renderer downloads favicons, it considers the entire list of
-// favicon candidates, if |download_largest_favicon_| is true, the largest
-// favicon will be used, otherwise the one that best matches the preferred size
-// is chosen (or the first one if there is no preferred  size). Once the
-// matching favicon has been determined, SetFavicon is called which updates
-// the page's favicon and notifies the database to save the favicon.
+// favicon candidates. Among those, the one that matches |target_spec_|
+// is chosen first. Once the matching favicon has been determined, SetFavicon
+// is called which updates the page's favicon and notifies the database to save
+// the favicon.
 
 class FaviconHandler {
  public:
@@ -130,19 +131,10 @@ class FaviconHandler {
   void OnUpdateFaviconURL(const GURL& page_url,
                           const std::vector<favicon::FaviconURL>& candidates);
 
-  // For testing.
-  const std::vector<favicon::FaviconURL>& image_urls() const {
-    return image_urls_;
-  }
-
   // Returns whether the handler is waiting for a download to complete or for
   // data from the FaviconService. Reserved for testing.
   bool HasPendingTasksForTest();
 
-  // Get the maximal icon size in pixels for a icon of type |icon_type| for the
-  // current platform.
-  static int GetMaximalIconSize(favicon_base::IconType icon_type);
-
  private:
   // Represents an in progress download of an image from the renderer.
   struct DownloadRequest {
@@ -155,22 +147,6 @@ class FaviconHandler {
     favicon_base::IconType icon_type;
   };
 
-  // Used to track a candidate for the favicon.
-  struct FaviconCandidate {
-    FaviconCandidate();
-    ~FaviconCandidate();
-
-    FaviconCandidate(const GURL& image_url,
-                     const gfx::Image& image,
-                     float score,
-                     favicon_base::IconType icon_type);
-
-    GURL image_url;
-    gfx::Image image;
-    float score;
-    favicon_base::IconType icon_type;
-  };
-
   // Returns the bit mask of favicon_base::IconType based on the handler's type.
   static int GetIconTypesFromHandlerType(
       FaviconDriverObserver::NotificationIconType handler_type);
@@ -183,11 +159,12 @@ class FaviconHandler {
   void OnFaviconDataForInitialURLFromFaviconService(const std::vector<
       favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results);
 
+  // Deques the next candidate to |current_candidate_| and starts processing it.
   // If the favicon currently mapped to |url_| has expired, downloads the
   // current candidate favicon from the renderer. Otherwise requests data for
   // the current favicon from history. If data is requested from history,
   // OnFaviconData() is called with the history data once it has been retrieved.
-  void DownloadCurrentCandidateOrAskFaviconService();
+  void DownloadNextCandidateOrAskFaviconService();
 
   // See description above class for details.
   void OnFaviconData(const std::vector<favicon_base::FaviconRawBitmapResult>&
@@ -208,7 +185,8 @@ class FaviconHandler {
 
   bool ShouldSaveFavicon();
 
-  // Updates |favicon_candidate_| and returns true if it is an exact match.
+  // Updates candidate queues and returns true if no more candidates should be
+  // processed (e.g. an exact match was found).
   bool UpdateFaviconCandidate(const GURL& image_url,
                               const gfx::Image& image,
                               float score,
@@ -231,17 +209,10 @@ class FaviconHandler {
                             favicon_base::IconType icon_type,
                             const gfx::Image& image);
 
-  // Return the current candidate if any.
-  favicon::FaviconURL* current_candidate() {
-    return current_candidate_index_ < image_urls_.size()
-               ? &image_urls_[current_candidate_index_]
-               : nullptr;
-  }
-
   // Returns the preferred size of the image. 0 means no preference (any size
   // will do).
   int preferred_icon_size() const {
-    return download_largest_icon_ ? 0 : gfx::kFaviconSize;
+    return target_size_spec_.WantsBestBitmapOnly() ? 0 : gfx::kFaviconSize;
   }
 
   // Used for FaviconService requests.
@@ -274,10 +245,18 @@ class FaviconHandler {
   // The combination of the supported icon types.
   const int icon_types_;
 
-  // Whether the largest icon should be downloaded.
-  const bool download_largest_icon_;
+  // The target "bucket" or requirements that the FaviconHandler will try to
+  // fulfill.
+  const FaviconSelector::TargetSizeSpec target_size_spec_;
+  std::unique_ptr<FaviconSelector> selector_;
+
+  // Current candidate being processed.
+  base::Optional<FaviconSelector::Candidate> current_candidate_;
 
-  // The prioritized favicon candidates from the page back from the renderer.
+  // Image of the best favicon processed so far.
+  gfx::Image best_favicon_image_;
+
+  // The filtered (per icon type) candidates provided by the page.
   std::vector<favicon::FaviconURL> image_urls_;
 
   // The icon URL and the icon type of the favicon in the most recent
@@ -292,16 +271,6 @@ class FaviconHandler {
   // This handler's delegate.
   Delegate* delegate_;
 
-  // The index of the favicon URL in |image_urls_| which is currently being
-  // requested from history or downloaded.
-  size_t current_candidate_index_;
-
-  // Best image we've seen so far.  As images are downloaded from the page they
-  // are stored here. When there is an exact match, or no more images are
-  // available the favicon service and the current page are updated (assuming
-  // the image is for a favicon).
-  FaviconCandidate best_favicon_candidate_;
-
   base::WeakPtrFactory<FaviconHandler> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FaviconHandler);