FaviconHandler test rewrite

components/favicon/core/favicon_handler_unittest.cc

Index: components/favicon/core/favicon_handler_unittest.cc
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index f4370bc599b41392c98a425a37e796e5a3514820..4d0b3b5d9c35908b49f21ad1c8c7214c1f4bbfc5 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -4,425 +4,414 @@
 
 #include "components/favicon/core/favicon_handler.h"
 
-#include <stddef.h>
-
+#include <map>
 #include <memory>
 #include <set>
+#include <string>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/macros.h"
-#include "components/favicon/core/favicon_driver.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
+#include "components/favicon/core/favicon_driver_observer.h"
+#include "components/favicon/core/test/mock_favicon_service.h"
+#include "components/favicon_base/favicon_types.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/layout.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
+
+using testing::_;
+using testing::ElementsAre;
 
 namespace favicon {
 namespace {
 
-// Fill the given bmp with valid png data.
-void FillDataToBitmap(int w, int h, SkBitmap* bmp) {
-  bmp->allocN32Pixels(w, h);
-
-  unsigned char* src_data =
-      reinterpret_cast<unsigned char*>(bmp->getAddr32(0, 0));
-  for (int i = 0; i < w * h; i++) {
-    src_data[i * 4 + 0] = static_cast<unsigned char>(i % 255);
-    src_data[i * 4 + 1] = static_cast<unsigned char>(i % 255);
-    src_data[i * 4 + 2] = static_cast<unsigned char>(i % 255);
-    src_data[i * 4 + 3] = static_cast<unsigned char>(i % 255);
-  }
+int GetNextDownloadId() {
+  static int id = 0;
+  ++id;
+  return id;
+}
+
+SkBitmap CreateBitmap(int width, int height, SkColor color) {
+  SkBitmap bmp;
+  bmp.allocN32Pixels(width, height);
+  bmp.eraseColor(color);
+  return bmp;
 }
 
 // Fill the given data buffer with valid png data.
-void FillBitmap(int w, int h, std::vector<unsigned char>* output) {
-  SkBitmap bitmap;
-  FillDataToBitmap(w, h, &bitmap);
+void FillBitmap(int edge_size, SkColor color, std::vector<unsigned char>* output) {
+  SkBitmap bitmap = CreateBitmap(edge_size, edge_size, color);
   gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, output);
 }
 
-void SetFaviconRawBitmapResult(
+std::vector<favicon_base::FaviconRawBitmapResult> CreateDbResultWithSpecs(
     const GURL& icon_url,
-    favicon_base::IconType icon_type,
+    favicon_base::IconType favicon_type,
     bool expired,
-    std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results) {
+    SkColor color) {
   scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes());
-  FillBitmap(gfx::kFaviconSize, gfx::kFaviconSize, &data->data());
+  FillBitmap(gfx::kFaviconSize, color, &data->data());
   favicon_base::FaviconRawBitmapResult bitmap_result;
   bitmap_result.expired = expired;
   bitmap_result.bitmap_data = data;
   // Use a pixel size other than (0,0) as (0,0) has a special meaning.
   bitmap_result.pixel_size = gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize);
-  bitmap_result.icon_type = icon_type;
+  bitmap_result.icon_type = favicon_type;
   bitmap_result.icon_url = icon_url;
+  return {bitmap_result};
+}
 
-  favicon_bitmap_results->push_back(bitmap_result);
+std::vector<favicon_base::FaviconRawBitmapResult> CreateDbResult(
+    const GURL& icon_url) {
+  return CreateDbResultWithSpecs(icon_url, favicon_base::FAVICON,
+                                 false /* expired */, SK_ColorWHITE);
 }
 
-void SetFaviconRawBitmapResult(
-    const GURL& icon_url,
-    std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results) {
-  SetFaviconRawBitmapResult(icon_url,
-                            favicon_base::FAVICON,
-                            false /* expired */,
-                            favicon_bitmap_results);
+// GMock matcher to check image color.
+// Example: ImageColorIs(SK_ColorYELLOW)
+MATCHER_P(ImageColorIs, other, "") {
+  SkBitmap bitmap = arg.AsBitmap();
+  if (bitmap.empty())
+    return false;
+
+  SkAutoLockPixels lock(bitmap);
+  return bitmap.getColor(1, 1) == other;
+}
+
+// GMock matcher to check that a gfx::Image is a single resolution image and has
+// the specified edge size.
+// Example: ImageIsSingleResolutionAndHasSize(64)
+MATCHER_P(ImageIsSingleResolutionAndHasSize, other, "") {
+  gfx::ImageSkia image_skia = arg.AsImageSkia();
+  return image_skia.image_reps().size() == 1u && arg.Width() == other &&
+         arg.Height() == other;
 }
 
-// This class is used to save the download request for verifying with test case.
-class DownloadHandler {
+// MockFaviconService subclass which returns database data stored via Store() to
+// GetFaviconForPageURL() and UpdateFaviconMappingsAndFetch(). If Store() has
+// not been called for a particular URL, the callback is called with empty
+// database results.
+// Extend testing::StrictMock<MockFaviconService> to ensure that only
+// FaviconService methods overridden by this class are called.
+class TestFaviconService : public testing::StrictMock<MockFaviconService> {
  public:
-  DownloadHandler() : callback_invoked_(false) {}
-  ~DownloadHandler() {}
-
-  void Reset() {
-    download_.reset();
-    callback_invoked_ = false;
-    // Does not affect |should_fail_download_icon_urls_| and
-    // |failed_download_icon_urls_|.
+  TestFaviconService() {}
+  ~TestFaviconService() override {}
+
+  // Stores favicon with bitmap data in |results| at |page_url| and |icon_url|.
+  void Store(const GURL& page_url,
+             const GURL& icon_url,
+             const std::vector<favicon_base::FaviconRawBitmapResult>& result) {
+    results_[page_url] = result;
+    results_[icon_url] = result;
   }
 
-  // Make downloads for any of |icon_urls| fail.
-  void FailDownloadForIconURLs(const std::set<GURL>& icon_urls) {
-    should_fail_download_icon_urls_ = icon_urls;
+  // Disables automatic callback for |url|. This is useful for emulating a
+  // db request taking a long time. The callback will for the db request will be
+  // stored in |manual_callback_|.
+  void SetRunCallbackManuallyForUrl(const GURL& url) {
+    manual_callback_url_ = url;
   }
 
-  // Returns whether a download for |icon_url| did fail.
-  bool DidFailDownloadForIconURL(const GURL& icon_url) const {
-    return failed_download_icon_urls_.count(icon_url);
+  base::CancelableTaskTracker::TaskId GetFaviconForPageURL(
+      const GURL& page_url,
+      int icon_types,
+      int desired_size_in_dip,
+      const favicon_base::FaviconResultsCallback& callback,
+      base::CancelableTaskTracker* tracker) override {
+    return GetFaviconForPageOrIconURL(page_url, callback, tracker);
   }
 
-  void AddDownload(int download_id,
-                   const GURL& image_url,
-                   const std::vector<int>& image_sizes,
-                   int max_image_size,
-                   FaviconHandler::Delegate::ImageDownloadCallback callback) {
-    download_.reset(new Download(download_id, image_url, image_sizes,
-                                 max_image_size, callback));
+  base::CancelableTaskTracker::TaskId UpdateFaviconMappingsAndFetch(
+      const GURL& page_url,
+      const std::vector<GURL>& icon_urls,
+      int icon_types,
+      int desired_size_in_dip,
+      const favicon_base::FaviconResultsCallback& callback,
+      base::CancelableTaskTracker* tracker) override {
+    EXPECT_EQ(1u, icon_urls.size());
+    return GetFaviconForPageOrIconURL(icon_urls[0], callback, tracker);
   }
 
-  void InvokeCallback();
-
-  bool HasDownload() const { return download_.get(); }
-  const GURL& GetImageUrl() const { return download_->image_url; }
-  void SetImageSizes(const std::vector<int>& sizes) {
-    download_->image_sizes = sizes; }
+  void SetFavicons(const GURL& page_url,
+                   const GURL& icon_url,
+                   favicon_base::IconType icon_type,
+                   const gfx::Image& image) override {
+    set_favicon_urls_.push_back(icon_url);
+    // Consider mocking SetFavicons() in a subclass and using EXPECT_CALL() if
+    // you need to verify additional parameters.
+  }
 
- private:
-  struct Download {
-    Download(int id,
-             GURL url,
-             const std::vector<int>& sizes,
-             int max_size,
-             FaviconHandler::Delegate::ImageDownloadCallback callback)
-        : download_id(id),
-          image_url(url),
-          image_sizes(sizes),
-          max_image_size(max_size),
-          callback(callback) {}
-    ~Download() {}
-    int download_id;
-    GURL image_url;
-    std::vector<int> image_sizes;
-    int max_image_size;
-    FaviconHandler::Delegate::ImageDownloadCallback callback;
-  };
+  void UnableToDownloadFavicon(const GURL& icon_url) override {
+    unable_to_download_icon_urls_.insert(icon_url);
+  }
 
-  std::unique_ptr<Download> download_;
-  bool callback_invoked_;
+  bool WasUnableToDownloadFavicon(const GURL& icon_url) const override {
+    return unable_to_download_icon_urls_.count(icon_url);
+  }
 
-  // The icon URLs for which the download should fail.
-  std::set<GURL> should_fail_download_icon_urls_;
+  // Returns pending and.depleted database request URLs.
+  const std::vector<GURL>& db_requests() const { return db_requests_; }
 
-  // The icon URLs for which the download did fail. This should be a subset of
-  // |should_fail_download_icon_urls_|.
-  std::set<GURL> failed_download_icon_urls_;
+  // Icons which were stored into the database via SetFavicons().
+  const std::vector<GURL>& set_favicon_urls() {
+    return set_favicon_urls_;
+  }
 
-  DISALLOW_COPY_AND_ASSIGN(DownloadHandler);
-};
+  // Callback for DownloadImage() request for |manual_callback_url_|.
+  base::Closure manual_callback_;
 
-// This class is used to save the history request for verifying with test case.
-// It also will be used to simulate the history response.
-class HistoryRequestHandler {
- public:
-  HistoryRequestHandler(const GURL& page_url,
-                        const GURL& icon_url,
-                        int icon_type,
-                        const favicon_base::FaviconResultsCallback& callback)
-      : page_url_(page_url),
-        icon_url_(icon_url),
-        icon_type_(icon_type),
-        callback_(callback) {
+ private:
+  base::CancelableTaskTracker::TaskId GetFaviconForPageOrIconURL(
+      const GURL& page_or_icon_url,
+      const favicon_base::FaviconResultsCallback& callback,
+      base::CancelableTaskTracker* tracker) {
+    db_requests_.push_back(page_or_icon_url);
+
+    base::Closure bound_callback =
+        base::Bind(callback, results_[page_or_icon_url]);
+    if (page_or_icon_url == manual_callback_url_) {
+      manual_callback_ = bound_callback;
+    } else {
+      tracker->PostTask(base::ThreadTaskRunnerHandle::Get().get(), FROM_HERE,
+                        bound_callback);
+    }
+    return 0;
   }
 
-  HistoryRequestHandler(const GURL& page_url,
-                        const GURL& icon_url,
-                        int icon_type,
-                        const std::vector<unsigned char>& bitmap_data,
-                        const gfx::Size& size)
-      : page_url_(page_url),
-        icon_url_(icon_url),
-        icon_type_(icon_type),
-        bitmap_data_(bitmap_data),
-        size_(size) {
-  }
+  // URL to disable automatic callbacks for.
+  GURL manual_callback_url_;
 
-  ~HistoryRequestHandler() {}
-  void InvokeCallback();
+  // Pending and.depleted database request URLs.
+  std::vector<GURL> db_requests_;
 
-  const GURL page_url_;
-  const GURL icon_url_;
-  const int icon_type_;
-  const std::vector<unsigned char> bitmap_data_;
-  const gfx::Size size_;
-  std::vector<favicon_base::FaviconRawBitmapResult> history_results_;
-  favicon_base::FaviconResultsCallback callback_;
+  // Icons which were stored in the database via SetFavicons().
+  std::vector<GURL> set_favicon_urls_;
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(HistoryRequestHandler);
+  std::map<GURL, std::vector<favicon_base::FaviconRawBitmapResult>> results_;
+  std::set<GURL> unable_to_download_icon_urls_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFaviconService);
 };
 
-class TestDelegate : public FaviconHandler::Delegate {
+// TestFaviconservice subclass where SetFavicons() is mocked.
+class ServiceMockSetFavicons : public TestFaviconService {
  public:
-  TestDelegate() : num_notifications_(0), download_id_(0) {}
+  ServiceMockSetFavicons() {}
+  ~ServiceMockSetFavicons() override {}
 
-  int DownloadImage(const GURL& image_url,
-                    int max_bitmap_size,
-                    ImageDownloadCallback callback) override {
-    // Do not do a download if downloading |image_url| failed previously. This
-    // emulates the behavior of FaviconDriver::DownloadImage()
-    if (download_handler_.DidFailDownloadForIconURL(image_url)) {
-      download_handler_.AddDownload(download_id_, image_url, std::vector<int>(),
-                                    0, callback);
-      return 0;
-    }
+  MOCK_METHOD4(SetFavicons,
+               void(const GURL& page_url,
+                    const GURL& icon_url,
+                    favicon_base::IconType icon_type,
+                    const gfx::Image& image));
 
-    download_id_++;
-    std::vector<int> sizes;
-    sizes.push_back(0);
-    download_handler_.AddDownload(download_id_, image_url, sizes,
-                                  max_bitmap_size, callback);
-    return download_id_;
-  }
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ServiceMockSetFavicons);
+};
 
-  bool IsOffTheRecord() override { return false; }
+struct WebImage {
+  WebImage() : http_status_code(404) {}
 
-  bool IsBookmarked(const GURL& url) override { return false; }
+  int http_status_code;
+  std::vector<SkBitmap> bitmaps;
+  std::vector<gfx::Size> original_bitmap_sizes;
+};
 
-  void OnFaviconUpdated(
-      const GURL& page_url,
-      FaviconDriverObserver::NotificationIconType notification_icon_type,
-      const GURL& icon_url,
-      bool icon_url_changed,
-      const gfx::Image& image) override {
-    ++num_notifications_;
-    icon_url_ = icon_url;
-    image_ = image;
+WebImage CreateWebImageWithSpecs(
+    int http_status_code,
+    const std::vector<gfx::Size>& bitmap_sizes,
+    const std::vector<gfx::Size>& original_bitmap_sizes,
+    SkColor color) {
+  WebImage image;
+  image.http_status_code = http_status_code;
+  for (const gfx::Size& bitmap_size : bitmap_sizes) {
+    image.bitmaps.push_back(
+        CreateBitmap(bitmap_size.width(), bitmap_size.height(), color));
   }
+  image.original_bitmap_sizes = original_bitmap_sizes;
+  return image;
+}
 
-  DownloadHandler* download_handler() { return &download_handler_; }
-  const GURL& icon_url() const { return icon_url_; }
-  const gfx::Image& image() const { return image_; }
-  size_t num_notifications() const { return num_notifications_; }
-  void ResetNumNotifications() { num_notifications_ = 0; }
-
- private:
-  GURL icon_url_;
-  gfx::Image image_;
-  size_t num_notifications_;
+WebImage CreateWebImageWithEdgeSize(int edge_size) {
+  return CreateWebImageWithSpecs(200, {gfx::Size(edge_size, edge_size)},
+                                 {gfx::Size(edge_size, edge_size)},
+                                 SK_ColorWHITE);
+}
 
-  // The unique id of a download request. It will be returned to a
-  // FaviconHandler.
-  int download_id_;
+WebImage CreateWebImageWithColor(SkColor color) {
+  return CreateWebImageWithSpecs(
+      200, {gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize)},
+      {gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize)}, color);
+}
 
-  DownloadHandler download_handler_;
+FaviconURL CreateFaviconURLWithEdgeSize(const GURL url, int edge_size) {
+  return FaviconURL(url, favicon_base::FAVICON, {gfx::Size(edge_size, edge_size)});
+}
 
-  DISALLOW_COPY_AND_ASSIGN(TestDelegate);
-};
+// FaviconHandler::Delegate subclass with setters for setting the data returned
+// to DownloadImage(). If data was not set for a particular URL, a 404 response
+// is returned to DownloadImage().
+class TestDelegate : public FaviconHandler::Delegate {
+ public:
+  TestDelegate() {}
+  virtual ~TestDelegate() {}
 
-}  // namespace
+  // Sets default download data for |url|.
+  void AddWebImage(const GURL& url) {
+    Add(url, CreateWebImageWithEdgeSize(gfx::kFaviconSize));
+  }
 
-// This class is used to catch the FaviconHandler's download and history
-// request, and also provide the methods to access the FaviconHandler
-// internals.
-class TestFaviconHandler : public FaviconHandler {
- public:
-  static int GetMaximalIconSize(favicon_base::IconType icon_type) {
-    return FaviconHandler::GetMaximalIconSize(icon_type);
+  // Sets download data for |favicons|. Extracts the url and downloaded bitmap
+  // data size from |favicons|.
+  void AddWebImagesForFaviconURLs(const std::vector<FaviconURL>& favicons) {
+    for (const FaviconURL& favicon : favicons) {
+      Add(favicon.icon_url,
+          CreateWebImageWithSpecs(200, favicon.icon_sizes, favicon.icon_sizes,
+                                  SK_ColorWHITE));
+    }
   }
 
-  TestFaviconHandler(FaviconHandler::Delegate* delegate,
-                     FaviconDriverObserver::NotificationIconType handler_type)
-      : FaviconHandler(nullptr, delegate, handler_type) {}
+  // Sets |image| as the download data for |url|.
+  void Add(const GURL& url, const WebImage& image) {
+    web_images_[url] = image;
+  }
 
-  ~TestFaviconHandler() override {}
+  // Returns pending and.depleted download URLs.
+  const std::vector<GURL>& downloads() const { return downloads_; }
 
-  HistoryRequestHandler* history_handler() {
-    return history_handler_.get();
+  // URLs for which OnFaviconUpdated() has been called.
+  const std::vector<GURL>& on_favicon_updated_urls() const {
+    return on_favicon_updated_urls_;
   }
 
-  // This method will take the ownership of the given handler.
-  void set_history_handler(HistoryRequestHandler* handler) {
-    history_handler_.reset(handler);
+  // Disables automatic callback for |url|. This is useful for emulating a
+  // download taking a long time. The callback will for DownloadImage() will be
+  // stored in |manual_callback_|.
+  void SetRunCallbackManuallyForUrl(const GURL& url) {
+    manual_callback_url_ = url;
   }
 
-  FaviconURL* current_candidate() {
-    return FaviconHandler::current_candidate();
+  int DownloadImage(const GURL& url,
+                    int max_image_size,
+                    ImageDownloadCallback callback) override {
+    downloads_.push_back(url);
+
+    const WebImage& image = web_images_[url];
+    int download_id = GetNextDownloadId();
+    base::Closure bound_callback =
+        base::Bind(callback, download_id, image.http_status_code, url,
+                   image.bitmaps, image.original_bitmap_sizes);
+    if (url == manual_callback_url_)
+      manual_callback_ = bound_callback;
+    else
+      base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, bound_callback);
+    return download_id;
   }
 
-  size_t current_candidate_index() const {
-    return current_candidate_index_;
-  }
+  bool IsOffTheRecord() override { return false; }
 
-  const FaviconCandidate& best_favicon_candidate() {
-    return best_favicon_candidate_;
-  }
+  bool IsBookmarked(const GURL& url) override { return false; }
 
- protected:
-  void UpdateFaviconMappingAndFetch(
+  void OnFaviconUpdated(
       const GURL& page_url,
+      FaviconDriverObserver::NotificationIconType notification_icon_type,
       const GURL& icon_url,
-      favicon_base::IconType icon_type,
-      const favicon_base::FaviconResultsCallback& callback,
-      base::CancelableTaskTracker* tracker) override {
-    history_handler_.reset(new HistoryRequestHandler(page_url, icon_url,
-                                                     icon_type, callback));
-  }
-
-  void GetFaviconFromFaviconService(
-      const GURL& icon_url,
-      favicon_base::IconType icon_type,
-      const favicon_base::FaviconResultsCallback& callback,
-      base::CancelableTaskTracker* tracker) override {
-    history_handler_.reset(new HistoryRequestHandler(GURL(), icon_url,
-                                                     icon_type, callback));
+      bool icon_url_changed,
+      const gfx::Image& image) override {
+    on_favicon_updated_urls_.push_back(icon_url);
+    // Consider mocking OnFaviconUpdated() in a subclass and using EXPECT_CALL()
+    // if you need to verify additional parameters.
   }
 
-  void GetFaviconForURLFromFaviconService(
-      const GURL& page_url,
-      int icon_types,
-      const favicon_base::FaviconResultsCallback& callback,
-      base::CancelableTaskTracker* tracker) override {
-    history_handler_.reset(new HistoryRequestHandler(page_url, GURL(),
-                                                     icon_types, callback));
-  }
+  // Callback for DownloadImage() request for |manual_callback_url_|.
+  base::Closure manual_callback_;
 
-  void SetHistoryFavicons(const GURL& page_url,
-                          const GURL& icon_url,
-                          favicon_base::IconType icon_type,
-                          const gfx::Image& image) override {
-    scoped_refptr<base::RefCountedMemory> bytes = image.As1xPNGBytes();
-    std::vector<unsigned char> bitmap_data(bytes->front(),
-                                           bytes->front() + bytes->size());
-    history_handler_.reset(new HistoryRequestHandler(
-        page_url, icon_url, icon_type, bitmap_data, image.Size()));
-  }
+ private:
+  // URL to disable automatic callbacks for.
+  GURL manual_callback_url_;
 
-  bool ShouldSaveFavicon() override { return true; }
+  // Pending and.deplated download URLs.
+  std::vector<GURL> downloads_;
 
-  GURL page_url_;
+  // Icon URLs for which OnFaviconUpdated() has been called.
+  std::vector<GURL> on_favicon_updated_urls_;
 
- private:
-  std::unique_ptr<HistoryRequestHandler> history_handler_;
+  std::map<GURL, WebImage> web_images_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestFaviconHandler);
+  DISALLOW_COPY_AND_ASSIGN(TestDelegate);
 };
 
-namespace {
-
-void HistoryRequestHandler::InvokeCallback() {
-  if (!callback_.is_null()) {
-    callback_.Run(history_results_);
-  }
-}
+// TestDelegate subclass where OnFaviconUpdated is mocked.
+class DelegateMockOnFaviconUpdated : public TestDelegate {
+ public:
+  DelegateMockOnFaviconUpdated() {}
+  ~DelegateMockOnFaviconUpdated() override {}
 
-void DownloadHandler::InvokeCallback() {
-  if (callback_invoked_)
-    return;
+  MOCK_METHOD5(
+      OnFaviconUpdated,
+      void(const GURL& page_url,
+           FaviconDriverObserver::NotificationIconType notification_icon_type,
+           const GURL& icon_url,
+           bool icon_url_changed,
+           const gfx::Image& image));
 
-  std::vector<gfx::Size> original_bitmap_sizes;
-  std::vector<SkBitmap> bitmaps;
-  if (should_fail_download_icon_urls_.count(download_->image_url)) {
-    failed_download_icon_urls_.insert(download_->image_url);
-  } else {
-    for (std::vector<int>::const_iterator i = download_->image_sizes.begin();
-         i != download_->image_sizes.end(); ++i) {
-      int original_size = (*i > 0) ? *i : gfx::kFaviconSize;
-      int downloaded_size = original_size;
-      if (download_->max_image_size != 0 &&
-          downloaded_size > download_->max_image_size) {
-        downloaded_size = download_->max_image_size;
-      }
-      SkBitmap bitmap;
-      FillDataToBitmap(downloaded_size, downloaded_size, &bitmap);
-      bitmaps.push_back(bitmap);
-      original_bitmap_sizes.push_back(gfx::Size(original_size, original_size));
-    }
-  }
-  download_->callback.Run(download_->download_id,
-                          /*=status_code=*/200, download_->image_url, bitmaps,
-                          original_bitmap_sizes);
-  callback_invoked_ = true;
-}
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DelegateMockOnFaviconUpdated);
+};
 
 class FaviconHandlerTest : public testing::Test {
- protected:
-  FaviconHandlerTest() {
-  }
+ public:
+  const GURL kPageUrl = GURL("https://www.google.de");
+  const std::vector<gfx::Size> kEmptySizes = std::vector<gfx::Size>();
 
+  FaviconHandlerTest()
+      : favicon_service_(new TestFaviconService), delegate_(new TestDelegate) {}
   ~FaviconHandlerTest() override {}
 
-  // Simulates requesting a favicon for |page_url| given:
-  // - We have not previously cached anything in history for |page_url| or for
-  //   any of |candidates|.
-  // - The page provides favicons at |candidate_icons|.
-  // - The favicons at |candidate_icons| have edge pixel sizes of
-  //   |candidate_icon_sizes|.
-  void DownloadTillDoneIgnoringHistory(
-      TestDelegate* delegate,
-      TestFaviconHandler* favicon_handler,
-      const GURL& page_url,
-      const std::vector<FaviconURL>& candidate_icons,
-      const int* candidate_icon_sizes) {
-    size_t old_num_notifications = delegate->num_notifications();
-
-    UpdateFaviconURL(delegate, favicon_handler, page_url, candidate_icons);
-    EXPECT_EQ(candidate_icons.size(), favicon_handler->image_urls().size());
-
-    DownloadHandler* download_handler = delegate->download_handler();
-    for (size_t i = 0; i < candidate_icons.size(); ++i) {
-      favicon_handler->history_handler()->history_results_.clear();
-      favicon_handler->history_handler()->InvokeCallback();
-      ASSERT_TRUE(download_handler->HasDownload());
-      EXPECT_EQ(download_handler->GetImageUrl(),
-                candidate_icons[i].icon_url);
-      std::vector<int> sizes;
-      sizes.push_back(candidate_icon_sizes[i]);
-      download_handler->SetImageSizes(sizes);
-      download_handler->InvokeCallback();
-
-      download_handler->Reset();
-
-      if (delegate->num_notifications() > old_num_notifications)
-        return;
-    }
+  // Runs the FaviconHandler for a navigation to |kPageUrl| and <link> tags
+  // |candidates|.
+  std::unique_ptr<FaviconHandler> RunHandlerWithCandidates(
+      FaviconDriverObserver::NotificationIconType handler_type,
+      const std::vector<favicon::FaviconURL>& candidates) {
+    std::unique_ptr<FaviconHandler> handler = base::MakeUnique<FaviconHandler>(
+        favicon_service_.get(), delegate_.get(), handler_type);
+    handler->FetchFavicon(kPageUrl);
+    base::RunLoop().RunUntilIdle();
+    handler->OnUpdateFaviconURL(kPageUrl, candidates);
+    base::RunLoop().RunUntilIdle();
+    return handler;
   }
 
-  void UpdateFaviconURL(TestDelegate* delegate,
-                        TestFaviconHandler* favicon_handler,
-                        const GURL& page_url,
-                        const std::vector<FaviconURL>& candidate_icons) {
-    delegate->ResetNumNotifications();
-
-    favicon_handler->FetchFavicon(page_url);
-    favicon_handler->history_handler()->InvokeCallback();
-
-    favicon_handler->OnUpdateFaviconURL(page_url, candidate_icons);
+  // Runs the FaviconHandler for a navigation to |kPageUrl| and
+  // <link> tags |candidates|. Makes all of the <link> tags
+  // <link rel="icon" sizes="">
+  std::unique_ptr<FaviconHandler> RunHandlerWithGURLCandidates(
+      FaviconDriverObserver::NotificationIconType handler_type,
+      const std::vector<GURL>& candidates) {
+    std::vector<favicon::FaviconURL> candidate_favicons;
+    for (const GURL& candidate : candidates) {
+      candidate_favicons.push_back(
+          favicon::FaviconURL(candidate, favicon_base::FAVICON, kEmptySizes));
+    }
+    return RunHandlerWithCandidates(handler_type, candidate_favicons);
   }
 
   void SetUp() override {
-    // The score computed by SelectFaviconFrames() is dependent on the supported
+    testing::Test::SetUp();
+
+    // The score.deputed by SelectFaviconFrames() is dependent on the supported
     // scale factors of the platform. It is used for determining the goodness of
     // a downloaded bitmap in FaviconHandler::OnDidDownloadFavicon().
     // Force the values of the scale factors so that the tests produce the same
@@ -431,916 +420,626 @@ class FaviconHandlerTest : public testing::Test {
     scale_factors.push_back(ui::SCALE_FACTOR_100P);
     scoped_set_supported_scale_factors_.reset(
         new ui::test::ScopedSetSupportedScaleFactors(scale_factors));
-    testing::Test::SetUp();
   }
 
+  std::unique_ptr<TestFaviconService> favicon_service_;
+  std::unique_ptr<TestDelegate> delegate_;
+
+ private:
   std::unique_ptr<ui::test::ScopedSetSupportedScaleFactors>
       scoped_set_supported_scale_factors_;
+
+  base::MessageLoopForUI message_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(FaviconHandlerTest);
 };
 
-TEST_F(FaviconHandlerTest, GetFaviconFromHistory) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
-
-  SetFaviconRawBitmapResult(icon_url, &history_handler->history_results_);
-
-  // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-
-  // 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.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(delegate.download_handler()->HasDownload());
+// Test that FaviconHandler process finishes when:
+// - The icon URL used by the page has changed.
+// AND
+// - FaviconService::GetFaviconForPageURL() callback returns before
+// FaviconHandler::OnUpdateFaviconURL() is called.
+TEST_F(FaviconHandlerTest, UpdateAndDownloadFaviconDbResultBeforeCandidates) {

[pkotwicz, 2017/03/10 05:47:01]

GetFaviconFromHistory

+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  GURL kNewIconUrl("https://www.google.de/new_favicon.ico");
+  favicon_service_->Store(
+      kPageUrl, kIconUrl,
+      CreateDbResultWithSpecs(kIconUrl, favicon_base::FAVICON,
+                              true /* expired */, SK_ColorWHITE));
+  delegate_->AddWebImage(kNewIconUrl);
+
+  FaviconHandler handler(favicon_service_.get(), delegate_.get(),
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+
+  handler.FetchFavicon(kPageUrl);
+  base::RunLoop().RunUntilIdle();
+  // We should send the old icon to the UI because the page load (a.k.a.
+  // OnUpdateFaviconURL() call) may not finish for a while.
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl));
+
+  favicon::FaviconURL new_favicon(kNewIconUrl, favicon_base::FAVICON,
+                                  kEmptySizes);
+  handler.OnUpdateFaviconURL(kPageUrl, {new_favicon});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_THAT(delegate_->downloads(), ElementsAre(kNewIconUrl));
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(kIconUrl, kNewIconUrl));
+  EXPECT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kNewIconUrl));
 }
 
-TEST_F(FaviconHandlerTest, DownloadFavicon) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
-
-  // Set icon data expired
-  SetFaviconRawBitmapResult(icon_url,
-                            favicon_base::FAVICON,
-                            true /* expired */,
-                            &history_handler->history_results_);
-  // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-
-  // 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.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 = delegate.download_handler();
-  EXPECT_TRUE(download_handler->HasDownload());
-
-  // Verify the download request.
-  EXPECT_EQ(icon_url, download_handler->GetImageUrl());
-
-  // Reset the history_handler to verify whether favicon is set.
-  helper.set_history_handler(nullptr);
-
-  // Smulates download done.
-  download_handler->InvokeCallback();
-
-  // New icon should be saved to history backend and navigation entry.
-  history_handler = helper.history_handler();
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(icon_url, history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
-  EXPECT_LT(0U, history_handler->bitmap_data_.size());
-  EXPECT_EQ(page_url, history_handler->page_url_);
-
-  // Verify NavigationEntry.
-  EXPECT_EQ(2u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-  EXPECT_FALSE(delegate.image().IsEmpty());
-  EXPECT_EQ(gfx::kFaviconSize, delegate.image().Width());
+// Test that FaviconHandler process finishes when:
+// - The icon URL used by the page has changed.
+// AND
+// - FaviconService::GetFaviconForPageURL() callback returns after
+// FaviconHandler::OnUpdateFaviconURL() is called.
+TEST_F(FaviconHandlerTest, UpdateAndDownloadFaviconDbResultAfterCandidates) {
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  GURL kNewIconUrl("https://www.google.de/new_favicon.ico");
+  favicon_service_->Store(
+      kPageUrl, kIconUrl,
+      CreateDbResultWithSpecs(kIconUrl, favicon_base::FAVICON,
+                              true /* expired */, SK_ColorWHITE));
+  favicon_service_->SetRunCallbackManuallyForUrl(kPageUrl);
+  delegate_->AddWebImage(kNewIconUrl);
+
+  FaviconHandler handler(favicon_service_.get(), delegate_.get(),
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+
+  handler.FetchFavicon(kPageUrl);
+  favicon::FaviconURL favicon(kNewIconUrl, favicon_base::FAVICON, kEmptySizes);
+  handler.OnUpdateFaviconURL(kPageUrl, {favicon});
+  base::RunLoop().RunUntilIdle();
+  // There is a pending db request for |kPageUrl|.
+  EXPECT_TRUE(delegate_->on_favicon_updated_urls().empty());
+  EXPECT_THAT(favicon_service_->db_requests(), ElementsAre(kPageUrl));
+
+  // Emulate FaviconService::GetFaviconForPageURL() request completing after a
+  // very long time.
+  favicon_service_->manual_callback_.Run();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_THAT(delegate_->downloads(), ElementsAre(kNewIconUrl));
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kNewIconUrl));
+  EXPECT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kNewIconUrl));
 }
 
-TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
-  const GURL new_icon_url("http://www.google.com/new_favicon");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
-
-  // Set valid icon data.
-  SetFaviconRawBitmapResult(icon_url, &history_handler->history_results_);
-
-  // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status.
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-
-  // Reset the history_handler to verify whether new icon is requested from
-  // 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.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_);
-
-  // Simulate not find icon.
-  history_handler->history_results_.clear();
-  history_handler->InvokeCallback();
-
-  // Favicon should request to download icon now.
-  DownloadHandler* download_handler = delegate.download_handler();
-  EXPECT_TRUE(download_handler->HasDownload());
-
-  // Verify the download request.
-  EXPECT_EQ(new_icon_url, download_handler->GetImageUrl());
-
-  // Reset the history_handler to verify whether favicon is set.
-  helper.set_history_handler(nullptr);
-
-  // Smulates download done.
-  download_handler->InvokeCallback();
-
-  // New icon should be saved to history backend and navigation entry.
-  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_LT(0U, history_handler->bitmap_data_.size());
-  EXPECT_EQ(page_url, history_handler->page_url_);
-
-  // Verify NavigationEntry.
-  EXPECT_EQ(2u, delegate.num_notifications());
-  EXPECT_EQ(new_icon_url, delegate.icon_url());
-  EXPECT_FALSE(delegate.image().IsEmpty());
-  EXPECT_EQ(gfx::kFaviconSize, delegate.image().Width());
+// Test that download data for icon URLs other than the current favicon
+// candidate URLs is ignored. This test tests the scenario where a download is
+// in flight when FaviconHandler::OnUpdateFaviconURL() is called.
+TEST_F(FaviconHandlerTest, ShouldIgnoreDownloadDataForNonCandidateUrls) {

[pkotwicz, 2017/03/10 05:47:02]

UpdateDuringDownloading

+  GURL kOtherPageUrl("https://www.bing.de");
+  GURL kIconUrl1("https://www.google.de/favicon1.png");
+  GURL kIconUrl2("https://www.google.de/favicon2.png");
+  GURL kIconUrl3("https://www.google.de/favicon3.png");
+  delegate_->AddWebImage(kIconUrl1);
+  delegate_->AddWebImage(kIconUrl2);
+  favicon_service_->Store(kOtherPageUrl, kIconUrl3, CreateDbResult(kIconUrl3));
+
+  delegate_->SetRunCallbackManuallyForUrl(kIconUrl1);
+
+  std::unique_ptr<FaviconHandler> handler = RunHandlerWithGURLCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl1, kIconUrl2});
+
+  // The FaviconHandler process should not have finished yet.
+  ASSERT_TRUE(delegate_->on_favicon_updated_urls().empty());
+  ASSERT_TRUE(favicon_service_->set_favicon_urls().empty());
+
+  ASSERT_THAT(favicon_service_->db_requests(), ElementsAre(kPageUrl, kIconUrl1));
+  // There should be a pending download for |kIconUrl1|.
+  ASSERT_THAT(delegate_->downloads(), ElementsAre(kIconUrl1));
+
+  favicon::FaviconURL favicon3(kIconUrl3, favicon_base::FAVICON, kEmptySizes);
+  handler->OnUpdateFaviconURL(kPageUrl, {favicon3});
+
+  // Emulate the download finishing. The downloaded icon should be thrown away
+  // due to the update.
+  delegate_->manual_callback_.Run();
+  base::RunLoop().RunUntilIdle();
+
+  // The FaviconHandler should have completed.
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl3));
+  EXPECT_TRUE(favicon_service_->set_favicon_urls().empty());
+
+  EXPECT_THAT(favicon_service_->db_requests(), ElementsAre(kPageUrl, kIconUrl1, kIconUrl3));
+  EXPECT_THAT(delegate_->downloads(), ElementsAre(kIconUrl1));
 }
 
-TEST_F(FaviconHandlerTest, FaviconInHistoryInvalid) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
+// Test that calling OnUpdateFaviconUrl() with the same icon URLs as before is a
+// no-op. This is important because OnUpdateFaviconUrl() is called when the page
+// finishes loading. This can occur several for pages with iframes.
+TEST_F(FaviconHandlerTest, UpdateSameIconURLsShouldBeNoop) {

[pkotwicz, 2017/03/10 05:47:01]

UpdateSameIconURLs

+  GURL kIconUrl10x10("https://www.google.de/favicon10x10.png");
+  GURL kIconUrl11x11("https://www.google.de/favicon11x11.png");
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(kIconUrl11x11, CreateWebImageWithEdgeSize(11));
+
+  FaviconHandler handler(favicon_service_.get(), delegate_.get(),
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(kPageUrl);
+  favicon::FaviconURL favicon10x10(kIconUrl10x10, favicon_base::FAVICON, kEmptySizes);
+  favicon::FaviconURL favicon11x11(kIconUrl11x11, favicon_base::FAVICON, kEmptySizes);
+  handler.OnUpdateFaviconURL(kPageUrl, {favicon10x10, favicon11x11});
+  base::RunLoop().RunUntilIdle();
+
+  ASSERT_THAT(favicon_service_->db_requests(),
+              ElementsAre(kPageUrl, kIconUrl10x10, kIconUrl11x11));
+  ASSERT_THAT(delegate_->downloads(),
+              ElementsAre(kIconUrl10x10, kIconUrl11x11));
+  ASSERT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl11x11));
+  ASSERT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kIconUrl11x11));
+
+  // Calling OnUpdateFaviconURL() with identical data should be a no-op.
+  handler.OnUpdateFaviconURL(kPageUrl, {favicon10x10, favicon11x11});
+  base::RunLoop().RunUntilIdle();
+
+  // No new db requests.
+  EXPECT_THAT(favicon_service_->db_requests(),
+              ElementsAre(kPageUrl, kIconUrl10x10, kIconUrl11x11));
+  // No new downloads.
+  EXPECT_THAT(delegate_->downloads(),
+              ElementsAre(kIconUrl10x10, kIconUrl11x11));
+  // No new UI notifications.
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl11x11));
+  // No new icons stored in the db.
+  EXPECT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kIconUrl11x11));
+}
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
+// Test that calling OnUpdateFaviconUrl() with the same icon URLs as the
+// previous call to OnUpdateFaviconUrl() while the FaviconHandler process is in
+// progresss does not restart the process from the beginning.
+TEST_F(FaviconHandlerTest,
+       UpdateSameIconURLsWhileHandlerProcessingShouldBeNoop) {

[pkotwicz, 2017/03/10 05:47:02]

UpdateSameIconURLs

+  GURL kIconUrl10x10("https://www.google.de/favicon10x10.png");
+  GURL kIconUrl11x11("https://www.google.de/favicon11x11.png");
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(kIconUrl11x11, CreateWebImageWithEdgeSize(11));
+
+  // We want to call FaviconService callback for |kIconUrl11x11| manually
+  // because we want to call OnUpdateFviconUrl() while the database request for
+  // the second candidate is in progress.
+  favicon_service_->SetRunCallbackManuallyForUrl(kIconUrl11x11);
+
+  FaviconHandler handler(favicon_service_.get(), delegate_.get(),
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(kPageUrl);
+  favicon::FaviconURL favicon10x10(kIconUrl10x10, favicon_base::FAVICON, kEmptySizes);
+  favicon::FaviconURL favicon11x11(kIconUrl11x11, favicon_base::FAVICON, kEmptySizes);
+  handler.OnUpdateFaviconURL(kPageUrl, {favicon10x10, favicon11x11});
+  base::RunLoop().RunUntilIdle();
+
+  // There should be a history request in progress for the second icon URL
+  // candidate |kIconUrl11x11|.
+  ASSERT_THAT(favicon_service_->db_requests(),
+              ElementsAre(kPageUrl, kIconUrl10x10, kIconUrl11x11));
+  // The download for |kIconUrl11x11| should not have occurred yet.
+  ASSERT_THAT(delegate_->downloads(), ElementsAre(kIconUrl10x10));
+  ASSERT_TRUE(delegate_->on_favicon_updated_urls().empty());
+  ASSERT_TRUE(favicon_service_->set_favicon_urls().empty());
+
+  // Calling OnUpdateFaviconURL() with identical data should be a no-op.
+  handler.OnUpdateFaviconURL(kPageUrl, {favicon10x10, favicon11x11});
+  favicon_service_->manual_callback_.Run();
+  base::RunLoop().RunUntilIdle();
+
+  // The FaviconHandler process should complete. There shouldn't be any extra
+  // database requests and downloads than if OnUpdateFaviconURL() had not been
+  // called.
+  ASSERT_THAT(favicon_service_->db_requests(),
+              ElementsAre(kPageUrl, kIconUrl10x10, kIconUrl11x11));
+  ASSERT_THAT(delegate_->downloads(),
+              ElementsAre(kIconUrl10x10, kIconUrl11x11));
+  ASSERT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl11x11));
+  ASSERT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kIconUrl11x11));
+}
 
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
+// Test that OnFaviconUpdated() is called to update the UI when the initial db
+// request completes if there is matching data in the database. This is nice
+// because we only get the icon URLs associated with the page once the page
+// loading stops which can take a while.
+TEST_F(FaviconHandlerTest, FaviconUpdatedWhenDbRequestCompletes) {
+  const GURL kIconUrl("https://www.google.de/favicon.ico");
+  favicon_service_->Store(kPageUrl, kIconUrl, CreateDbResult(kIconUrl));
+  FaviconHandler handler(favicon_service_.get(), delegate_.get(),
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+
+  handler.FetchFavicon(kPageUrl);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl));
+}
 
-  // Set non empty but invalid data.
-  favicon_base::FaviconRawBitmapResult bitmap_result;
-  bitmap_result.expired = false;
-  // Empty bitmap data is invalid.
-  bitmap_result.bitmap_data = new base::RefCountedBytes();
-  bitmap_result.pixel_size = gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize);
-  bitmap_result.icon_type = favicon_base::FAVICON;
-  bitmap_result.icon_url = icon_url;
-  history_handler->history_results_.clear();
-  history_handler->history_results_.push_back(bitmap_result);
-
-  // Send history response.
-  history_handler->InvokeCallback();
-  // The NavigationEntry should not be set yet as the history data is invalid.
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_EQ(GURL(), delegate.icon_url());
-
-  // Reset the history_handler to verify whether new icon is requested from
-  // history.
-  helper.set_history_handler(nullptr);
-
-  // Simulates update with matching favicon URL.
-  std::vector<FaviconURL> urls;
-  urls.push_back(
-      FaviconURL(icon_url, favicon_base::FAVICON, std::vector<gfx::Size>()));
-  helper.OnUpdateFaviconURL(page_url, urls);
-
-  // A download for the favicon should be requested, and we should not do
-  // another history request.
-  DownloadHandler* download_handler = delegate.download_handler();
-  EXPECT_TRUE(download_handler->HasDownload());
-  EXPECT_EQ(nullptr, helper.history_handler());
-
-  // Verify the download request.
-  EXPECT_EQ(icon_url, download_handler->GetImageUrl());
-
-  // Simulates download done.
-  download_handler->InvokeCallback();
-
-  // New icon should be saved to history backend and navigation entry.
-  history_handler = helper.history_handler();
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(icon_url, history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
-  EXPECT_LT(0U, history_handler->bitmap_data_.size());
-  EXPECT_EQ(page_url, history_handler->page_url_);
-
-  // Verify NavigationEntry.
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-  EXPECT_FALSE(delegate.image().IsEmpty());
-  EXPECT_EQ(gfx::kFaviconSize, delegate.image().Width());
+// Test that no downloads are done and that nothing is written to the database
+// if there is data for |kPageUrl| in the database.
+TEST_F(FaviconHandlerTest, PageUrlInDb) {
+  const GURL kIconUrl("https://www.google.de/favicon.ico");
+  favicon_service_->Store(kPageUrl, kIconUrl, CreateDbResult(kIconUrl));
+
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               {kIconUrl});
+  EXPECT_TRUE(delegate_->downloads().empty());
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl));
+  EXPECT_TRUE(favicon_service_->set_favicon_urls().empty());
 }
 
+// Test that no downloads are done if a user visits a page which changed its
+// favicon URL to a favicon URL which is already cached in the database.
 TEST_F(FaviconHandlerTest, UpdateFavicon) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
-  const GURL new_icon_url("http://www.google.com/new_favicon");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
-
-  SetFaviconRawBitmapResult(icon_url, &history_handler->history_results_);
-
-  // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status.
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-
-  // Reset the history_handler to verify whether new icon is requested from
-  // 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.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_);
-
-  // Simulate find icon.
-  SetFaviconRawBitmapResult(new_icon_url, &history_handler->history_results_);
-  history_handler->InvokeCallback();
-
-  // Shouldn't request download favicon
-  EXPECT_FALSE(delegate.download_handler()->HasDownload());
-
-  // Verify the favicon status.
-  EXPECT_EQ(2u, delegate.num_notifications());
-  EXPECT_EQ(new_icon_url, delegate.icon_url());
-  EXPECT_FALSE(delegate.image().IsEmpty());
+  GURL kDifferentPageUrl("https://www.bing.com");
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  GURL kNewIconUrl("https://www.google.de/new_favicon.ico");
+  favicon_service_->Store(kPageUrl, kIconUrl, CreateDbResult(kIconUrl));
+  favicon_service_->Store(kDifferentPageUrl, kNewIconUrl,
+                          CreateDbResult(kNewIconUrl));
+
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               {kNewIconUrl});
+
+  EXPECT_TRUE(delegate_->downloads().empty());
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(kIconUrl, kNewIconUrl));
+  EXPECT_TRUE(favicon_service_->set_favicon_urls().empty());
+}
+
+// Test that the icon is redownloaded if the icon cached for the page URL has
+// expired.
+TEST_F(FaviconHandlerTest, ExpiredDbPageUrl) {

[pkotwicz, 2017/03/10 05:47:02]

DownloadFavicon

+  // Replace |delegate_| with implementation which mocks out OnFaviconUpdated()
+  // so that we can use EXPECT_CALL().
+  using StrictDelegate = testing::StrictMock<DelegateMockOnFaviconUpdated>;
+  delegate_.reset(new StrictDelegate);
+  StrictDelegate* strict_delegate = static_cast<StrictDelegate*>(delegate_.get());
+
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  favicon_service_->Store(
+      kPageUrl, kIconUrl,
+      CreateDbResultWithSpecs(kIconUrl, favicon_base::FAVICON,
+                              true /* expired */, SK_ColorGRAY));
+  delegate_->Add(kIconUrl, CreateWebImageWithColor(SK_ColorYELLOW));
+
+  testing::InSequence seq;
+  EXPECT_CALL(*strict_delegate,
+              OnFaviconUpdated(_, _, kIconUrl, _, ImageColorIs(SK_ColorGRAY)));
+  EXPECT_CALL(*strict_delegate, OnFaviconUpdated(_, _, kIconUrl, _,
+                                                 ImageColorIs(SK_ColorYELLOW)));
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               {kIconUrl});
+
+  // We know from |kPageUrl| database request that |kIconUrl| has expired. A
+  // second request for |kIconUrl| should not have been made because it is
+  // redundant.
+  EXPECT_THAT(favicon_service_->db_requests(), ElementsAre(kPageUrl));
+
+  EXPECT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kIconUrl));
 }
 
-TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
-  const GURL new_icon_url("http://www.google.com/new_favicon");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::TOUCH_LARGEST);
-  std::set<GURL> fail_downloads;
-  fail_downloads.insert(icon_url);
-  delegate.download_handler()->FailDownloadForIconURLs(fail_downloads);
-
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON | favicon_base::TOUCH_ICON,
-            history_handler->icon_type_);
-
-  // Icon not found.
-  history_handler->history_results_.clear();
-  // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status.
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_EQ(GURL(), delegate.icon_url());
-
-  // Reset the history_handler to verify whether new icon is requested from
-  // history.
-  helper.set_history_handler(nullptr);
-
-  // Simulates update with the different favicon url.
-  std::vector<FaviconURL> urls;
-  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.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 = delegate.download_handler();
-  EXPECT_TRUE(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.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_);
-
-  // Reset download handler
-  download_handler->Reset();
-
-  // Simulates getting a expired icon from history.
-  SetFaviconRawBitmapResult(new_icon_url,
-                            favicon_base::TOUCH_ICON,
-                            true /* expired */,
-                            &history_handler->history_results_);
-  history_handler->InvokeCallback();
-
-  // Verify the download request.
-  EXPECT_TRUE(delegate.download_handler()->HasDownload());
-  EXPECT_EQ(new_icon_url, download_handler->GetImageUrl());
-
-  helper.set_history_handler(nullptr);
-
-  // Simulates icon being downloaded.
-  download_handler->InvokeCallback();
-
-  // New icon should be saved to history backend.
-  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_LT(0U, history_handler->bitmap_data_.size());
-  EXPECT_EQ(page_url, history_handler->page_url_);
+// Test that the icon is redownloaded if the icon cached for the icon URL has
+// expired.
+TEST_F(FaviconHandlerTest, ExpiredDbIconUrl) {
+  // Replace |delegate_| with implementation which mocks out OnFaviconUpdated()
+  // so that we can use EXPECT_CALL().
+  using StrictDelegate = testing::StrictMock<DelegateMockOnFaviconUpdated>;
+  delegate_.reset(new StrictDelegate);
+  StrictDelegate* strict_delegate = static_cast<StrictDelegate*>(delegate_.get());
+
+  GURL kDifferentPageUrl("https://www.bing.de");
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  // There isn't an icon in the database for |kPageUrl|.
+  favicon_service_->Store(
+      kDifferentPageUrl, kIconUrl,
+      CreateDbResultWithSpecs(kIconUrl, favicon_base::FAVICON,
+                              true /* expired */, SK_ColorGRAY));
+  delegate_->Add(kIconUrl, CreateWebImageWithColor(SK_ColorYELLOW));
+
+  testing::InSequence seq;
+  EXPECT_CALL(*strict_delegate,
+              OnFaviconUpdated(_, _, kIconUrl, _, ImageColorIs(SK_ColorGRAY)));
+  EXPECT_CALL(*strict_delegate, OnFaviconUpdated(_, _, kIconUrl, _,
+                                                 ImageColorIs(SK_ColorYELLOW)));
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               {kIconUrl});
+
+  // The database contains the same data for both |kPageUrl| and |kIconUrl|. A
+  // database request should not have been done for |kIconUrl|.
+  EXPECT_THAT(favicon_service_->db_requests(), testing::Contains(kIconUrl));
+
+  EXPECT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kIconUrl));
 }
 
-TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
-  const GURL page_url("http://www.google.com");
-  const GURL icon_url("http://www.google.com/favicon");
-  const GURL new_icon_url("http://www.google.com/new_favicon");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::TOUCH_LARGEST);
-
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
-  // Ensure the data given to history is correct.
-  ASSERT_TRUE(history_handler);
-  EXPECT_EQ(page_url, history_handler->page_url_);
-  EXPECT_EQ(GURL(), history_handler->icon_url_);
-  EXPECT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON | favicon_base::TOUCH_ICON,
-            history_handler->icon_type_);
-
-  // Icon not found.
-  history_handler->history_results_.clear();
-  // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status.
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_EQ(GURL(), delegate.icon_url());
-
-  // Reset the history_handler to verify whether new icon is requested from
-  // history.
-  helper.set_history_handler(nullptr);
-
-  // Simulates update with the different favicon url.
-  std::vector<FaviconURL> urls;
-  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.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_);
-
-  // Simulate not find icon.
-  history_handler->history_results_.clear();
-  history_handler->InvokeCallback();
-
-  // Should request download favicon.
-  DownloadHandler* download_handler = delegate.download_handler();
-  EXPECT_TRUE(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);
-  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.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_);
-
-  // Reset the history_handler to verify whether favicon is request from
-  // history.
-  // Save the callback for late use.
-  favicon_base::FaviconResultsCallback callback = history_handler->callback_;
-  helper.set_history_handler(nullptr);
-
-  // Simulates download succeed.
-  download_handler->InvokeCallback();
-  // The downloaded icon should be thrown away as there is favicon update.
-  EXPECT_FALSE(helper.history_handler());
-
-  download_handler->Reset();
-
-  // Simulates getting the icon from history.
-  std::unique_ptr<HistoryRequestHandler> handler;
-  handler.reset(new HistoryRequestHandler(
-      page_url, latest_icon_url, favicon_base::TOUCH_ICON, callback));
-  SetFaviconRawBitmapResult(latest_icon_url,
-                            favicon_base::TOUCH_ICON,
-                            false /* expired */,
-                            &handler->history_results_);
-  handler->InvokeCallback();
-
-  // No download request.
-  EXPECT_FALSE(download_handler->HasDownload());
+// If there is data for the page URL in history which is invalid, test that:
+// - the invalid data is not sent to the UI.
+// - the icon is redownloaded.
+TEST_F(FaviconHandlerTest, InvalidDataForPageUrlInDb) {

[pkotwicz, 2017/03/10 05:47:01]

FaviconInHistoryInvalid

+  // Replace |delegate_| with implementation which mocks out OnFaviconUpdated()
+  // so that we can use EXPECT_CALL().
+  using StrictDelegate = testing::StrictMock<DelegateMockOnFaviconUpdated>;
+  delegate_.reset(new StrictDelegate);
+  StrictDelegate* strict_delegate = static_cast<StrictDelegate*>(delegate_.get());
+
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+    // Set non empty but invalid data.
+  favicon_base::FaviconRawBitmapResult db_result;
+  db_result.expired = false;
+  // Empty bitmap data is invalid.
+  db_result.bitmap_data = new base::RefCountedBytes();
+  db_result.pixel_size = gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize);
+  db_result.icon_type = favicon_base::FAVICON;
+  db_result.icon_url = kIconUrl;
+  favicon_service_->Store(kPageUrl, kIconUrl, {db_result});
+
+  delegate_->Add(kIconUrl, CreateWebImageWithColor(SK_ColorYELLOW));
+
+  // OnFaviconUpdated() should be called with the downloaded image but not
+
+  EXPECT_CALL(
+      *strict_delegate,
+      OnFaviconUpdated(_, _, kIconUrl, _, ImageColorIs(SK_ColorYELLOW)));
+
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               {kIconUrl});
+
+  // The database contains the same data for both |kPageUrl| and |kIconUrl|. A
+  // database request should not have been done for |kIconUrl|.
+  EXPECT_THAT(favicon_service_->db_requests(), testing::ElementsAre(kPageUrl));
+
+  EXPECT_THAT(favicon_service_->set_favicon_urls(), ElementsAre(kIconUrl));
 }
 
-// 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>()));
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  // 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);
-  delegate.download_handler()->SetImageSizes(std::vector<int>(1u, 10));
-  delegate.download_handler()->InvokeCallback();
-  delegate.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 that an icon is not redownloaded if a previous attempt returned a 404.
+TEST_F(FaviconHandlerTest, ShouldNotRedownloadIfPreviouslyGot404) {
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  delegate_->Add(kIconUrl, CreateWebImageWithSpecs(404, kEmptySizes,
+                                                   kEmptySizes, SK_ColorWHITE));
+
+  ASSERT_FALSE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
+
+  // Run handler in order to do initial unsuccessful download.
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl});
+  ASSERT_THAT(delegate_->downloads(), testing::ElementsAre(kIconUrl));
+  ASSERT_TRUE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
+
+  // Test that a new download is not done now that FaviconService knows that
+  // downloading |kIconUrl| reesults in a 404.
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl});
+  EXPECT_THAT(delegate_->downloads(), testing::ElementsAre(kIconUrl));
+  // There should be no change in status to the return value of
+  // FaviconService::WasUnableToDownload().
+  EXPECT_TRUE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
 }
 
-// Fixes crbug.com/544560
-TEST_F(FaviconHandlerTest,
-       OnFaviconAvailableNotificationSentAfterIconURLChange) {
-  const GURL kPageURL("http://www.page_which_animates_favicon.com");
-  const GURL kIconURL1("http://wwww.page_which_animates_favicon.com/frame1.png");
-  const GURL kIconURL2("http://wwww.page_which_animates_favicon.com/frame2.png");
-
-  TestDelegate delegate;
-  TestFaviconHandler helper(&delegate, FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  // Initial state:
-  // - The database does not know about |kPageURL|.
-  // - The page uses |kIconURL1| and |kIconURL2|.
-  // - The database knows about both |kIconURL1| and |kIconURl2|. Both icons
-  //   are expired in the database.
-  helper.FetchFavicon(kPageURL);
-  ASSERT_TRUE(helper.history_handler());
-  helper.history_handler()->InvokeCallback();
-  {
-    std::vector<FaviconURL> icon_urls;
-    icon_urls.push_back(
-        FaviconURL(kIconURL1, favicon_base::FAVICON, std::vector<gfx::Size>()));
-    icon_urls.push_back(
-        FaviconURL(kIconURL2, favicon_base::FAVICON, std::vector<gfx::Size>()));
-    helper.OnUpdateFaviconURL(kPageURL, icon_urls);
-  }
+// Test that WasUnableToDownloadFavicon() is not called if a download returns
+// HTTP status 200.
+TEST_F(FaviconHandlerTest, ShouldNotCallUnableToDownloadFaviconFor200) {
+  GURL kIconUrl("https://www.google.de/200.png");
+  delegate_->Add(kIconUrl, CreateWebImageWithSpecs(200, kEmptySizes,
+                                                   kEmptySizes, SK_ColorWHITE));
 
-  // FaviconHandler should request from history and download |kIconURL1| and
-  // |kIconURL2|. |kIconURL1| is the better match. A
-  // FaviconDriver::OnFaviconUpdated() notification should be sent for
-  // |kIconURL1|.
-  ASSERT_EQ(0u, delegate.num_notifications());
-  ASSERT_TRUE(helper.history_handler());
-  SetFaviconRawBitmapResult(kIconURL1,
-                            favicon_base::FAVICON,
-                            true /* expired */,
-                            &helper.history_handler()->history_results_);
-  helper.history_handler()->InvokeCallback();
-  helper.set_history_handler(nullptr);
-  ASSERT_TRUE(delegate.download_handler()->HasDownload());
-  delegate.download_handler()->SetImageSizes(std::vector<int>(1u, 15));
-  delegate.download_handler()->InvokeCallback();
-  delegate.download_handler()->Reset();
-
-  ASSERT_TRUE(helper.history_handler());
-  helper.history_handler()->InvokeCallback();
-  SetFaviconRawBitmapResult(kIconURL2,
-                            favicon_base::FAVICON,
-                            true /* expired */,
-                            &helper.history_handler()->history_results_);
-  helper.history_handler()->InvokeCallback();
-  helper.set_history_handler(nullptr);
-  ASSERT_TRUE(delegate.download_handler()->HasDownload());
-  delegate.download_handler()->SetImageSizes(std::vector<int>(1u, 10));
-  delegate.download_handler()->InvokeCallback();
-  delegate.download_handler()->Reset();
-
-  ASSERT_LT(0u, delegate.num_notifications());
-  ASSERT_EQ(kIconURL1, delegate.icon_url());
-
-  // Clear the history handler because SetHistoryFavicons() sets it.
-  helper.set_history_handler(nullptr);
-
-  // Simulate the page changing it's icon URL to just |kIconURL2| via
-  // Javascript.
-  helper.OnUpdateFaviconURL(
-      kPageURL,
-      std::vector<FaviconURL>(1u, FaviconURL(kIconURL2, favicon_base::FAVICON,
-                                             std::vector<gfx::Size>())));
-
-  // FaviconHandler should request from history and download |kIconURL2|. A
-  // FaviconDriver::OnFaviconUpdated() notification should be sent for
-  // |kIconURL2|.
-  delegate.ResetNumNotifications();
-  ASSERT_TRUE(helper.history_handler());
-  SetFaviconRawBitmapResult(kIconURL2,
-                            favicon_base::FAVICON,
-                            true /* expired */,
-                            &helper.history_handler()->history_results_);
-  helper.history_handler()->InvokeCallback();
-  helper.set_history_handler(nullptr);
-  ASSERT_TRUE(delegate.download_handler()->HasDownload());
-  delegate.download_handler()->InvokeCallback();
-  delegate.download_handler()->Reset();
-  ASSERT_LT(0u, delegate.num_notifications());
-  EXPECT_EQ(kIconURL2, delegate.icon_url());
+  ASSERT_FALSE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
+
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               {kIconUrl});
+  EXPECT_THAT(delegate_->downloads(), testing::ElementsAre(kIconUrl));
+  EXPECT_FALSE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
 }
 
-// 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>()),
-      FaviconURL(GURL("http://www.google.com/b"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      FaviconURL(GURL("http://www.google.com/c"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      FaviconURL(GURL("http://www.google.com/d"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      FaviconURL(GURL("http://www.google.com/e"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>())};
-
-  // Set the supported scale factors to 1x and 2x. This affects the behavior of
-  // SelectFaviconFrames().
-  std::vector<ui::ScaleFactor> scale_factors;
-  scale_factors.push_back(ui::SCALE_FACTOR_100P);
-  scale_factors.push_back(ui::SCALE_FACTOR_200P);
-  ui::test::ScopedSetSupportedScaleFactors scoped_supported(scale_factors);
-
-  // 1) Test that if there are several single resolution favicons to choose from
-  // that the largest exact match is chosen.
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&delegate1,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  const int kSizes1[] = { 16, 24, 32, 48, 256 };
-  std::vector<FaviconURL> urls1(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSizes1));
-  DownloadTillDoneIgnoringHistory(&delegate1, &handler1, kPageURL, urls1,
-                                  kSizes1);
-
-  EXPECT_EQ(nullptr, handler1.current_candidate());
-  EXPECT_EQ(1u, delegate1.num_notifications());
-  EXPECT_FALSE(delegate1.image().IsEmpty());
-  EXPECT_EQ(gfx::kFaviconSize, delegate1.image().Width());
-
-  size_t expected_index = 2u;
-  EXPECT_EQ(32, kSizes1[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate1.icon_url());
-
-  // 2) Test that if there are several single resolution favicons to choose
-  // from, the exact match is preferred even if it results in upsampling.
-  TestDelegate delegate2;
-  TestFaviconHandler handler2(&delegate2,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  const int kSizes2[] = { 16, 24, 48, 256 };
-  std::vector<FaviconURL> urls2(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSizes2));
-  DownloadTillDoneIgnoringHistory(&delegate2, &handler2, kPageURL, urls2,
-                                  kSizes2);
-  EXPECT_EQ(1u, delegate2.num_notifications());
-  expected_index = 0u;
-  EXPECT_EQ(16, kSizes2[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate2.icon_url());
-
-  // 3) Test that favicons which need to be upsampled a little or downsampled
-  // a little are preferred over huge favicons.
-  TestDelegate delegate3;
-  TestFaviconHandler handler3(&delegate3,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  const int kSizes3[] = { 256, 48 };
-  std::vector<FaviconURL> urls3(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSizes3));
-  DownloadTillDoneIgnoringHistory(&delegate3, &handler3, kPageURL, urls3,
-                                  kSizes3);
-  EXPECT_EQ(1u, delegate3.num_notifications());
-  expected_index = 1u;
-  EXPECT_EQ(48, kSizes3[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate3.icon_url());
-
-  TestDelegate delegate4;
-  TestFaviconHandler handler4(&delegate4,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
-
-  const int kSizes4[] = { 17, 256 };
-  std::vector<FaviconURL> urls4(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSizes4));
-  DownloadTillDoneIgnoringHistory(&delegate4, &handler4, kPageURL, urls4,
-                                  kSizes4);
-  EXPECT_EQ(1u, delegate4.num_notifications());
-  expected_index = 0u;
-  EXPECT_EQ(17, kSizes4[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate4.icon_url());
+// Test that WasUnableToDownloadFavicon() is not called if a download returns
+// HTTP status 503.
+TEST_F(FaviconHandlerTest, ShouldNotCallUnableToDownloadFaviconFor503) {
+  GURL kIconUrl("https://www.google.de/503.png");
+  delegate_->Add(kIconUrl, CreateWebImageWithSpecs(200, kEmptySizes,
+                                                   kEmptySizes, SK_ColorWHITE));
+
+  ASSERT_FALSE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
+
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl});
+  EXPECT_THAT(delegate_->downloads(), testing::ElementsAre(kIconUrl));
+  EXPECT_FALSE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
 }
 
-// Test that the best favicon is selected when:
-// - The page provides several favicons.
-// - Downloading one of the page's icon URLs previously returned a 404.
-// - None of the favicons are cached in the Favicons database.
-TEST_F(FaviconHandlerTest, MultipleFavicons404) {
-  const GURL kPageURL("http://www.google.com");
-  const GURL k404IconURL("http://www.google.com/404.png");
-  const FaviconURL k404FaviconURL(
-      k404IconURL, favicon_base::FAVICON, std::vector<gfx::Size>());
-  const FaviconURL kFaviconURLs[] = {
-      FaviconURL(GURL("http://www.google.com/a"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      k404FaviconURL,
-      FaviconURL(GURL("http://www.google.com/c"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-  };
+// Test that WasUnableToDownloadFavicon() is called if a download returns HTTP
+// status 404.
+TEST_F(FaviconHandlerTest, ShouldCallUnableToDownloadFaviconFor404) {

[pkotwicz, 2017/03/10 05:47:01]

ContentFaviconDriverTest.ShouldNotRequestRepeatedlyIfUnavailable

+  GURL kIconUrl("https://www.google.de/404.png");
+  delegate_->Add(kIconUrl, CreateWebImageWithSpecs(404, kEmptySizes,
+                                                   kEmptySizes, SK_ColorWHITE));
+
+  ASSERT_FALSE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
 
-  TestDelegate delegate;
-  TestFaviconHandler handler(&delegate,
-                             FaviconDriverObserver::NON_TOUCH_16_DIP);
-  DownloadHandler* download_handler = delegate.download_handler();
-
-  std::set<GURL> k404URLs;
-  k404URLs.insert(k404IconURL);
-  download_handler->FailDownloadForIconURLs(k404URLs);
-
-  // Make the initial download for |k404IconURL| fail.
-  const int kSizes1[] = { 0 };
-  std::vector<FaviconURL> urls1(1u, k404FaviconURL);
-  DownloadTillDoneIgnoringHistory(&delegate, &handler, kPageURL, urls1,
-                                  kSizes1);
-  EXPECT_TRUE(download_handler->DidFailDownloadForIconURL(k404IconURL));
-
-  // 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(&delegate, &handler, kPageURL, urls2,
-                                  kSizes2);
-
-  EXPECT_EQ(nullptr, handler.current_candidate());
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_FALSE(delegate.image().IsEmpty());
-  int expected_index = 2u;
-  EXPECT_EQ(16, kSizes2[expected_index]);
-  EXPECT_EQ(kFaviconURLs[expected_index].icon_url, delegate.icon_url());
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl});
+  EXPECT_THAT(delegate_->downloads(), testing::ElementsAre(kIconUrl));
+  EXPECT_TRUE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
 }
 
-// Test that no favicon is selected when:
-// - The page provides several favicons.
-// - Downloading the page's icons has previously returned a 404.
-// - None of the favicons are cached in the Favicons database.
-TEST_F(FaviconHandlerTest, MultipleFaviconsAll404) {
-  const GURL kPageURL("http://www.google.com");
-  const GURL k404IconURL1("http://www.google.com/4041.png");
-  const GURL k404IconURL2("http://www.google.com/4042.png");
-  const FaviconURL kFaviconURLs[] = {
-      FaviconURL(k404IconURL1,
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      FaviconURL(k404IconURL2,
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-  };
+// Test that an icon is not downloaded if a previous attempt failed.
+TEST_F(FaviconHandlerTest, ShouldNotDownloadIfPreviouslyUnableToDownload) {
+  GURL kIconUrl("https://www.google.de/favicon.ico");
+  delegate_->AddWebImage(kIconUrl);
+
+  favicon_service_->UnableToDownloadFavicon(kIconUrl);
+  ASSERT_TRUE(favicon_service_->WasUnableToDownloadFavicon(kIconUrl));
+
+  RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl});
+  EXPECT_TRUE(delegate_->downloads().empty());
+}
+
+// Test that FaviconHandler completes and the best icon is selected when:
+// - The page provides several icons.
+// - FaviconHandler was previously unable to download the first candidate.
+// - None of the favicons are cached in the favicons database.
+TEST_F(FaviconHandlerTest, ShouldCompleteWhenUnableToDownloadFirstCandidate) {

[pkotwicz, 2017/03/10 05:47:02]

MultipleFavicons404

+  const GURL kUnableToDownloadIconUrl("https://www.google.de/12x12.png");
+  const GURL kIconUrl10x10("https://www.google.de/favicon10x10.png");
+  const GURL kIconUrl11x11("https://www.google.de/favicon11x11.png");
+  delegate_->Add(kUnableToDownloadIconUrl, CreateWebImageWithEdgeSize(12));
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(kIconUrl11x11, CreateWebImageWithEdgeSize(11));
+
+  favicon_service_->UnableToDownloadFavicon(kUnableToDownloadIconUrl);
+  ASSERT_TRUE(
+      favicon_service_->WasUnableToDownloadFavicon(kUnableToDownloadIconUrl));
+
+  RunHandlerWithGURLCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP,
+      {kUnableToDownloadIconUrl, kIconUrl10x10, kIconUrl11x11});
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl11x11));
+}
+
+// Test that FaviconHandler completes and the best icon is selected when:
+// - The page provides several icons.
+// - FaviconHandler was previously unable to download the last candidate.
+// - None of the favicons are cached in the favicons database.
+TEST_F(FaviconHandlerTest, ShouldCompleteWhenUnableToDownloadLastCandidate) {

[pkotwicz, 2017/03/10 05:47:02]

MultipleFavicons404

+  const GURL kIconUrl10x10("https://www.google.de/favicon10x10.png");
+  const GURL kIconUrl11x11("https://www.google.de/favicon11x11.png");
+  const GURL kUnableToDownloadIconUrl("https://www.google.de/12x12.png");
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(kIconUrl11x11, CreateWebImageWithEdgeSize(11));
+  delegate_->Add(kUnableToDownloadIconUrl, CreateWebImageWithEdgeSize(12));
+
+  favicon_service_->UnableToDownloadFavicon(kUnableToDownloadIconUrl);
+  ASSERT_TRUE(
+      favicon_service_->WasUnableToDownloadFavicon(kUnableToDownloadIconUrl));
+
+  RunHandlerWithGURLCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP,
+      {kUnableToDownloadIconUrl, kIconUrl10x10, kIconUrl11x11});
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl11x11));
+}
+
+// Fixes crbug.de/544560
+// Tests that Delegate::OnFaviconUpdated() is called if:
+// - The best icon on the initial page is not the last icon.
+// - All of the initial page's icons are downloaded.
+// AND
+// - JavaScript modifies the page's <link rel="icon"> tags to contain only the
+// last icon.
+TEST_F(FaviconHandlerTest, ShouldCallOnFaviconAvailableAfterIconURLChange) {

[pkotwicz, 2017/03/10 05:47:02]

OnFaviconAvailableNotificationSentAfterIconURLChange

+  const GURL kIconUrl11x11("https://wwww.google.de/favicon11x11.png");
+  const GURL kIconUrl10x10("https://wwww.google.de/favicon10x10.png");
+  delegate_->Add(kIconUrl11x11, CreateWebImageWithEdgeSize(11));
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+
+  std::unique_ptr<FaviconHandler> handler = RunHandlerWithGURLCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP, {kIconUrl11x11, kIconUrl10x10});
+
+  // |kIconUrl10x10| should have been fetched last.
+  EXPECT_THAT(favicon_service_->db_requests(), ElementsAre(kPageUrl, kIconUrl11x11, kIconUrl10x10));
+  EXPECT_THAT(delegate_->downloads(), ElementsAre(kIconUrl11x11, kIconUrl10x10));
+  // |kIconUrl11x11| is the better candidate and should have been selected.
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl11x11));
+
+  // Simulate the page changing it's <link rel="icon"> tags via JavaScript to
+  // just |kIconUrl10x10|.
+  FaviconURL favicon10x10(kIconUrl10x10, favicon_base::FAVICON, kEmptySizes);
+  handler->OnUpdateFaviconURL(kPageUrl, {favicon10x10});
+  base::RunLoop().RunUntilIdle();
+
+  // OnFaviconUpdated() should be called for |kIconUrl10x10|.
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(kIconUrl11x11, kIconUrl10x10));
+}
 
-  TestDelegate delegate;
-  TestFaviconHandler handler(&delegate,
-                             FaviconDriverObserver::NON_TOUCH_16_DIP);
-  DownloadHandler* download_handler = delegate.download_handler();
-
-  std::set<GURL> k404URLs;
-  k404URLs.insert(k404IconURL1);
-  k404URLs.insert(k404IconURL2);
-  download_handler->FailDownloadForIconURLs(k404URLs);
-
-  // Make the initial downloads for |kFaviconURLs| fail.
-  for (const FaviconURL& favicon_url : kFaviconURLs) {
-    const int kSizes[] = { 0 };
-    std::vector<FaviconURL> urls(1u, favicon_url);
-    DownloadTillDoneIgnoringHistory(&delegate, &handler, kPageURL, urls,
-                                    kSizes);
+// Test that when FaviconHandler is created with
+// FaviconDriverObserver::NON_TOUCH_16_DIP that the FaviconHandler stops
+// downloading icons when it finds an exact match.
+TEST_F(FaviconHandlerTest, StopDownloadingWhenGetExactMatch) {
+  const GURL kIconUrl10x10("https://www.google.de/a");
+  const GURL kIconUrl72x72("https://www.google.de/b");
+  const GURL kIconUrl16x16("https://www.google.de/c");
+  const GURL kIconUrl48x48("https://www.google.de/d");
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(kIconUrl72x72, CreateWebImageWithEdgeSize(72));
+  delegate_->Add(kIconUrl16x16, CreateWebImageWithEdgeSize(16));
+  delegate_->Add(kIconUrl48x48, CreateWebImageWithEdgeSize(48));
+
+  RunHandlerWithGURLCandidates(
+      FaviconDriverObserver::NON_TOUCH_16_DIP,
+      {kIconUrl10x10, kIconUrl72x72, kIconUrl16x16, kIconUrl48x48});
+
+  // The handler should only stop downloading when it finds an exact match, not
+  // when it downloads a bitmap greater than the desired size.
+  EXPECT_THAT(delegate_->downloads(),
+              ElementsAre(kIconUrl10x10, kIconUrl72x72, kIconUrl16x16));
+
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(kIconUrl16x16));
+}
+
+// ****************************************************************************
+// The goal of these tests is to be more of an integration test than
+// SelectFaviconFrameTest.*
+
+// Tests that running the FaviconHandler:
+// - on a page with <link rel="icon"> tags with no "sizes" information
+// - where the sizes of the downloaded bitmaps are |download_edge_sizes|
+// - where none of the icons are cached in history
+// calls OnFaviconUpdated() with the bitmap with edge size
+// |notification_edge_size|.
+void TestHandlerWithDownloadDataSendsNotification(
+    FaviconHandlerTest* tester,
+    const std::vector<int>& download_edge_sizes,
+    int notification_edge_size) {
+  std::vector<GURL> candidate_icon_urls;
+  GURL notification_icon_url;
+  for (int download_edge_size : download_edge_sizes) {
+    GURL candidate_icon_url(base::StringPrintf("https://www.google.de/icon%d.png",
+                                          download_edge_size));
+    candidate_icon_urls.push_back(candidate_icon_url);
+    tester->delegate_->Add(candidate_icon_url,
+                           CreateWebImageWithEdgeSize(download_edge_size));
+
+    if (download_edge_size == notification_edge_size)
+      notification_icon_url = candidate_icon_url;
   }
-  EXPECT_TRUE(download_handler->DidFailDownloadForIconURL(k404IconURL1));
-  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(&delegate, &handler, kPageURL, urls, kSizes);
-
-  EXPECT_EQ(nullptr, handler.current_candidate());
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_TRUE(delegate.image().IsEmpty());
+
+  tester->RunHandlerWithGURLCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
+                                       candidate_icon_urls);
+  EXPECT_THAT(tester->delegate_->on_favicon_updated_urls(),
+              ElementsAre(notification_icon_url));
+}
+
+// Tests that running FaviconHandler
+// - on an OS which supports the 1x and 2x scale factor
+// - on a page with <link rel="icon"> tags with no "sizes" information
+// selects the largest exact match. Note that a 32x32 PNG image is not a "true
+// exact match" on an OS which supports an 1x and 2x. A "true exact match" is a
+// .ico file with 16x16 and 32x32 bitmaps.
+TEST_F(FaviconHandlerTest, ChooseLargestExactMatch) {

[pkotwicz, 2017/03/10 05:47:02]

MultipleFavicons

+  ui::test::ScopedSetSupportedScaleFactors scoped_supported(
+      {ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_200P});
+  TestHandlerWithDownloadDataSendsNotification(this, {16, 24, 32, 48, 256}, 32);
+}
+
+// Tests that running FaviconHandler
+// - on an OS which supports the 1x and 2x scale factor
+// - on a page with <link rel="icon"> tags with no "sizes" information
+// selects the exact match even if it requires in upsampling.
+TEST_F(FaviconHandlerTest, ChooseExactMatchDespiteUpsampling) {

[pkotwicz, 2017/03/10 05:47:02]

MultipleFavicons

+  ui::test::ScopedSetSupportedScaleFactors scoped_supported(
+      {ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_200P});
+  TestHandlerWithDownloadDataSendsNotification(this, {16, 24, 48, 256}, 16);
 }
 
-// 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());
-
-  FaviconURL favicon_url(kInvalidFormatURL, favicon_base::FAVICON,
-                         std::vector<gfx::Size>());
-
-  TestDelegate delegate;
-  TestFaviconHandler handler(&delegate,
-                             FaviconDriverObserver::NON_TOUCH_16_DIP);
-  UpdateFaviconURL(&delegate, &handler, kPageURL,
-                   std::vector<FaviconURL>(1u, favicon_url));
-  EXPECT_EQ(0u, handler.image_urls().size());
+// Tests that running FaviconHandler
+// - on an OS which supports the 1x and 2x scale factor
+// - on a page with <link rel="icon"> tags with no "sizes" information
+// selects an icon which needs to be downsampled a little over a huge icon.
+TEST_F(FaviconHandlerTest, ChooseMinorDownsamplingOverHugeIcon) {

[pkotwicz, 2017/03/10 05:47:02]

MultipleFavicons

+  ui::test::ScopedSetSupportedScaleFactors scoped_supported(
+      {ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_200P});
+  TestHandlerWithDownloadDataSendsNotification(this, {48, 256}, 48);
 }
 
-TEST_F(FaviconHandlerTest, TestSortFavicon) {
-  const GURL kPageURL("http://www.google.com");
+TEST_F(FaviconHandlerTest, ChooseMinorUpsamplingOverHugeIcon) {

[pkotwicz, 2017/03/10 05:47:02]

MultipleFavicons

+  ui::test::ScopedSetSupportedScaleFactors scoped_supported(
+      {ui::SCALE_FACTOR_100P, ui::SCALE_FACTOR_200P});
+  TestHandlerWithDownloadDataSendsNotification(this, {17, 256}, 17);
+}
+
+// ****************************************************************************
+// Tests for when FaviconHandler is constructed with NON_TOUCH_LARGEST.
+
+// Test the sorting done as a result of FaviconHandler::OnUpdateFaviconURL().
+// by FaviconHandlers constructed with NON_TOUCH_LARGEST.
+TEST_F(FaviconHandlerTest, TestLargestHandlerSortsCandidates) {

[pkotwicz, 2017/03/10 05:47:02]

TestSortFavicon

+  const GURL kPageURL("https://www.google.de");
   std::vector<gfx::Size> icon1;
   icon1.push_back(gfx::Size(1024, 1024));
   icon1.push_back(gfx::Size(512, 512));
@@ -1353,26 +1052,21 @@ TEST_F(FaviconHandlerTest, TestSortFavicon) {
   icon3.push_back(gfx::Size(16, 16));
   icon3.push_back(gfx::Size(14, 14));
 
-  const FaviconURL kSourceIconURLs[] = {
-      FaviconURL(GURL("http://www.google.com/a"), favicon_base::FAVICON, icon1),
-      FaviconURL(GURL("http://www.google.com/b"), favicon_base::FAVICON, icon2),
-      FaviconURL(GURL("http://www.google.com/c"), favicon_base::FAVICON, icon3),
-      FaviconURL(GURL("http://www.google.com/d"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      FaviconURL(GURL("http://www.google.com/e"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>())};
-
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
-  std::vector<FaviconURL> urls1(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  const std::vector<FaviconURL> kSourceFavicons = {
+      FaviconURL(GURL("https://www.google.de/a"), favicon_base::FAVICON, icon1),
+      FaviconURL(GURL("https://www.google.de/b"), favicon_base::FAVICON, icon2),
+      FaviconURL(GURL("https://www.google.de/c"), favicon_base::FAVICON, icon3),
+      FaviconURL(GURL("https://www.google.de/d"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("https://www.google.de/e"), favicon_base::FAVICON,
+                 kEmptySizes)
+  };
+
+  std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates(
+      FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceFavicons);
 
   struct ExpectedResult {
-    // The favicon's index in kSourceIconURLs.
+    // The favicon's index in kSourceFavicons.
     size_t favicon_index;
     // Width of largest bitmap.
     int width;
@@ -1389,21 +1083,24 @@ TEST_F(FaviconHandlerTest, TestSortFavicon) {
     {3, -1},
     {4, -1},
   };
-  const std::vector<FaviconURL>& icons = handler1.image_urls();
+  const std::vector<FaviconURL>& icons = handler->image_urls();
   ASSERT_EQ(5u, icons.size());
   for (size_t i = 0; i < icons.size(); ++i) {
-    EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
+    EXPECT_EQ(kSourceFavicons[results[i].favicon_index].icon_url,
               icons[i].icon_url);
     if (results[i].width != -1)
       EXPECT_EQ(results[i].width, icons[i].icon_sizes[0].width());
   }
 }
 
+// Test that when FaviconHandler is constructed with NON_TOUCH_LARGEST that
+// icons with "sizes" specified in the <link rel="icon"> tag are downloaded
+// first. That icons with "sizes" specified are downloaded in decreasing order
+// of the "sizes" parameter.
 TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
-  const GURL kPageURL("http://www.google.com");
   std::vector<gfx::Size> icon1;
-  icon1.push_back(gfx::Size(1024, 1024));
-  icon1.push_back(gfx::Size(512, 512));
+  icon1.push_back(gfx::Size(1024,1024));
+  icon1.push_back(gfx::Size(512,512));
 
   std::vector<gfx::Size> icon2;
   icon2.push_back(gfx::Size(15, 15));
@@ -1413,41 +1110,26 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
   icon3.push_back(gfx::Size(16, 16));
   icon3.push_back(gfx::Size(512, 512));
 
-  const FaviconURL kSourceIconURLs[] = {
-      FaviconURL(
-          GURL("http://www.google.com/a"), favicon_base::FAVICON, icon1),
-      FaviconURL(
-          GURL("http://www.google.com/b"), favicon_base::FAVICON, icon2),
-      FaviconURL(
-          GURL("http://www.google.com/c"), favicon_base::FAVICON, icon3),
-      FaviconURL(GURL("http://www.google.com/d"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>()),
-      FaviconURL(GURL("http://www.google.com/e"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>())};
-
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
-
-  std::set<GURL> fail_icon_urls;
-  for (size_t i = 0; i < arraysize(kSourceIconURLs); ++i) {
-    fail_icon_urls.insert(kSourceIconURLs[i].icon_url);
-  }
-  delegate1.download_handler()->FailDownloadForIconURLs(fail_icon_urls);
+  const std::vector<FaviconURL> kSourceFavicons = {
+      FaviconURL(GURL("https://www.google.de/a"), favicon_base::FAVICON, icon1),
+      FaviconURL(GURL("https://www.google.de/b"), favicon_base::FAVICON, icon2),
+      FaviconURL(GURL("https://www.google.de/c"), favicon_base::FAVICON, icon3),
+      FaviconURL(GURL("https://www.google.de/d"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("https://www.google.de/e"), favicon_base::FAVICON,
+                 kEmptySizes)
+  };
 
-  std::vector<FaviconURL> urls1(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  // Simulate all the downloads failing in order to check the order of the icons
+  // being downloaded.
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST,
+                           kSourceFavicons);
 
-  // Simulate the download failed, to check whether the icons were requested
-  // to download according their size.
   struct ExpectedResult {
-    // The favicon's index in kSourceIconURLs.
+    // The favicon's index in kSourceFavicons.
     size_t favicon_index;
-    // Width of largest bitmap.
-    int width;
+    // Edge size of largest bitmap.
+    int edge_size;
   } results[] = {
     {0, 1024},
     {2, 512},
@@ -1457,221 +1139,241 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
     {4, -1},
   };
 
-  for (int i = 0; i < 5; ++i) {
-    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());
+  std::vector<GURL> downloads = delegate_->downloads();
+  ASSERT_EQ(downloads.size(), arraysize(results));
+  for (size_t i = 0; i < arraysize(results); ++i) {
+    const ExpectedResult& result = results[i];
+    FaviconURL favicon = kSourceFavicons[result.favicon_index];
+    EXPECT_EQ(favicon.icon_url, downloads[i]);
+    if (result.edge_size > 0) {
+      EXPECT_THAT(favicon.icon_sizes, testing::Contains(gfx::Size(
+                                          result.edge_size, result.edge_size)));
     }
+  }
+}
 
-    // Simulate no favicon from history.
-    handler1.history_handler()->history_results_.clear();
-    handler1.history_handler()->InvokeCallback();
+// Test that FaviconHandler selects the largest bitmap.
+TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
+  const std::vector<FaviconURL> kSourceFavicons = {
+      CreateFaviconURLWithEdgeSize(GURL("https://www.google.de/a"), 72),
+      CreateFaviconURLWithEdgeSize(GURL("https://www.google.de/b"), 256),
+  };
 
-    // Verify download request
-    ASSERT_TRUE(delegate1.download_handler()->HasDownload());
-    EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
-              delegate1.download_handler()->GetImageUrl());
+  delegate_->AddWebImagesForFaviconURLs(kSourceFavicons);
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceFavicons);
 
-    delegate1.download_handler()->InvokeCallback();
-    delegate1.download_handler()->Reset();
-  }
+  // The 256x256 icon should be selected.
+  FaviconURL expected_favicon = kSourceFavicons[1];
+  ASSERT_THAT(expected_favicon.icon_sizes, testing::Contains(gfx::Size(256, 256)));
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(expected_favicon.icon_url));
 }
 
-TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
-  const GURL kPageURL("http://www.google.com");
-
-  std::vector<gfx::Size> one_icon;
-  one_icon.push_back(gfx::Size(15, 15));
-
-  std::vector<gfx::Size> two_icons;
-  two_icons.push_back(gfx::Size(14, 14));
-  two_icons.push_back(gfx::Size(16, 16));
-
-  const FaviconURL kSourceIconURLs[] = {
-      FaviconURL(
-          GURL("http://www.google.com/b"), favicon_base::FAVICON, one_icon),
-      FaviconURL(
-          GURL("http://www.google.com/c"), favicon_base::FAVICON, two_icons)};
-
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
-  std::vector<FaviconURL> urls1(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
-
-  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],
-            handler1.current_candidate()->icon_sizes[0]);
-
-  // Simulate no favicon from history.
-  handler1.history_handler()->history_results_.clear();
-  handler1.history_handler()->InvokeCallback();
-
-  // Verify download request
-  ASSERT_TRUE(delegate1.download_handler()->HasDownload());
-  EXPECT_EQ(kSourceIconURLs[i].icon_url,
-            delegate1.download_handler()->GetImageUrl());
-
-  // Give the correct download result.
-  std::vector<int> sizes;
-  for (std::vector<gfx::Size>::const_iterator j =
-           kSourceIconURLs[i].icon_sizes.begin();
-       j != kSourceIconURLs[i].icon_sizes.end(); ++j)
-    sizes.push_back(j->width());
-
-  delegate1.download_handler()->SetImageSizes(sizes);
-  delegate1.download_handler()->InvokeCallback();
-
-  // Verify the largest bitmap has been saved into history.
-  EXPECT_EQ(kSourceIconURLs[i].icon_url, handler1.history_handler()->icon_url_);
-  EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b],
-            handler1.history_handler()->size_);
-  // Verify NotifyFaviconAvailable().
-  EXPECT_EQ(1u, delegate1.num_notifications());
-  EXPECT_EQ(kSourceIconURLs[i].icon_url, delegate1.icon_url());
-  EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b], delegate1.image().Size());
+// Test running FaviconHandler when the largest bitmap is in a multi resolution
+// .ico file. Test that OnFaviconUpdated() is called with the largest bitmap in
+// the .ico file not a multi resolution image.
+TEST_F(FaviconHandlerTest, TestSelectLargestFaviconFromMultiResolutionIco) {

[pkotwicz, 2017/03/10 05:47:01]

TestSelectLargestFavicon

+  // Replace |delegate_| with implementation which mocks out OnFaviconUpdated()
+  // so that we can use EXPECT_CALL().
+  using StrictDelegate = testing::StrictMock<DelegateMockOnFaviconUpdated>;
+  delegate_.reset(new StrictDelegate);
+  StrictDelegate* strict_delegate = static_cast<StrictDelegate*>(delegate_.get());
+
+  EXPECT_CALL(
+      *strict_delegate,
+      OnFaviconUpdated(_, _, _, _, ImageIsSingleResolutionAndHasSize(17)));
+
+  std::vector<gfx::Size> one_size = {gfx::Size(15,15)};
+  std::vector<gfx::Size> two_sizes = {gfx::Size(14, 14), gfx::Size(17, 17)};
+  std::vector<FaviconURL> kSourceFavicons = {
+      FaviconURL(GURL("https://www.google.de/a"), favicon_base::FAVICON,
+                 one_size),
+      FaviconURL(GURL("https://www.google.de/b"), favicon_base::FAVICON,
+                 two_sizes),
+  };
+  delegate_->AddWebImagesForFaviconURLs(kSourceFavicons);
+
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceFavicons);
 }
 
+// WebContents::DownloadImage() downsamples the downloaded bitmap sent to the
+// callback if the downloaded bitmap is larger than the |max_bitmap_size| passed
+// to WebContents::DownloadImage(). Test that FaviconHandler selects the bitmap
+// whose size before downsampling was the largest.
 TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {

[pkotwicz, 2017/03/10 05:47:01]

TestFaviconWasScaledAfterDownload

-  const GURL kPageURL("http://www.google.com");
   const int kMaximalSize =
-    TestFaviconHandler::GetMaximalIconSize(favicon_base::FAVICON);
+      FaviconHandler::GetMaximalIconSize(favicon_base::FAVICON);
+  const GURL kIconUrlMaxPlus1("https://www.google.de/a");
+  const GURL kIconUrlMaxPlus2("https://www.google.de/b");
+  // Emulate WebContents::DownloadImage() downsampling the bitmaps to
+  // |kMaximalSize|.
+  delegate_->Add(kIconUrlMaxPlus1, CreateWebImageWithEdgeSize(kMaximalSize));
+  delegate_->Add(kIconUrlMaxPlus2, CreateWebImageWithEdgeSize(kMaximalSize));
+
+  const std::vector<FaviconURL> kSourceFavicons = {
+      CreateFaviconURLWithEdgeSize(kIconUrlMaxPlus1, kMaximalSize + 1),
+      CreateFaviconURLWithEdgeSize(kIconUrlMaxPlus2, kMaximalSize + 2),
+  };
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceFavicons);
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(kIconUrlMaxPlus2));
+}
 
-  std::vector<gfx::Size> icon1;
-  icon1.push_back(gfx::Size(kMaximalSize + 1, kMaximalSize + 1));
+// Test that if the size of the first downloaded bitmap is smaller than the size
+// of the 2nd largest bitmap according to the "sizes" parameter in the
+// <link rel="icon"> tags that we download the 2nd largest bitmap.
+TEST_F(FaviconHandlerTest, TestFirstDownloadSmallerThanExpected) {
+  const GURL kIconUrl1x1("https://www.google.de/a");
+  const GURL kIconUrl18x18("https://www.google.de/b");
+  const GURL kIconUrl17x17("https://www.google.de/c");
+  delegate_->Add(kIconUrl1x1, CreateWebImageWithEdgeSize(1));
+  delegate_->Add(kIconUrl18x18, CreateWebImageWithEdgeSize(18));
+  delegate_->Add(kIconUrl18x18, CreateWebImageWithEdgeSize(17));
+
+  std::vector<FaviconURL> kSourceFavicons = {
+    CreateFaviconURLWithEdgeSize(kIconUrl1x1, 192),
+    CreateFaviconURLWithEdgeSize(kIconUrl18x18, 18),
+    CreateFaviconURLWithEdgeSize(kIconUrl17x17, 17),
+  };
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceFavicons);
 
-  std::vector<gfx::Size> icon2;
-  icon2.push_back(gfx::Size(kMaximalSize + 2, kMaximalSize + 2));
-
-  const FaviconURL kSourceIconURLs[] = {
-      FaviconURL(
-          GURL("http://www.google.com/b"), favicon_base::FAVICON, icon1),
-      FaviconURL(
-          GURL("http://www.google.com/c"), favicon_base::FAVICON, icon2)};
-
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
-  std::vector<FaviconURL> urls1(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
-
-  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],
-            handler1.current_candidate()->icon_sizes[0]);
-
-  // Simulate no favicon from history.
-  handler1.history_handler()->history_results_.clear();
-  handler1.history_handler()->InvokeCallback();
-
-  // Verify download request
-  ASSERT_TRUE(delegate1.download_handler()->HasDownload());
-  EXPECT_EQ(kSourceIconURLs[i].icon_url,
-            delegate1.download_handler()->GetImageUrl());
-
-  // Give the scaled download bitmap.
-  std::vector<int> sizes;
-  sizes.push_back(kMaximalSize);
-
-  delegate1.download_handler()->SetImageSizes(sizes);
-  delegate1.download_handler()->InvokeCallback();
-
-  // Verify the largest bitmap has been saved into history though it was
-  // scaled down to maximal size and smaller than icon1 now.
-  EXPECT_EQ(kSourceIconURLs[i].icon_url, handler1.history_handler()->icon_url_);
-  EXPECT_EQ(gfx::Size(kMaximalSize, kMaximalSize),
-            handler1.history_handler()->size_);
+  // |kIconUrl1x1| should have been downloaded first because the website says
+  // that it is huge. |kIconUrl18x18| should be also downloaded because the site
+  // says that it is bigger than 1x1 px.
+  EXPECT_THAT(delegate_->downloads(), ElementsAre(kIconUrl1x1, kIconUrl18x18));
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl18x18));
 }
 
-TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
-  const GURL kPageURL("http://www.google.com");
+// Test that if the size of the first downloaded bitmap is smaller than expected
+// but bigger than the size of the 2nd largest bitmap according to the "sizes"
+// parameter in the <link rel="icon"> tags that we don't download the 2nd
+// largest bitmap.
+TEST_F(FaviconHandlerTest, TestFirstDownloadSmallerThanExpectedButStillBiggest) {
+  const GURL kIconUrl20x20("https://www.google.de/a");
+  const GURL kIconUrl18x18("https://www.google.de/b");
+  delegate_->Add(kIconUrl20x20, CreateWebImageWithEdgeSize(20));
+  delegate_->Add(kIconUrl18x18, CreateWebImageWithEdgeSize(18));
+
+  std::vector<FaviconURL> kSourceFavicons = {
+    CreateFaviconURLWithEdgeSize(kIconUrl20x20, 192),
+    CreateFaviconURLWithEdgeSize(kIconUrl18x18, 18),
+  };
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceFavicons);
 
-  std::vector<gfx::Size> icon1;
-  icon1.push_back(gfx::Size(16, 16));
-  const int actual_size1 = 10;
+  // |kIconUrl20x20| should have been downloaded first because the website says
+  // that it is huge. |kIconUrl18x18| should not be downloaded because the site
+  // says that it is smaller than 20x20 px.
+  EXPECT_THAT(delegate_->downloads(), ElementsAre(kIconUrl20x20));
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl20x20));
+}
 
-  std::vector<gfx::Size> icon2;
-  icon2.push_back(gfx::Size(15, 15));
-  const int actual_size2 = 12;
-
-  const FaviconURL kSourceIconURLs[] = {
-      FaviconURL(GURL("http://www.google.com/b"), favicon_base::FAVICON, icon1),
-      FaviconURL(GURL("http://www.google.com/c"), favicon_base::FAVICON, icon2),
-      FaviconURL(GURL("http://www.google.com/d"),
-                 favicon_base::FAVICON,
-                 std::vector<gfx::Size>())};
-
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
-  std::vector<FaviconURL> urls1(kSourceIconURLs,
-                                kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
-  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(delegate1.download_handler()->HasDownload());
-  EXPECT_EQ(kSourceIconURLs[0].icon_url,
-            delegate1.download_handler()->GetImageUrl());
-
-  // Give the incorrect size.
-  std::vector<int> sizes;
-  sizes.push_back(actual_size1);
-  delegate1.download_handler()->SetImageSizes(sizes);
-  delegate1.download_handler()->InvokeCallback();
-  delegate1.download_handler()->Reset();
-
-  // Simulate no favicon from history.
-  handler1.history_handler()->history_results_.clear();
-  handler1.history_handler()->InvokeCallback();
-
-  // Verify the 2nd icon was request to download
-  ASSERT_TRUE(delegate1.download_handler()->HasDownload());
-  EXPECT_EQ(kSourceIconURLs[1].icon_url,
-            delegate1.download_handler()->GetImageUrl());
-
-  // Very the best candidate is icon1
-  EXPECT_EQ(kSourceIconURLs[0].icon_url,
-            handler1.best_favicon_candidate().image_url);
-  EXPECT_EQ(gfx::Size(actual_size1, actual_size1),
-            handler1.best_favicon_candidate().image.Size());
-
-  // Give the incorrect size.
-  sizes.clear();
-  sizes.push_back(actual_size2);
-  delegate1.download_handler()->SetImageSizes(sizes);
-  delegate1.download_handler()->InvokeCallback();
-  delegate1.download_handler()->Reset();
-
-  // Verify icon2 has been saved into history.
-  EXPECT_EQ(kSourceIconURLs[1].icon_url, handler1.history_handler()->icon_url_);
-  EXPECT_EQ(gfx::Size(actual_size2, actual_size2),
-            handler1.history_handler()->size_);
+// Test that if several icons are downloaded because the icons are smaller than
+// expected that OnFaviconUpdated() is called with the largest downloaded
+// bitmap.
+TEST_F(FaviconHandlerTest, TestDownloadManySelectLargest) {

[pkotwicz, 2017/03/10 05:47:02]

TestKeepDownloadedLargestFavicon

+  const GURL kIconUrl10x10("https://www.google.de/a");
+  const GURL kIconUrl12x12("https://www.google.de/b");
+  delegate_->Add(kIconUrl10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(kIconUrl12x12, CreateWebImageWithEdgeSize(12));
+
+  const std::vector<FaviconURL> kSourceFavicons = {
+      CreateFaviconURLWithEdgeSize(kIconUrl10x10, 16),
+      CreateFaviconURLWithEdgeSize(kIconUrl12x12, 15),
+  };
+  RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_LARGEST,
+                           kSourceFavicons);
+
+  // |kIconUrl10x10| should have been downloaded first because the website says
+  // that itis the biggest.
+  EXPECT_THAT(delegate_->downloads(),
+              ElementsAre(kIconUrl10x10, kIconUrl12x12));
+
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(), ElementsAre(kIconUrl12x12));
+}
+
+// ************************************************************************************************
+// Tests for when there are icon candidates with types other than favicon_base::FAVICON.
+
+// Test that a FaviconHandler constructed with FaviconDriverObserver::TOUCH_LARGEST only downloads
+// icons of type TOUCH_ICON and TOUCH_ICON_PRECOMPOSED.
+TEST_F(FaviconHandlerTest, OnlyDownloadMatchingIconType) {

[pkotwicz, 2017/03/10 05:47:01]

Download2ndFaviconURLCandidate

+  favicon::FaviconURL favicon(GURL("https://www.google.de/a"), favicon_base::FAVICON,
+                      kEmptySizes);
+  favicon::FaviconURL touch_icon(GURL("https://www.google.de/b"),
+                                 favicon_base::TOUCH_ICON, kEmptySizes);
+  favicon::FaviconURL touch_precomposed_icon(
+      GURL("https://www.google.de/c"), favicon_base::TOUCH_PRECOMPOSED_ICON,
+      kEmptySizes);
+
+  // Run FaviconHandler. All downloads will return 404. This is fine because we
+  // want to know which icons get downloaded.
+  RunHandlerWithCandidates(FaviconDriverObserver::TOUCH_LARGEST,
+                           {favicon, touch_icon, touch_precomposed_icon});
+  EXPECT_THAT(delegate_->downloads(),
+              ElementsAre(touch_icon.icon_url, touch_precomposed_icon.icon_url));
+}
+
+// Test that FaviconHandler stores the selected icon in the database with the
+// selected icon's type.
+TEST_F(FaviconHandlerTest, ShouldCallSetFaviconsWithCorrectIconType) {
+  // Replace |favicon_service_| with implementation which mocks out SetFavicons()
+  // so that we can use EXPECT_CALL().
+  using StrictService = testing::StrictMock<ServiceMockSetFavicons>;
+  favicon_service_.reset(new StrictService);
+  StrictService* strict_service =
+      static_cast<StrictService*>(favicon_service_.get());
+
+  GURL touch_icon_url10x10("https://www.google.de/a");
+  GURL touch_precomposed_icon_url11x11("https://www.google.de/b");
+  delegate_->Add(touch_icon_url10x10, CreateWebImageWithEdgeSize(10));
+  delegate_->Add(touch_precomposed_icon_url11x11,
+                 CreateWebImageWithEdgeSize(11));
+
+  // SetFavicons() should be called with favicon_base::TOUCH_PRECOMPOSED_ICON.
+  EXPECT_CALL(*strict_service,
+              SetFavicons(_, touch_precomposed_icon_url11x11,
+                          favicon_base::TOUCH_PRECOMPOSED_ICON, _));
+
+  // Set incorrect size for |touch_icon_url10x10| so that both icons get
+  // downloaded.
+  std::vector<FaviconURL> kSourceFavicons = {
+      FaviconURL(touch_icon_url10x10, favicon_base::TOUCH_ICON,
+                 {gfx::Size(48, 48)}),
+      FaviconURL(touch_precomposed_icon_url11x11,
+                 favicon_base::TOUCH_PRECOMPOSED_ICON, {gfx::Size(11, 11)}),
+  };
+  RunHandlerWithCandidates(FaviconDriverObserver::TOUCH_LARGEST, kSourceFavicons);
+
+  EXPECT_THAT(delegate_->downloads(),
+              ElementsAre(touch_icon_url10x10, touch_precomposed_icon_url11x11));
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(touch_precomposed_icon_url11x11));
+}
+
+// Test that a FaviconHandler constructed with
+// FaviconDriverObserver::TOUCH_LARGEST does not resize downloaded bitmaps
+// before storing them into the database.
+TEST_F(FaviconHandlerTest, LargestHandlerShouldNotResizeDownloads) {
+  // Replace |favicon_service_| with implementation which mocks out SetFavicons()
+  // so that we can use EXPECT_CALL().
+  using StrictService = testing::StrictMock<ServiceMockSetFavicons>;
+  favicon_service_.reset(new StrictService);
+  StrictService* strict_service =
+      static_cast<StrictService*>(favicon_service_.get());
+
+  std::vector<FaviconURL> kSourceFavicons = {
+      FaviconURL(GURL("https://www.google.de/a"), favicon_base::TOUCH_ICON,
+                 {gfx::Size(183, 183)})
+  };
+  delegate_->AddWebImagesForFaviconURLs(kSourceFavicons);
+
+  // gfx::Image passed to SetFavicons() should have unresized bitmap.
+  EXPECT_CALL(*strict_service,
+              SetFavicons(_, _, _, ImageIsSingleResolutionAndHasSize(183)));
+
+  RunHandlerWithCandidates(FaviconDriverObserver::TOUCH_LARGEST, kSourceFavicons);
+  EXPECT_THAT(delegate_->on_favicon_updated_urls(),
+              ElementsAre(kSourceFavicons[0].icon_url));
 }
 
 }  // namespace