Have AppService re-use ArcAppIcon code

The AppService (and specifically app_service/arc_apps.cc) currently uses
the ARC_GET_INSTANCE_FOR_METHOD macro directly, which is really a
private implementation detail of the ArcAppListPrefs class and its
friends, such as ArcAppIcon and ArcAppIconLoader.

A follow-up CL, adding ArcAppIcon support for providing compressed
icons, will let us delete the ARC_GET_INSTANCE_FOR_METHOD macros in
app_service/arc_apps.cc.

BUG=826982

Change-Id: Ifc5a88deea2693c2fe4c6c1ae3fe0b33661dbee3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758024Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689728}

chrome/browser/BUILD.gn

@@ -3568,6 +3568,8 @@ jumbo_split_static_library("browser") {
       "apps/app_service/arc_apps.h",
       "apps/app_service/arc_apps_factory.cc",
       "apps/app_service/arc_apps_factory.h",
+      "apps/app_service/arc_icon_once_loader.cc",
+      "apps/app_service/arc_icon_once_loader.h",
       "apps/app_service/built_in_chromeos_apps.cc",
       "apps/app_service/built_in_chromeos_apps.h",
       "apps/app_service/crostini_apps.cc",

chrome/browser/apps/app_service/app_icon_factory.cc

@@ -41,30 +41,6 @@
 
 namespace {
 
-void ApplyIconEffects(apps::IconEffects icon_effects,
-                      int size_hint_in_dip,
-                      gfx::ImageSkia* image_skia) {
-  extensions::ChromeAppIcon::ResizeFunction resize_function;
-#if defined(OS_CHROMEOS)
-  if (icon_effects & apps::IconEffects::kResizeAndPad) {
-    // TODO(crbug.com/826982): khmel@ notes that MD post-processing is not
-    // always applied: "See legacy code:
-    // https://cs.chromium.org/search/?q=ChromeAppIconLoader&type=cs In one
-    // cases MD design is used in another not."
-    resize_function =
-        base::BindRepeating(&app_list::MaybeResizeAndPadIconForMd);
-  }
-#endif
-
-  bool apply_chrome_badge = icon_effects & apps::IconEffects::kBadge;
-  bool app_launchable = !(icon_effects & apps::IconEffects::kGray);
-  bool from_bookmark = icon_effects & apps::IconEffects::kRoundCorners;
-
-  extensions::ChromeAppIcon::ApplyEffects(size_hint_in_dip, resize_function,
-                                          apply_chrome_badge, app_launchable,
-                                          from_bookmark, image_skia);
-}
-
 std::vector<uint8_t> ReadFileAsCompressedData(const base::FilePath path) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
@@ -217,7 +193,7 @@ void RunCallbackWithImageSkia(int size_hint_in_dip,
     // an uncompressed icon, return the uncompressed result; otherwise, encode
     // the icon to a compressed icon, return the compressed result.
     if (icon_effects) {
-      ApplyIconEffects(icon_effects, size_hint_in_dip, &processed_image);
+      apps::ApplyIconEffects(icon_effects, size_hint_in_dip, &processed_image);
     }
 
     if (icon_compression == apps::mojom::IconCompression::kUncompressed) {
@@ -315,6 +291,30 @@ void RunCallbackWithFallback(
 
 namespace apps {
 
+void ApplyIconEffects(IconEffects icon_effects,
+                      int size_hint_in_dip,
+                      gfx::ImageSkia* image_skia) {
+  extensions::ChromeAppIcon::ResizeFunction resize_function;
+#if defined(OS_CHROMEOS)
+  if (icon_effects & IconEffects::kResizeAndPad) {
+    // TODO(crbug.com/826982): MD post-processing is not always applied: "See
+    // legacy code:
+    // https://cs.chromium.org/search/?q=ChromeAppIconLoader&type=cs In one
+    // cases MD design is used in another not."
+    resize_function =
+        base::BindRepeating(&app_list::MaybeResizeAndPadIconForMd);
+  }
+#endif
+
+  const bool apply_chrome_badge = icon_effects & IconEffects::kBadge;
+  const bool app_launchable = !(icon_effects & IconEffects::kGray);
+  const bool from_bookmark = icon_effects & IconEffects::kRoundCorners;
+
+  extensions::ChromeAppIcon::ApplyEffects(size_hint_in_dip, resize_function,
+                                          apply_chrome_badge, app_launchable,
+                                          from_bookmark, image_skia);
+}
+
 void LoadIconFromExtension(apps::mojom::IconCompression icon_compression,
                            int size_hint_in_dip,
                            content::BrowserContext* context,

chrome/browser/apps/app_service/app_icon_factory.h

@@ -35,6 +35,13 @@ enum IconEffects : uint32_t {
   kRoundCorners = 0x08,  // Bookmark apps get round corners.
 };
 
+// Modifies |image_skia| to apply icon post-processing effects like badging and
+// desaturation to gray.
+void ApplyIconEffects(IconEffects icon_effects,
+                      int size_hint_in_dip,
+                      gfx::ImageSkia* image_skia);
+
+// Loads an icon from an extension.
 void LoadIconFromExtension(apps::mojom::IconCompression icon_compression,
                            int size_hint_in_dip,
                            content::BrowserContext* context,
@@ -42,6 +49,8 @@ void LoadIconFromExtension(apps::mojom::IconCompression icon_compression,
                            IconEffects icon_effects,
                            apps::mojom::Publisher::LoadIconCallback callback);
 
+// Loads an icon from a FilePath. If that fails, it calls the fallback.
+//
 // The file named by |path| might be empty, not found or otherwise unreadable.
 // If so, "fallback(callback)" is run. If the file is non-empty and readable,
 // just "callback" is run, even if that file doesn't contain a valid image.
@@ -54,6 +63,8 @@ void LoadIconFromFileWithFallback(
     base::OnceCallback<void(apps::mojom::Publisher::LoadIconCallback)>
         fallback);
 
+// Loads an icon from a compiled-into-the-binary resource, with a resource_id
+// named IDR_XXX, for some value of XXX.
 void LoadIconFromResource(apps::mojom::IconCompression icon_compression,
                           int size_hint_in_dip,
                           int resource_id,

chrome/browser/apps/app_service/arc_apps.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_dialog.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_icon.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_icon_descriptor.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
 #include "chrome/grit/component_extension_resources.h"
@@ -28,50 +29,45 @@
 #include "content/public/browser/system_connector.h"
 #include "extensions/grit/extensions_browser_resources.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
-#include "services/data_decoder/public/cpp/decode_image.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/geometry/size.h"
 
-// TODO(crbug.com/826982): investigate re-using the ArcAppIcon class directly.
-// This may or may not be difficult: see
-// (https://chromium-review.googlesource.com/c/chromium/src/+/1482350/7#message-b45fa253ea01b523e8389b30d74ce805b0e05f77)
-// and
-// (https://chromium-review.googlesource.com/c/chromium/src/+/1482350/7#message-52080b7d348d7806c818aa395392ff1385d1784e).
-
 // TODO(crbug.com/826982): consider that, per khmel@, "App icon can be
 // overwritten (setTaskDescription) or by assigning the icon for the app
 // window. In this case some consumers (Shelf for example) switch to
 // overwritten icon... IIRC this applies to shelf items and ArcAppWindow icon".
 
-// TODO(crbug.com/826982): consider that, per khmel@, "We may change the way
-// how we handle icons in ARC++ container. That means view of the icon can be
-// changed. We support invalidation of icon scales way, similar to the case
-// above... We have methods to notify about new icon arrival. IIRC ArcAppIcon
-// already handles this".
+namespace {
 
-// TODO(crbug.com/826982): consider that, per khmel@, "Functionality to detect
-// icons cannot be decoded correctly and issue request to refresh the icon
-// scale from ARC++ container... The logic here is wider. We request new copy
-// of icon in this case".
+void OnArcAppIconCompletelyLoaded(
+    int32_t size_hint_in_dip,
+    apps::IconEffects icon_effects,
+    apps::mojom::Publisher::LoadIconCallback callback,
+    ArcAppIcon* icon) {
+  if (!icon) {
+    std::move(callback).Run(apps::mojom::IconValue::New());
+    return;
+  }
 
-namespace {
+  apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
+  iv->icon_compression = apps::mojom::IconCompression::kUncompressed;
+  iv->uncompressed = icon->image_skia();
+  iv->is_placeholder_icon = false;
+
+  if (icon_effects != apps::IconEffects::kNone) {
+    apps::ApplyIconEffects(icon_effects, size_hint_in_dip, &iv->uncompressed);
+  }
+
+  std::move(callback).Run(std::move(iv));
+}
 
 // ArcApps::LoadIcon (via ArcApps::LoadIconFromVM) runs a series of callbacks,
 // defined here in back-to-front order so that e.g. the compiler knows
-// LoadIcon2's signature when compiling LoadIcon1 (which binds LoadIcon2).
+// LoadIcon1's signature when compiling LoadIcon0 (which binds LoadIcon1).
 //
 //  - LoadIcon0 is called back when the AppConnectionHolder is connected.
 //  - LoadIcon1 is called back when the compressed (PNG) image is loaded.
-//  - LoadIcon2 is called back when the uncompressed image is loaded.
-
-void LoadIcon2(apps::mojom::Publisher::LoadIconCallback callback,
-               const SkBitmap& bitmap) {
-  apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
-  iv->icon_compression = apps::mojom::IconCompression::kUncompressed;
-  iv->uncompressed = gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, 0.0f));
-  std::move(callback).Run(std::move(iv));
-}
 
 void LoadIcon1(apps::mojom::IconCompression icon_compression,
                apps::mojom::Publisher::LoadIconCallback callback,
@@ -82,17 +78,15 @@ void LoadIcon1(apps::mojom::IconCompression icon_compression,
       break;
 
     case apps::mojom::IconCompression::kUncompressed:
-      data_decoder::DecodeImage(
-          content::GetSystemConnector(), icon_png_data,
-          data_decoder::mojom::ImageCodec::DEFAULT, false,
-          data_decoder::kDefaultMaxSizeInBytes, gfx::Size(),
-          base::BindOnce(&LoadIcon2, std::move(callback)));
+      LOG(ERROR) << "unexpected ArcApps icon_compression";
+      std::move(callback).Run(apps::mojom::IconValue::New());
       break;
 
     case apps::mojom::IconCompression::kCompressed:
       apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
       iv->icon_compression = apps::mojom::IconCompression::kCompressed;
       iv->compressed = icon_png_data;
+      iv->is_placeholder_icon = false;
       std::move(callback).Run(std::move(iv));
       break;
   }
@@ -110,6 +104,10 @@ void LoadIcon0(apps::mojom::IconCompression icon_compression,
   // request to ARC++ container to extract the real data. This logic is
   // isolated inside ArcAppListPrefs and I don't think that anybody else should
   // call mojom RequestAppIcon".
+  //
+  // Note that this TODO will be obsolete when the LoadIcon0 code is deleted,
+  // which depends on ArcAppIcon being able to provide *compressed* icons
+  // (PNG-encoded bytes, not RGBA pixels).
   if (app_connection_holder) {
     if (icon_resource_id.empty()) {
       auto* app_instance =
@@ -170,7 +168,7 @@ ArcApps* ArcApps::CreateForTesting(Profile* profile,
 ArcApps::ArcApps(Profile* profile) : ArcApps(profile, nullptr) {}
 
 ArcApps::ArcApps(Profile* profile, apps::AppServiceProxy* proxy)
-    : binding_(this), profile_(profile) {
+    : binding_(this), profile_(profile), arc_icon_once_loader_(profile) {
   if (!arc::IsArcAllowedForProfile(profile_) ||
       (arc::ArcServiceManager::Get() == nullptr)) {
     return;
@@ -236,6 +234,7 @@ void ArcApps::Shutdown() {
       holder->RemoveObserver(this);
     }
     prefs->RemoveObserver(this);
+    arc_icon_once_loader_.StopObserving(prefs);
   }
 }
 
@@ -514,6 +513,21 @@ void ArcApps::LoadIconFromVM(const std::string app_id,
     return;
   }
 
+  // TODO(crbug.com/826982): drop the "kUncompressed only" condition, and
+  // always use the arc_icon_once_loader_, once the ArcAppIcon class supports
+  // providing *compressed* icons (i.e. PNG-encoded bytes, not RGBA pixels).
+  //
+  // Once that happens, we can delete the rest of this function, the LoadIcon0
+  // code, the pending_load_icon_calls_ mechanism and any mention in this file
+  // (or its .h) of arc::ConnectionHolder or arc::ConnectionObserver.
+  if (icon_compression == apps::mojom::IconCompression::kUncompressed) {
+    arc_icon_once_loader_.LoadIcon(
+        app_id, size_hint_in_dip,
+        base::BindOnce(&OnArcAppIconCompletelyLoaded, size_hint_in_dip,
+                       icon_effects, std::move(callback)));
+    return;
+  }
+
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_);
   if (prefs) {
     std::unique_ptr<ArcAppListPrefs::AppInfo> app_info = prefs->GetApp(app_id);

chrome/browser/apps/app_service/arc_apps.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/apps/app_service/app_icon_factory.h"
+#include "chrome/browser/apps/app_service/arc_icon_once_loader.h"
 #include "chrome/browser/apps/app_service/icon_key_util.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "chrome/services/app_service/public/mojom/app_service.mojom.h"
@@ -115,6 +116,7 @@ class ArcApps : public KeyedService,
   mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
 
   Profile* profile_;
+  ArcIconOnceLoader arc_icon_once_loader_;
 
   std::vector<base::OnceCallback<void(AppConnectionHolder*)>>
       pending_load_icon_calls_;

chrome/browser/apps/app_service/arc_icon_once_loader.cc

+// Copyright (c) 2019 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 <utility>
+
+#include "chrome/browser/apps/app_service/arc_icon_once_loader.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_icon.h"
+
+namespace apps {
+
+// A part of an ArcIconOnceLoader, for a specific size_in_dip. This two-level
+// structure (an ArcIconOnceLoader contains multiple SizeSpecificLoader
+// instances) is needed because each ArcAppIcon is for a specific size_in_dip.
+class ArcIconOnceLoader::SizeSpecificLoader : public ArcAppIcon::Observer {
+ public:
+  SizeSpecificLoader(Profile* profile, int32_t size_in_dip);
+  ~SizeSpecificLoader() override;
+
+  void LoadIcon(const std::string& app_id,
+                base::OnceCallback<void(ArcAppIcon*)> callback);
+  void Remove(const std::string& app_id);
+  void Reload(const std::string& app_id, ui::ScaleFactor scale_factor);
+
+  // ArcAppIcon::Observer overrides.
+  void OnIconUpdated(ArcAppIcon* icon) override;
+
+ private:
+  Profile* const profile_;
+  const int32_t size_in_dip_;
+
+  // Maps App IDs to their icon loaders (for a specific size in DIPs).
+  std::map<std::string, std::unique_ptr<ArcAppIcon>> icons_;
+
+  // Maps App IDs to callbacks to run when an icon is completely loaded.
+  std::multimap<std::string, base::OnceCallback<void(ArcAppIcon*)>> callbacks_;
+
+  DISALLOW_COPY_AND_ASSIGN(SizeSpecificLoader);
+};
+
+ArcIconOnceLoader::SizeSpecificLoader::SizeSpecificLoader(Profile* profile,
+                                                          int32_t size_in_dip)
+    : profile_(profile), size_in_dip_(size_in_dip) {}
+
+ArcIconOnceLoader::SizeSpecificLoader::~SizeSpecificLoader() {
+  for (auto& kv_pair : callbacks_) {
+    std::move(kv_pair.second).Run(nullptr);
+  }
+}
+
+void ArcIconOnceLoader::SizeSpecificLoader::LoadIcon(
+    const std::string& app_id,
+    base::OnceCallback<void(ArcAppIcon*)> callback) {
+  auto iter = icons_.find(app_id);
+  if ((iter != icons_.end()) &&
+      iter->second->EverySupportedScaleFactorIsLoaded()) {
+    std::move(callback).Run(iter->second.get());
+    return;
+  }
+
+  callbacks_.insert(std::make_pair(app_id, std::move(callback)));
+
+  if (iter != icons_.end()) {
+    return;
+  }
+  iter = icons_
+             .insert(std::make_pair(
+                 app_id, std::make_unique<ArcAppIcon>(profile_, app_id,
+                                                      size_in_dip_, this)))
+             .first;
+  iter->second->image_skia().EnsureRepsForSupportedScales();
+}
+
+void ArcIconOnceLoader::SizeSpecificLoader::Remove(const std::string& app_id) {
+  auto iter = icons_.find(app_id);
+  if (iter != icons_.end()) {
+    icons_.erase(iter);
+  }
+}
+
+void ArcIconOnceLoader::SizeSpecificLoader::Reload(
+    const std::string& app_id,
+    ui::ScaleFactor scale_factor) {
+  auto iter = icons_.find(app_id);
+  if (iter != icons_.end()) {
+    iter->second->LoadForScaleFactor(scale_factor);
+  }
+}
+
+void ArcIconOnceLoader::SizeSpecificLoader::OnIconUpdated(ArcAppIcon* icon) {
+  if (!icon || !icon->EverySupportedScaleFactorIsLoaded()) {
+    return;
+  }
+  auto range = callbacks_.equal_range(icon->app_id());
+  auto count = std::distance(range.first, range.second);
+  if (count <= 0) {
+    return;
+  }
+
+  // Optimize / simplify the common case.
+  if (count == 1) {
+    base::OnceCallback<void(ArcAppIcon*)> callback =
+        std::move(range.first->second);
+    callbacks_.erase(range.first, range.second);
+    std::move(callback).Run(icon);
+    return;
+  }
+
+  // Run every callback in |range|. This is subtle, because an arbitrary
+  // callback could invoke further methods on |this|, which could mutate
+  // |callbacks_|, invalidating |range|'s iterators.
+  //
+  // Thus, we first gather the callbacks, then erase the |range|, then run the
+  // callbacks.
+  std::vector<base::OnceCallback<void(ArcAppIcon*)>> callbacks_to_run;
+  callbacks_to_run.reserve(count);
+  for (auto iter = range.first; iter != range.second; ++iter) {
+    callbacks_to_run.push_back(std::move(iter->second));
+  }
+  callbacks_.erase(range.first, range.second);
+  for (auto& callback : callbacks_to_run) {
+    std::move(callback).Run(icon);
+  }
+}
+
+ArcIconOnceLoader::ArcIconOnceLoader(Profile* profile) : profile_(profile) {
+  ArcAppListPrefs::Get(profile)->AddObserver(this);
+}
+
+ArcIconOnceLoader::~ArcIconOnceLoader() = default;
+
+void ArcIconOnceLoader::StopObserving(ArcAppListPrefs* prefs) {
+  prefs->RemoveObserver(this);
+}
+
+void ArcIconOnceLoader::LoadIcon(
+    const std::string& app_id,
+    int32_t size_in_dip,
+    base::OnceCallback<void(ArcAppIcon*)> callback) {
+  auto iter = size_specific_loaders_.find(size_in_dip);
+  if (iter == size_specific_loaders_.end()) {
+    iter = size_specific_loaders_
+               .insert(std::make_pair(
+                   size_in_dip,
+                   std::make_unique<SizeSpecificLoader>(profile_, size_in_dip)))
+               .first;
+  }
+  iter->second->LoadIcon(app_id, std::move(callback));
+}
+
+void ArcIconOnceLoader::OnAppRemoved(const std::string& app_id) {
+  for (auto& iter : size_specific_loaders_) {
+    iter.second->Remove(app_id);
+  }
+}
+
+void ArcIconOnceLoader::OnAppIconUpdated(
+    const std::string& app_id,
+    const ArcAppIconDescriptor& descriptor) {
+  auto iter = size_specific_loaders_.find(descriptor.dip_size);
+  if (iter != size_specific_loaders_.end()) {
+    iter->second->Reload(app_id, descriptor.scale_factor);
+  }
+}
+
+}  // namespace apps

chrome/browser/apps/app_service/arc_icon_once_loader.h

+// Copyright (c) 2019 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.
+
+#ifndef CHROME_BROWSER_APPS_APP_SERVICE_ARC_ICON_ONCE_LOADER_H_
+#define CHROME_BROWSER_APPS_APP_SERVICE_ARC_ICON_ONCE_LOADER_H_
+
+#include <map>
+#include <memory>
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_icon_descriptor.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
+
+class ArcAppIcon;
+class Profile;
+
+namespace apps {
+
+// An icon loader specific to ARC++ apps, similar to the ArcAppIconLoader
+// class, except that it works with base::OnceCallback's (such as those used by
+// Mojo IPC). As the name implies, a base::OnceCallback can be run only once.
+// The ArcAppIconLoader class, like any AppIconLoader sub-class, assumes that
+// AppIconLoaderDelegate::OnAppImageUpdated can be called multiple times.
+//
+// Another minor difference is that this class works with multiple icon sizes.
+// Each AppIconLoader instance is for only one icon size.
+class ArcIconOnceLoader : public ArcAppListPrefs::Observer {
+ public:
+  explicit ArcIconOnceLoader(Profile* profile);
+  ~ArcIconOnceLoader() override;
+
+  void StopObserving(ArcAppListPrefs* prefs);
+
+  // Runs |callback| when the corresponding ARC++ icon is completely loaded,
+  // which may occur before LoadIcon returns, if that icon is cached.
+  //
+  // The callback may be run with a nullptr argument, if the icon could not be
+  // loaded.
+  void LoadIcon(const std::string& app_id,
+                int32_t size_in_dip,
+                base::OnceCallback<void(ArcAppIcon*)> callback);
+
+  // ArcAppListPrefs::Observer overrides.
+  void OnAppRemoved(const std::string& app_id) override;
+  void OnAppIconUpdated(const std::string& app_id,
+                        const ArcAppIconDescriptor& descriptor) override;
+
+ private:
+  class SizeSpecificLoader;
+
+  Profile* const profile_;
+  std::map<int32_t, std::unique_ptr<SizeSpecificLoader>> size_specific_loaders_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcIconOnceLoader);
+};
+
+}  // namespace apps
+
+#endif  // CHROME_BROWSER_APPS_APP_SERVICE_ARC_ICON_ONCE_LOADER_H_

chrome/browser/ui/app_list/arc/arc_app_icon.cc

@@ -91,6 +91,9 @@ struct ArcAppIcon::ReadResult {
 ////////////////////////////////////////////////////////////////////////////////
 // ArcAppIcon::Source
 
+// Initializes the ImageSkia with placeholder bitmaps, decoded from
+// compiled-into-the-binary resources such as IDR_APP_DEFAULT_ICON, and
+// schedules the asynchronous loading of the app's actual bitmaps.
 class ArcAppIcon::Source : public gfx::ImageSkiaSource {
  public:
   Source(const base::WeakPtr<ArcAppIcon>& host, int resource_size_in_dip);
@@ -256,11 +259,19 @@ ArcAppIcon::ArcAppIcon(content::BrowserContext* context,
                                          resource_size_in_dip);
   gfx::Size resource_size(resource_size_in_dip, resource_size_in_dip);
   image_skia_ = gfx::ImageSkia(std::move(source), resource_size);
+
+  const std::vector<ui::ScaleFactor>& scale_factors =
+      ui::GetSupportedScaleFactors();
+  incomplete_scale_factors_.insert(scale_factors.begin(), scale_factors.end());
 }
 
 ArcAppIcon::~ArcAppIcon() {
 }
 
+bool ArcAppIcon::EverySupportedScaleFactorIsLoaded() const {
+  return incomplete_scale_factors_.empty();
+}
+
 void ArcAppIcon::LoadForScaleFactor(ui::ScaleFactor scale_factor) {
   // We provide Play Store icon from Chrome resources and it is not expected
   // that we have external load request.
@@ -382,6 +393,8 @@ void ArcAppIcon::Update(ui::ScaleFactor scale_factor, const SkBitmap& bitmap) {
   image_skia_.AddRepresentation(image_rep);
   image_skia_.RemoveUnsupportedRepresentationsForScale(image_rep.scale());
 
+  incomplete_scale_factors_.erase(scale_factor);
+
   observer_->OnIconUpdated(this);
 }
 

chrome/browser/ui/app_list/arc/arc_app_icon.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_APP_LIST_ARC_ARC_APP_ICON_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -15,6 +16,10 @@
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 
+namespace apps {
+class ArcIconOnceLoader;
+}
+
 namespace base {
 class FilePath;
 }
@@ -26,6 +31,11 @@ class BrowserContext;
 // A class that provides an ImageSkia for UI code to use. It handles ARC app
 // icon resource loading, screen scale factor change etc. UI code that uses
 // ARC app icon should host this class.
+//
+// Icon images are sometimes subject to post-processing effects, such as
+// desaturating (graying out) disabled apps. Applying those effects are the
+// responsibility of code that uses this ArcAppIcon class, not the
+// responsibility of ArcAppIcon itself.
 class ArcAppIcon {
  public:
   class Observer {
@@ -44,6 +54,10 @@ class ArcAppIcon {
              Observer* observer);
   ~ArcAppIcon();
 
+  // Whether every supported scale factor was successfully loaded. "Supported"
+  // is in the same sense as ui::GetSupportedScaleFactors().
+  bool EverySupportedScaleFactorIsLoaded() const;
+
   const std::string& app_id() const { return app_id_; }
   const gfx::ImageSkia& image_skia() const { return image_skia_; }
 
@@ -62,7 +76,7 @@ class ArcAppIcon {
 
  private:
   friend class ArcAppIconLoader;
-  friend class ArcAppModelBuilder;
+  friend class apps::ArcIconOnceLoader;
 
   class Source;
   class DecodeRequest;
@@ -103,6 +117,7 @@ class ArcAppIcon {
   Observer* const observer_;
 
   gfx::ImageSkia image_skia_;
+  std::set<ui::ScaleFactor> incomplete_scale_factors_;
 
   // Contains pending image decode requests.
   std::vector<std::unique_ptr<DecodeRequest>> decode_requests_;