Improve FaviconHandler tests by testing public API

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 ef4a1c4dfc0c5832df19900ed2f24867b2510f61..cc9d7ec5b50293937dc9090830761db7d06abf47 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -6,6 +6,8 @@
 
 #include <stddef.h>
 
+#include <list>
+#include <map>
 #include <memory>
 #include <set>
 #include <vector>
@@ -24,357 +26,170 @@
 namespace favicon {
 namespace {
 
-// Fill the given bmp with valid png data.
-void FillDataToBitmap(int w, int h, SkBitmap* bmp) {
-  bmp->allocN32Pixels(w, h);
+using favicon_base::FaviconRawBitmapResult;
+using testing::Contains;
+using testing::InSequence;
+using testing::Invoke;
+using testing::Key;
+using testing::Return;
+using testing::SaveArg;
+using testing::WithArg;
+using testing::_;
+
+using URLVector = std::vector<GURL>;
+
+MATCHER_P2(ImageSizeIs, width, height, "") {
+  return arg.Size() == gfx::Size(width, height);
+}
+
+// Fill the given bmp with some test data.
+SkBitmap CreateBitmap(int w, int h) {
+  SkBitmap bmp;
+  bmp.allocN32Pixels(w, h);
 
   unsigned char* src_data =
-      reinterpret_cast<unsigned char*>(bmp->getAddr32(0, 0));
+      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);
   }
+  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);
+  SkBitmap bitmap = CreateBitmap(w, h);
   gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, output);
 }
 
-void SetFaviconRawBitmapResult(
+std::vector<FaviconRawBitmapResult> CreateFaviconRawBitmapResult(
     const GURL& icon_url,
     favicon_base::IconType icon_type,
-    bool expired,
-    std::vector<favicon_base::FaviconRawBitmapResult>* favicon_bitmap_results) {
+    bool expired) {
   scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes());
   FillBitmap(gfx::kFaviconSize, gfx::kFaviconSize, &data->data());
-  favicon_base::FaviconRawBitmapResult bitmap_result;
+  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_url = icon_url;
-
-  favicon_bitmap_results->push_back(bitmap_result);
+  return {bitmap_result};
 }
 
-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);
-}
-
-// This class is used to save the download request for verifying with test case.
-class DownloadHandler {
+// Convenience class to queue image download requests. MockDelegate below will
+// forward all download requests to this class.
+class ImageDownloadTester {
  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_|.
-  }
-
-  // Make downloads for any of |icon_urls| fail.
-  void FailDownloadForIconURLs(const std::set<GURL>& icon_urls) {
-    should_fail_download_icon_urls_ = icon_urls;
-  }
+  using ImageDownloadCallback = FaviconHandler::Delegate::ImageDownloadCallback;
 
-  // Returns whether a download for |icon_url| did fail.
-  bool DidFailDownloadForIconURL(const GURL& icon_url) const {
-    return failed_download_icon_urls_.count(icon_url);
-  }
-
-  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));
-  }
-
-  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; }
-
- 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;
-  };
-
-  std::unique_ptr<Download> download_;
-  bool callback_invoked_;
-
-  // The icon URLs for which the download should fail.
-  std::set<GURL> should_fail_download_icon_urls_;
-
-  // 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_;
-
-  DISALLOW_COPY_AND_ASSIGN(DownloadHandler);
-};
-
-// 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) {
-  }
+  ImageDownloadTester() : download_id_(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) {
-  }
-
-  ~HistoryRequestHandler() {}
-  void InvokeCallback();
-
-  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_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(HistoryRequestHandler);
-};
-
-class TestDelegate : public FaviconHandler::Delegate {
- public:
-  TestDelegate() : num_notifications_(0), download_id_(0) {}
-
-  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::StartDownload()
-    if (download_handler_.DidFailDownloadForIconURL(image_url)) {
-      download_handler_.AddDownload(download_id_, image_url, std::vector<int>(),
-                                    0, callback);
-      return 0;
-    }
-
-    download_id_++;
-    std::vector<int> sizes;
-    sizes.push_back(0);
-    download_handler_.AddDownload(download_id_, image_url, sizes,
-                                  max_bitmap_size, callback);
+  int DownloadImage(const GURL& url,
+                    int max_image_size,
+                    ImageDownloadCallback callback) {
+    ++download_id_;
+    pending_downloads_[url].emplace_back(download_id_, callback);
+    LOG(INFO) << "Returning download id " << download_id_;
     return download_id_;
   }
 
-  bool IsOffTheRecord() override { return false; }
-
-  bool IsBookmarked(const GURL& url) override { return false; }
-
-  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;
+  // Causes the first queued download for |image_url| to be finished. Returns
+  // false if none was queued.
+  bool TestDownloadFinished(int http_status_code,
+                            const GURL& url,
+                            const std::vector<SkBitmap>& bitmaps,
+                            const std::vector<gfx::Size>& original_bitmap_sizes)
+      WARN_UNUSED_RESULT {
+    if (pending_downloads_[url].empty()) {
+      EXPECT_THAT(pending_downloads_, Contains(Key(url)));
+      return false;
+    }
+    int id = pending_downloads_[url].front().first;
+    ImageDownloadCallback callback = pending_downloads_[url].front().second;
+    pending_downloads_[url].pop_front();
+    LOG(INFO) << "Returning bitmaps " << bitmaps.size();
+    callback.Run(id, http_status_code, url, bitmaps, original_bitmap_sizes);
+    return true;
   }
 
-  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_;
-
-  // The unique id of a download request. It will be returned to a
-  // FaviconHandler.
   int download_id_;
-  DownloadHandler download_handler_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestDelegate);
+  // Map keyed by image URL. Values are <id, callback> pairs.
+  std::map<GURL, std::list<std::pair<int, ImageDownloadCallback>>>
+      pending_downloads_;
 };
 
-}  // namespace
-
-// 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 {
+// Convenience class to "queue" requestes to FaviconService. MockFaviconService
+// will forward requests to this class.
+class FaviconServiceTester {
  public:
-  static int GetMaximalIconSize(favicon_base::IconType icon_type) {
-    return FaviconHandler::GetMaximalIconSize(icon_type);
-  }
-
-  TestFaviconHandler(FaviconService* service,
-                     FaviconHandler::Delegate* delegate,
-                     FaviconDriverObserver::NotificationIconType handler_type)
-      : FaviconHandler(service, delegate, handler_type) {}
-
-  ~TestFaviconHandler() override {}
-
-  HistoryRequestHandler* history_handler() {
-    return history_handler_.get();
-  }
-
-  // This method will take the ownership of the given handler.
-  void set_history_handler(HistoryRequestHandler* handler) {
-    history_handler_.reset(handler);
-  }
-
-  FaviconURL* current_candidate() {
-    return FaviconHandler::current_candidate();
+  base::CancelableTaskTracker::TaskId SaveCallback(
+      favicon_base::FaviconResultsCallback callback) {
+    EXPECT_TRUE(callback);
+    EXPECT_FALSE(callback_) << "Single in-flight FaviconRequest expected";
+    callback_ = callback;
+    return 1;
   }
 
-  size_t current_candidate_index() const {
-    return current_candidate_index_;
+  // Runs a previously saved callback. Returns false if none was saved.
+  bool TestRunCallback(const std::vector<FaviconRawBitmapResult>& results)
+      WARN_UNUSED_RESULT {
+    if (!callback_)
+      return false;
+    favicon_base::FaviconResultsCallback callback_copy = callback_;
+    callback_.Reset();
+    callback_copy.Run(results);
+    return true;
   }
 
-  const FaviconCandidate& best_favicon_candidate() {
-    return best_favicon_candidate_;
-  }
-
- protected:
-  void UpdateFaviconMappingAndFetch(
-      const GURL& page_url,
-      const GURL& icon_url,
-      favicon_base::IconType icon_type,
-      const favicon_base::FaviconResultsCallback& callback,
-      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));
-  }
-
-  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));
-  }
-
-  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()));
-  }
-
-  bool ShouldSaveFavicon() override { return true; }
-
-  GURL page_url_;
-
  private:
-  std::unique_ptr<HistoryRequestHandler> history_handler_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestFaviconHandler);
+  // We actually support one single request in flight;
+  favicon_base::FaviconResultsCallback callback_;
 };
 
-namespace {
-
-void HistoryRequestHandler::InvokeCallback() {
-  if (!callback_.is_null()) {
-    callback_.Run(history_results_);
-  }
-}
-
-void DownloadHandler::InvokeCallback() {
-  if (callback_invoked_)
-    return;
-
-  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;
-}
+class MockDelegate : public FaviconHandler::Delegate {
+ public:
+  MOCK_METHOD3(DownloadImage,
+               int(const GURL& url,
+                   int max_image_size,
+                   ImageDownloadCallback callback));
+  // TODO(mastiz): Consider parameterizing tests to cover incognito.
+  MOCK_METHOD0(IsOffTheRecord, bool());
+  MOCK_METHOD1(IsBookmarked, bool(const GURL& url));
+  MOCK_METHOD5(OnFaviconUpdated,
+               void(const GURL& page_url,
+                    FaviconDriverObserver::NotificationIconType type,
+                    const GURL& icon_url,
+                    bool icon_url_changed,
+                    const gfx::Image& image));
+};
 
 class FaviconHandlerTest : public testing::Test {
  protected:
+  const std::vector<gfx::Size> kEmptySizes;
+  const std::vector<FaviconRawBitmapResult> kEmptyBitmapResult;
+
   FaviconHandlerTest() {
+    // Forward ImageDownload calls to ImageDownloadTester where they get queued.
+    ON_CALL(delegate_, DownloadImage(_, _, _))
+        .WillByDefault(Invoke(&image_download_tester_,
+                              &ImageDownloadTester::DownloadImage));
+    // Forward FaviconService calls to FaviconServiceTester.
+    ON_CALL(favicon_service_, GetFaviconForPageURL(_, _, _, _, _))
+        .WillByDefault(WithArg<3>(Invoke(&favicon_service_tester_,
+                                         &FaviconServiceTester::SaveCallback)));
+    ON_CALL(favicon_service_, GetFavicon(_, _, _, _, _))
+        .WillByDefault(WithArg<3>(Invoke(&favicon_service_tester_,
+                                         &FaviconServiceTester::SaveCallback)));
+    ON_CALL(favicon_service_, UpdateFaviconMappingsAndFetch(_, _, _, _, _, _))
+        .WillByDefault(WithArg<4>(Invoke(&favicon_service_tester_,
+                                         &FaviconServiceTester::SaveCallback)));
   }
 
-  ~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|.
@@ -382,11 +197,16 @@ class FaviconHandlerTest : public testing::Test {
   // - The favicons at |candidate_icons| have edge pixel sizes of
   //   |candidate_icon_sizes|.
   void DownloadTillDoneIgnoringHistory(
-      TestDelegate* delegate,
-      TestFaviconHandler* favicon_handler,
+      MockDelegate* delegate,
+      FaviconHandler* favicon_handler,
       const GURL& page_url,
       const std::vector<FaviconURL>& candidate_icons,
       const int* candidate_icon_sizes) {
+    /*
+    EXPECT_CALL(favicon_service_,
+                GetFaviconForPageURL(page_url, favicon_base::FAVICON, _, _, _))
+        .WillOnce(SaveArg<3>(&get_favicon_callback));
+
     size_t old_num_notifications = delegate->num_notifications();
 
     UpdateFaviconURL(delegate, favicon_handler, page_url, candidate_icons);
@@ -409,16 +229,24 @@ class FaviconHandlerTest : public testing::Test {
       if (delegate->num_notifications() > old_num_notifications)
         return;
     }
+    */
   }
 
-  void UpdateFaviconURL(TestDelegate* delegate,
-                        TestFaviconHandler* favicon_handler,
+  void UpdateFaviconURL(FaviconHandler* favicon_handler,
                         const GURL& page_url,
                         const std::vector<FaviconURL>& candidate_icons) {
-    delegate->ResetNumNotifications();
+    EXPECT_CALL(favicon_service_, WasUnableToDownloadFavicon(_))
+        .Times(candidate_icons.size())
+        .WillRepeatedly(Return(false));
+
+    favicon_base::FaviconResultsCallback get_favicon_callback;
+    // EXPECT_CALL(favicon_service_,
+    //            GetFaviconForPageURL(page_url, favicon_base::FAVICON, _, _,
+    //            _))
+    //    .WillOnce(SaveArg<3>(&get_favicon_callback));
 
     favicon_handler->FetchFavicon(page_url);
-    favicon_handler->history_handler()->InvokeCallback();
+    get_favicon_callback.Run(std::vector<FaviconRawBitmapResult>());
 
     favicon_handler->OnUpdateFaviconURL(page_url, candidate_icons);
   }
@@ -439,116 +267,73 @@ class FaviconHandlerTest : public testing::Test {
   std::unique_ptr<ui::test::ScopedSetSupportedScaleFactors>
       scoped_set_supported_scale_factors_;
   testing::StrictMock<MockFaviconService> favicon_service_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(FaviconHandlerTest);
+  ImageDownloadTester image_download_tester_;
+  FaviconServiceTester favicon_service_tester_;
+  testing::NiceMock<MockDelegate> delegate_;
 };
 
 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(&favicon_service_, &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_);
-
+  // Favicon shouldn't request to download icon.
+  EXPECT_CALL(delegate_, DownloadImage(_, _, _)).Times(0);
+
+  InSequence seq;
+  EXPECT_CALL(favicon_service_,
+              GetFaviconForPageURL(page_url, favicon_base::FAVICON, _, _, _));

[pkotwicz, 2017/02/16 16:36:17]

I don't think that which FaviconService method got called is interesting. 

It is useful to test that there was just one db request

[mastiz, 2017/02/21 16:22:16]

I propose as follows: I'll try to finalize these patch with overspecification of
calls to FaviconService, and then we can trivially choose which ones get
simplified (i.e. use _, don't consider order, etc.), prior to landing.

+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             page_url, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                             icon_url, /*icon_url_changed=*/true, _));
+
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(page_url);
   // Send history response.
-  history_handler->InvokeCallback();
-  // Verify FaviconHandler status
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
-
+  EXPECT_TRUE(
+      favicon_service_tester_.TestRunCallback(CreateFaviconRawBitmapResult(
+          icon_url, favicon_base::FAVICON, /*expired=*/false)));

[pkotwicz, 2017/02/17 15:46:24]

The test right now tests the order of the db request calls and download calls.
In some tests we want to verify order. We want to ensure that we have
FaviconHandler::OnFaviconDataForInitialURLFromFaviconService() getting called
before and after FaviconHandler::OnUpdateFaviconURL()

Right now the sequence of events if there are two icon URLs is
FaviconHandler::FetchFavicon()
FaviconHandler::OnUpdateFaviconURL()
FaviconHandler::GetFaviconForURLFromFaviconService()
FaviconHandler::UpdateFaviconMappingAndFetch()
FaviconHandler::ScheduleDownload()
FaviconHandler::UpdateFaviconMappingAndFetch()
FaviconHandler::ScheduleDownload()

In my ideal world, very few tests would need to change if the implementation
changes the order to
FaviconHandler::FetchFavicon()
FaviconHandler::OnUpdateFaviconURL()
FaviconHandler::GetFaviconForURLFromFaviconService()
FaviconHandler::UpdateFaviconMappingAndFetch()
FaviconHandler::UpdateFaviconMappingAndFetch()
FaviconHandler::ScheduleDownload()
FaviconHandler::ScheduleDownload()

The tests are right now white box tests and I am hoping that more of them can be
black box tests (or a fast integration tests). In order to make them black box
tests I think that FaviconServiceTester needs to be slightly more complex

FaviconServiceTester {
 public:
  void Store(const std::const GURL& page_url, const GURL& icon_url, const
std::vector<FaviconRawBitmapResult>& results);
  void RunPendingCallbacks()
  int num_db_requests() const {}
}

I'd want tests to be able to call RunAllPending() the way I implemented it in
https://codereview.chromium.org/2696423002/diff/1/a.c

- I see how gmock is useful for checking calls to SetFavicons()
OnFaviconUpdated()

- Most tests don't care which FaviconService getters were invoked. We do care
about the # of db accesses. That means that in most tests we should be able to
remove
EXPECT_CALL(favicon_service_, GetFaviconForPageURL(page_url,
favicon_base::FAVICON, _, _, _)) statements

I think that implies that MockFaviconService cannot be a StrictMock

Random: One of my coworkers pointed me to this thread
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/gmock|...
apparently the usage of GMock in Chromium is a subject of debate.  There seems
to be consensus that the GMock matchers are useful

[mastiz, 2017/02/21 16:22:15]

I agree that this is a implementation detail.

However, I'm not aware of a technique that ignores order for a particular pair
of events. I believe the two options are:
1. Ignore order for all calls in some tests.
2. Always enforce order and accept the burden of trivially updating tests if
such change occurs.
RunAllPending() essentially means we don't want to simulate a particular order
of execution. For these, I would use the real FaviconService and then
RunUntilIdle(). WDYT?

I however think it'd be a bit confusing to use a different FaviconService
depending on the test. Perhaps we can do this in FaviconDriverImplTest?
I would expect you actually want to be a bit more careful with *writes*. Hence,
you probably want to whitelist (i.e. not be strict about) one or two methods?

This can be easily achieved by:
EXPECT_CALL(favicon_service_, GetFaviconForPageURL(_, _, _, _,
_)).Times(AtLeast(1));
I indeed followed this thread very closely. From what I understand, the general
guidance is to avoid test doubles and use production implementations
(FaviconService->HistoryService->HistoryBackend).

I think we should do this in FaviconDriverImplTest,
ContentFaviconDriverImplTest, or both.

WDYT?

   // 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());
+  handler.OnUpdateFaviconURL(
+      page_url, {
+                    FaviconURL(icon_url, favicon_base::FAVICON, kEmptySizes),
+                });
 }
 
-TEST_F(FaviconHandlerTest, DownloadFavicon) {
+TEST_F(FaviconHandlerTest, RedownloadExpiredFavicon) {
   const GURL page_url("http://www.google.com");
   const GURL icon_url("http://www.google.com/favicon");
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&favicon_service_, &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());
-
+  InSequence seq;
+  EXPECT_CALL(favicon_service_,
+              GetFaviconForPageURL(page_url, favicon_base::FAVICON, _, _, _));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             page_url, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                             icon_url, /*icon_url_changed=*/true, _));

[pkotwicz, 2017/02/16 16:36:17]

This method should have been called twice

[mastiz, 2017/02/21 16:22:15]

There's one more occurrence below, with a different value for icon_url_changed.

+  EXPECT_CALL(delegate_, DownloadImage(icon_url, _, _));
+  // New icon should be saved to favicon service.
+  EXPECT_CALL(favicon_service_,
+              SetFavicons(page_url, icon_url, favicon_base::FAVICON,
+                          ImageSizeIs(16, 16)));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             page_url, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                             icon_url, /*icon_url_changed=*/false, _));
+
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(page_url);
+  // Send history response with expired icon data.
+  EXPECT_TRUE(
+      favicon_service_tester_.TestRunCallback(CreateFaviconRawBitmapResult(
+          icon_url, favicon_base::FAVICON, /*expired=*/true)));
   // 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());
+  handler.OnUpdateFaviconURL(
+      page_url, {
+                    FaviconURL(icon_url, favicon_base::FAVICON, kEmptySizes),
+                });
+  // Simulates download done.
+  EXPECT_TRUE(image_download_tester_.TestDownloadFinished(
+      200, icon_url, {CreateBitmap(16, 16)}, {gfx::Size(16, 16)}));

[pkotwicz, 2017/02/16 16:36:17]

Doesn't this duplicate the check on line 313?

[mastiz, 2017/02/21 16:22:15]

This is the price you pay for controlling when exactly each operation (in this
case download) finishes. As discussed earlier, this seems desirable for quite
some tests in this set.

Since you seem to like RunUntilIdle-like approaches, I rewrote tests with this
other technique, please see tests suffixed with _New.

 }
 
 TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
@@ -556,163 +341,96 @@ TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&favicon_service_, &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_);
-
+  InSequence seq;
+  EXPECT_CALL(favicon_service_,
+              GetFaviconForPageURL(page_url, favicon_base::FAVICON, _, _, _));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             page_url, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                             icon_url, /*icon_url_changed=*/true, _));
+  EXPECT_CALL(favicon_service_,
+              UpdateFaviconMappingsAndFetch(page_url, URLVector{new_icon_url},
+                                            favicon_base::FAVICON, _, _, _));
+  EXPECT_CALL(delegate_, DownloadImage(new_icon_url, _, _));
+  // New icon should be saved to favicon service.
+  EXPECT_CALL(favicon_service_,
+              SetFavicons(page_url, new_icon_url, favicon_base::FAVICON,
+                          ImageSizeIs(16, 16)));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             page_url, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                             new_icon_url, /*icon_url_changed=*/true, _));
+
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(page_url);
   // 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);
-
+  EXPECT_TRUE(
+      favicon_service_tester_.TestRunCallback(CreateFaviconRawBitmapResult(
+          icon_url, favicon_base::FAVICON, /*expired=*/false)));
   // 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_);
-
+  handler.OnUpdateFaviconURL(
+      page_url,
+      {
+          FaviconURL(new_icon_url, favicon_base::FAVICON, kEmptySizes),
+      });
   // 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());
+  EXPECT_TRUE(favicon_service_tester_.TestRunCallback(kEmptyBitmapResult));
+  // Simulates download done.
+  EXPECT_TRUE(image_download_tester_.TestDownloadFinished(
+      200, new_icon_url, {CreateBitmap(16, 16)}, {gfx::Size(16, 16)}));
 }
 
 TEST_F(FaviconHandlerTest, FaviconInHistoryInvalid) {
   const GURL page_url("http://www.google.com");
   const GURL icon_url("http://www.google.com/favicon");
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&favicon_service_, &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_);
-
+  InSequence seq;
+  EXPECT_CALL(favicon_service_,
+              GetFaviconForPageURL(page_url, favicon_base::FAVICON, _, _, _));
+  EXPECT_CALL(delegate_, DownloadImage(icon_url, _, _));
+  // Icon should be saved to favicon service.
+  EXPECT_CALL(favicon_service_,
+              SetFavicons(page_url, icon_url, favicon_base::FAVICON,
+                          ImageSizeIs(16, 16)));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(
+                             page_url, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                             icon_url, /*icon_url_changed=*/true, _));
+
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(page_url);
   // Set non empty but invalid data.
-  favicon_base::FaviconRawBitmapResult bitmap_result;
+  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());
-
+  EXPECT_TRUE(favicon_service_tester_.TestRunCallback(
+      std::vector<FaviconRawBitmapResult>{bitmap_result}));
+  // Simulates update favicon url.
+  handler.OnUpdateFaviconURL(
+      page_url, {
+                    FaviconURL(icon_url, favicon_base::FAVICON, kEmptySizes),
+                });
   // 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());
+  EXPECT_TRUE(image_download_tester_.TestDownloadFinished(
+      200, icon_url, {CreateBitmap(16, 16)}, {gfx::Size(16, 16)}));
 }
 
+#ifdef MASTIZ_WIP
+
 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(&favicon_service_, &delegate,
-                            FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
 
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
+  handler.FetchFavicon(page_url);
+  HistoryRequestHandler* history_handler = handler.history_handler();
   // Ensure the data given to history is correct.
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ -724,27 +442,26 @@ TEST_F(FaviconHandlerTest, UpdateFavicon) {
   // Send history response.
   history_handler->InvokeCallback();
   // Verify FaviconHandler status.
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_EQ(icon_url, delegate.icon_url());
+  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);
+  handler.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);
+  urls.push_back(FaviconURL(new_icon_url, favicon_base::FAVICON, kEmptySizes));
+  handler.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);
+  EXPECT_EQ(1u, handler.image_urls().size());
+  ASSERT_TRUE(handler.current_candidate());
+  ASSERT_EQ(new_icon_url, handler.current_candidate()->icon_url);
+  ASSERT_EQ(favicon_base::FAVICON, handler.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
-  history_handler = helper.history_handler();
+  history_handler = handler.history_handler();
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(new_icon_url, history_handler->icon_url_);
   EXPECT_EQ(favicon_base::FAVICON, history_handler->icon_type_);
@@ -755,12 +472,12 @@ TEST_F(FaviconHandlerTest, UpdateFavicon) {
   history_handler->InvokeCallback();
 
   // Shouldn't request download favicon
-  EXPECT_FALSE(delegate.download_handler()->HasDownload());
+  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());
+  EXPECT_EQ(2u, delegate_.num_notifications());
+  EXPECT_EQ(new_icon_url, delegate_.icon_url());
+  EXPECT_FALSE(delegate_.image().IsEmpty());
 }
 
 TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
@@ -768,15 +485,15 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&favicon_service_, &delegate,
-                            FaviconDriverObserver::TOUCH_LARGEST);
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::TOUCH_LARGEST);
   std::set<GURL> fail_downloads;
   fail_downloads.insert(icon_url);
-  delegate.download_handler()->FailDownloadForIconURLs(fail_downloads);
+  delegate_.download_handler()->FailDownloadForIconURLs(fail_downloads);
 
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
+  handler.FetchFavicon(page_url);
+  HistoryRequestHandler* history_handler = handler.history_handler();
   // Ensure the data given to history is correct.
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ -789,34 +506,32 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   // Send history response.
   history_handler->InvokeCallback();
   // Verify FaviconHandler status.
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_EQ(GURL(), delegate.icon_url());
+  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);
+  handler.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);
+  urls.push_back(
+      FaviconURL(icon_url, favicon_base::TOUCH_PRECOMPOSED_ICON, kEmptySizes));
+  urls.push_back(
+      FaviconURL(new_icon_url, favicon_base::TOUCH_ICON, kEmptySizes));
+  urls.push_back(FaviconURL(new_icon_url, favicon_base::FAVICON, kEmptySizes));
+  handler.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);
+  EXPECT_EQ(2u, handler.image_urls().size());
+  EXPECT_EQ(0u, handler.current_candidate_index());
+  ASSERT_TRUE(handler.current_candidate());
+  ASSERT_EQ(icon_url, handler.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
-            helper.current_candidate()->icon_type);
+            handler.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
-  history_handler = helper.history_handler();
+  history_handler = handler.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_);
@@ -827,7 +542,7 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   history_handler->InvokeCallback();
 
   // Should request download favicon.
-  DownloadHandler* download_handler = delegate.download_handler();
+  DownloadHandler* download_handler = delegate_.download_handler();
   EXPECT_TRUE(download_handler->HasDownload());
 
   // Verify the download request.
@@ -835,17 +550,17 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
 
   // Reset the history_handler to verify whether favicon is request from
   // history.
-  helper.set_history_handler(nullptr);
+  handler.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);
+  EXPECT_EQ(1u, handler.current_candidate_index());
+  ASSERT_TRUE(handler.current_candidate());
+  EXPECT_EQ(new_icon_url, handler.current_candidate()->icon_url);
+  EXPECT_EQ(favicon_base::TOUCH_ICON, handler.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
-  history_handler = helper.history_handler();
+  history_handler = handler.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_);
@@ -862,16 +577,16 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   history_handler->InvokeCallback();
 
   // Verify the download request.
-  EXPECT_TRUE(delegate.download_handler()->HasDownload());
+  EXPECT_TRUE(delegate_.download_handler()->HasDownload());
   EXPECT_EQ(new_icon_url, download_handler->GetImageUrl());
 
-  helper.set_history_handler(nullptr);
+  handler.set_history_handler(nullptr);
 
   // Simulates icon being downloaded.
   download_handler->InvokeCallback();
 
   // New icon should be saved to history backend.
-  history_handler = helper.history_handler();
+  history_handler = handler.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_);
@@ -884,12 +599,12 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&favicon_service_, &delegate,
-                            FaviconDriverObserver::TOUCH_LARGEST);
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::TOUCH_LARGEST);
 
-  helper.FetchFavicon(page_url);
-  HistoryRequestHandler* history_handler = helper.history_handler();
+  handler.FetchFavicon(page_url);
+  HistoryRequestHandler* history_handler = handler.history_handler();
   // Ensure the data given to history is correct.
   ASSERT_TRUE(history_handler);
   EXPECT_EQ(page_url, history_handler->page_url_);
@@ -902,33 +617,31 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   // Send history response.
   history_handler->InvokeCallback();
   // Verify FaviconHandler status.
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_EQ(GURL(), delegate.icon_url());
+  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);
+  handler.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);
+  urls.push_back(
+      FaviconURL(icon_url, favicon_base::TOUCH_PRECOMPOSED_ICON, kEmptySizes));
+  urls.push_back(
+      FaviconURL(new_icon_url, favicon_base::TOUCH_ICON, kEmptySizes));
+  urls.push_back(FaviconURL(new_icon_url, favicon_base::FAVICON, kEmptySizes));
+  handler.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);
+  EXPECT_EQ(2u, handler.image_urls().size());
+  ASSERT_EQ(0u, handler.current_candidate_index());
+  ASSERT_EQ(icon_url, handler.current_candidate()->icon_url);
   ASSERT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
-            helper.current_candidate()->icon_type);
+            handler.current_candidate()->icon_type);
 
   // Favicon should be requested from history.
-  history_handler = helper.history_handler();
+  history_handler = handler.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_);
@@ -939,7 +652,7 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   history_handler->InvokeCallback();
 
   // Should request download favicon.
-  DownloadHandler* download_handler = delegate.download_handler();
+  DownloadHandler* download_handler = delegate_.download_handler();
   EXPECT_TRUE(download_handler->HasDownload());
 
   // Verify the download request.
@@ -947,20 +660,20 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
 
   // Reset the history_handler to verify whether favicon is request from
   // history.
-  helper.set_history_handler(nullptr);
+  handler.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);
+  latest_urls.push_back(
+      FaviconURL(latest_icon_url, favicon_base::TOUCH_ICON, kEmptySizes));
+  handler.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);
+  EXPECT_EQ(1u, handler.image_urls().size());
+  EXPECT_EQ(0u, handler.current_candidate_index());
+  EXPECT_EQ(latest_icon_url, handler.current_candidate()->icon_url);
+  EXPECT_EQ(favicon_base::TOUCH_ICON, handler.current_candidate()->icon_type);
 
   // Whether new icon is requested from history
-  history_handler = helper.history_handler();
+  history_handler = handler.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_);
@@ -970,12 +683,12 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   // history.
   // Save the callback for late use.
   favicon_base::FaviconResultsCallback callback = history_handler->callback_;
-  helper.set_history_handler(nullptr);
+  handler.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());
+  EXPECT_FALSE(handler.history_handler());
 
   download_handler->Reset();
 
@@ -1001,50 +714,48 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLs) {
   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_base::FAVICON, kEmptySizes));
   favicon_urls.push_back(FaviconURL(GURL("http://www.google.com/favicon2"),
-                                    favicon_base::FAVICON,
-                                    std::vector<gfx::Size>()));
+                                    favicon_base::FAVICON, kEmptySizes));
 
-  TestDelegate delegate;
-  TestFaviconHandler helper(&favicon_service_, &delegate,
-                            FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &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);
+  handler.FetchFavicon(page_url);
+  handler.history_handler()->InvokeCallback();
+  handler.set_history_handler(nullptr);
 
   // Got icon URLs.
-  helper.OnUpdateFaviconURL(page_url, favicon_urls);
+  handler.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_EQ(2u, handler.image_urls().size());
+  ASSERT_EQ(0u, handler.current_candidate_index());
+  HistoryRequestHandler* history_handler = handler.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());
+  handler.OnUpdateFaviconURL(page_url, favicon_urls);
+  EXPECT_EQ(history_handler, handler.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();
+  handler.history_handler()->InvokeCallback();
+  handler.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_EQ(1u, handler.current_candidate_index());
+  history_handler = handler.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());
+  handler.OnUpdateFaviconURL(page_url, favicon_urls);
+  EXPECT_EQ(history_handler, handler.history_handler());
 }
 
 // Fixes crbug.com/544560
@@ -1054,86 +765,83 @@ TEST_F(FaviconHandlerTest,
   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(&favicon_service_, &delegate,
-                            FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &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();
+  handler.FetchFavicon(kPageURL);
+  ASSERT_TRUE(handler.history_handler());
+  handler.history_handler()->InvokeCallback();
   {
     std::vector<FaviconURL> icon_urls;
     icon_urls.push_back(
-        FaviconURL(kIconURL1, favicon_base::FAVICON, std::vector<gfx::Size>()));
+        FaviconURL(kIconURL1, favicon_base::FAVICON, kEmptySizes));
     icon_urls.push_back(
-        FaviconURL(kIconURL2, favicon_base::FAVICON, std::vector<gfx::Size>()));
-    helper.OnUpdateFaviconURL(kPageURL, icon_urls);
+        FaviconURL(kIconURL2, favicon_base::FAVICON, kEmptySizes));
+    handler.OnUpdateFaviconURL(kPageURL, icon_urls);
   }
 
   // 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,
+  ASSERT_EQ(0u, delegate_.num_notifications());
+  ASSERT_TRUE(handler.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,
+                            &handler.history_handler()->history_results_);
+  handler.history_handler()->InvokeCallback();
+  handler.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(handler.history_handler());
+  handler.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();
+                            &handler.history_handler()->history_results_);
+  handler.history_handler()->InvokeCallback();
+  handler.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());
+  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);
+  handler.set_history_handler(nullptr);
 
   // Simulate the page changing it's icon URL to just |kIconURL2| via
   // Javascript.
-  helper.OnUpdateFaviconURL(
+  handler.OnUpdateFaviconURL(
       kPageURL,
-      std::vector<FaviconURL>(1u, FaviconURL(kIconURL2, favicon_base::FAVICON,
-                                             std::vector<gfx::Size>())));
+      std::vector<FaviconURL>(
+          1u, FaviconURL(kIconURL2, favicon_base::FAVICON, kEmptySizes)));
 
   // 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,
+  delegate_.ResetNumNotifications();
+  ASSERT_TRUE(handler.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());
+                            &handler.history_handler()->history_results_);
+  handler.history_handler()->InvokeCallback();
+  handler.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());
 }
 
 // Test the favicon which is selected when the web page provides several
@@ -1143,21 +851,16 @@ TEST_F(FaviconHandlerTest,
 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>())};
+      FaviconURL(GURL("http://www.google.com/a"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/b"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/c"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/d"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/e"), favicon_base::FAVICON,
+                 kEmptySizes)};
 
   // Set the supported scale factors to 1x and 2x. This affects the behavior of
   // SelectFaviconFrames().
@@ -1168,70 +871,70 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
 
   // 1) Test that if there are several single resolution favicons to choose from
   // that the largest exact match is chosen.
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&favicon_service_, &delegate1,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_1;
+  FaviconHandler handler1(&favicon_service_, &delegate_1,
+                          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,
+  DownloadTillDoneIgnoringHistory(&delegate_1, &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());
+  EXPECT_EQ(1u, delegate_1.num_notifications());
+  EXPECT_FALSE(delegate_1.image().IsEmpty());
+  EXPECT_EQ(gfx::kFaviconSize, delegate_1.image().Width());
 
   size_t expected_index = 2u;
   EXPECT_EQ(32, kSizes1[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate1.icon_url());
+  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate_1.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(&favicon_service_, &delegate2,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_2;
+  FaviconHandler handler2(&favicon_service_, &delegate_2,
+                          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,
+  DownloadTillDoneIgnoringHistory(&delegate_2, &handler2, kPageURL, urls2,
                                   kSizes2);
-  EXPECT_EQ(1u, delegate2.num_notifications());
+  EXPECT_EQ(1u, delegate_2.num_notifications());
   expected_index = 0u;
   EXPECT_EQ(16, kSizes2[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate2.icon_url());
+  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate_2.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(&favicon_service_, &delegate3,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_3;
+  FaviconHandler handler3(&favicon_service_, &delegate_3,
+                          FaviconDriverObserver::NON_TOUCH_16_DIP);
 
   const int kSizes3[] = { 256, 48 };
   std::vector<FaviconURL> urls3(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes3));
-  DownloadTillDoneIgnoringHistory(&delegate3, &handler3, kPageURL, urls3,
+  DownloadTillDoneIgnoringHistory(&delegate_3, &handler3, kPageURL, urls3,
                                   kSizes3);
-  EXPECT_EQ(1u, delegate3.num_notifications());
+  EXPECT_EQ(1u, delegate_3.num_notifications());
   expected_index = 1u;
   EXPECT_EQ(48, kSizes3[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate3.icon_url());
+  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate_3.icon_url());
 
-  TestDelegate delegate4;
-  TestFaviconHandler handler4(&favicon_service_, &delegate4,
-                              FaviconDriverObserver::NON_TOUCH_16_DIP);
+  MockDelegate_ delegate_4;
+  FaviconHandler handler4(&favicon_service_, &delegate_4,
+                          FaviconDriverObserver::NON_TOUCH_16_DIP);
 
   const int kSizes4[] = { 17, 256 };
   std::vector<FaviconURL> urls4(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes4));
-  DownloadTillDoneIgnoringHistory(&delegate4, &handler4, kPageURL, urls4,
+  DownloadTillDoneIgnoringHistory(&delegate_4, &handler4, kPageURL, urls4,
                                   kSizes4);
-  EXPECT_EQ(1u, delegate4.num_notifications());
+  EXPECT_EQ(1u, delegate_4.num_notifications());
   expected_index = 0u;
   EXPECT_EQ(17, kSizes4[expected_index]);
-  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate4.icon_url());
+  EXPECT_EQ(kSourceIconURLs[expected_index].icon_url, delegate_4.icon_url());
 }
 
 // Test that the best favicon is selected when:
@@ -1241,22 +944,19 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
 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 k404FaviconURL(k404IconURL, favicon_base::FAVICON,
+                                  kEmptySizes);
   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>()),
+      FaviconURL(GURL("http://www.google.com/a"), favicon_base::FAVICON,
+                 kEmptySizes),
+      k404FaviconURL, FaviconURL(GURL("http://www.google.com/c"),
+                                 favicon_base::FAVICON, kEmptySizes),
   };
 
-  TestDelegate delegate;
-  TestFaviconHandler handler(&favicon_service_, &delegate,
-                             FaviconDriverObserver::NON_TOUCH_16_DIP);
-  DownloadHandler* download_handler = delegate.download_handler();
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  DownloadHandler* download_handler = delegate_.download_handler();
 
   std::set<GURL> k404URLs;
   k404URLs.insert(k404IconURL);
@@ -1265,7 +965,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons404) {
   // Make the initial download for |k404IconURL| fail.
   const int kSizes1[] = { 0 };
   std::vector<FaviconURL> urls1(1u, k404FaviconURL);
-  DownloadTillDoneIgnoringHistory(&delegate, &handler, kPageURL, urls1,
+  DownloadTillDoneIgnoringHistory(&delegate_, &handler, kPageURL, urls1,
                                   kSizes1);
   EXPECT_TRUE(download_handler->DidFailDownloadForIconURL(k404IconURL));
 
@@ -1275,15 +975,15 @@ TEST_F(FaviconHandlerTest, MultipleFavicons404) {
   const int kSizes2[] = { 10, 0, 16 };
   std::vector<FaviconURL> urls2(kFaviconURLs,
                                 kFaviconURLs + arraysize(kFaviconURLs));
-  DownloadTillDoneIgnoringHistory(&delegate, &handler, kPageURL, urls2,
+  DownloadTillDoneIgnoringHistory(&delegate_, &handler, kPageURL, urls2,
                                   kSizes2);
 
   EXPECT_EQ(nullptr, handler.current_candidate());
-  EXPECT_EQ(1u, delegate.num_notifications());
-  EXPECT_FALSE(delegate.image().IsEmpty());
+  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());
+  EXPECT_EQ(kFaviconURLs[expected_index].icon_url, delegate_.icon_url());
 }
 
 // Test that no favicon is selected when:
@@ -1295,18 +995,14 @@ TEST_F(FaviconHandlerTest, MultipleFaviconsAll404) {
   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>()),
+      FaviconURL(k404IconURL1, favicon_base::FAVICON, kEmptySizes),
+      FaviconURL(k404IconURL2, favicon_base::FAVICON, kEmptySizes),
   };
 
-  TestDelegate delegate;
-  TestFaviconHandler handler(&favicon_service_, &delegate,
-                             FaviconDriverObserver::NON_TOUCH_16_DIP);
-  DownloadHandler* download_handler = delegate.download_handler();
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  DownloadHandler* download_handler = delegate_.download_handler();
 
   std::set<GURL> k404URLs;
   k404URLs.insert(k404IconURL1);
@@ -1317,7 +1013,7 @@ TEST_F(FaviconHandlerTest, MultipleFaviconsAll404) {
   for (const FaviconURL& favicon_url : kFaviconURLs) {
     const int kSizes[] = { 0 };
     std::vector<FaviconURL> urls(1u, favicon_url);
-    DownloadTillDoneIgnoringHistory(&delegate, &handler, kPageURL, urls,
+    DownloadTillDoneIgnoringHistory(&delegate_, &handler, kPageURL, urls,
                                     kSizes);
   }
   EXPECT_TRUE(download_handler->DidFailDownloadForIconURL(k404IconURL1));
@@ -1329,11 +1025,11 @@ TEST_F(FaviconHandlerTest, MultipleFaviconsAll404) {
   const int kSizes[] = { 0, 0 };
   std::vector<FaviconURL> urls(kFaviconURLs,
                                kFaviconURLs + arraysize(kFaviconURLs));
-  DownloadTillDoneIgnoringHistory(&delegate, &handler, kPageURL, urls, kSizes);
+  DownloadTillDoneIgnoringHistory(&delegate_, &handler, kPageURL, urls, kSizes);
 
   EXPECT_EQ(nullptr, handler.current_candidate());
-  EXPECT_EQ(0u, delegate.num_notifications());
-  EXPECT_TRUE(delegate.image().IsEmpty());
+  EXPECT_EQ(0u, delegate_.num_notifications());
+  EXPECT_TRUE(delegate_.image().IsEmpty());
 }
 
 // Test that no favicon is selected when the page's only icon uses an invalid
@@ -1343,28 +1039,27 @@ TEST_F(FaviconHandlerTest, FaviconInvalidURL) {
   const GURL kInvalidFormatURL("invalid");
   ASSERT_TRUE(kInvalidFormatURL.is_empty());
 
-  FaviconURL favicon_url(kInvalidFormatURL, favicon_base::FAVICON,
-                         std::vector<gfx::Size>());
+  FaviconURL favicon_url(kInvalidFormatURL, favicon_base::FAVICON, kEmptySizes);
 
-  TestDelegate delegate;
-  TestFaviconHandler handler(&favicon_service_, &delegate,
-                             FaviconDriverObserver::NON_TOUCH_16_DIP);
-  UpdateFaviconURL(&delegate, &handler, kPageURL,
+  MockDelegate_ delegate_;
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  UpdateFaviconURL(&delegate_, &handler, kPageURL,
                    std::vector<FaviconURL>(1u, favicon_url));
   EXPECT_EQ(0u, handler.image_urls().size());
 }
 
 TEST_F(FaviconHandlerTest, TestSortFavicon) {
   const GURL kPageURL("http://www.google.com");
-  std::vector<gfx::Size> icon1;
+  kEmptySizes icon1;
   icon1.push_back(gfx::Size(1024, 1024));
   icon1.push_back(gfx::Size(512, 512));
 
-  std::vector<gfx::Size> icon2;
+  kEmptySizes icon2;
   icon2.push_back(gfx::Size(15, 15));
   icon2.push_back(gfx::Size(16, 16));
 
-  std::vector<gfx::Size> icon3;
+  kEmptySizes icon3;
   icon3.push_back(gfx::Size(16, 16));
   icon3.push_back(gfx::Size(14, 14));
 
@@ -1372,19 +1067,17 @@ TEST_F(FaviconHandlerTest, TestSortFavicon) {
       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(&favicon_service_, &delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
+      FaviconURL(GURL("http://www.google.com/d"), favicon_base::FAVICON,
+                 kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/e"), favicon_base::FAVICON,
+                 kEmptySizes)};
+
+  MockDelegate_ delegate_1;
+  FaviconHandler handler1(&favicon_service_, &delegate_1,
+                          FaviconDriverObserver::NON_TOUCH_LARGEST);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  UpdateFaviconURL(&delegate_1, &handler1, kPageURL, urls1);
 
   struct ExpectedResult {
     // The favicon's index in kSourceIconURLs.
@@ -1416,45 +1109,40 @@ TEST_F(FaviconHandlerTest, TestSortFavicon) {
 
 TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
   const GURL kPageURL("http://www.google.com");
-  std::vector<gfx::Size> icon1;
+  kEmptySizes icon1;
   icon1.push_back(gfx::Size(1024, 1024));
   icon1.push_back(gfx::Size(512, 512));
 
-  std::vector<gfx::Size> icon2;
+  kEmptySizes icon2;
   icon2.push_back(gfx::Size(15, 15));
   icon2.push_back(gfx::Size(14, 14));
 
-  std::vector<gfx::Size> icon3;
+  kEmptySizes icon3;
   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(&favicon_service_, &delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
+      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,
+                 kEmptySizes),
+      FaviconURL(GURL("http://www.google.com/e"), favicon_base::FAVICON,
+                 kEmptySizes)};
+
+  MockDelegate_ delegate_1;
+  FaviconHandler handler1(&favicon_service_, &delegate_1,
+                          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);
+  delegate_1.download_handler()->FailDownloadForIconURLs(fail_icon_urls);
 
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  UpdateFaviconURL(&delegate_1, &handler1, kPageURL, urls1);
 
   // Simulate the download failed, to check whether the icons were requested
   // to download according their size.
@@ -1485,22 +1173,22 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
     handler1.history_handler()->InvokeCallback();
 
     // Verify download request
-    ASSERT_TRUE(delegate1.download_handler()->HasDownload());
+    ASSERT_TRUE(delegate_1.download_handler()->HasDownload());
     EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
-              delegate1.download_handler()->GetImageUrl());
+              delegate_1.download_handler()->GetImageUrl());
 
-    delegate1.download_handler()->InvokeCallback();
-    delegate1.download_handler()->Reset();
+    delegate_1.download_handler()->InvokeCallback();
+    delegate_1.download_handler()->Reset();
   }
 }
 
 TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
   const GURL kPageURL("http://www.google.com");
 
-  std::vector<gfx::Size> one_icon;
+  kEmptySizes one_icon;
   one_icon.push_back(gfx::Size(15, 15));
 
-  std::vector<gfx::Size> two_icons;
+  kEmptySizes two_icons;
   two_icons.push_back(gfx::Size(14, 14));
   two_icons.push_back(gfx::Size(16, 16));
 
@@ -1510,12 +1198,12 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
       FaviconURL(
           GURL("http://www.google.com/c"), favicon_base::FAVICON, two_icons)};
 
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&favicon_service_, &delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
+  MockDelegate_ delegate_1;
+  FaviconHandler handler1(&favicon_service_, &delegate_1,
+                          FaviconDriverObserver::NON_TOUCH_LARGEST);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  UpdateFaviconURL(&delegate_1, &handler1, kPageURL, urls1);
 
   ASSERT_EQ(2u, handler1.image_urls().size());
 
@@ -1535,39 +1223,38 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
   handler1.history_handler()->InvokeCallback();
 
   // Verify download request
-  ASSERT_TRUE(delegate1.download_handler()->HasDownload());
+  ASSERT_TRUE(delegate_1.download_handler()->HasDownload());
   EXPECT_EQ(kSourceIconURLs[i].icon_url,
-            delegate1.download_handler()->GetImageUrl());
+            delegate_1.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();
+  for (kEmptySizes::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();
+  delegate_1.download_handler()->SetImageSizes(sizes);
+  delegate_1.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());
+  EXPECT_EQ(1u, delegate_1.num_notifications());
+  EXPECT_EQ(kSourceIconURLs[i].icon_url, delegate_1.icon_url());
+  EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b], delegate_1.image().Size());
 }
 
 TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
   const GURL kPageURL("http://www.google.com");
   const int kMaximalSize =
-    TestFaviconHandler::GetMaximalIconSize(favicon_base::FAVICON);
+      FaviconHandler::GetMaximalIconSize(favicon_base::FAVICON);
 
-  std::vector<gfx::Size> icon1;
+  kEmptySizes icon1;
   icon1.push_back(gfx::Size(kMaximalSize + 1, kMaximalSize + 1));
 
-  std::vector<gfx::Size> icon2;
+  kEmptySizes icon2;
   icon2.push_back(gfx::Size(kMaximalSize + 2, kMaximalSize + 2));
 
   const FaviconURL kSourceIconURLs[] = {
@@ -1576,12 +1263,12 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
       FaviconURL(
           GURL("http://www.google.com/c"), favicon_base::FAVICON, icon2)};
 
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&favicon_service_, &delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
+  MockDelegate_ delegate_1;
+  FaviconHandler handler1(&favicon_service_, &delegate_1,
+                          FaviconDriverObserver::NON_TOUCH_LARGEST);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  UpdateFaviconURL(&delegate_1, &handler1, kPageURL, urls1);
 
   ASSERT_EQ(2u, handler1.image_urls().size());
 
@@ -1601,16 +1288,16 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
   handler1.history_handler()->InvokeCallback();
 
   // Verify download request
-  ASSERT_TRUE(delegate1.download_handler()->HasDownload());
+  ASSERT_TRUE(delegate_1.download_handler()->HasDownload());
   EXPECT_EQ(kSourceIconURLs[i].icon_url,
-            delegate1.download_handler()->GetImageUrl());
+            delegate_1.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();
+  delegate_1.download_handler()->SetImageSizes(sizes);
+  delegate_1.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.
@@ -1622,27 +1309,26 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
 TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
   const GURL kPageURL("http://www.google.com");
 
-  std::vector<gfx::Size> icon1;
+  kEmptySizes icon1;
   icon1.push_back(gfx::Size(16, 16));
   const int actual_size1 = 10;
 
-  std::vector<gfx::Size> icon2;
+  kEmptySizes 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>())};
+      FaviconURL(GURL("http://www.google.com/d"), favicon_base::FAVICON,
+                 kEmptySizes)};
 
-  TestDelegate delegate1;
-  TestFaviconHandler handler1(&favicon_service_, &delegate1,
-                              FaviconDriverObserver::NON_TOUCH_LARGEST);
+  MockDelegate_ delegate_1;
+  FaviconHandler handler1(&favicon_service_, &delegate_1,
+                          FaviconDriverObserver::NON_TOUCH_LARGEST);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&delegate1, &handler1, kPageURL, urls1);
+  UpdateFaviconURL(&delegate_1, &handler1, kPageURL, urls1);
   ASSERT_EQ(3u, handler1.image_urls().size());
 
   // Simulate no favicon from history.
@@ -1650,25 +1336,25 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
   handler1.history_handler()->InvokeCallback();
 
   // Verify the first icon was request to download
-  ASSERT_TRUE(delegate1.download_handler()->HasDownload());
+  ASSERT_TRUE(delegate_1.download_handler()->HasDownload());
   EXPECT_EQ(kSourceIconURLs[0].icon_url,
-            delegate1.download_handler()->GetImageUrl());
+            delegate_1.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();
+  delegate_1.download_handler()->SetImageSizes(sizes);
+  delegate_1.download_handler()->InvokeCallback();
+  delegate_1.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());
+  ASSERT_TRUE(delegate_1.download_handler()->HasDownload());
   EXPECT_EQ(kSourceIconURLs[1].icon_url,
-            delegate1.download_handler()->GetImageUrl());
+            delegate_1.download_handler()->GetImageUrl());
 
   // Very the best candidate is icon1
   EXPECT_EQ(kSourceIconURLs[0].icon_url,
@@ -1679,9 +1365,9 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
   // Give the incorrect size.
   sizes.clear();
   sizes.push_back(actual_size2);
-  delegate1.download_handler()->SetImageSizes(sizes);
-  delegate1.download_handler()->InvokeCallback();
-  delegate1.download_handler()->Reset();
+  delegate_1.download_handler()->SetImageSizes(sizes);
+  delegate_1.download_handler()->InvokeCallback();
+  delegate_1.download_handler()->Reset();
 
   // Verify icon2 has been saved into history.
   EXPECT_EQ(kSourceIconURLs[1].icon_url, handler1.history_handler()->icon_url_);
@@ -1689,5 +1375,7 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
             handler1.history_handler()->size_);
 }
 
+#endif
+
 }  // namespace
 }  // namespace favicon