Add the Chrome badge to the icon when install the ARC app.

After install the ARC app, update the equivalent Chrome app to add the
badge on the icon. When the ARC app is removed, update the equivalent
Chrome app to remove the badge on the icon.

This is the follow up for crrev.com/c/1833006 to implement the the
change in extension_apps.

This CL breaks unit tests when testing multiple profiles:
NoteTakingHelperTest.AddProfileWithPlayStoreEnabled
WebAppInstallTaskTest.InstallWebAppWithParams_GuestProfile
The reason is IsArcAllowedForProfile returns true for any profile under
tests. crbug.com/1010137 is used to fix that. Since that is
out of the scope of this CL, for this CL, a work around is implementted
to look up |user_to_profile_for_testing_| map before going to the
default primary user.

Bug: b/141388525

Change-Id: Ic11965d72ec6ad1636cd0765f57cadc377bd5479
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824307
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704093}

chrome/browser/apps/app_service/app_service_proxy.cc

@@ -12,7 +12,6 @@
 #include "chrome/browser/apps/app_service/app_icon_source.h"
 #include "chrome/browser/apps/app_service/app_service_metrics.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
-#include "chrome/browser/chromeos/extensions/gfx_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/services/app_service/app_service_impl.h"
 #include "chrome/services/app_service/public/cpp/intent_util.h"
@@ -247,6 +246,18 @@ std::vector<std::string> AppServiceProxy::GetAppIdsForIntent(
   return app_ids;
 }
 
+void AppServiceProxy::SetArcIsRegistered() {
+#if defined(OS_CHROMEOS)
+  if (arc_is_registered_) {
+    return;
+  }
+
+  arc_is_registered_ = true;
+  extension_apps_->ObserveArc();
+  extension_web_apps_->ObserveArc();
+#endif
+}
+
 void AppServiceProxy::AddAppIconSource(Profile* profile) {
   // Make the chrome://app-icon/ resource available.
   content::URLDataSource::Add(profile,
@@ -271,16 +282,4 @@ void AppServiceProxy::Clone(
   receivers_.Add(this, std::move(receiver));
 }
 
-#if defined(OS_CHROMEOS)
-void AppServiceProxy::ApplyChromeBadge(Profile* profile,
-                                       const std::string& arc_package_name) {
-  const std::vector<std::string> extension_ids =
-      extensions::util::GetEquivalentInstalledExtensions(profile,
-                                                         arc_package_name);
-  for (auto app_id : extension_ids) {
-    this->extension_apps_->ApplyChromeBadge(app_id);
-  }
-}
-#endif  // OS_CHROMEOS
-
 }  // namespace apps

chrome/browser/apps/app_service/app_service_proxy.h

@@ -75,10 +75,7 @@ class AppServiceProxy : public KeyedService,
   void ReInitializeCrostiniForTesting(Profile* profile);
   std::vector<std::string> GetAppIdsForUrl(const GURL& url);
   std::vector<std::string> GetAppIdsForIntent(apps::mojom::IntentPtr intent);
-
-#if defined(OS_CHROMEOS)
-  void ApplyChromeBadge(Profile* profile, const std::string& arc_package_name);
-#endif
+  void SetArcIsRegistered();
 
  private:
   // An adapter, presenting an IconLoader interface based on the underlying
@@ -177,6 +174,7 @@ class AppServiceProxy : public KeyedService,
   std::unique_ptr<CrostiniApps> crostini_apps_;
   std::unique_ptr<ExtensionApps> extension_apps_;
   std::unique_ptr<ExtensionApps> extension_web_apps_;
+  bool arc_is_registered_ = false;
 #endif  // OS_CHROMEOS
 
   Profile* profile_;

chrome/browser/apps/app_service/arc_apps.cc

@@ -132,6 +132,7 @@ ArcApps::ArcApps(Profile* profile, apps::AppServiceProxy* proxy)
     return;
   }
   prefs->AddObserver(this);
+  proxy->SetArcIsRegistered();
 
   auto* intent_helper_bridge =
       arc::ArcIntentHelperBridge::GetForBrowserContext(profile_);
@@ -185,7 +186,6 @@ void ArcApps::Connect(
           prefs->GetApp(app_id);
       if (app_info) {
         apps.push_back(Convert(prefs, app_id, *app_info));
-        ApplyChromeBadge(app_info->package_name);
       }
     }
   }
@@ -392,7 +392,6 @@ void ArcApps::OnAppLastLaunchTimeUpdated(const std::string& app_id) {
 
 void ArcApps::OnPackageInstalled(
     const arc::mojom::ArcPackageInfo& package_info) {
-  ApplyChromeBadge(package_info.package_name);
   ConvertAndPublishPackageApps(package_info);
 }
 
@@ -402,11 +401,6 @@ void ArcApps::OnPackageModified(
   ConvertAndPublishPackageApps(package_info, update_icon);
 }
 
-void ArcApps::OnPackageRemoved(const std::string& package_name,
-                               bool uninstalled) {
-  ApplyChromeBadge(package_name);
-}
-
 void ArcApps::OnPackageListInitialRefreshed() {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_);
   if (!prefs) {
@@ -631,12 +625,4 @@ void ArcApps::UpdateAppIntentFilters(
   }
 }
 
-void ArcApps::ApplyChromeBadge(const std::string& arc_package_name) {
-  apps::AppServiceProxy* proxy =
-      apps::AppServiceProxyFactory::GetForProfile(profile_);
-  if (proxy) {
-    proxy->ApplyChromeBadge(profile_, arc_package_name);
-  }
-}
-
 }  // namespace apps

chrome/browser/apps/app_service/arc_apps.h

@@ -90,8 +90,6 @@ class ArcApps : public KeyedService,
       const arc::mojom::ArcPackageInfo& package_info) override;
   void OnPackageModified(
       const arc::mojom::ArcPackageInfo& package_info) override;
-  void OnPackageRemoved(const std::string& package_name,
-                        bool uninstalled) override;
   void OnPackageListInitialRefreshed() override;
 
   // arc::ArcIntentHelperObserver overrides.
@@ -111,7 +109,6 @@ class ArcApps : public KeyedService,
   void ConvertAndPublishPackageApps(
       const arc::mojom::ArcPackageInfo& package_info,
       bool update_icon = true);
-  void ApplyChromeBadge(const std::string& arc_package_name);
   void UpdateAppIntentFilters(
       std::string package_name,
       arc::ArcIntentHelperBridge* intent_helper_bridge,

chrome/browser/apps/app_service/extension_apps.cc

@@ -16,6 +16,7 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/apps/app_service/app_icon_factory.h"
 #include "chrome/browser/apps/launch_service/launch_service.h"
+#include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/extensions/gfx_utils.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -38,6 +39,7 @@
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/services/app_service/public/cpp/intent_filter_util.h"
 #include "chrome/services/app_service/public/mojom/types.mojom.h"
+#include "components/arc/arc_service_manager.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
@@ -155,22 +157,48 @@ ExtensionApps::ExtensionApps(
       prefs_observer_(this),
       registry_observer_(this),
       content_settings_observer_(this),
-      app_type_(app_type) {
+      app_type_(app_type),
+      arc_prefs_(nullptr) {
   Initialize(app_service);
 }
 
-ExtensionApps::~ExtensionApps() = default;
+ExtensionApps::~ExtensionApps() {
+  // In unit tests, AppServiceProxy might be ReInitializeForTesting, so
+  // ExtensionApps might be destroyed without calling Shutdown, so arc_prefs_
+  // needs to be removed from observer in the destructor function.
+  if (arc_prefs_) {
+    arc_prefs_->RemoveObserver(this);
+    arc_prefs_ = nullptr;
+  }
+}
 
 void ExtensionApps::FlushMojoCallsForTesting() {
   receiver_.FlushForTesting();
 }
 
 void ExtensionApps::Shutdown() {
+  if (arc_prefs_) {
+    arc_prefs_->RemoveObserver(this);
+    arc_prefs_ = nullptr;
+  }
+
   if (profile_) {
     content_settings_observer_.RemoveAll();
   }
 }
 
+void ExtensionApps::ObserveArc() {
+  // Observe the ARC apps to set the badge on the equivalent Chrome app's icon.
+  if (arc_prefs_) {
+    arc_prefs_->RemoveObserver(this);
+  }
+
+  arc_prefs_ = ArcAppListPrefs::Get(profile_);
+  if (arc_prefs_) {
+    arc_prefs_->AddObserver(this);
+  }
+}
+
 void ExtensionApps::Initialize(
     const mojo::Remote<apps::mojom::AppService>& app_service) {
   DCHECK(profile_);
@@ -551,6 +579,25 @@ void ExtensionApps::Publish(apps::mojom::AppPtr app) {
   }
 }
 
+void ExtensionApps::OnPackageInstalled(
+    const arc::mojom::ArcPackageInfo& package_info) {
+  ApplyChromeBadge(package_info.package_name);
+}
+
+void ExtensionApps::OnPackageRemoved(const std::string& package_name,
+                                     bool uninstalled) {
+  ApplyChromeBadge(package_name);
+}
+
+void ExtensionApps::OnPackageListInitialRefreshed() {
+  if (!arc_prefs_) {
+    return;
+  }
+  for (const auto& app_name : arc_prefs_->GetPackagesFromPrefs()) {
+    ApplyChromeBadge(app_name);
+  }
+}
+
 // static
 bool ExtensionApps::IsBlacklisted(const std::string& app_id) {
   // We blacklist (meaning we don't publish the app, in the App Service sense)
@@ -721,7 +768,7 @@ apps::mojom::AppPtr ExtensionApps::Convert(
   app->short_name = extension->short_name();
   app->description = extension->description();
   app->version = extension->GetVersionForDisplay();
-  app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffect(extension));
+  app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffects(extension));
 
   if (profile_) {
     auto* prefs = extensions::ExtensionPrefs::Get(profile_);
@@ -790,7 +837,7 @@ bool ExtensionApps::RunExtensionEnableFlow(
   return true;
 }
 
-IconEffects ExtensionApps::GetIconEffect(
+IconEffects ExtensionApps::GetIconEffects(
     const extensions::Extension* extension) {
   IconEffects icon_effects = IconEffects::kNone;
 #if defined(OS_CHROMEOS)
@@ -810,21 +857,27 @@ IconEffects ExtensionApps::GetIconEffect(
   return icon_effects;
 }
 
-void ExtensionApps::ApplyChromeBadge(const std::string& app_id) {
+void ExtensionApps::ApplyChromeBadge(const std::string& package_name) {
+  const std::vector<std::string> extension_ids =
+      extensions::util::GetEquivalentInstalledExtensions(profile_,
+                                                         package_name);
   extensions::ExtensionRegistry* registry =
       extensions::ExtensionRegistry::Get(profile_);
-  const extensions::Extension* extension =
-      registry->GetInstalledExtension(app_id);
-  if (!extension || !Accepts(extension)) {
-    return;
-  }
 
-  apps::mojom::AppPtr app = apps::mojom::App::New();
-  app->app_type = app_type_;
-  app->app_id = extension->id();
-  app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffect(extension));
+  for (auto& app_id : extension_ids) {
+    const extensions::Extension* extension =
+        registry->GetInstalledExtension(app_id);
+    if (!extension || !Accepts(extension)) {
+      continue;
+    }
 
-  Publish(std::move(app));
+    apps::mojom::AppPtr app = apps::mojom::App::New();
+    app->app_type = app_type_;
+    app->app_id = extension->id();
+    app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffects(extension));
+
+    Publish(std::move(app));
+  }
 }
 
 }  // namespace apps

chrome/browser/apps/app_service/extension_apps.h

@@ -12,6 +12,7 @@
 #include "base/scoped_observer.h"
 #include "chrome/browser/apps/app_service/app_icon_factory.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/content_settings/core/browser/content_settings_observer.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -41,7 +42,8 @@ class ExtensionAppsEnableFlow;
 class ExtensionApps : public apps::mojom::Publisher,
                       public extensions::ExtensionPrefsObserver,
                       public extensions::ExtensionRegistryObserver,
-                      public content_settings::Observer {
+                      public content_settings::Observer,
+                      public ArcAppListPrefs::Observer {
  public:
   ExtensionApps(const mojo::Remote<apps::mojom::AppService>& app_service,
                 Profile* profile,
@@ -52,7 +54,7 @@ class ExtensionApps : public apps::mojom::Publisher,
 
   void Shutdown();
 
-  void ApplyChromeBadge(const std::string& app_id);
+  void ObserveArc();
 
  private:
   void Initialize(const mojo::Remote<apps::mojom::AppService>& app_service);
@@ -105,6 +107,13 @@ class ExtensionApps : public apps::mojom::Publisher,
                               const extensions::Extension* extension,
                               extensions::UninstallReason reason) override;
 
+  // ArcAppListPrefs::Observer overrides.
+  void OnPackageInstalled(
+      const arc::mojom::ArcPackageInfo& package_info) override;
+  void OnPackageRemoved(const std::string& package_name,
+                        bool uninstalled) override;
+  void OnPackageListInitialRefreshed() override;
+
   void Publish(apps::mojom::AppPtr app);
 
   // Checks if extension is disabled and if enable flow should be started.
@@ -133,7 +142,14 @@ class ExtensionApps : public apps::mojom::Publisher,
                      apps::mojom::Readiness readiness,
                      std::vector<apps::mojom::AppPtr>* apps_out);
 
-  IconEffects GetIconEffect(const extensions::Extension* extension);
+  // Calculate the icon effects for the extension.
+  IconEffects GetIconEffects(const extensions::Extension* extension);
+
+  // Get the equivalent Chrome app from |arc_package_name| and set the Chrome
+  // app badge on the icon effects for the equivalent Chrome apps. If the
+  // equivalent ARC app is installed, add the Chrome app badge, otherwise,
+  // remove the Chrome app badge.
+  void ApplyChromeBadge(const std::string& arc_package_name);
 
   mojo::Receiver<apps::mojom::Publisher> receiver_{this};
   mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
@@ -155,6 +171,8 @@ class ExtensionApps : public apps::mojom::Publisher,
   using EnableFlowPtr = std::unique_ptr<ExtensionAppsEnableFlow>;
   std::map<std::string, EnableFlowPtr> enable_flow_map_;
 
+  ArcAppListPrefs* arc_prefs_;
+
   base::WeakPtrFactory<ExtensionApps> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionApps);