ash: HiDPI user avatar for SessionController

ui/gfx/image/mojo/image_traits_unittest.cc

Index: ui/gfx/image/mojo/image_traits_unittest.cc
diff --git a/ui/gfx/image/mojo/image_traits_unittest.cc b/ui/gfx/image/mojo/image_traits_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f7832f1d97d2031c24dd3a38e65ba9ea2ab1c4d8
--- /dev/null
+++ b/ui/gfx/image/mojo/image_traits_unittest.cc
@@ -0,0 +1,194 @@
+// Copyright 2017 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 <vector>
+
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/geometry/size.h"
+#include "ui/gfx/image/image_skia_operations.h"
+#include "ui/gfx/image/image_skia_source.h"
+#include "ui/gfx/image/mojo/image_traits_test_service.mojom.h"
+#include "ui/gfx/skia_util.h"
+
+namespace gfx {
+
+namespace {
+
+// Returns true if all reps in |reps| are present in |image|.
+bool ContainsSameRepresentation(const std::vector<ImageSkiaRep>& reps,

[msw, 2017/03/23 20:47:52]

nit: look at gfx::test::ImageSkiaStructureMatches, maybe that'd be useful?

[xiyuan, 2017/03/24 05:45:31]

Acknowledged.

+                                const ImageSkia& image) {
+  for (const auto& rep : reps) {

[msw, 2017/03/23 20:47:52]

Should we check that image.image_reps().size() == reps.size()?
(I guess that wouldn't work for the null/empty reps that were omitted)

Similarly, I wonder if we should check that |image| doesn't contain additional
reps that are missing from |reps|, but that wouldn't be the case for null/empty
reps that were omitted.

Maybe we could just check that all other reps of |image| not in |reps| are
null/empty?

[xiyuan, 2017/03/24 05:45:27]

Done. Revised code to get image_reps() from |image|, strip out null/empty ones,
then compare the two list.

+    if (!gfx::BitmapsAreEqual(
+            rep.sk_bitmap(),
+            image.GetRepresentation(rep.scale()).sk_bitmap())) {

[msw, 2017/03/23 20:47:52]

Should we be checking HasRepresentation before calling GetRepresentation? (we
likely wouldn't want this to generate any reps that didn't exist before)

[xiyuan, 2017/03/24 05:45:24]

Good point. GetRepresentation should not be used. Code revised and should no
longer has the problem.

+      return false;
+    }
+  }
+  return true;
+}
+
+class TestImageSkiaSource : public ImageSkiaSource {

[msw, 2017/03/23 20:47:52]

nit: add a comment

[xiyuan, 2017/03/24 05:45:26]

Done.

+ public:
+  explicit TestImageSkiaSource(const gfx::Size& dip_size)
+      : dip_size_(dip_size) {}
+  ~TestImageSkiaSource() override = default;
+
+  // ImageSkiaSource:
+  ImageSkiaRep GetImageForScale(float scale) override {
+    return ImageSkiaRep(ScaleToCeiledSize(dip_size_, scale), scale);
+  }
+
+ private:
+  const gfx::Size dip_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestImageSkiaSource);
+};
+
+class ImageTraitsTest : public testing::Test,

[msw, 2017/03/23 20:47:52]

See my earlier comment; hopefully the test service approach isn't needed.

[xiyuan, 2017/03/24 05:45:28]

Tried it and handles are not supported in Deserialize(Serialize()) API.

+                        public mojom::ImageTraitsTestService {
+ public:
+  ImageTraitsTest() = default;
+
+  // testing::Test:
+  void SetUp() override {
+    service_ = bindings_.CreateInterfacePtrAndBind(this);
+  }
+
+  mojom::ImageTraitsTestServicePtr& service() { return service_; }
+
+ private:
+  // mojom::ImageTraitsTestService:
+  void EchoImageSkiaRep(const ImageSkiaRep& in,
+                        const EchoImageSkiaRepCallback& callback) override {
+    callback.Run(in);
+  }
+  void EchoImageSkia(const ImageSkia& in,
+                     const EchoImageSkiaCallback& callback) override {
+    callback.Run(in);
+  }
+
+  base::MessageLoop loop_;
+  mojo::BindingSet<ImageTraitsTestService> bindings_;
+  mojom::ImageTraitsTestServicePtr service_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageTraitsTest);
+};
+
+}  // namespace
+
+TEST_F(ImageTraitsTest, NullImageSkiaRep) {
+  ImageSkiaRep null_rep;
+  ASSERT_TRUE(null_rep.is_null());
+
+  ImageSkiaRep output(gfx::Size(1, 1), 1.0f);
+  ASSERT_FALSE(output.is_null());
+  service()->EchoImageSkiaRep(null_rep, &output);
+  EXPECT_TRUE(output.is_null());
+}
+
+TEST_F(ImageTraitsTest, EmptyImageSkiaRepIsNull) {
+  SkBitmap empty_bitmap;
+  empty_bitmap.allocN32Pixels(0, 0);

[msw, 2017/03/23 20:47:52]

optional nit: check if the bitmap is null?

[xiyuan, 2017/03/24 05:45:31]

Done.

+  ImageSkiaRep image_rep(empty_bitmap, 1.0f);

[msw, 2017/03/23 20:47:52]

nit: after this, ASSERT_TRUE(image_rep.is_null()); optionally name |empty_rep|

[xiyuan, 2017/03/24 05:45:23]

This is the tricky part, might be a bug with current code. 

ImageSkiaRep with empty SkBitmap is not considered as null. is_null() returns
false. Thus it would be included in the reps returned from
ImageSkia::image_reps().

+
+  ImageSkiaRep output(gfx::Size(1, 1), 1.0f);
+  ASSERT_FALSE(output.is_null());
+  service()->EchoImageSkiaRep(image_rep, &output);
+  EXPECT_TRUE(output.is_null());
+}
+
+TEST_F(ImageTraitsTest, ImageSkiaRep) {
+  ImageSkiaRep image_rep(gfx::Size(2, 4), 2.0f);
+
+  ImageSkiaRep output;
+  service()->EchoImageSkiaRep(image_rep, &output);
+
+  EXPECT_FALSE(output.is_null());
+  EXPECT_EQ(image_rep.scale(), output.scale());
+  EXPECT_EQ(image_rep.pixel_size(), output.pixel_size());
+  EXPECT_EQ(image_rep.GetWidth(), output.GetWidth());
+  EXPECT_EQ(image_rep.GetHeight(), output.GetHeight());
+  EXPECT_TRUE(gfx::BitmapsAreEqual(image_rep.sk_bitmap(), output.sk_bitmap()));
+}
+
+TEST_F(ImageTraitsTest, NullImageSkia) {
+  ImageSkia null_image;
+  ASSERT_TRUE(null_image.isNull());
+
+  ImageSkia output(ImageSkiaRep(gfx::Size(1, 1), 1.0f));
+  ASSERT_FALSE(output.isNull());
+  service()->EchoImageSkia(null_image, &output);
+  EXPECT_TRUE(output.isNull());
+}
+
+TEST_F(ImageTraitsTest, ImageSkiaWithNoRepsTreatedAsNull) {

[msw, 2017/03/23 20:47:52]

Hmm, I wonder if this behavior will bite us later...

[xiyuan, 2017/03/24 05:45:30]

Possible. But there is not much we could do at this level. Only the caller of us
knows how to fill up the reps.

+  const gfx::Size kSize(1, 2);
+  ImageSkia image(new TestImageSkiaSource(kSize), kSize);
+  ASSERT_FALSE(image.isNull());
+
+  ImageSkia output(ImageSkiaRep(gfx::Size(1, 1), 1.0f));
+  ASSERT_FALSE(output.isNull());
+  service()->EchoImageSkia(image, &output);
+

[msw, 2017/03/23 20:47:52]

optional nit: remove blank line

[xiyuan, 2017/03/24 05:45:29]

Done.

+  EXPECT_TRUE(output.isNull());
+}
+
+TEST_F(ImageTraitsTest, ImageSkia) {
+  const gfx::Size kSize(1, 2);
+  ImageSkia image(new TestImageSkiaSource(kSize), kSize);
+  image.GetRepresentation(1.0f);
+  image.GetRepresentation(2.0f);
+
+  ImageSkia output;
+  service()->EchoImageSkia(image, &output);
+
+  EXPECT_FALSE(output.isNull());
+  const std::vector<ImageSkiaRep> output_reps = output.image_reps();
+  EXPECT_EQ(2u, output_reps.size());
+  EXPECT_TRUE(ContainsSameRepresentation(output_reps, image));
+}
+
+TEST_F(ImageTraitsTest, EmptyRepStripped) {
+  const gfx::Size kSize(1, 2);
+  ImageSkia image(new TestImageSkiaSource(kSize), kSize);
+  image.GetRepresentation(1.0f);
+
+  SkBitmap empty_bitmap;
+  empty_bitmap.allocN32Pixels(0, 0);
+  image.AddRepresentation(ImageSkiaRep(empty_bitmap, 2.0f));
+
+  ImageSkia output;
+  service()->EchoImageSkia(image, &output);
+
+  EXPECT_FALSE(output.isNull());
+  const std::vector<ImageSkiaRep> output_reps = output.image_reps();
+  EXPECT_EQ(1u, output_reps.size());
+  EXPECT_TRUE(ContainsSameRepresentation(output_reps, image));
+}
+
+TEST_F(ImageTraitsTest, ImageSkiaWithOperations) {
+  const gfx::Size kSize(32, 32);
+  ImageSkia image(new TestImageSkiaSource(kSize), kSize);
+
+  const gfx::Size kNewSize(16, 16);
+  ImageSkia resized = ImageSkiaOperations::CreateResizedImage(
+      image, skia::ImageOperations::RESIZE_BEST, kNewSize);
+  resized.GetRepresentation(1.0f);
+  resized.GetRepresentation(2.0f);
+
+  ImageSkia output;
+  service()->EchoImageSkia(resized, &output);
+
+  EXPECT_FALSE(output.isNull());
+  EXPECT_EQ(kNewSize, output.size());
+  const std::vector<ImageSkiaRep> output_reps = output.image_reps();
+  EXPECT_EQ(2u, output_reps.size());
+  EXPECT_TRUE(ContainsSameRepresentation(output_reps, resized));
+}
+
+}  // namespace gfx