Add an App Service icon factory implementation

BUG=826982

Change-Id: Iede4849e94bbecc9340dcd3d612e419531185fa9
Reviewed-on: https://chromium-review.googlesource.com/c/1347955
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611565}

chrome/browser/BUILD.gn

@@ -2593,6 +2593,8 @@ jumbo_split_static_library("browser") {
     sources += [
       "accessibility/invert_bubble_prefs.cc",
       "accessibility/invert_bubble_prefs.h",
+      "apps/app_service/app_icon_factory.cc",
+      "apps/app_service/app_icon_factory.h",
       "apps/app_service/app_service_proxy.cc",
       "apps/app_service/app_service_proxy.h",
       "apps/app_service/app_service_proxy_factory.cc",

chrome/browser/apps/app_service/app_icon_factory.cc

+// Copyright 2018 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 "chrome/browser/apps/app_service/app_icon_factory.h"
+
+#include <cmath>
+#include <utility>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/files/file_util.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/image_loader.h"
+#include "extensions/common/manifest_handlers/icons_handler.h"
+#include "skia/ext/image_operations.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_skia_operations.h"
+
+namespace {
+
+float GetDeviceScaleFactor() {
+  display::Screen* screen = display::Screen::GetScreen();
+  if (!screen) {
+    return 1.0f;
+  }
+  return screen->GetPrimaryDisplay().device_scale_factor();
+}
+
+int ConvertDipToPx(int dip) {
+  return base::saturated_cast<int>(
+      std::floor(static_cast<float>(dip) * GetDeviceScaleFactor()));
+}
+
+std::string ReadExtensionResource(extensions::ExtensionResource ext_resource) {
+  std::string data;
+  base::ReadFileToString(ext_resource.GetFilePath(), &data);
+  return data;
+}
+
+// Runs |callback| passing an IconValuePtr with a compressed image.
+void RunCallbackWithCompressedImage(apps::LoadIconRepeatingCallback callback,
+                                    std::string data) {
+  apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
+  iv->icon_compression = data.empty()
+                             ? apps::mojom::IconCompression::kUnknown
+                             : apps::mojom::IconCompression::kCompressed;
+  iv->compressed = std::vector<uint8_t>(data.begin(), data.end());
+  callback.Run(std::move(iv));
+}
+
+// Runs |callback| passing an IconValuePtr with an uncompressed image.
+void RunCallbackWithUncompressedImage(apps::LoadIconRepeatingCallback callback,
+                                      const gfx::Image& image) {
+  apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
+  iv->icon_compression = apps::mojom::IconCompression::kUncompressed;
+  iv->uncompressed = image.AsImageSkia();
+  callback.Run(std::move(iv));
+}
+
+}  // namespace
+
+namespace apps {
+
+void LoadIconFromExtension(apps::mojom::IconCompression icon_compression,
+                           int size_hint_in_dip,
+                           LoadIconRepeatingCallback callback,
+                           content::BrowserContext* context,
+                           const std::string& extension_id) {
+  int size_hint_in_px = ConvertDipToPx(size_hint_in_dip);
+
+  const extensions::Extension* extension =
+      extensions::ExtensionSystem::Get(context)
+          ->extension_service()
+          ->GetInstalledExtension(extension_id);
+  if (extension) {
+    extensions::ExtensionResource ext_resource =
+        extensions::IconsInfo::GetIconResource(extension, size_hint_in_px,
+                                               ExtensionIconSet::MATCH_BIGGER);
+
+    switch (icon_compression) {
+      case apps::mojom::IconCompression::kUnknown:
+        break;
+
+      case apps::mojom::IconCompression::kUncompressed:
+        extensions::ImageLoader::Get(context)->LoadImageAsync(
+            extension, std::move(ext_resource),
+            gfx::Size(size_hint_in_px, size_hint_in_px),
+            base::BindRepeating(&RunCallbackWithUncompressedImage, callback));
+        return;
+
+      case apps::mojom::IconCompression::kCompressed:
+        base::PostTaskWithTraitsAndReplyWithResult(
+            FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
+            base::BindOnce(&ReadExtensionResource, std::move(ext_resource)),
+            base::BindOnce(&RunCallbackWithCompressedImage, callback));
+        return;
+    }
+  }
+
+  std::move(callback).Run(apps::mojom::IconValue::New());
+}
+
+void LoadIconFromResource(apps::mojom::IconCompression icon_compression,
+                          int size_hint_in_dip,
+                          apps::mojom::Publisher::LoadIconCallback callback,
+                          int resource_id) {
+  if (resource_id != 0) {
+    switch (icon_compression) {
+      case apps::mojom::IconCompression::kUnknown:
+        break;
+
+      case apps::mojom::IconCompression::kUncompressed: {
+        gfx::ImageSkia* unscaled =
+            ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+                resource_id);
+
+        apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
+        iv->icon_compression = apps::mojom::IconCompression::kUncompressed;
+        iv->uncompressed = gfx::ImageSkiaOperations::CreateResizedImage(
+            *unscaled, skia::ImageOperations::RESIZE_BEST,
+            gfx::Size(size_hint_in_dip, size_hint_in_dip));
+        std::move(callback).Run(std::move(iv));
+        return;
+      }
+
+      case apps::mojom::IconCompression::kCompressed: {
+        base::StringPiece data =
+            ui::ResourceBundle::GetSharedInstance().GetRawDataResource(
+                resource_id);
+
+        apps::mojom::IconValuePtr iv = apps::mojom::IconValue::New();
+        iv->icon_compression = apps::mojom::IconCompression::kCompressed;
+        iv->compressed = std::vector<uint8_t>(data.begin(), data.end());
+        std::move(callback).Run(std::move(iv));
+        return;
+      }
+    }
+  }
+
+  std::move(callback).Run(apps::mojom::IconValue::New());
+}
+
+}  // namespace apps

chrome/browser/apps/app_service/app_icon_factory.h

+// Copyright 2018 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_APP_ICON_FACTORY_H_
+#define CHROME_BROWSER_APPS_APP_SERVICE_APP_ICON_FACTORY_H_
+
+#include <string>
+
+#include "base/callback_forward.h"
+#include "chrome/services/app_service/public/mojom/app_service.mojom.h"
+#include "chrome/services/app_service/public/mojom/types.mojom.h"
+#include "ui/gfx/image/image_skia.h"
+
+namespace content {
+class BrowserContext;
+}
+
+namespace apps {
+
+// TODO: delete this. See below.
+using LoadIconRepeatingCallback =
+    base::RepeatingCallback<void(apps::mojom::IconValuePtr icon_value)>;
+
+void LoadIconFromExtension(
+    apps::mojom::IconCompression icon_compression,
+    int size_hint_in_dip,
+    // TODO: the next line should be:
+    //
+    // apps::mojom::Publisher::LoadIconCallback callback,
+    //
+    // In other words, we should drop the "Repeating". This will require fixing
+    // extensions::ImageLoader to take a base::OnceCallback instead of an
+    // (implicitly repeating) base::Callback.
+    LoadIconRepeatingCallback callback,
+    content::BrowserContext* context,
+    const std::string& extension_id);
+
+void LoadIconFromResource(apps::mojom::IconCompression icon_compression,
+                          int size_hint_in_dip,
+                          apps::mojom::Publisher::LoadIconCallback callback,
+                          int resource_id);
+
+}  // namespace apps
+
+#endif  // CHROME_BROWSER_APPS_APP_SERVICE_APP_ICON_FACTORY_H_

chrome/browser/apps/app_service/app_service_proxy.cc

@@ -44,6 +44,30 @@ AppRegistryCache& AppServiceProxy::Cache() {
   return cache_;
 }
 
+void AppServiceProxy::LoadIcon(
+    const std::string& app_id,
+    apps::mojom::IconCompression icon_compression,
+    int32_t size_hint_in_dip,
+    apps::mojom::Publisher::LoadIconCallback callback) {
+  bool found = false;
+  cache_.ForOneApp(app_id, [this, &icon_compression, &size_hint_in_dip,
+                            &callback, &found](const apps::AppUpdate& update) {
+    apps::mojom::IconKeyPtr icon_key = update.IconKey();
+    if (icon_key.is_null()) {
+      return;
+    }
+    found = true;
+    app_service_->LoadIcon(update.AppType(), update.AppId(),
+                           std::move(icon_key), icon_compression,
+                           size_hint_in_dip, std::move(callback));
+  });
+
+  if (!found) {
+    // On failure, we still run the callback, with the zero IconValue.
+    std::move(callback).Run(apps::mojom::IconValue::New());
+  }
+}
+
 void AppServiceProxy::OnApps(std::vector<apps::mojom::AppPtr> deltas) {
   cache_.OnApps(std::move(deltas));
 }

chrome/browser/apps/app_service/app_service_proxy.h

@@ -8,6 +8,7 @@
 #include "base/macros.h"
 #include "chrome/services/app_service/public/cpp/app_registry_cache.h"
 #include "chrome/services/app_service/public/mojom/app_service.mojom.h"
+#include "chrome/services/app_service/public/mojom/types.mojom.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 
@@ -35,6 +36,11 @@ class AppServiceProxy : public KeyedService, public apps::mojom::Subscriber {
 
   AppRegistryCache& Cache();
 
+  void LoadIcon(const std::string& app_id,
+                apps::mojom::IconCompression icon_compression,
+                int32_t size_hint_in_dip,
+                apps::mojom::Publisher::LoadIconCallback callback);
+
  private:
   // apps::mojom::Subscriber overrides.
   void OnApps(std::vector<apps::mojom::AppPtr> deltas) override;

chrome/browser/apps/app_service/built_in_chromeos_apps.cc

@@ -7,8 +7,12 @@
 #include <utility>
 #include <vector>
 
+#include "chrome/browser/apps/app_service/app_icon_factory.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 
+// TODO: delete this when we no longer refer to IDR_APP_DEFAULT_ICON below.
+#include "extensions/grit/extensions_browser_resources.h"
+
 namespace apps {
 
 BuiltInChromeOsApps::BuiltInChromeOsApps() : binding_(this) {}
@@ -32,6 +36,12 @@ void BuiltInChromeOsApps::Connect(apps::mojom::SubscriberPtr subscriber,
   auto app = apps::mojom::App::New();
   app->app_type = apps::mojom::AppType::kBuiltIn;
   app->app_id = "placeholder";
+  app->readiness = apps::mojom::Readiness::kReady;
+  app->name = "Holder of the Place";
+  app->icon_key = apps::mojom::IconKey::New();
+  app->icon_key->icon_type = apps::mojom::IconType::kResource;
+  app->icon_key->u_key = IDR_APP_DEFAULT_ICON;
+  app->show_in_launcher = apps::mojom::OptionalBool::kTrue;
   apps.push_back(std::move(app));
 
   subscriber->OnApps(std::move(apps));
@@ -43,4 +53,22 @@ void BuiltInChromeOsApps::Connect(apps::mojom::SubscriberPtr subscriber,
   // lifetime of the Chrome OS session. There won't be any further updates.
 }
 
+void BuiltInChromeOsApps::LoadIcon(
+    const std::string& app_id,
+    apps::mojom::IconKeyPtr icon_key,
+    apps::mojom::IconCompression icon_compression,
+    int32_t size_hint_in_dip,
+    LoadIconCallback callback) {
+  if (!icon_key.is_null() &&
+      (icon_key->icon_type == apps::mojom::IconType::kResource) &&
+      (icon_key->u_key != 0) && (icon_key->u_key <= INT_MAX)) {
+    int resource_id = static_cast<int>(icon_key->u_key);
+    LoadIconFromResource(icon_compression, size_hint_in_dip,
+                         std::move(callback), resource_id);
+    return;
+  }
+  // On failure, we still run the callback, with the zero IconValue.
+  std::move(callback).Run(apps::mojom::IconValue::New());
+}
+
 }  // namespace apps

chrome/browser/apps/app_service/built_in_chromeos_apps.h

@@ -25,6 +25,11 @@ class BuiltInChromeOsApps : public apps::mojom::Publisher {
   // apps::mojom::Publisher overrides.
   void Connect(apps::mojom::SubscriberPtr subscriber,
                apps::mojom::ConnectOptionsPtr opts) override;
+  void LoadIcon(const std::string& app_id,
+                apps::mojom::IconKeyPtr icon_key,
+                apps::mojom::IconCompression icon_compression,
+                int32_t size_hint_in_dip,
+                LoadIconCallback callback) override;
 
   mojo::Binding<apps::mojom::Publisher> binding_;
 

chrome/services/app_service/README.md

@@ -253,7 +253,7 @@ methods. An `AppUpdate` is a pair of `App` values: old state and delta.
 
 Icon data (even compressed as a PNG) is bulky, relative to the rest of the
 `App` type. `Publisher`s will generally serve icon data lazily, on demand,
-especially as the desired icon resolutions (e.g. 64px or 256px) aren't known
+especially as the desired icon resolutions (e.g. 64dip or 256dip) aren't known
 up-front. Instead of sending an icon at all possible resolutions, the
 `Publisher` sends an `IconKey`: enough information (when combined with the
 `AppType app_type` and `string app_id`) to load the icon at given resoultions.
@@ -278,15 +278,23 @@ statically built resource ID.
 
     interface AppService {
       // App Icon Factory methods.
-      LoadIcon(AppType app_type,
-          string app_id, IconKey icon_key, LoadIconOptions? opts) => (gfx.mojom.ImageSkia image);
+      LoadIcon(
+          AppType app_type,
+          string app_id,
+          IconKey icon_key,
+          IconCompression icon_compression,
+          int32 size_hint_in_dip) => (IconValue icon_value);
 
       // Some additional methods; not App Icon Factory related.
     };
 
     interface Publisher {
       // App Icon Factory methods.
-      LoadIcon(string app_id, IconKey icon_key, LoadIconOptions? opts) => (gfx.mojom.ImageSkia image);
+      LoadIcon(
+          string app_id,
+          IconKey icon_key,
+          IconCompression icon_compression,
+          int32 size_hint_in_dip) => (IconValue icon_value);
 
       // Some additional methods; not App Icon Factory related.
     };
@@ -304,9 +312,16 @@ statically built resource ID.
       string s_key;
     };
 
-    struct LoadIconOptions {
-      // TBD: desired resolutions (e.g. 64px or 256px). This could move out of
-      // LoadIconOptions to be a mandatory (not optional) LoadIcon arg.
+    enum IconCompression {
+      kUnknown,
+      kUncompressed,
+      kCompressed,
+    };
+
+    struct IconValue {
+      IconCompression icon_compression;
+      gfx.mojom.ImageSkia? uncompressed;
+      array<uint8>? compressed;
     };
 
 TBD: post-processing effects like rounded corners, badges or grayed out (for
@@ -368,4 +383,4 @@ TBD: details.
 
 ---
 
-Updated on 2018-11-16.
+Updated on 2018-11-22.

chrome/services/app_service/app_service_impl.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 
 namespace {
@@ -38,22 +39,22 @@ void AppServiceImpl::RegisterPublisher(apps::mojom::PublisherPtr publisher,
   });
 
   // Check that no previous publisher has registered for the same app_type.
-  CHECK(publishers_by_type_.find(app_type) == publishers_by_type_.end());
+  CHECK(publishers_.find(app_type) == publishers_.end());
 
   // Add the new publisher to the set.
-  //
-  // We also track publishers by their app_type, so that we can forward other
-  // App Service method calls on to one particular publisher, instead of
-  // broadcasting to all publishers.
-  publishers_by_type_[app_type] = publishers_.AddPtr(std::move(publisher));
+  publisher.set_connection_error_handler(
+      base::BindOnce(&AppServiceImpl::OnPublisherDisconnected,
+                     base::Unretained(this), app_type));
+  auto result = publishers_.emplace(app_type, std::move(publisher));
+  CHECK(result.second);
 }
 
 void AppServiceImpl::RegisterSubscriber(apps::mojom::SubscriberPtr subscriber,
                                         apps::mojom::ConnectOptionsPtr opts) {
   // Connect the new subscriber with every registered publisher.
-  publishers_.ForAllPtrs([&subscriber](auto* publisher) {
-    ::Connect(publisher, subscriber.get());
-  });
+  for (const auto& iter : publishers_) {
+    ::Connect(iter.second.get(), subscriber.get());
+  }
 
   // TODO: store the opts somewhere.
 
@@ -61,4 +62,23 @@ void AppServiceImpl::RegisterSubscriber(apps::mojom::SubscriberPtr subscriber,
   subscribers_.AddPtr(std::move(subscriber));
 }
 
+void AppServiceImpl::LoadIcon(apps::mojom::AppType app_type,
+                              const std::string& app_id,
+                              apps::mojom::IconKeyPtr icon_key,
+                              apps::mojom::IconCompression icon_compression,
+                              int32_t size_hint_in_dip,
+                              LoadIconCallback callback) {
+  auto iter = publishers_.find(app_type);
+  if (iter == publishers_.end()) {
+    std::move(callback).Run(apps::mojom::IconValue::New());
+    return;
+  }
+  iter->second->LoadIcon(app_id, std::move(icon_key), icon_compression,
+                         size_hint_in_dip, std::move(callback));
+}
+
+void AppServiceImpl::OnPublisherDisconnected(apps::mojom::AppType app_type) {
+  publishers_.erase(app_type);
+}
+
 }  // namespace apps

chrome/services/app_service/app_service_impl.h

@@ -31,13 +31,20 @@ class AppServiceImpl : public apps::mojom::AppService {
                          apps::mojom::AppType app_type) override;
   void RegisterSubscriber(apps::mojom::SubscriberPtr subscriber,
                           apps::mojom::ConnectOptionsPtr opts) override;
+  void LoadIcon(apps::mojom::AppType app_type,
+                const std::string& app_id,
+                apps::mojom::IconKeyPtr icon_key,
+                apps::mojom::IconCompression icon_compression,
+                int32_t size_hint_in_dip,
+                LoadIconCallback callback) override;
 
  private:
-  mojo::BindingSet<apps::mojom::AppService> bindings_;
+  void OnPublisherDisconnected(apps::mojom::AppType app_type);
 
-  std::map<apps::mojom::AppType, mojo::InterfacePtrSetElementId>
-      publishers_by_type_;
-  mojo::InterfacePtrSet<apps::mojom::Publisher> publishers_;
+  mojo::BindingSet<apps::mojom::AppService> bindings_;
+  // publishers_ is a std::map, not a mojo::InterfacePtrSet, since we want to
+  // be able to find *the* publisher for a given apps::mojom::AppType.
+  std::map<apps::mojom::AppType, apps::mojom::PublisherPtr> publishers_;
   mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
 
   DISALLOW_COPY_AND_ASSIGN(AppServiceImpl);

chrome/services/app_service/app_service_impl_unittest.cc

@@ -4,11 +4,14 @@
 
 #include <set>
 #include <sstream>
+#include <utility>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "chrome/services/app_service/app_service_impl.h"
+#include "chrome/services/app_service/public/mojom/types.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace apps {
@@ -33,6 +36,8 @@ class FakePublisher : public apps::mojom::Publisher {
     }
   }
 
+  std::string load_icon_app_id_;
+
  private:
   void Connect(apps::mojom::SubscriberPtr subscriber,
                apps::mojom::ConnectOptionsPtr opts) override {
@@ -40,6 +45,15 @@ class FakePublisher : public apps::mojom::Publisher {
     subscribers_.AddPtr(std::move(subscriber));
   }
 
+  void LoadIcon(const std::string& app_id,
+                apps::mojom::IconKeyPtr icon_key,
+                apps::mojom::IconCompression icon_compression,
+                int32_t size_hint_in_dip,
+                LoadIconCallback callback) override {
+    load_icon_app_id_ = app_id;
+    std::move(callback).Run(apps::mojom::IconValue::New());
+  }
+
   void CallOnApps(apps::mojom::Subscriber* subscriber,
                   std::vector<std::string>& app_ids) {
     std::vector<apps::mojom::AppPtr> apps;
@@ -98,6 +112,8 @@ class AppServiceImplTest : public testing::Test {
 };
 
 TEST_F(AppServiceImplTest, PubSub) {
+  const int size_hint_in_dip = 64;
+
   AppServiceImpl impl;
 
   // Start with one subscriber.
@@ -154,6 +170,34 @@ TEST_F(AppServiceImplTest, PubSub) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ("$&ABCDEFGmnopqr", sub0.AppIdsSeen());
   EXPECT_EQ("$&ABCDEFGmnopqr", sub1.AppIdsSeen());
+
+  // Call LoadIcon on the impl twice.
+  //
+  // The first time (i == 0), it should be forwarded onto the AppType::kBuiltIn
+  // publisher (which is pub1) and no other publisher.
+  //
+  // The second time (i == 1), passing AppType::kUnknown, none of the
+  // publishers' LoadIcon's should fire, but the callback should still be run.
+  for (int i = 0; i < 2; i++) {
+    auto app_type = i == 0 ? apps::mojom::AppType::kBuiltIn
+                           : apps::mojom::AppType::kUnknown;
+
+    bool callback_ran = false;
+    pub0.load_icon_app_id_ = "-";
+    pub1.load_icon_app_id_ = "-";
+    pub2.load_icon_app_id_ = "-";
+    impl.LoadIcon(
+        app_type, "o", apps::mojom::IconKey::New(),
+        apps::mojom::IconCompression::kUncompressed, size_hint_in_dip,
+        base::BindOnce(
+            [](bool* ran, apps::mojom::IconValuePtr iv) { *ran = true; },
+            &callback_ran));
+    base::RunLoop().RunUntilIdle();
+    EXPECT_TRUE(callback_ran);
+    EXPECT_EQ("-", pub0.load_icon_app_id_);
+    EXPECT_EQ(i == 0 ? "o" : "-", pub1.load_icon_app_id_);
+    EXPECT_EQ("-", pub2.load_icon_app_id_);
+  }
 }
 
 }  // namespace apps

chrome/services/app_service/public/cpp/app_registry_cache.h

@@ -68,6 +68,21 @@ class AppRegistryCache {
     }
   }
 
+  // Calls f, a void-returning function whose arguments are (const
+  // apps::AppUpdate&), on the app in the cache with the given app_id. It will
+  // return true (and call f) if there is such an app, otherwise it will return
+  // false (and not call f). The AppUpdate argument to f has the same semantics
+  // as for ForEachApp, above.
+  template <typename FunctionType>
+  bool ForOneApp(const std::string& app_id, FunctionType f) {
+    auto iter = states_.find(app_id);
+    if (iter != states_.end()) {
+      f(apps::AppUpdate(iter->second, iter->second));
+      return true;
+    }
+    return false;
+  }
+
  private:
   base::ObserverList<Observer> observers_;
 

chrome/services/app_service/public/cpp/app_registry_cache_unittest.cc

@@ -77,6 +77,22 @@ TEST_F(AppRegistryCacheTest, ForEachApp) {
   EXPECT_NE(updated_names_.end(), updated_names_.find("banana"));
   EXPECT_NE(updated_names_.end(), updated_names_.find("cherry"));
   EXPECT_NE(updated_names_.end(), updated_names_.find("durian"));
+
+  // Test that ForOneApp succeeds for "c" and fails for "e".
+
+  bool found_c = false;
+  EXPECT_TRUE(cache.ForOneApp("c", [&found_c](const apps::AppUpdate& update) {
+    found_c = true;
+    EXPECT_EQ("c", update.AppId());
+  }));
+  EXPECT_TRUE(found_c);
+
+  bool found_e = false;
+  EXPECT_FALSE(cache.ForOneApp("e", [&found_e](const apps::AppUpdate& update) {
+    found_e = true;
+    EXPECT_EQ("e", update.AppId());
+  }));
+  EXPECT_FALSE(found_e);
 }
 
 TEST_F(AppRegistryCacheTest, Observer) {

chrome/services/app_service/public/cpp/app_update.cc

@@ -21,6 +21,9 @@ void AppUpdate::Merge(apps::mojom::App* state,
   if (delta->name.has_value()) {
     state->name = delta->name;
   }
+  if (!delta->icon_key.is_null()) {
+    state->icon_key = delta->icon_key.Clone();
+  }
   if (delta->show_in_launcher != apps::mojom::OptionalBool::kUnknown) {
     state->show_in_launcher = delta->show_in_launcher;
   }
@@ -68,6 +71,21 @@ bool AppUpdate::NameChanged() const {
   return delta_->name.has_value() && (delta_->name != state_->name);
 }
 
+apps::mojom::IconKeyPtr AppUpdate::IconKey() const {
+  if (!delta_->icon_key.is_null()) {
+    return delta_->icon_key.Clone();
+  }
+  if (!state_->icon_key.is_null()) {
+    return state_->icon_key.Clone();
+  }
+  return apps::mojom::IconKeyPtr();
+}
+
+bool AppUpdate::IconKeyChanged() const {
+  return !delta_->icon_key.is_null() &&
+         !delta_->icon_key.Equals(state_->icon_key);
+}
+
 apps::mojom::OptionalBool AppUpdate::ShowInLauncher() const {
   return (delta_->show_in_launcher != apps::mojom::OptionalBool::kUnknown)
              ? delta_->show_in_launcher

chrome/services/app_service/public/cpp/app_update.h

@@ -52,6 +52,9 @@ class AppUpdate {
   const std::string& Name() const;
   bool NameChanged() const;
 
+  apps::mojom::IconKeyPtr IconKey() const;
+  bool IconKeyChanged() const;
+
   apps::mojom::OptionalBool ShowInLauncher() const;
   bool ShowInLauncherChanged() const;
 

chrome/services/app_service/public/cpp/app_update_unittest.cc

@@ -110,4 +110,29 @@ TEST_F(AppUpdateTest, BasicOps) {
   EXPECT_EQ("Inigo Montoya", state->name);
   EXPECT_EQ(apps::mojom::OptionalBool::kUnknown, u.ShowInLauncher());
   EXPECT_EQ(apps::mojom::OptionalBool::kUnknown, state->show_in_launcher);
+
+  // IconKey tests.
+
+  EXPECT_TRUE(u.IconKey().is_null());
+  EXPECT_FALSE(u.IconKeyChanged());
+
+  state->icon_key = apps::mojom::IconKey::New(apps::mojom::IconType::kResource,
+                                              100, std::string());
+
+  EXPECT_FALSE(u.IconKey().is_null());
+  EXPECT_EQ(apps::mojom::IconType::kResource, u.IconKey()->icon_type);
+  EXPECT_FALSE(u.IconKeyChanged());
+
+  delta->icon_key = apps::mojom::IconKey::New(apps::mojom::IconType::kExtension,
+                                              0, "one hundred");
+
+  EXPECT_FALSE(u.IconKey().is_null());
+  EXPECT_EQ(apps::mojom::IconType::kExtension, u.IconKey()->icon_type);
+  EXPECT_TRUE(u.IconKeyChanged());
+
+  apps::AppUpdate::Merge(state.get(), delta);
+
+  EXPECT_FALSE(u.IconKey().is_null());
+  EXPECT_EQ(apps::mojom::IconType::kExtension, u.IconKey()->icon_type);
+  EXPECT_FALSE(u.IconKeyChanged());
 }

chrome/services/app_service/public/mojom/BUILD.gn

@@ -13,7 +13,6 @@ mojom("mojom") {
   public_deps = [
     ":constants",
     ":types",
-    "//mojo/public/mojom/base",
   ]
 }
 
@@ -27,4 +26,9 @@ mojom("types") {
   sources = [
     "types.mojom",
   ]
+
+  public_deps = [
+    "//mojo/public/mojom/base",
+    "//ui/gfx/image/mojo:interfaces",
+  ]
 }

chrome/services/app_service/public/mojom/app_service.mojom

@@ -17,11 +17,26 @@ interface AppService {
   // App Registry methods.
   RegisterPublisher(Publisher publisher, AppType app_type);
   RegisterSubscriber(Subscriber subscriber, ConnectOptions? opts);
+
+  // App Icon Factory methods.
+  LoadIcon(
+      AppType app_type,
+      string app_id,
+      IconKey icon_key,
+      IconCompression icon_compression,
+      int32 size_hint_in_dip) => (IconValue icon_value);
 };
 
 interface Publisher {
   // App Registry methods.
   Connect(Subscriber subscriber, ConnectOptions? opts);
+
+  // App Icon Factory methods.
+  LoadIcon(
+      string app_id,
+      IconKey icon_key,
+      IconCompression icon_compression,
+      int32 size_hint_in_dip) => (IconValue icon_value);
 };
 
 interface Subscriber {

chrome/services/app_service/public/mojom/types.mojom

@@ -4,6 +4,8 @@
 
 module apps.mojom;
 
+import "ui/gfx/image/mojo/image.mojom";
+
 // Information about an app. See chrome/services/app_service/README.md.
 struct App {
   AppType app_type;
@@ -13,10 +15,9 @@ struct App {
 
   Readiness readiness;
   string? name;
+  IconKey? icon_key;
   OptionalBool show_in_launcher;
 
-  // TODO: add more fields, e.g. icon_key.
-
   // When adding new fields, also update the Merge method and other helpers in
   // chrome/services/app_service/public/cpp/app_update.*
 };
@@ -73,3 +74,28 @@ struct AppInfo {
   // The app's current preferred state.
   AppPreferred preferred;
 };
+
+enum IconType {
+  kUnknown,
+  kExtension,
+  kResource,
+};
+
+struct IconKey {
+  IconType icon_type;
+  // The semantics of u_key and s_key depend on the icon_type.
+  uint64 u_key;
+  string s_key;
+};
+
+enum IconCompression {
+  kUnknown,
+  kUncompressed,
+  kCompressed,
+};
+
+struct IconValue {
+  IconCompression icon_compression;
+  gfx.mojom.ImageSkia? uncompressed;
+  array<uint8>? compressed;
+};