[LargeIconService] Allow decoding of images in the service

components/favicon/core/large_icon_service_unittest.cc

Index: components/favicon/core/large_icon_service_unittest.cc
diff --git a/components/favicon/core/large_icon_service_unittest.cc b/components/favicon/core/large_icon_service_unittest.cc
index fd9cfa4df6bd53efd5ccef0cea22f9f95c75f7b6..134fc2078ce26742fe4fcf565850cbd7f2ad19fa 100644
--- a/components/favicon/core/large_icon_service_unittest.cc
+++ b/components/favicon/core/large_icon_service_unittest.cc
@@ -20,6 +20,7 @@
 #include "components/favicon/core/test/mock_favicon_service.h"
 #include "components/favicon_base/fallback_icon_style.h"
 #include "components/favicon_base/favicon_types.h"
+#include "components/image_fetcher/core/image_decoder.h"
 #include "components/image_fetcher/core/image_fetcher.h"
 #include "components/image_fetcher/core/request_metadata.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -29,6 +30,7 @@
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
 namespace favicon {
@@ -97,6 +99,18 @@ class MockImageFetcher : public image_fetcher::ImageFetcher {
   MOCK_METHOD0(GetImageDecoder, image_fetcher::ImageDecoder*());
 };
 
+class FakeImageDecoder : public image_fetcher::ImageDecoder {
+ public:
+  void DecodeImage(
+      const std::string& image_data,
+      const gfx::Size& desired_image_frame_size,
+      const image_fetcher::ImageDecodedCallback& callback) override {
+    callback.Run(gfx::Image::CreateFrom1xPNGBytes(
+        reinterpret_cast<const unsigned char*>(image_data.data()),
+        image_data.size()));
+  }
+};
+
 class LargeIconServiceTest : public testing::Test {
  public:
   LargeIconServiceTest()
@@ -104,12 +118,14 @@ class LargeIconServiceTest : public testing::Test {
         large_icon_service_(&mock_favicon_service_,
                             base::ThreadTaskRunnerHandle::Get(),
                             base::WrapUnique(mock_image_fetcher_)),
-        is_callback_invoked_(false) {}
-
-  ~LargeIconServiceTest() override {
+        is_callback_invoked_(false) {
+    ON_CALL(*mock_image_fetcher_, GetImageDecoder())
+        .WillByDefault(Return(&fake_image_decoder_));
   }
 
-  void ResultCallback(const favicon_base::LargeIconResult& result) {
+  ~LargeIconServiceTest() override {}
+
+  void RawBitmapResultCallback(const favicon_base::LargeIconResult& result) {
     is_callback_invoked_ = true;
 
     // Checking presence and absence of results.
@@ -122,8 +138,25 @@ class LargeIconServiceTest : public testing::Test {
       // Not actually checking bitmap content.
     }
     if (expected_fallback_icon_style_.get()) {
-      EXPECT_EQ(*expected_fallback_icon_style_,
-                *result.fallback_icon_style);
+      EXPECT_EQ(*expected_fallback_icon_style_, *result.fallback_icon_style);
+    }
+  }
+
+  void ImageResultCallback(const favicon_base::LargeIconImageResult& result) {
+    is_callback_invoked_ = true;
+
+    // Checking presence and absence of results.
+    EXPECT_EQ(expected_bitmap_.is_valid(), !result.image.IsEmpty());
+    EXPECT_EQ(expected_fallback_icon_style_ != nullptr,
+              result.fallback_icon_style != nullptr);
+
+    if (expected_bitmap_.is_valid()) {
+      EXPECT_EQ(expected_bitmap_.pixel_size,
+                result.image.ToImageSkia()->size());
+      // Not actually checking bitmap content.
+    }
+    if (expected_fallback_icon_style_.get()) {
+      EXPECT_EQ(*expected_fallback_icon_style_, *result.fallback_icon_style);
     }
   }
 
@@ -139,6 +172,7 @@ class LargeIconServiceTest : public testing::Test {
   base::MessageLoopForIO loop_;
 
   NiceMock<MockImageFetcher>* mock_image_fetcher_;
+  FakeImageDecoder fake_image_decoder_;
   testing::NiceMock<MockFaviconService> mock_favicon_service_;
   LargeIconService large_icon_service_;
   base::CancelableTaskTracker cancelable_task_tracker_;
@@ -153,100 +187,215 @@ class LargeIconServiceTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(LargeIconServiceTest);
 };
 
-TEST_F(LargeIconServiceTest, SameSize) {
+TEST_F(LargeIconServiceTest, RawBitmapSameSize) {

[jkrcal, 2017/03/30 13:10:46]

I am duplicating most of the tests. I was thinking about parameterized tests for
this but I think it would hurt readability which is not worth for k=2. WDYT?

[pkotwicz, 2017/03/30 19:14:23]

I don't think that duplicating the tests is worth it. Since
GetLargeIconImageOrFallbackStyle() uses GetLargeIconOrFallbackStyle(), in my
opinion you just need to test GetLargeIconOrFallbackStyle()

[jkrcal, 2017/03/31 12:31:11]

In the current version, the new function is not a wrapper anymore, so I think
the tests are worth it, in the end.

   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 24, kTestColor));
   expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl),
       24,  // |min_source_size_in_pixel|
       24,  // |desired_size_in_pixel|
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, ImageSameSize) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 24, kTestColor));
+  expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl),
+      24,  // |min_source_size_in_pixel|
+      24,  // |desired_size_in_pixel|
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
-TEST_F(LargeIconServiceTest, ScaleDown) {
+TEST_F(LargeIconServiceTest, RawBitmapScaleDown) {
   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(32, 32, kTestColor));
   expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 24, 24,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
-TEST_F(LargeIconServiceTest, ScaleUp) {
+TEST_F(LargeIconServiceTest, ImageScaleDown) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(32, 32, kTestColor));
+  expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 24, 24,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, RawBitmapScaleUp) {
   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(16, 16, kTestColor));
   expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl),
       14,  // Lowered requirement so stored bitmap is admitted.
       24,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, ImageScaleUp) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(16, 16, kTestColor));
+  expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl),
+      14,  // Lowered requirement so stored bitmap is admitted.
+      24,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
 // |desired_size_in_pixel| == 0 means retrieve original image without scaling.
-TEST_F(LargeIconServiceTest, NoScale) {
+TEST_F(LargeIconServiceTest, RawBitmapNoScale) {
   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 24, kTestColor));
   expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 16, 0,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+// |desired_size_in_pixel| == 0 means retrieve original image without scaling.
+TEST_F(LargeIconServiceTest, ImageNoScale) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 24, kTestColor));
+  expected_bitmap_ = CreateTestBitmapResult(24, 24, kTestColor);
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 16, 0,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
-TEST_F(LargeIconServiceTest, FallbackSinceIconTooSmall) {
+TEST_F(LargeIconServiceTest, RawBitmapFallbackSinceIconTooSmall) {
   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(16, 16, kTestColor));
   expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
   expected_fallback_icon_style_->background_color = kTestColor;
   expected_fallback_icon_style_->is_default_background_color = false;
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 24, 24,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, ImageFallbackSinceIconTooSmall) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(16, 16, kTestColor));
+  expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
+  expected_fallback_icon_style_->background_color = kTestColor;
+  expected_fallback_icon_style_->is_default_background_color = false;
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 24, 24,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
-TEST_F(LargeIconServiceTest, FallbackSinceIconNotSquare) {
+TEST_F(LargeIconServiceTest, RawBitmapFallbackSinceIconNotSquare) {
   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 32, kTestColor));
   expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
   expected_fallback_icon_style_->background_color = kTestColor;
   expected_fallback_icon_style_->is_default_background_color = false;
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 24, 24,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
-TEST_F(LargeIconServiceTest, FallbackSinceIconMissing) {
+TEST_F(LargeIconServiceTest, ImageFallbackSinceIconNotSquare) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 32, kTestColor));
+  expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
+  expected_fallback_icon_style_->background_color = kTestColor;
+  expected_fallback_icon_style_->is_default_background_color = false;
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 24, 24,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, RawBitmapFallbackSinceIconMissing) {
   InjectMockResult(GURL(kDummyUrl), favicon_base::FaviconRawBitmapResult());
   // Expect default fallback style, including background.
   expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 24, 24,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, ImageFallbackSinceIconMissing) {
+  InjectMockResult(GURL(kDummyUrl), favicon_base::FaviconRawBitmapResult());
+  // Expect default fallback style, including background.
+  expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 24, 24,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
 }
 
-TEST_F(LargeIconServiceTest, FallbackSinceIconMissingNoScale) {
+TEST_F(LargeIconServiceTest, RawBitmapFallbackSinceIconMissingNoScale) {
   InjectMockResult(GURL(kDummyUrl), favicon_base::FaviconRawBitmapResult());
   // Expect default fallback style, including background.
   expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 24, 0,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, ImageFallbackSinceIconMissingNoScale) {
+  InjectMockResult(GURL(kDummyUrl), favicon_base::FaviconRawBitmapResult());
+  // Expect default fallback style, including background.
+  expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 24, 0,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);
@@ -254,14 +403,29 @@ TEST_F(LargeIconServiceTest, FallbackSinceIconMissingNoScale) {
 
 // Oddball case where we demand a high resolution icon to scale down. Generates
 // fallback even though an icon with the final size is available.
-TEST_F(LargeIconServiceTest, FallbackSinceTooPicky) {
+TEST_F(LargeIconServiceTest, RawBitmapFallbackSinceTooPicky) {
   InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 24, kTestColor));
   expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
   expected_fallback_icon_style_->background_color = kTestColor;
   expected_fallback_icon_style_->is_default_background_color = false;
   large_icon_service_.GetLargeIconOrFallbackStyle(
       GURL(kDummyUrl), 32, 24,
-      base::Bind(&LargeIconServiceTest::ResultCallback, base::Unretained(this)),
+      base::Bind(&LargeIconServiceTest::RawBitmapResultCallback,
+                 base::Unretained(this)),
+      &cancelable_task_tracker_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(is_callback_invoked_);
+}
+
+TEST_F(LargeIconServiceTest, ImageFallbackSinceTooPicky) {
+  InjectMockResult(GURL(kDummyUrl), CreateTestBitmapResult(24, 24, kTestColor));
+  expected_fallback_icon_style_.reset(new favicon_base::FallbackIconStyle);
+  expected_fallback_icon_style_->background_color = kTestColor;
+  expected_fallback_icon_style_->is_default_background_color = false;
+  large_icon_service_.GetLargeIconImageOrFallbackStyle(
+      GURL(kDummyUrl), 32, 24,
+      base::Bind(&LargeIconServiceTest::ImageResultCallback,
+                 base::Unretained(this)),
       &cancelable_task_tracker_);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(is_callback_invoked_);