[Promise Icons] Support the removal of promise apps

This CL provides functions that allow a promise app to be deleted
through AppServiceProxy via the PromiseAppService. To do this,
we remove the promise app from the PromiseAppRegistryCache using
PromiseStatus::kRemove, and then delete all associated icons from the
PromiseAppIconCache.

Bug: b/288832707
Change-Id: Ic9895aa756848cd8241396d92a6e7f7650e0c969
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4671709
Reviewed-by: Maggie Cai <mxcai@chromium.org>
Commit-Queue: Vivian Pao <vpao@google.com>
Cr-Commit-Position: refs/heads/main@{#1167272}

chrome/browser/apps/app_service/app_service_proxy_ash.cc

@@ -20,6 +20,7 @@
 #include "chrome/browser/apps/app_service/metrics/app_platform_metrics.h"
 #include "chrome/browser/apps/app_service/metrics/app_platform_metrics_service.h"
 #include "chrome/browser/apps/app_service/metrics/app_service_metrics.h"
+#include "chrome/browser/apps/app_service/package_id.h"
 #include "chrome/browser/apps/app_service/promise_apps/promise_app.h"
 #include "chrome/browser/apps/app_service/promise_apps/promise_app_registry_cache.h"
 #include "chrome/browser/apps/app_service/promise_apps/promise_app_service.h"
@@ -408,7 +409,11 @@ void AppServiceProxyAsh::OnPromiseApp(PromiseAppPtr delta) {
   if (!promise_app_service_) {
     return;
   }
-  PromiseAppService()->OnPromiseApp(std::move(delta));
+  promise_app_service_->OnPromiseApp(std::move(delta));
+}
+
+void AppServiceProxyAsh::RemovePromiseApp(const PackageId& package_id) {
+  promise_app_service_->RemovePromiseApp(package_id);
 }
 
 void AppServiceProxyAsh::RegisterShortcutPublisher(

chrome/browser/apps/app_service/app_service_proxy_ash.h

@@ -52,6 +52,7 @@ class AppPlatformMetricsService;
 class InstanceRegistryUpdater;
 class BrowserAppInstanceRegistry;
 class BrowserAppInstanceTracker;
+class PackageId;
 class PromiseAppRegistryCache;
 class PromiseAppService;
 class ShortcutPublisher;
@@ -156,6 +157,9 @@ class AppServiceProxyAsh : public AppServiceProxyBase,
   // Add or update a promise app in the Promise App Registry Cache.
   void OnPromiseApp(PromiseAppPtr delta);
 
+  // Remove all details of a promise app from the Promise App Service.
+  void RemovePromiseApp(const PackageId& package_id);
+
   // Registers `publisher` with the App Service as exclusively publishing
   // shortcut to app type `app_type`. `publisher` must have a lifetime equal to
   // or longer than this object.

chrome/browser/apps/app_service/promise_apps/promise_app.cc

@@ -11,7 +11,7 @@
 
 namespace apps {
 
-APP_ENUM_TO_STRING(PromiseStatus, kUnknown, kPending, kInstalling)
+APP_ENUM_TO_STRING(PromiseStatus, kUnknown, kPending, kInstalling, kRemove)
 
 PromiseApp::PromiseApp(const apps::PackageId& package_id)
     : package_id(package_id) {}

chrome/browser/apps/app_service/promise_apps/promise_app.h

@@ -18,6 +18,7 @@ enum class PromiseStatus {
   kUnknown,
   kPending,     // Waiting for the installation process to start.
   kInstalling,  // Installing app package.
+  kRemove,      // Marking the promise app for deletion.
 };
 
 std::string EnumToString(PromiseStatus);

chrome/browser/apps/app_service/promise_apps/promise_app_icon_cache.cc

@@ -36,6 +36,13 @@ bool PromiseAppIconCache::DoesPackageIdHaveIcons(const PackageId& package_id) {
   return icon_cache_.contains(package_id);
 }
 
+void PromiseAppIconCache::RemoveIconsForPackageId(const PackageId& package_id) {
+  if (!icon_cache_.contains(package_id)) {
+    return;
+  }
+  icon_cache_.erase(package_id);
+}
+
 std::vector<PromiseAppIcon*> PromiseAppIconCache::GetIconsForTesting(
     const PackageId& package_id) {
   std::vector<PromiseAppIcon*> icons;

chrome/browser/apps/app_service/promise_apps/promise_app_icon_cache.h

@@ -31,6 +31,9 @@ class PromiseAppIconCache {
   // Checks whether there is at least one icon for a package ID.
   bool DoesPackageIdHaveIcons(const PackageId& package_id);
 
+  // Removes the icons cached for a specified package ID.
+  void RemoveIconsForPackageId(const PackageId& package_id);
+
   // For testing only. Retrieves pointers to all the registered icons for a
   // package ID.
   std::vector<PromiseAppIcon*> GetIconsForTesting(const PackageId& package_id);

chrome/browser/apps/app_service/promise_apps/promise_app_icon_cache_unittest.cc

@@ -78,4 +78,23 @@ TEST_F(PromiseAppIconCacheTest, SaveMultipleIcons) {
                                    gfx::test::CreateBitmap(1024, 1024)));
 }
 
+TEST_F(PromiseAppIconCacheTest, RemoveIconsForPackageId) {
+  PromiseAppIconPtr icon_small = CreateIcon(/*width=*/100);
+  PromiseAppIconPtr icon_med = CreateIcon(/*width=*/200);
+  PromiseAppIconPtr icon_large = CreateIcon(/*width=*/300);
+
+  icon_cache()->SaveIcon(kTestPackageId, std::move(icon_small));
+  icon_cache()->SaveIcon(kTestPackageId, std::move(icon_med));
+  icon_cache()->SaveIcon(kTestPackageId, std::move(icon_large));
+
+  // Confirm we have 3 icons.
+  EXPECT_EQ(icon_cache()->GetIconsForTesting(kTestPackageId).size(), 3u);
+
+  // Remove all icons for package ID.
+  icon_cache()->RemoveIconsForPackageId(kTestPackageId);
+
+  // Confirm we have no icons.
+  EXPECT_EQ(icon_cache()->GetIconsForTesting(kTestPackageId).size(), 0u);
+}
+
 }  // namespace apps

chrome/browser/apps/app_service/promise_apps/promise_app_registry_cache.cc

@@ -44,7 +44,10 @@ void PromiseAppRegistryCache::OnPromiseApp(PromiseAppPtr delta) {
     observer.OnPromiseAppUpdate(PromiseAppUpdate(state, delta.get()));
   }
 
-  if (state) {
+  if (delta->status == PromiseStatus::kRemove &&
+      promise_app_map_.contains(delta->package_id)) {
+    promise_app_map_.erase(delta->package_id);
+  } else if (state) {
     // Update the existing promise app if it exists.
     PromiseAppUpdate::Merge(state, delta.get());
   } else {

chrome/browser/apps/app_service/promise_apps/promise_app_registry_cache_unittest.cc

@@ -88,6 +88,23 @@ TEST_F(PromiseAppRegistryCacheTest, GetAllPromiseApps) {
   EXPECT_EQ(promise_app_list[1]->package_id, package_id_2);
 }
 
+TEST_F(PromiseAppRegistryCacheTest, RemovePromiseApp) {
+  // Register a promise app.
+  auto promise_app = std::make_unique<PromiseApp>(kTestPackageId);
+  cache()->OnPromiseApp(std::move(promise_app));
+
+  // Confirm that the promise app is registered.
+  EXPECT_TRUE(cache()->HasPromiseApp(kTestPackageId));
+
+  // Update the promise app with a kRemove status.
+  auto delta = std::make_unique<PromiseApp>(kTestPackageId);
+  delta->status = PromiseStatus::kRemove;
+  cache()->OnPromiseApp(std::move(delta));
+
+  // Confirm that the promise app was removed.
+  EXPECT_FALSE(cache()->HasPromiseApp(kTestPackageId));
+}
+
 class PromiseAppRegistryCacheObserverTest : public testing::Test,
                                             PromiseAppRegistryCache::Observer {
  public:

chrome/browser/apps/app_service/promise_apps/promise_app_service.cc

@@ -117,6 +117,14 @@ void PromiseAppService::OnPromiseApp(PromiseAppPtr delta) {
                      weak_ptr_factory_.GetWeakPtr(), package_id));
 }
 
+void PromiseAppService::RemovePromiseApp(const PackageId& package_id) {
+  PromiseAppPtr promise_app = std::make_unique<PromiseApp>(package_id);
+  promise_app->status = PromiseStatus::kRemove;
+  promise_app->should_show = false;
+  OnPromiseApp(std::move(promise_app));
+  promise_app_icon_cache_->RemoveIconsForPackageId(package_id);
+}
+
 void PromiseAppService::SetSkipAlmanacForTesting(bool skip_almanac) {
   skip_almanac_for_testing_ = skip_almanac;
 }

chrome/browser/apps/app_service/promise_apps/promise_app_service.h

@@ -64,6 +64,10 @@ class PromiseAppService {
   // request to the Almanac API to retrieve additional promise app info.
   void OnPromiseApp(PromiseAppPtr delta);
 
+  // Remove all details about a promise app from the PromiseAppRegistryCache and
+  // PromiseAppIconCache.
+  void RemovePromiseApp(const PackageId& package_id);
+
   // Allows us to skip Almanac implementation when running unit tests that don't
   // care about Almanac responses.
   void SetSkipAlmanacForTesting(bool skip_almanac);

chrome/browser/apps/app_service/promise_apps/promise_app_service_unittest.cc

@@ -58,6 +58,13 @@ class PromiseAppServiceTest : public testing::Test,
 
   PromiseAppService* service() { return service_.get(); }
 
+  PromiseAppIconPtr CreateIcon(int width) {
+    PromiseAppIconPtr icon = std::make_unique<PromiseAppIcon>();
+    icon->icon = gfx::test::CreateBitmap(width, width);
+    icon->width_in_pixels = width;
+    return icon;
+  }
+
   // Create a data string that represents an image of the requested dimensions.
   // This is used to produce mock content for the url_loader_factory.
   std::string CreateImageString(int width) {
@@ -234,4 +241,33 @@ TEST_F(PromiseAppServiceTest, OnPromiseApp_FailedIconDownload) {
   // Icon cache should still be empty.
   EXPECT_FALSE(icon_cache()->DoesPackageIdHaveIcons(kTestPackageId));
 }
+
+TEST_F(PromiseAppServiceTest, RemovePromiseApp) {
+  // Register test promise app.
+  PromiseAppPtr promise_app = std::make_unique<PromiseApp>(kTestPackageId);
+  promise_app->should_show = true;
+  promise_app->status = PromiseStatus::kInstalling;
+  service()->OnPromiseApp(std::move(promise_app));
+
+  // Confirm that the promise app gets registered.
+  const PromiseApp* promise_app_registered =
+      cache()->GetPromiseAppForTesting(kTestPackageId);
+  EXPECT_TRUE(promise_app_registered);
+  EXPECT_TRUE(promise_app_registered->should_show.has_value());
+  EXPECT_TRUE(promise_app_registered->should_show.value());
+  EXPECT_EQ(promise_app_registered->status, PromiseStatus::kInstalling);
+
+  // Add test icons.
+  icon_cache()->SaveIcon(kTestPackageId, CreateIcon(100));
+  icon_cache()->SaveIcon(kTestPackageId, CreateIcon(200));
+  EXPECT_TRUE(icon_cache()->DoesPackageIdHaveIcons(kTestPackageId));
+
+  // Remove the promise app.
+  service()->RemovePromiseApp(kTestPackageId);
+
+  // Confirm that the promise app is now absent from the Promise App Registry
+  // and Promise App Icon Cache.
+  EXPECT_FALSE(cache()->HasPromiseApp(kTestPackageId));
+  EXPECT_FALSE(icon_cache()->DoesPackageIdHaveIcons(kTestPackageId));
+}
 }  // namespace apps