Refactor extension app icon.

chrome/browser/extensions/extension_app_icon_unittest.cc

+// 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 <memory>
+
+#include "base/macros.h"
+#include "chrome/browser/extensions/extension_app_icon.h"
+#include "chrome/browser/extensions/extension_app_icon_delegate.h"
+#include "chrome/browser/extensions/extension_app_icon_loader.h"
+#include "chrome/browser/extensions/extension_app_icon_service.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_service_test_base.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/app_icon_loader_delegate.h"
+#include "chrome/common/chrome_constants.h"
+#include "extensions/common/constants.h"
+#include "ui/app_list/app_list_item.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_test.h"
+#include "chrome/browser/ui/app_list/extension_app_model_builder.h"
+#include "chrome/browser/ui/app_list/search/extension_app_result.h"
+#include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
+#include "components/arc/test/fake_app_instance.h"
+#endif
+
+namespace extensions {
+
+namespace {
+
+constexpr char kTestAppId[] = "emfkafnhnpcmabnnkckkchdilgeoekbo";
+
+// Receives icon image updates from ExtensionAppIcon.
+class TestAppIcon : public ExtensionAppIconDelegate {
+ public:
+  TestAppIcon(content::BrowserContext* context,
+              const std::string& app_id,
+              int size) {
+    app_icon_ =
+        ExtensionAppIconService::Get(context)->CreateIcon(this, app_id, size);
+    DCHECK(app_icon_);
+  }
+
+  ~TestAppIcon() override = default;
+
+  int icon_update_count() const { return icon_update_count_; }
+  ExtensionAppIcon* app_icon() { return app_icon_.get(); }
+  const gfx::ImageSkia& image_skia() const { return app_icon_->image_skia(); }
+
+  void WaitIconUpdated() {
+    base::RunLoop run_loop;
+    icon_updated_callback_ = run_loop.QuitClosure();
+    run_loop.Run();
+    icon_updated_callback_.Reset();

[xiyuan, 2017/04/20 16:36:32]

nit: We can avoid this call by making |icon_updated_callback_| a
base::OnceClosure. And invoke it std::move(icon_updated_callback_).Run() would
auto reset it.

[khmel, 2017/04/20 17:46:18]

I also found base::ResetAndReturn approach. Would it look well here?

[xiyuan, 2017/04/20 17:52:46]

That works.

+  }
+
+ private:
+  void OnIconUpdated(ExtensionAppIcon* icon) override {
+    ++icon_update_count_;
+    if (!icon_updated_callback_.is_null())
+      icon_updated_callback_.Run();
+  }
+
+  std::unique_ptr<ExtensionAppIcon> app_icon_;
+
+  int icon_update_count_ = 0;
+
+  base::Closure icon_updated_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestAppIcon);
+};
+
+// Receives icon image updates from ExtensionAppIconLoader.
+class TestAppIconLoader : public AppIconLoaderDelegate {
+ public:
+  TestAppIconLoader() = default;
+  ~TestAppIconLoader() override = default;
+
+  // AppIconLoaderDelegate:
+  void OnAppImageUpdated(const std::string& app_id,
+                         const gfx::ImageSkia& image) override {
+    image_skia_ = image;
+  }
+
+  const gfx::ImageSkia& image_skia() { return image_skia_; }
+
+ private:
+  gfx::ImageSkia image_skia_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestAppIconLoader);
+};
+
+// Returns true if provided |image| consists from only empty pixels.
+bool IsBlankImage(const gfx::ImageSkia& image) {
+  if (!image.width() || !image.height())
+    return false;
+
+  const SkBitmap* bitmap = image.bitmap();
+  DCHECK(bitmap);
+  DCHECK_EQ(bitmap->width(), image.width());
+  DCHECK_EQ(bitmap->height(), image.height());
+
+  for (int x = 0; x < bitmap->width(); ++x) {
+    for (int y = 0; y < bitmap->height(); ++y) {
+      if (*bitmap->getAddr32(x, y))
+        return false;
+    }
+  }
+  return true;
+}
+
+// Returns true if provided |image| is grayscale.
+bool IsGrayscaleImage(const gfx::ImageSkia& image) {
+  const SkBitmap* bitmap = image.bitmap();
+  DCHECK(bitmap);
+  for (int x = 0; x < bitmap->width(); ++x) {
+    for (int y = 0; y < bitmap->height(); ++y) {
+      const unsigned int pixel = *bitmap->getAddr32(x, y);
+      if ((pixel & 0xff) != ((pixel >> 8) & 0xff) ||
+          (pixel & 0xff) != ((pixel >> 16) & 0xff)) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+// Returns true if provided |image1| and |image2| are equal.
+bool AreEqual(const gfx::ImageSkia& image1, const gfx::ImageSkia& image2) {

[xiyuan, 2017/04/20 16:36:32]

You can use gfx::test::AreImagesEqual for this.

e.g.
https://cs.chromium.org/chromium/src/ash/system/user/tray_user_unittest.cc?rc...

[khmel, 2017/04/20 17:46:18]

Done.

+  const SkBitmap* bitmap1 = image1.bitmap();
+  DCHECK(bitmap1);
+  const SkBitmap* bitmap2 = image2.bitmap();
+  DCHECK(bitmap2);
+
+  if (bitmap1->width() != bitmap2->width() ||
+      bitmap1->height() != bitmap2->height()) {
+    return false;
+  }
+
+  for (int x = 0; x < bitmap1->width(); ++x) {
+    for (int y = 0; y < bitmap2->height(); ++y) {
+      if (*bitmap1->getAddr32(x, y) != *bitmap2->getAddr32(x, y))
+        return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
+class ExtensionAppIconTest : public ExtensionServiceTestBase {
+ public:
+  ExtensionAppIconTest() = default;
+  ~ExtensionAppIconTest() override = default;
+
+  // ExtensionServiceTestBase:
+  void SetUp() override {
+    ExtensionServiceTestBase::SetUp();
+
+    const base::FilePath source_install_dir =
+        data_dir().AppendASCII("app_list").AppendASCII("Extensions");
+    const base::FilePath pref_path =
+        source_install_dir.DirName().Append(chrome::kPreferencesFilename);
+    InitializeInstalledExtensionService(pref_path, source_install_dir);
+    service_->Init();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ExtensionAppIconTest);
+};
+
+TEST_F(ExtensionAppIconTest, IconLifeCycle) {
+  TestAppIcon reference_icon(profile(), kTestAppId,
+                             extension_misc::EXTENSION_ICON_MEDIUM);
+  EXPECT_EQ(1, reference_icon.icon_update_count());
+  // By default no representation in image.
+  EXPECT_FALSE(reference_icon.image_skia().HasRepresentation(1.0f));
+
+  // Accessing to bitmap requests default representations.
+  // Defualt blank image must be provided without an update.
+  EXPECT_TRUE(reference_icon.image_skia().bitmap());

[xiyuan, 2017/04/20 16:36:32]

nit: avoid bitmap() since it is going way. Use GetRepresentation(1.0f) instead.

[khmel, 2017/04/20 17:46:18]

Done.

+  EXPECT_EQ(1, reference_icon.icon_update_count());
+  EXPECT_TRUE(reference_icon.image_skia().HasRepresentation(1.0f));
+  EXPECT_EQ(extension_misc::EXTENSION_ICON_MEDIUM,
+            reference_icon.image_skia().width());
+  EXPECT_EQ(extension_misc::EXTENSION_ICON_MEDIUM,
+            reference_icon.image_skia().height());
+  EXPECT_TRUE(IsBlankImage(reference_icon.image_skia()));
+
+  // Wait until real image is loaded.
+  reference_icon.WaitIconUpdated();
+  EXPECT_EQ(2, reference_icon.icon_update_count());
+  EXPECT_FALSE(IsBlankImage(reference_icon.image_skia()));
+  EXPECT_FALSE(IsGrayscaleImage(reference_icon.image_skia()));
+
+  const gfx::ImageSkia image_before_disable = reference_icon.image_skia();
+  // Disable extension. This should update icon and provide grayed image inline.
+  // Note update might be sent twice in CrOS.
+  service()->DisableExtension(kTestAppId, Extension::DISABLE_CORRUPTED);
+  const int update_count_after_disable = reference_icon.icon_update_count();
+  EXPECT_NE(2, update_count_after_disable);
+  EXPECT_FALSE(IsBlankImage(reference_icon.image_skia()));
+  EXPECT_TRUE(IsGrayscaleImage(reference_icon.image_skia()));
+
+  // Reenable extension. It should match previous enabled image
+  service()->EnableExtension(kTestAppId);
+  EXPECT_NE(update_count_after_disable, reference_icon.icon_update_count());
+  EXPECT_TRUE(AreEqual(reference_icon.image_skia(), image_before_disable));
+}
+
+#if defined(OS_CHROMEOS)
+class ExtensionAppIconWithModelTest : public ExtensionAppIconTest {
+ public:
+  ExtensionAppIconWithModelTest() = default;
+  ~ExtensionAppIconWithModelTest() override = default;
+
+ protected:
+  void CreateBuilder() {
+    model_.reset(new app_list::AppListModel);
+    controller_.reset(new test::TestAppListControllerDelegate);
+    builder_.reset(new ExtensionAppModelBuilder(controller_.get()));
+    builder_->InitializeWithProfile(profile(), model_.get());
+  }
+
+  void ResetBuilder() {
+    builder_.reset();
+    controller_.reset();
+    model_.reset();
+  }
+
+  app_list::AppListItem* FindAppListItem(const std::string& app_id) {
+    return model_->FindItem(app_id);
+  }
+
+  test::TestAppListControllerDelegate* app_list_controller() {
+    return controller_.get();
+  }
+
+ private:
+  std::unique_ptr<app_list::AppListModel> model_;
+  std::unique_ptr<test::TestAppListControllerDelegate> controller_;
+  std::unique_ptr<ExtensionAppModelBuilder> builder_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionAppIconWithModelTest);
+};
+
+// Validates that icons are the same for different consumers.
+TEST_F(ExtensionAppIconWithModelTest, IconsTheSame) {
+  CreateBuilder();
+
+  TestAppIcon reference_icon(profile(), kTestAppId,
+                             extension_misc::EXTENSION_ICON_MEDIUM);
+
+  // Wait until reference data is loaded.
+  EXPECT_TRUE(reference_icon.image_skia().bitmap());

[xiyuan, 2017/04/20 16:36:31]

nit: GetRepresentation(1.0f)

[khmel, 2017/04/20 17:46:18]

Done.

+  while (IsBlankImage(reference_icon.image_skia()))

[xiyuan, 2017/04/20 16:36:31]

Why using a while loop instead of WaitIconUpdated ?

[khmel, 2017/04/20 17:46:18]

Hmmm...., you are right.

+    base::RunLoop().RunUntilIdle();
+
+  app_list::ExtensionAppResult search(profile(), kTestAppId,
+                                      app_list_controller(), true);
+  while (!AreEqual(reference_icon.image_skia(), search.icon()))
+    base::RunLoop().RunUntilIdle();
+
+  app_list::AppListItem* app_list_item = FindAppListItem(kTestAppId);
+  ASSERT_TRUE(app_list_item);
+  while (!AreEqual(reference_icon.image_skia(), app_list_item->icon()))
+    base::RunLoop().RunUntilIdle();
+
+  TestAppIconLoader loader_delegate;
+  ExtensionAppIconLoader loader(
+      profile(), extension_misc::EXTENSION_ICON_MEDIUM, &loader_delegate);
+  loader.FetchImage(kTestAppId);
+  while (!AreEqual(reference_icon.image_skia(), loader_delegate.image_skia()))
+    base::RunLoop().RunUntilIdle();
+
+  ResetBuilder();
+}
+
+TEST_F(ExtensionAppIconWithModelTest, ChromeBadging) {
+  ArcAppTest arc_test;
+  arc_test.SetUp(profile());
+
+  TestAppIcon reference_icon(profile(), kTestAppId,
+                             extension_misc::EXTENSION_ICON_MEDIUM);
+  // Wait until reference data is loaded.
+  EXPECT_TRUE(reference_icon.image_skia().bitmap());

[xiyuan, 2017/04/20 16:36:31]

nit: GetRepresentation(1.0f)

[khmel, 2017/04/20 17:46:18]

Done.

+  while (IsBlankImage(reference_icon.image_skia()))
+    base::RunLoop().RunUntilIdle();
+
+  const int icon_update_count_before_badging =
+      reference_icon.icon_update_count();
+  const gfx::ImageSkia image_before_badging = reference_icon.image_skia();
+
+  // Badging should be applied once package is installed.
+  arc_test.app_instance()->RefreshAppList();
+  std::vector<arc::mojom::AppInfo> fake_apps = arc_test.fake_apps();
+  fake_apps[0].package_name = arc_test.fake_packages()[0].package_name;
+  arc_test.app_instance()->SendRefreshAppList(fake_apps);
+  arc_test.app_instance()->SendRefreshPackageList(arc_test.fake_packages());
+  EXPECT_EQ(icon_update_count_before_badging + 1,

[xiyuan, 2017/04/20 16:36:32]

nit: Other tests are using a GetCountAndReset that returns the current count and
resets it to 0 on return so that we can EXPECT_EQ on delta.

[khmel, 2017/04/20 17:46:18]

Done.

+            reference_icon.icon_update_count());
+  EXPECT_FALSE(AreEqual(reference_icon.image_skia(), image_before_badging));
+
+  // Opts out the Play Store. Badge should be gone and icon image is the same
+  // as it was before badging.
+  arc::SetArcPlayStoreEnabledForProfile(profile(), false);
+  EXPECT_EQ(icon_update_count_before_badging + 2,
+            reference_icon.icon_update_count());
+  EXPECT_TRUE(AreEqual(reference_icon.image_skia(), image_before_badging));
+}
+#endif
+
+}  // namespace extensions