Add baked-in favicons for default popular sites on NTP

components/ntp_tiles/icon_cacher_impl_unittest.cc

 // Copyright 2016 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/ntp_tiles/icon_cacher_impl.h"
 
 #include <utility>
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/favicon/core/favicon_client.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon/core/favicon_util.h"
 #include "components/history/core/browser/history_database_params.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image_unittest_util.h"
 
 using ::testing::_;
 using ::testing::InSequence;
 using ::testing::MockFunction;
 using ::testing::Return;
 
 namespace ntp_tiles {
 namespace {
 
 class MockImageFetcher : public image_fetcher::ImageFetcher {
  public:
   MOCK_METHOD1(SetImageFetcherDelegate,
                void(image_fetcher::ImageFetcherDelegate* delegate));
   MOCK_METHOD1(SetDataUseServiceName,
                void(image_fetcher::ImageFetcher::DataUseServiceName name));
   MOCK_METHOD3(StartOrQueueNetworkRequest,
                void(const std::string& id,
                     const GURL& image_url,
                     base::Callback<void(const std::string& id,
                                         const gfx::Image& image)> callback));
 };
 
+// This class provides methods to inject an image resource where a real resource
+// would be necessary otherwise. All other methods have return values that allow
+// the normal implementation to proceed.
+class MockResourceDelegate : public ui::ResourceBundle::Delegate {

[sfiera, 2017/02/16 18:54:16]

This class is (mostly) not a mock—please don't mix patterns. I think this class
would be a good candidate for a regular fake, with a std::map<int, gfx::Image>
used to implement GetImageNamed().

[fhorschig, 2017/02/17 16:24:03]

Done.

+ public:
+  ~MockResourceDelegate() override {}
+  base::FilePath GetPathForResourcePack(const base::FilePath& pack_path,
+                                        ui::ScaleFactor scale_factor) override {
+    return pack_path;  // Continue Loading pack file.
+  }
+
+  base::FilePath GetPathForLocalePack(const base::FilePath& pack_path,
+                                      const std::string& locale) override {
+    return pack_path;  // Continue Loading pack file.
+  }
+
+  MOCK_METHOD1(GetImageNamed, gfx::Image(int));
+
+  gfx::Image GetNativeImageNamed(int resource_id) override {
+    return gfx::Image();  // Attempt retrieval of the default resource.
+  }
+
+  base::RefCountedMemory* LoadDataResourceBytes(
+      int resource_id,
+      ui::ScaleFactor scale_factor) override {
+    return nullptr;  // Attempt retrieval of the default resource.
+  }
+
+  bool GetRawDataResource(int resource_id,
+                          ui::ScaleFactor scale_factor,
+                          base::StringPiece* value) override {
+    return false;  // Attempt retrieval of the default resource.
+  }
+
+  bool GetLocalizedString(int message_id, base::string16* value) override {
+    return false;  // Attempt retrieval of the default resource.
+  }
+};
+
 class IconCacherTest : public ::testing::Test {
  protected:
   IconCacherTest()
       : site_(base::string16(),  // title, unused
               GURL("http://url.google/"),
               GURL("http://url.google/icon.png"),
               GURL("http://url.google/favicon.ico"),
               GURL()),  // thumbnail, unused
         image_fetcher_(new ::testing::StrictMock<MockImageFetcher>),
         favicon_service_(/*favicon_client=*/nullptr, &history_service_) {
@@ skipping 134 matching lines @@
     EXPECT_CALL(done, Call(false)).WillOnce(Quit(&loop));
   }
 
   IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
   cacher.StartFetch(site_, BindMockFunction(&done));
   loop.Run();
   EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::FAVICON));
   EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
 }
 
+TEST_F(IconCacherTest, ProvidesDefaultIconAndSucceedsWithFetching) {
+  if (ui::ResourceBundle::HasSharedInstance()) {

[sfiera, 2017/02/16 18:54:16]

I'd put this as setup for IconCacherTest. Even if it's not necessary for the
other tests, I like to start reading each test with the assumption of a clean
slate.

[fhorschig, 2017/02/17 16:24:03]

Done.

+    ui::ResourceBundle::CleanupSharedInstance();
+  }
+
+  // Returns true after the global resource loader instance has been created.
+  MockResourceDelegate mock_resource;
+  ui::ResourceBundle::InitSharedInstanceWithLocale(
+      "en-US", &mock_resource, ui::ResourceBundle::LOAD_COMMON_RESOURCES);
+  MockFunction<void(bool)> done;
+  base::RunLoop loop_for_static;
+  base::RunLoop loop_for_fetch;
+  {
+    InSequence s;
+    EXPECT_CALL(*image_fetcher_,
+                SetDataUseServiceName(
+                    data_use_measurement::DataUseUserData::NTP_TILES));
+    EXPECT_CALL(mock_resource, GetImageNamed(12345))
+        .WillOnce(Return(gfx::test::CreateImage(64, 64)));
+    EXPECT_CALL(done, Call(true)).WillOnce(Quit(&loop_for_static));
+    EXPECT_CALL(*image_fetcher_,
+                StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
+        .WillOnce(PassFetch(128, 128));
+    EXPECT_CALL(done, Call(true)).WillOnce(Quit(&loop_for_fetch));
+  }
+
+  IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
+  site_.default_resource_id = 12345;
+  cacher.StartFetch(site_, BindMockFunction(&done));
+
+  loop_for_static.Run();  // Wait for the default image.
+  EXPECT_TRUE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));

[sfiera, 2017/02/16 18:54:16]

Can you add a check for the size here, so that we can distinguish which version
we see here and below?

[fhorschig, 2017/02/17 16:24:03]

Done.

+
+  // Let the fetcher continue and wait for the second call of the callback.
+  loop_for_fetch.Run();  // Wait for the updated image.
+  EXPECT_TRUE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
+}
+
 }  // namespace
 }  // namespace ntp_tiles