[Refactor] Avoid favicon code duplication between webuis and ui menu

We introduce a new layer FaviconRequestHandler that is used by both
FaviconSource and RecentTabsMenuModel to request favicons by pageurl.
The layer first queries the local storage (FaviconService) and if that
fails then queries sync storage (FaviconCache). No behavior is changed.

Bug: 955475
Change-Id: I4836cd21cc6a7f30147c84f5de04fdef3d1a32ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1603563
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Auto-Submit: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#661297}

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc

@@ -132,6 +132,18 @@ gfx::Image CreateFavicon(const gfx::VectorIcon& icon) {
                                 ui::NativeTheme::kColorId_DefaultIconColor)));
 }
 
+bool GetSyncedFaviconForPageURL(
+    sync_sessions::SessionSyncService* session_sync_service,
+    const GURL& page_url,
+    scoped_refptr<base::RefCountedMemory>* sync_bitmap) {
+  if (!session_sync_service)
+    return false;
+  sync_sessions::OpenTabsUIDelegate* open_tabs =
+      session_sync_service->GetOpenTabsUIDelegate();
+  return open_tabs &&
+         open_tabs->GetSyncedFaviconForPageURL(page_url.spec(), sync_bitmap);
+}
+
 }  // namespace
 
 enum RecentTabAction {
@@ -565,62 +577,37 @@ void RecentTabsSubMenuModel::AddDeviceFavicon(
 void RecentTabsSubMenuModel::AddTabFavicon(int command_id, const GURL& url) {
   int index_in_menu = GetIndexOfCommandId(command_id);
 
-  // Start to fetch the favicon from local history asynchronously.
   // Set default icon first.
   SetIcon(index_in_menu, favicon::GetDefaultFavicon());
-  // Start request to fetch actual icon if possible.
-  favicon::FaviconService* favicon_service =
-      FaviconServiceFactory::GetForProfile(browser_->profile(),
-                                           ServiceAccessType::EXPLICIT_ACCESS);
-  if (!favicon_service)
-    return;
 
   bool is_local_tab = command_id < kFirstOtherDevicesTabCommandId;
-  favicon_service->GetFaviconImageForPageURL(
+  favicon_request_handler_.GetFaviconImageForPageURL(
       url,
-      base::Bind(&RecentTabsSubMenuModel::OnFaviconDataAvailable,
-                 weak_ptr_factory_.GetWeakPtr(), url, command_id),
+      base::BindRepeating(&RecentTabsSubMenuModel::OnFaviconDataAvailable,
+                          base::Unretained(this), command_id),
+      favicon::FaviconRequestOrigin::RECENTLY_CLOSED_TABS,
+      FaviconServiceFactory::GetForProfile(browser_->profile(),
+                                           ServiceAccessType::EXPLICIT_ACCESS),
+      base::BindOnce(&GetSyncedFaviconForPageURL,
+                     base::Unretained(session_sync_service_)),
       is_local_tab ? &local_tab_cancelable_task_tracker_
                    : &other_devices_tab_cancelable_task_tracker_);
 }
 
 void RecentTabsSubMenuModel::OnFaviconDataAvailable(
-    const GURL& page_url,
     int command_id,
     const favicon_base::FaviconImageResult& image_result) {
-  int index_in_menu = GetIndexOfCommandId(command_id);
-  if (!image_result.image.IsEmpty()) {
-    favicon::RecordFaviconRequestMetric(
-        favicon::FaviconRequestOrigin::RECENTLY_CLOSED_TABS,
-        favicon::FaviconAvailability::kLocal);
-    DCHECK_GT(index_in_menu, -1);
-    SetIcon(index_in_menu, image_result.image);
-    ui::MenuModelDelegate* delegate = menu_model_delegate();
-    if (delegate)
-      delegate->OnIconChanged(index_in_menu);
+  if (image_result.image.IsEmpty()) {
+    // Default icon has already been set.
     return;
   }
-
-  // If tab has synced favicon, use it.
-  // Note that currently, other devices' tabs only have favicons if
-  // --sync-tab-favicons switch is on; according to zea@, this flag is now
-  // automatically enabled for iOS and android, and they're looking into
-  // enabling it for other platforms.
-  sync_sessions::OpenTabsUIDelegate* open_tabs = GetOpenTabsUIDelegate();
-  scoped_refptr<base::RefCountedMemory> favicon_png;
-  if (open_tabs &&
-      open_tabs->GetSyncedFaviconForPageURL(page_url.spec(), &favicon_png)) {
-    favicon::RecordFaviconRequestMetric(
-        favicon::FaviconRequestOrigin::RECENTLY_CLOSED_TABS,
-        favicon::FaviconAvailability::kSync);
-    gfx::Image image = gfx::Image::CreateFrom1xPNGBytes(favicon_png);
-    SetIcon(index_in_menu, image);
-    return;
-  }
-
-  favicon::RecordFaviconRequestMetric(
-      favicon::FaviconRequestOrigin::RECENTLY_CLOSED_TABS,
-      favicon::FaviconAvailability::kNotAvailable);
+  int index_in_menu = GetIndexOfCommandId(command_id);
+  DCHECK_GT(index_in_menu, -1);
+  SetIcon(index_in_menu, image_result.image);
+  ui::MenuModelDelegate* delegate = menu_model_delegate();
+  if (delegate)
+    delegate->OnIconChanged(index_in_menu);
+  return;
 }
 
 int RecentTabsSubMenuModel::CommandIdToTabVectorIndex(

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.h

@@ -16,6 +16,7 @@
 #include "base/scoped_observer.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "base/timer/elapsed_timer.h"
+#include "components/favicon/core/favicon_request_handler.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/sessions/core/session_id.h"
 #include "components/sessions/core/tab_restore_service.h"
@@ -119,7 +120,6 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
   // OnFaviconDataAvailable() will be invoked when the favicon is ready.
   void AddTabFavicon(int command_id, const GURL& url);
   void OnFaviconDataAvailable(
-      const GURL& page_url,
       int command_id,
       const favicon_base::FaviconImageResult& image_result);
 
@@ -188,6 +188,7 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
   std::unique_ptr<base::CallbackList<void()>::Subscription>
       foreign_session_updated_subscription_;
 
+  favicon::FaviconRequestHandler favicon_request_handler_;
   base::WeakPtrFactory<RecentTabsSubMenuModel> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(RecentTabsSubMenuModel);

chrome/browser/ui/webui/favicon_source.cc

@@ -38,6 +38,21 @@ favicon::FaviconRequestOrigin ParseFaviconRequestOrigin(const GURL& url) {
     return favicon::FaviconRequestOrigin::HISTORY;
   return favicon::FaviconRequestOrigin::UNKNOWN;
 }
+
+bool GetSyncedFaviconForPageURL(
+    Profile* profile,
+    const GURL& page_url,
+    scoped_refptr<base::RefCountedMemory>* sync_bitmap) {
+  sync_sessions::SessionSyncService* session_sync_service =
+      SessionSyncServiceFactory::GetInstance()->GetForProfile(profile);
+  if (!session_sync_service)
+    return false;
+  sync_sessions::OpenTabsUIDelegate* open_tabs =
+      session_sync_service->GetOpenTabsUIDelegate();
+  return open_tabs &&
+         open_tabs->GetSyncedFaviconForPageURL(page_url.spec(), sync_bitmap);
+}
+
 }  // namespace
 
 FaviconSource::IconRequest::IconRequest()
@@ -112,7 +127,7 @@ void FaviconSource::StartDataRequest(
     // IconType.
     favicon_service->GetRawFavicon(
         url, favicon_base::IconType::kFavicon, desired_size_in_pixel,
-        base::Bind(
+        base::BindRepeating(
             &FaviconSource::OnFaviconDataAvailable, base::Unretained(this),
             IconRequest(callback, url, parsed.size_in_dip,
                         parsed.device_scale_factor, unsafe_request_origin)),
@@ -135,21 +150,14 @@ void FaviconSource::StartDataRequest(
       }
     }
 
-    // |url| is an origin, and it may not have had a favicon associated with it.
-    // A trickier case is when |url| only has domain-scoped cookies, but
-    // visitors are redirected to HTTPS on visiting. Then |url| defaults to a
-    // HTTP scheme, but the favicon will be associated with the HTTPS URL and
-    // hence won't be found if we include the scheme in the lookup. Set
-    // |fallback_to_host|=true so the favicon database will fall back to
-    // matching only the hostname to have the best chance of finding a favicon.
-    const bool fallback_to_host = true;
-    favicon_service->GetRawFaviconForPageURL(
-        url, {favicon_base::IconType::kFavicon}, desired_size_in_pixel,
-        fallback_to_host,
-        base::Bind(
+    favicon_request_handler_.GetRawFaviconForPageURL(
+        url, desired_size_in_pixel,
+        base::BindRepeating(
             &FaviconSource::OnFaviconDataAvailable, base::Unretained(this),
             IconRequest(callback, url, parsed.size_in_dip,
                         parsed.device_scale_factor, unsafe_request_origin)),
+        unsafe_request_origin, favicon_service,
+        base::BindOnce(&GetSyncedFaviconForPageURL, base::Unretained(profile_)),
         &cancelable_task_tracker_);
   }
 }
@@ -182,23 +190,6 @@ bool FaviconSource::ShouldServiceRequest(
                                              render_process_id);
 }
 
-bool FaviconSource::HandleMissingResource(const IconRequest& request) {
-  // If the favicon is not available, try to use the synced favicon.
-  sync_sessions::SessionSyncService* service =
-      SessionSyncServiceFactory::GetInstance()->GetForProfile(profile_);
-  sync_sessions::OpenTabsUIDelegate* open_tabs =
-      service ? service->GetOpenTabsUIDelegate() : nullptr;
-
-  scoped_refptr<base::RefCountedMemory> response;
-  if (open_tabs &&
-      open_tabs->GetSyncedFaviconForPageURL(request.request_path.spec(),
-                                            &response)) {
-    request.callback.Run(response.get());
-    return true;
-  }
-  return false;
-}
-
 ui::NativeTheme* FaviconSource::GetNativeTheme() {
   return ui::NativeTheme::GetInstanceForNativeUi();
 }
@@ -207,18 +198,9 @@ void FaviconSource::OnFaviconDataAvailable(
     const IconRequest& request,
     const favicon_base::FaviconRawBitmapResult& bitmap_result) {
   if (bitmap_result.is_valid()) {
-    favicon::RecordFaviconRequestMetric(request.icon_request_origin,
-                                        favicon::FaviconAvailability::kLocal);
     // Forward the data along to the networking system.
     request.callback.Run(bitmap_result.bitmap_data.get());
-  } else if (HandleMissingResource(request)) {
-    favicon::RecordFaviconRequestMetric(request.icon_request_origin,
-                                        favicon::FaviconAvailability::kSync);
-    // The response was already treated by HandleMissingResource.
   } else {
-    favicon::RecordFaviconRequestMetric(
-        request.icon_request_origin,
-        favicon::FaviconAvailability::kNotAvailable);
     SendDefaultResponse(request);
   }
 }

chrome/browser/ui/webui/favicon_source.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "components/favicon/core/favicon_request_handler.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon_base/favicon_request_metrics.h"
 #include "content/public/browser/url_data_source.h"
@@ -91,12 +92,6 @@ class FaviconSource : public content::URLDataSource {
     favicon::FaviconRequestOrigin icon_request_origin;
   };
 
-  // Called when the favicon data is missing to perform additional checks to
-  // locate the resource.
-  // |request| contains information for the failed request.
-  // Returns true if the missing resource is found.
-  virtual bool HandleMissingResource(const IconRequest& request);
-
   // Exposed for testing.
   virtual ui::NativeTheme* GetNativeTheme();
   virtual base::RefCountedMemory* LoadIconBytes(const IconRequest& request,
@@ -126,6 +121,7 @@ class FaviconSource : public content::URLDataSource {
   void SendDefaultResponse(const IconRequest& request);
 
   base::CancelableTaskTracker cancelable_task_tracker_;
+  favicon::FaviconRequestHandler favicon_request_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(FaviconSource);
 };

components/favicon/core/BUILD.gn

@@ -14,6 +14,8 @@ static_library("core") {
     "favicon_driver_observer.h",
     "favicon_handler.cc",
     "favicon_handler.h",
+    "favicon_request_handler.cc",
+    "favicon_request_handler.h",
     "favicon_server_fetcher_params.cc",
     "favicon_server_fetcher_params.h",
     "favicon_service.cc",
@@ -51,6 +53,7 @@ source_set("unit_tests") {
   sources = [
     "fallback_url_util_unittest.cc",
     "favicon_handler_unittest.cc",
+    "favicon_request_handler_unittest.cc",
     "favicon_service_impl_unittest.cc",
     "large_icon_service_impl_unittest.cc",
   ]

components/favicon/core/favicon_request_handler.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/favicon/core/favicon_request_handler.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "components/favicon/core/favicon_service.h"
+#include "components/favicon_base/favicon_types.h"
+
+namespace favicon {
+
+namespace {
+
+// Parameter used for local bitmap queries by page url. The url is an origin,
+// and it may not have had a favicon associated with it. A trickier case is when
+// it only has domain-scoped cookies, but visitors are redirected to HTTPS on
+// visiting. It defaults to a HTTP scheme, but the favicon will be associated
+// with the HTTPS URL and hence won't be found if we include the scheme in the
+// lookup. Set |fallback_to_host|=true so the favicon database will fall back to
+// matching only the hostname to have the best chance of finding a favicon.
+// TODO(victorvianna): Consider passing this as a parameter in the API.
+const bool kFallbackToHost = true;
+
+// Parameter used for local bitmap queries by page url.
+favicon_base::IconTypeSet GetIconTypesForLocalQuery() {
+  return favicon_base::IconTypeSet{favicon_base::IconType::kFavicon};
+}
+
+}  // namespace
+
+FaviconRequestHandler::FaviconRequestHandler() {}
+
+FaviconRequestHandler::~FaviconRequestHandler() {}
+
+void FaviconRequestHandler::GetRawFaviconForPageURL(
+    const GURL& page_url,
+    int desired_size_in_pixel,
+    const favicon_base::FaviconRawBitmapCallback& callback,
+    FaviconRequestOrigin request_origin,
+    FaviconService* favicon_service,
+    FaviconRequestHandler::SyncedFaviconGetter synced_favicon_getter,
+    base::CancelableTaskTracker* tracker) {
+  if (!favicon_service) {
+    RecordFaviconRequestMetric(request_origin,
+                               FaviconAvailability::kNotAvailable);
+    callback.Run(favicon_base::FaviconRawBitmapResult());
+    return;
+  }
+
+  // First attempt to find the icon locally.
+  favicon_service->GetRawFaviconForPageURL(
+      page_url, GetIconTypesForLocalQuery(), desired_size_in_pixel,
+      kFallbackToHost,
+      base::BindRepeating(&FaviconRequestHandler::OnBitmapLocalDataAvailable,
+                          weak_ptr_factory_.GetWeakPtr(), page_url,
+                          /*response_callback=*/callback, request_origin,
+                          base::Passed(&synced_favicon_getter)),
+      tracker);
+}
+
+void FaviconRequestHandler::GetFaviconImageForPageURL(
+    const GURL& page_url,
+    const favicon_base::FaviconImageCallback& callback,
+    FaviconRequestOrigin request_origin,
+    FaviconService* favicon_service,
+    FaviconRequestHandler::SyncedFaviconGetter synced_favicon_getter,
+    base::CancelableTaskTracker* tracker) {
+  if (!favicon_service) {
+    RecordFaviconRequestMetric(request_origin,
+                               FaviconAvailability::kNotAvailable);
+    callback.Run(favicon_base::FaviconImageResult());
+    return;
+  }
+
+  // First attempt to find the icon locally.
+  favicon_service->GetFaviconImageForPageURL(
+      page_url,
+      base::BindRepeating(&FaviconRequestHandler::OnImageLocalDataAvailable,
+                          weak_ptr_factory_.GetWeakPtr(), page_url,
+                          /*response_callback=*/callback, request_origin,
+                          base::Passed(&synced_favicon_getter)),
+      tracker);
+}
+
+void FaviconRequestHandler::OnBitmapLocalDataAvailable(
+    const GURL& page_url,
+    const favicon_base::FaviconRawBitmapCallback& response_callback,
+    FaviconRequestOrigin origin,
+    FaviconRequestHandler::SyncedFaviconGetter synced_favicon_getter,
+    const favicon_base::FaviconRawBitmapResult& bitmap_result) const {
+  if (bitmap_result.is_valid()) {
+    RecordFaviconRequestMetric(origin, FaviconAvailability::kLocal);
+    response_callback.Run(bitmap_result);
+    return;
+  }
+
+  scoped_refptr<base::RefCountedMemory> sync_bitmap;
+  if (std::move(synced_favicon_getter).Run(page_url, &sync_bitmap)) {
+    RecordFaviconRequestMetric(origin, FaviconAvailability::kSync);
+    favicon_base::FaviconRawBitmapResult sync_bitmap_result;
+    sync_bitmap_result.bitmap_data = sync_bitmap;
+    response_callback.Run(sync_bitmap_result);
+    return;
+  }
+
+  // If sync does not have the favicon, send empty response.
+  RecordFaviconRequestMetric(origin, FaviconAvailability::kNotAvailable);
+  response_callback.Run(favicon_base::FaviconRawBitmapResult());
+}
+
+void FaviconRequestHandler::OnImageLocalDataAvailable(
+    const GURL& page_url,
+    const favicon_base::FaviconImageCallback& response_callback,
+    FaviconRequestOrigin origin,
+    FaviconRequestHandler::SyncedFaviconGetter synced_favicon_getter,
+    const favicon_base::FaviconImageResult& image_result) const {
+  if (!image_result.image.IsEmpty()) {
+    RecordFaviconRequestMetric(origin, FaviconAvailability::kLocal);
+    response_callback.Run(image_result);
+    return;
+  }
+
+  scoped_refptr<base::RefCountedMemory> sync_bitmap;
+  if (std::move(synced_favicon_getter).Run(page_url, &sync_bitmap)) {
+    RecordFaviconRequestMetric(origin, FaviconAvailability::kSync);
+    favicon_base::FaviconImageResult sync_image_result;
+    // Convert bitmap to image.
+    sync_image_result.image =
+        gfx::Image::CreateFrom1xPNGBytes(sync_bitmap.get());
+    response_callback.Run(sync_image_result);
+    return;
+  }
+
+  // If sync does not have the favicon, send empty response.
+  RecordFaviconRequestMetric(origin, FaviconAvailability::kNotAvailable);
+  response_callback.Run(favicon_base::FaviconImageResult());
+}
+
+}  // namespace favicon

components/favicon/core/favicon_request_handler.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_FAVICON_CORE_FAVICON_REQUEST_HANDLER_H_
+#define COMPONENTS_FAVICON_CORE_FAVICON_REQUEST_HANDLER_H_
+
+#include "base/memory/ref_counted_memory.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/task/cancelable_task_tracker.h"
+#include "components/favicon_base/favicon_callback.h"
+#include "components/favicon_base/favicon_request_metrics.h"
+#include "url/gurl.h"
+
+namespace favicon {
+
+class FaviconService;
+
+// Class for handling favicon requests by page url, forwarding them to local
+// storage or sync accordingly.
+class FaviconRequestHandler {
+ public:
+  // Callback that requests the synced bitmap for the page url given in the
+  // the first argument, storing the result in the second argument. Returns
+  // whether the request succeeded.
+  // TODO(victorvianna): Make this return a pointer instead of a bool.
+  using SyncedFaviconGetter =
+      base::OnceCallback<bool(const GURL&,
+                              scoped_refptr<base::RefCountedMemory>*)>;
+
+  FaviconRequestHandler();
+
+  ~FaviconRequestHandler();
+
+  // Requests favicon bitmap at |page_url| of size |desired_size_in_pixel|.
+  // Tries to fetch the icon from local storage and falls back to sync if it's
+  // not found.
+  void GetRawFaviconForPageURL(
+      const GURL& page_url,
+      int desired_size_in_pixel,
+      const favicon_base::FaviconRawBitmapCallback& callback,
+      FaviconRequestOrigin request_origin,
+      FaviconService* favicon_service,
+      SyncedFaviconGetter synced_favicon_getter,
+      base::CancelableTaskTracker* tracker);
+
+  // Requests favicon image at |page_url|. Tries to fetch the icon from local
+  // storage and falls back to sync if it's not found.
+  void GetFaviconImageForPageURL(
+      const GURL& page_url,
+      const favicon_base::FaviconImageCallback& callback,
+      FaviconRequestOrigin request_origin,
+      FaviconService* favicon_service,
+      SyncedFaviconGetter synced_favicon_getter,
+      base::CancelableTaskTracker* tracker);
+
+ private:
+  // Called after the first attempt to retrieve the icon bitmap from local
+  // storage. If request succeeded, sends the result. Otherwise attempts to
+  // retrieve from sync.
+  void OnBitmapLocalDataAvailable(
+      const GURL& page_url,
+      const favicon_base::FaviconRawBitmapCallback& response_callback,
+      FaviconRequestOrigin origin,
+      SyncedFaviconGetter synced_favicon_getter,
+      const favicon_base::FaviconRawBitmapResult& bitmap_result) const;
+
+  // Called after the first attempt to retrieve the icon image from local
+  // storage. If request succeeded, sends the result. Otherwise attempts to
+  // retrieve from sync.
+  void OnImageLocalDataAvailable(
+      const GURL& page_url,
+      const favicon_base::FaviconImageCallback& response_callback,
+      FaviconRequestOrigin origin,
+      SyncedFaviconGetter synced_favicon_getter,
+      const favicon_base::FaviconImageResult& image_result) const;
+
+  base::WeakPtrFactory<FaviconRequestHandler> weak_ptr_factory_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(FaviconRequestHandler);
+};
+
+}  // namespace favicon
+
+#endif  // COMPONENTS_FAVICON_CORE_FAVICON_REQUEST_HANDLER_H_

components/favicon/core/favicon_request_handler_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/favicon/core/favicon_request_handler.h"
+
+#include "base/bind.h"
+#include "base/task/cancelable_task_tracker.h"
+#include "base/test/mock_callback.h"
+#include "components/favicon/core/test/mock_favicon_service.h"
+#include "components/favicon_base/favicon_request_metrics.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/gfx/codec/png_codec.h"
+#include "ui/gfx/image/image.h"
+
+namespace favicon {
+namespace {
+
+using testing::_;
+using testing::Invoke;
+
+const char kDummyPageUrl[] = "https://www.example.com";
+const int kDesiredSizeInPixel = 16;
+const SkColor kTestColor = SK_ColorRED;
+base::CancelableTaskTracker::TaskId kDummyTaskId;
+const FaviconRequestOrigin kOrigin = FaviconRequestOrigin::UNKNOWN;
+
+scoped_refptr<base::RefCountedBytes> CreateTestBitmapBytes() {
+  scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes());
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(16, 16);
+  bitmap.eraseColor(kTestColor);
+  gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &data->data());
+  return data;
+}
+
+favicon_base::FaviconRawBitmapResult CreateTestBitmapResult() {
+  favicon_base::FaviconRawBitmapResult result;
+  result.bitmap_data = CreateTestBitmapBytes();
+  return result;
+}
+
+favicon_base::FaviconImageResult CreateTestImageResult() {
+  favicon_base::FaviconImageResult result;
+  result.image = gfx::Image::CreateFrom1xPNGBytes(CreateTestBitmapBytes());
+  return result;
+}
+
+ACTION_P(ReturnBitmapFromLocal, bitmap) {
+  arg4.Run(bitmap);
+  return kDummyTaskId;
+}
+
+ACTION_P(ReturnImageFromLocal, image) {
+  arg1.Run(image);
+  return kDummyTaskId;
+}
+
+ACTION_P(ReturnFaviconFromSync, should_return_valid) {
+  if (should_return_valid) {
+    *arg1 = CreateTestBitmapBytes();
+  }
+  return should_return_valid;
+}
+
+void StoreBitmap(favicon_base::FaviconRawBitmapResult* destination,
+                 const favicon_base::FaviconRawBitmapResult& result) {
+  *destination = result;
+}
+
+void StoreImage(favicon_base::FaviconImageResult* destination,
+                const favicon_base::FaviconImageResult& result) {
+  *destination = result;
+}
+
+class FaviconRequestHandlerTest : public ::testing::Test {
+ public:
+  FaviconRequestHandlerTest() = default;
+
+ protected:
+  testing::NiceMock<MockFaviconService> mock_favicon_service_;
+  FaviconRequestHandler favicon_request_handler_;
+  base::MockCallback<FaviconRequestHandler::SyncedFaviconGetter>
+      synced_favicon_getter_;
+  base::CancelableTaskTracker tracker_;
+};
+
+TEST_F(FaviconRequestHandlerTest, ShouldGetEmptyBitmap) {
+  EXPECT_CALL(mock_favicon_service_,
+              GetRawFaviconForPageURL(GURL(kDummyPageUrl), _,
+                                      kDesiredSizeInPixel, _, _, &tracker_))
+      .WillOnce(ReturnBitmapFromLocal(favicon_base::FaviconRawBitmapResult()));
+  EXPECT_CALL(synced_favicon_getter_, Run(GURL(kDummyPageUrl), _))
+      .WillOnce(ReturnFaviconFromSync(/*should_return_valid=*/false));
+  favicon_base::FaviconRawBitmapResult result;
+  favicon_request_handler_.GetRawFaviconForPageURL(
+      GURL(kDummyPageUrl), kDesiredSizeInPixel,
+      base::BindRepeating(&StoreBitmap, &result), kOrigin,
+      &mock_favicon_service_, synced_favicon_getter_.Get(), &tracker_);
+  EXPECT_FALSE(result.is_valid());
+}
+
+TEST_F(FaviconRequestHandlerTest, ShouldGetSyncBitmap) {
+  EXPECT_CALL(mock_favicon_service_,
+              GetRawFaviconForPageURL(GURL(kDummyPageUrl), _,
+                                      kDesiredSizeInPixel, _, _, &tracker_))
+      .WillOnce(ReturnBitmapFromLocal(favicon_base::FaviconRawBitmapResult()));
+  EXPECT_CALL(synced_favicon_getter_, Run(GURL(kDummyPageUrl), _))
+      .WillOnce(ReturnFaviconFromSync(/*should_return_valid=*/true));
+  favicon_base::FaviconRawBitmapResult result;
+  favicon_request_handler_.GetRawFaviconForPageURL(
+      GURL(kDummyPageUrl), kDesiredSizeInPixel,
+      base::BindRepeating(&StoreBitmap, &result), kOrigin,
+      &mock_favicon_service_, synced_favicon_getter_.Get(), &tracker_);
+  EXPECT_TRUE(result.is_valid());
+}
+
+TEST_F(FaviconRequestHandlerTest, ShouldGetLocalBitmap) {
+  EXPECT_CALL(mock_favicon_service_,
+              GetRawFaviconForPageURL(GURL(kDummyPageUrl), _,
+                                      kDesiredSizeInPixel, _, _, &tracker_))
+      .WillOnce(ReturnBitmapFromLocal(CreateTestBitmapResult()));
+  EXPECT_CALL(synced_favicon_getter_, Run(GURL(kDummyPageUrl), _)).Times(0);
+  favicon_base::FaviconRawBitmapResult result;
+  favicon_request_handler_.GetRawFaviconForPageURL(
+      GURL(kDummyPageUrl), kDesiredSizeInPixel,
+      base::BindRepeating(&StoreBitmap, &result), kOrigin,
+      &mock_favicon_service_, synced_favicon_getter_.Get(), &tracker_);
+  EXPECT_TRUE(result.is_valid());
+}
+
+TEST_F(FaviconRequestHandlerTest, ShouldGetEmptyImage) {
+  EXPECT_CALL(mock_favicon_service_,
+              GetFaviconImageForPageURL(GURL(kDummyPageUrl), _, &tracker_))
+      .WillOnce(ReturnImageFromLocal(favicon_base::FaviconImageResult()));
+  EXPECT_CALL(synced_favicon_getter_, Run(GURL(kDummyPageUrl), _))
+      .WillOnce(ReturnFaviconFromSync(/*should_return_valid=*/false));
+  favicon_base::FaviconImageResult result;
+  favicon_request_handler_.GetFaviconImageForPageURL(
+      GURL(kDummyPageUrl), base::BindRepeating(&StoreImage, &result), kOrigin,
+      &mock_favicon_service_, synced_favicon_getter_.Get(), &tracker_);
+  EXPECT_TRUE(result.image.IsEmpty());
+}
+
+TEST_F(FaviconRequestHandlerTest, ShouldGetSyncImage) {
+  EXPECT_CALL(mock_favicon_service_,
+              GetFaviconImageForPageURL(GURL(kDummyPageUrl), _, &tracker_))
+      .WillOnce(ReturnImageFromLocal(favicon_base::FaviconImageResult()));
+  EXPECT_CALL(synced_favicon_getter_, Run(GURL(kDummyPageUrl), _))
+      .WillOnce(ReturnFaviconFromSync(/*should_return_valid=*/true));
+  favicon_base::FaviconImageResult result;
+  favicon_request_handler_.GetFaviconImageForPageURL(
+      GURL(kDummyPageUrl), base::BindRepeating(&StoreImage, &result), kOrigin,
+      &mock_favicon_service_, synced_favicon_getter_.Get(), &tracker_);
+  EXPECT_FALSE(result.image.IsEmpty());
+}
+
+TEST_F(FaviconRequestHandlerTest, ShouldGetLocalImage) {
+  EXPECT_CALL(mock_favicon_service_,
+              GetFaviconImageForPageURL(GURL(kDummyPageUrl), _, &tracker_))
+      .WillOnce(ReturnImageFromLocal(CreateTestImageResult()));
+  EXPECT_CALL(synced_favicon_getter_, Run(GURL(kDummyPageUrl), _)).Times(0);
+  favicon_base::FaviconImageResult result;
+  favicon_request_handler_.GetFaviconImageForPageURL(
+      GURL(kDummyPageUrl), base::BindRepeating(&StoreImage, &result), kOrigin,
+      &mock_favicon_service_, synced_favicon_getter_.Get(), &tracker_);
+  EXPECT_FALSE(result.image.IsEmpty());
+}
+
+}  // namespace
+}  // namespace favicon