Split FaviconService and FaviconServiceImpl.

ios/chrome/app/spotlight/spotlight_manager_unittest.mm

Index: ios/chrome/app/spotlight/spotlight_manager_unittest.mm
diff --git a/ios/chrome/app/spotlight/spotlight_manager_unittest.mm b/ios/chrome/app/spotlight/spotlight_manager_unittest.mm
index dc5ffc86dcaaa16026ccc3a5fed631e0815574cc..3a4a96179fa54e07e2bce1140358840015b8a8d7 100644
--- a/ios/chrome/app/spotlight/spotlight_manager_unittest.mm
+++ b/ios/chrome/app/spotlight/spotlight_manager_unittest.mm
@@ -16,9 +16,8 @@
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/test/bookmark_test_helpers.h"
 #include "components/bookmarks/test/test_bookmark_client.h"
-#include "components/favicon/core/favicon_client.h"
-#include "components/favicon/core/favicon_service.h"
 #include "components/favicon/core/large_icon_service.h"
+#include "components/favicon/core/test/mock_favicon_service.h"
 #include "components/favicon_base/fallback_icon_style.h"
 #import "ios/chrome/app/spotlight/bookmarks_spotlight_manager.h"
 #import "ios/chrome/app/spotlight/spotlight_manager.h"
@@ -27,6 +26,7 @@
 #include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
 #include "ios/public/provider/chrome/browser/spotlight/spotlight_provider.h"
 #import "net/base/mac/url_conversions.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gtest_mac.h"
 
@@ -34,6 +34,10 @@
 #error "This file requires ARC support."
 #endif
 
+using favicon::PostCallbackWithResult;
+using testing::WithArg;
+using testing::_;
+
 const char kDummyIconUrl[] = "http://www.example.com/touch_icon.png";
 
 favicon_base::FaviconRawBitmapResult CreateTestBitmap(int w, int h) {
@@ -59,59 +63,25 @@ favicon_base::FaviconRawBitmapResult CreateTestBitmap(int w, int h) {
   return result;
 }
 
-// A mock FaviconService that emits pre-programmed response.
-class MockFaviconService : public favicon::FaviconService {
- public:
-  MockFaviconService() : FaviconService(nullptr, nullptr) {}
-
-  ~MockFaviconService() override {}
-
-  base::CancelableTaskTracker::TaskId GetLargestRawFaviconForPageURL(
-      const GURL& page_url,
-      const std::vector<int>& icon_types,
-      int minimum_size_in_pixels,
-      const favicon_base::FaviconRawBitmapCallback& callback,
-      base::CancelableTaskTracker* tracker) override {
-    favicon_base::FaviconRawBitmapResult mock_result = CreateTestBitmap(24, 24);
-    return tracker->PostTask(base::ThreadTaskRunnerHandle::Get().get(),
-                             FROM_HERE, base::Bind(callback, mock_result));
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockFaviconService);
-};
-
-// This class provides access to LargeIconService internals, using the current
-// thread's task runner for testing.
-class TestLargeIconService : public favicon::LargeIconService {
- public:
-  explicit TestLargeIconService(MockFaviconService* mock_favicon_service)
-      : LargeIconService(mock_favicon_service,
-                         base::ThreadTaskRunnerHandle::Get()) {}
-  ~TestLargeIconService() override {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestLargeIconService);
-};
-
 class SpotlightManagerTest : public testing::Test {
  protected:
   SpotlightManagerTest() {
-    mock_favicon_service_.reset(new MockFaviconService());
-    large_icon_service_.reset(
-        new TestLargeIconService(mock_favicon_service_.get()));
     model_ = bookmarks::TestBookmarkClient::CreateModel();
-    mock_favicon_service_.reset(new MockFaviconService());
-    large_icon_service_.reset(
-        new TestLargeIconService(mock_favicon_service_.get()));
+    large_icon_service_.reset(new favicon::LargeIconService(
+        &mock_favicon_service_, base::ThreadTaskRunnerHandle::Get()));
     bookmarksSpotlightManager_ = [[BookmarksSpotlightManager alloc]
         initWithLargeIconService:large_icon_service_.get()
                    bookmarkModel:model_.get()];
+
+    EXPECT_CALL(mock_favicon_service_,
+                GetLargestRawFaviconForPageURL(_, _, _, _, _))
+        .WillRepeatedly(
+            WithArg<3>(PostCallbackWithResult(CreateTestBitmap(24, 24))));

[pkotwicz, 2017/02/16 04:07:27]

This is ok because there aren't a lot of places where we implement
FaviconService methods via the EXPECT_CALL() macro. However, I wouldn't want to
implement FaviconService via the EXPECT_CALL macro in many dozens of places.

[mastiz, 2017/02/16 09:46:25]

The upside is, only a single method can be implemented. It also allows tests to
optionally specify parameter expectations, as it is the case today in
favicon_view_provider_unittest.mm, which wants to verify that the passed in URL
is kTestFaviconURL.

   }
 
   base::MessageLoop loop_;
-  std::unique_ptr<MockFaviconService> mock_favicon_service_;
-  std::unique_ptr<TestLargeIconService> large_icon_service_;
+  testing::StrictMock<favicon::MockFaviconService> mock_favicon_service_;
+  std::unique_ptr<favicon::LargeIconService> large_icon_service_;
   base::CancelableTaskTracker cancelable_task_tracker_;
   std::unique_ptr<bookmarks::BookmarkModel> model_;
   BookmarksSpotlightManager* bookmarksSpotlightManager_;