Factor out an IncrementingIconKeyFactory

BUG=826982

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

chrome/browser/BUILD.gn

@@ -3308,6 +3308,8 @@ jumbo_split_static_library("browser") {
       "apps/app_service/crostini_apps.h",
       "apps/app_service/extension_apps.cc",
       "apps/app_service/extension_apps.h",
+      "apps/app_service/icon_key_util.cc",
+      "apps/app_service/icon_key_util.h",
       "apps/app_service/launch_util.cc",
       "apps/app_service/launch_util.h",
       "ash_service_registry.cc",

chrome/browser/apps/app_service/arc_apps.cc

@@ -130,7 +130,7 @@ ArcApps* ArcApps::Get(Profile* profile) {
 }
 
 ArcApps::ArcApps(Profile* profile)
-    : binding_(this), profile_(profile), prefs_(nullptr), next_u_key_(1) {
+    : binding_(this), profile_(profile), prefs_(nullptr) {
   if (!arc::IsArcAllowedForProfile(profile_) ||
       (arc::ArcServiceManager::Get() == nullptr)) {
     return;
@@ -476,11 +476,7 @@ apps::mojom::AppPtr ArcApps::Convert(const std::string& app_id,
 }
 
 apps::mojom::IconKeyPtr ArcApps::NewIconKey(const std::string& app_id) {
-  auto icon_key = apps::mojom::IconKey::New();
-  icon_key->icon_type = apps::mojom::IconType::kArc;
-  icon_key->s_key = app_id;
-  icon_key->u_key = next_u_key_++;
-  return icon_key;
+  return icon_key_factory_.MakeIconKey(apps::mojom::IconType::kArc, app_id);
 }
 
 void ArcApps::Publish(apps::mojom::AppPtr app) {

chrome/browser/apps/app_service/arc_apps.h

@@ -12,6 +12,7 @@
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.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"
 #include "components/arc/connection_observer.h"
@@ -105,10 +106,7 @@ class ArcApps : public KeyedService,
   std::vector<base::OnceCallback<void(AppConnectionHolder*)>>
       pending_load_icon_calls_;
 
-  // |next_u_key_| is incremented every time Convert returns a valid AppPtr, so
-  // that when an app's icon has changed, this apps::mojom::Publisher sends a
-  // different IconKey even though the IconKey's s_key hasn't changed.
-  uint64_t next_u_key_;
+  apps_util::IncrementingIconKeyFactory icon_key_factory_;
 
   base::WeakPtrFactory<ArcApps> weak_ptr_factory_{this};
 

chrome/browser/apps/app_service/crostini_apps.cc

@@ -21,8 +21,7 @@
 
 namespace apps {
 
-CrostiniApps::CrostiniApps()
-    : binding_(this), registry_(nullptr), next_u_key_(1) {}
+CrostiniApps::CrostiniApps() : binding_(this), registry_(nullptr) {}
 
 CrostiniApps::~CrostiniApps() {
   if (registry_) {
@@ -217,8 +216,6 @@ apps::mojom::AppPtr CrostiniApps::Convert(
 }
 
 apps::mojom::IconKeyPtr CrostiniApps::NewIconKey(const std::string& app_id) {
-  auto icon_key = apps::mojom::IconKey::New();
-
   // Treat the Crostini Terminal as a special case, loading an icon defined by
   // a resource instead of asking the Crostini VM (or the cache of previous
   // responses from the Crostini VM). Presumably this is for bootstrapping: the
@@ -226,15 +223,14 @@ apps::mojom::IconKeyPtr CrostiniApps::NewIconKey(const std::string& app_id) {
   // should be showable even before the user has installed their first Crostini
   // app and before bringing up an Crostini VM for the first time.
   if (app_id == crostini::kCrostiniTerminalId) {
+    auto icon_key = apps::mojom::IconKey::New();
     icon_key->icon_type = apps::mojom::IconType::kResource;
     icon_key->u_key = IDR_LOGO_CROSTINI_TERMINAL;
-  } else {
-    icon_key->icon_type = apps::mojom::IconType::kCrostini;
-    icon_key->s_key = app_id;
-    icon_key->u_key = next_u_key_++;
+    return icon_key;
   }
 
-  return icon_key;
+  return icon_key_factory_.MakeIconKey(apps::mojom::IconType::kCrostini,
+                                       app_id);
 }
 
 void CrostiniApps::PublishAppID(const std::string& app_id,

chrome/browser/apps/app_service/crostini_apps.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/apps/app_service/icon_key_util.h"
 #include "chrome/browser/chromeos/crostini/crostini_registry_service.h"
 #include "chrome/services/app_service/public/mojom/app_service.mojom.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -86,10 +87,7 @@ class CrostiniApps : public KeyedService,
 
   crostini::CrostiniRegistryService* registry_;
 
-  // |next_u_key_| is incremented every time Convert returns a valid AppPtr, so
-  // that when an app's icon has changed, this apps::mojom::Publisher sends a
-  // different IconKey even though the IconKey's s_key hasn't changed.
-  uint64_t next_u_key_;
+  apps_util::IncrementingIconKeyFactory icon_key_factory_;
 
   base::WeakPtrFactory<CrostiniApps> weak_ptr_factory_{this};
 

chrome/browser/apps/app_service/extension_apps.cc

@@ -62,7 +62,6 @@ namespace apps {
 ExtensionApps::ExtensionApps()
     : binding_(this),
       profile_(nullptr),
-      next_u_key_(1),
       observer_(this),
       app_type_(apps::mojom::AppType::kUnknown) {}
 
@@ -426,10 +425,8 @@ apps::mojom::AppPtr ExtensionApps::Convert(
   app->name = extension->name();
   app->short_name = extension->short_name();
 
-  app->icon_key = apps::mojom::IconKey::New();
-  app->icon_key->icon_type = apps::mojom::IconType::kExtension;
-  app->icon_key->s_key = extension->id();
-  app->icon_key->u_key = next_u_key_++;
+  app->icon_key = icon_key_factory_.MakeIconKey(
+      apps::mojom::IconType::kExtension, extension->id());
 
   if (profile_) {
     auto* prefs = extensions::ExtensionPrefs::Get(profile_);

chrome/browser/apps/app_service/extension_apps.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/scoped_observer.h"
+#include "chrome/browser/apps/app_service/icon_key_util.h"
 #include "chrome/services/app_service/public/mojom/app_service.mojom.h"
 #include "components/content_settings/core/browser/content_settings_observer.h"
 #include "extensions/browser/extension_registry_observer.h"
@@ -101,15 +102,12 @@ class ExtensionApps : public apps::mojom::Publisher,
 
   Profile* profile_;
 
-  // |next_u_key_| is incremented every time Convert returns a valid AppPtr, so
-  // that when an app's icon has changed, this apps::mojom::Publisher sends a
-  // different IconKey even though the IconKey's s_key hasn't changed.
-  uint64_t next_u_key_;
-
   ScopedObserver<extensions::ExtensionRegistry,
                  extensions::ExtensionRegistryObserver>
       observer_;
 
+  apps_util::IncrementingIconKeyFactory icon_key_factory_;
+
   apps::mojom::AppType app_type_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionApps);

chrome/browser/apps/app_service/icon_key_util.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 "chrome/browser/apps/app_service/icon_key_util.h"
+
+namespace apps_util {
+
+IncrementingIconKeyFactory::IncrementingIconKeyFactory() : u_key_(0) {}
+
+apps::mojom::IconKeyPtr IncrementingIconKeyFactory::MakeIconKey(
+    apps::mojom::IconType icon_type,
+    const std::string& s_key,
+    uint8_t flags) {
+  // The flags occupy the low 8 bits.
+  u_key_ += 1 << 8;
+
+  auto icon_key = apps::mojom::IconKey::New();
+  icon_key->icon_type = icon_type;
+  icon_key->s_key = s_key;
+  icon_key->u_key = u_key_ | static_cast<uint64_t>(flags);
+  return icon_key;
+}
+
+}  // namespace apps_util

chrome/browser/apps/app_service/icon_key_util.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_ICON_KEY_UTIL_H_
+#define CHROME_BROWSER_APPS_APP_SERVICE_ICON_KEY_UTIL_H_
+
+// Utility classes for providing an App Service IconKey.
+
+#include <string>
+
+#include "base/macros.h"
+#include "chrome/services/app_service/public/mojom/types.mojom.h"
+
+namespace apps_util {
+
+// Converts strings (such as App IDs) to IconKeys, such that passing the same
+// string twice to MakeIconKey will result in different IconKeys (different not
+// just in the pointer sense, but their IconKey.u_key values will also differ).
+//
+// Callers (which are presumably App Service app publishers) can therefore
+// publish such IconKeys whenever an app's icon changes, even though the App ID
+// itself doesn't change, and App Service app subscribers will notice (and
+// reload) the new icon from the new (changed) icon key.
+//
+// The low 8 bits (a uint8_t) of the resultant IconKey's u_key are reserved for
+// caller-specific flags. For example, colorful/gray icons for enabled/disabled
+// states of the same app can be distinguished in one of those bits.
+class IncrementingIconKeyFactory {
+ public:
+  IncrementingIconKeyFactory();
+
+  apps::mojom::IconKeyPtr MakeIconKey(apps::mojom::IconType icon_type,
+                                      const std::string& s_key,
+                                      uint8_t flags = 0);
+
+ private:
+  uint64_t u_key_;
+
+  DISALLOW_COPY_AND_ASSIGN(IncrementingIconKeyFactory);
+};
+
+}  // namespace apps_util
+
+#endif  // CHROME_BROWSER_APPS_APP_SERVICE_ICON_KEY_UTIL_H_