Update paused app to use PausedApps.

This is the follow up based on CL:2050217 review comments.

BUG=1051066

Change-Id: I22011a7bfded29ad0b976a648a8429be40484ebb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2053664Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741269}

chrome/browser/BUILD.gn

@@ -3791,6 +3791,8 @@ jumbo_static_library("browser") {
       "apps/app_service/icon_key_util.h",
       "apps/app_service/menu_util.cc",
       "apps/app_service/menu_util.h",
+      "apps/app_service/paused_apps.cc",
+      "apps/app_service/paused_apps.h",
       "apps/app_service/web_apps.cc",
       "apps/app_service/web_apps.h",
       "component_updater/cros_component_installer_chromeos.cc",

chrome/browser/apps/app_service/arc_apps.cc

@@ -471,22 +471,19 @@ void ArcApps::Uninstall(const std::string& app_id,
 }
 
 void ArcApps::PauseApp(const std::string& app_id) {
-  if (base::Contains(paused_apps_, app_id)) {
+  if (!paused_apps_.MaybeAddApp(app_id)) {
     return;
   }
 
-  paused_apps_.insert(app_id);
   SetIconEffect(app_id);
-
   CloseTasks(app_id);
 }
 
 void ArcApps::UnpauseApps(const std::string& app_id) {
-  if (!base::Contains(paused_apps_, app_id)) {
+  if (!paused_apps_.MaybeRemoveApp(app_id)) {
     return;
   }
 
-  paused_apps_.erase(app_id);
   SetIconEffect(app_id);
 }
 
@@ -645,7 +642,7 @@ void ArcApps::OnAppStatesChanged(const std::string& app_id,
 }
 
 void ArcApps::OnAppRemoved(const std::string& app_id) {
-  paused_apps_.erase(app_id);
+  paused_apps_.MaybeRemoveApp(app_id);
   if (base::Contains(app_id_to_task_ids_, app_id)) {
     for (int task_id : app_id_to_task_ids_[app_id]) {
       task_id_to_app_id_.erase(task_id);
@@ -897,7 +894,7 @@ apps::mojom::AppPtr ArcApps::Convert(ArcAppListPrefs* prefs,
   app->short_name = app->name;
   app->publisher_id = app_info.package_name;
 
-  auto paused = base::Contains(paused_apps_, app_id)
+  auto paused = paused_apps_.IsPaused(app_id)
                     ? apps::mojom::OptionalBool::kTrue
                     : apps::mojom::OptionalBool::kFalse;
 
@@ -989,7 +986,7 @@ void ArcApps::SetIconEffect(const std::string& app_id) {
     icon_effects =
         static_cast<IconEffects>(icon_effects | IconEffects::kBlocked);
   }
-  if (paused_apps_.find(app_id) != paused_apps_.end()) {
+  if (paused_apps_.IsPaused(app_id)) {
     icon_effects =
         static_cast<IconEffects>(icon_effects | IconEffects::kPaused);
   }

chrome/browser/apps/app_service/arc_apps.h

@@ -18,6 +18,7 @@
 #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/apps/app_service/paused_apps.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/intent_helper/arc_intent_helper_bridge.h"
@@ -148,7 +149,7 @@ class ArcApps : public KeyedService,
 
   apps_util::IncrementingIconKeyFactory icon_key_factory_;
 
-  std::set<std::string> paused_apps_;
+  PausedApps paused_apps_;
 
   AppIdToTaskIds app_id_to_task_ids_;
   TaskIdToAppId task_id_to_app_id_;

chrome/browser/apps/app_service/extension_apps.cc

@@ -584,11 +584,10 @@ void ExtensionApps::Uninstall(const std::string& app_id,
 }
 
 void ExtensionApps::PauseApp(const std::string& app_id) {
-  if (base::Contains(paused_apps_, app_id)) {
+  if (!paused_apps_.MaybeAddApp(app_id)) {
     return;
   }
 
-  paused_apps_.insert(app_id);
   SetIconEffect(app_id);
 
   if (instance_registry_->GetWindows(app_id).empty()) {
@@ -622,11 +621,10 @@ void ExtensionApps::PauseApp(const std::string& app_id) {
 }
 
 void ExtensionApps::UnpauseApps(const std::string& app_id) {
-  if (!base::Contains(paused_apps_, app_id)) {
+  if (!paused_apps_.MaybeRemoveApp(app_id)) {
     return;
   }
 
-  paused_apps_.erase(app_id);
   SetIconEffect(app_id);
 
   for (auto* browser : *BrowserList::GetInstance()) {
@@ -940,7 +938,7 @@ void ExtensionApps::OnExtensionUninstalled(
   }
 
   enable_flow_map_.erase(extension->id());
-  paused_apps_.erase(extension->id());
+  paused_apps_.MaybeRemoveApp(extension->id());
 
   // Construct an App with only the information required to identify an
   // uninstallation.
@@ -1179,7 +1177,10 @@ 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(GetIconEffects(extension));
+
+  bool paused = paused_apps_.IsPaused(extension->id());
+  app->icon_key =
+      icon_key_factory_.MakeIconKey(GetIconEffects(extension, paused));
 
   if (profile_) {
     auto* prefs = extensions::ExtensionPrefs::Get(profile_);
@@ -1202,9 +1203,8 @@ apps::mojom::AppPtr ExtensionApps::Convert(
                              : apps::mojom::OptionalBool::kFalse;
   app->recommendable = apps::mojom::OptionalBool::kTrue;
   app->searchable = apps::mojom::OptionalBool::kTrue;
-  app->paused = base::Contains(paused_apps_, extension->id())
-                    ? apps::mojom::OptionalBool::kTrue
-                    : apps::mojom::OptionalBool::kFalse;
+  app->paused = paused ? apps::mojom::OptionalBool::kTrue
+                       : apps::mojom::OptionalBool::kFalse;
   SetShowInFields(app, extension, profile_);
 
   // Get the intent filters for PWAs.
@@ -1252,7 +1252,8 @@ bool ExtensionApps::RunExtensionEnableFlow(
 }
 
 IconEffects ExtensionApps::GetIconEffects(
-    const extensions::Extension* extension) {
+    const extensions::Extension* extension,
+    bool paused) {
   IconEffects icon_effects = IconEffects::kNone;
 #if defined(OS_CHROMEOS)
   icon_effects =
@@ -1270,7 +1271,7 @@ IconEffects ExtensionApps::GetIconEffects(
     icon_effects =
         static_cast<IconEffects>(icon_effects | IconEffects::kRoundCorners);
   }
-  if (base::Contains(paused_apps_, extension->id())) {
+  if (paused) {
     icon_effects =
         static_cast<IconEffects>(icon_effects | IconEffects::kPaused);
   }
@@ -1300,7 +1301,8 @@ void ExtensionApps::SetIconEffect(const std::string& app_id) {
   apps::mojom::AppPtr app = apps::mojom::App::New();
   app->app_type = app_type_;
   app->app_id = app_id;
-  app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffects(extension));
+  app->icon_key = icon_key_factory_.MakeIconKey(
+      GetIconEffects(extension, paused_apps_.IsPaused(app_id)));
   Publish(std::move(app));
 }
 

chrome/browser/apps/app_service/extension_apps.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_APPS_APP_SERVICE_EXTENSION_APPS_H_
 
 #include <map>
-#include <set>
 #include <string>
 
 #include "base/macros.h"
@@ -14,6 +13,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/apps/app_service/paused_apps.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "chrome/services/app_service/public/cpp/instance.h"
 #include "chrome/services/app_service/public/cpp/instance_registry.h"
@@ -188,7 +188,8 @@ class ExtensionApps : public apps::mojom::Publisher,
                      std::vector<apps::mojom::AppPtr>* apps_out);
 
   // Calculate the icon effects for the extension.
-  IconEffects GetIconEffects(const extensions::Extension* extension);
+  IconEffects GetIconEffects(const extensions::Extension* extension,
+                             bool paused);
 
   // 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
@@ -229,7 +230,7 @@ class ExtensionApps : public apps::mojom::Publisher,
   using EnableFlowPtr = std::unique_ptr<ExtensionAppsEnableFlow>;
   std::map<std::string, EnableFlowPtr> enable_flow_map_;
 
-  std::set<std::string> paused_apps_;
+  PausedApps paused_apps_;
 
   std::map<extensions::AppWindow*, aura::Window*> app_window_to_aura_window_;
 

chrome/browser/apps/app_service/paused_apps.cc

+// Copyright (c) 2020 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/paused_apps.h"
+
+#include "base/stl_util.h"
+
+namespace apps {
+
+PausedApps::PausedApps() = default;
+
+PausedApps::~PausedApps() = default;
+
+bool PausedApps::MaybeAddApp(const std::string& app_id) {
+  auto ret = paused_apps_.insert(app_id);
+  return ret.second;
+}
+
+bool PausedApps::MaybeRemoveApp(const std::string& app_id) {
+  return paused_apps_.erase(app_id) != 0;
+}
+
+bool PausedApps::IsPaused(const std::string& app_id) {
+  return base::Contains(paused_apps_, app_id);
+}
+
+}  // namespace apps

chrome/browser/apps/app_service/paused_apps.h

+// Copyright (c) 2020 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_PAUSED_APPS_H_
+#define CHROME_BROWSER_APPS_APP_SERVICE_PAUSED_APPS_H_
+
+#include <set>
+#include <string>
+
+namespace apps {
+
+// PausedApps saves apps which have been paused for a specific publisher. The
+// paused app can't be launched, until it is unpaused.
+class PausedApps {
+ public:
+  PausedApps();
+  ~PausedApps();
+
+  PausedApps(const PausedApps&) = delete;
+  PausedApps& operator=(const PausedApps&) = delete;
+
+  // Returns true if the app was added to the paused set, and false if it was
+  // already there.
+  bool MaybeAddApp(const std::string& app_id);
+
+  // Returns true if the app was removed from the paused set, and false if it
+  // wasn't there.
+  bool MaybeRemoveApp(const std::string& app_id);
+
+  bool IsPaused(const std::string& app_id);
+
+ private:
+  std::set<std::string> paused_apps_;
+};
+
+}  // namespace apps
+
+#endif  // CHROME_BROWSER_APPS_APP_SERVICE_PAUSED_APPS_H_

chrome/browser/apps/app_service/web_apps.cc

@@ -342,11 +342,10 @@ void WebApps::Uninstall(const std::string& app_id,
 }
 
 void WebApps::PauseApp(const std::string& app_id) {
-  if (base::Contains(paused_apps_, app_id)) {
+  if (!paused_apps_.MaybeAddApp(app_id)) {
     return;
   }
 
-  paused_apps_.insert(app_id);
   SetIconEffect(app_id);
 
   for (auto* browser : *BrowserList::GetInstance()) {
@@ -360,11 +359,10 @@ void WebApps::PauseApp(const std::string& app_id) {
 }
 
 void WebApps::UnpauseApps(const std::string& app_id) {
-  if (!base::Contains(paused_apps_, app_id)) {
+  if (!paused_apps_.MaybeRemoveApp(app_id)) {
     return;
   }
 
-  paused_apps_.erase(app_id);
   SetIconEffect(app_id);
 }
 
@@ -465,7 +463,7 @@ void WebApps::OnWebAppWillBeUninstalled(const web_app::AppId& app_id) {
     return;
   }
 
-  paused_apps_.erase(web_app->app_id());
+  paused_apps_.MaybeRemoveApp(app_id);
 
   // Construct an App with only the information required to identify an
   // uninstallation.
@@ -591,7 +589,10 @@ apps::mojom::AppPtr WebApps::Convert(const web_app::WebApp* web_app,
   app->name = web_app->name();
   app->short_name = web_app->name();
   app->description = web_app->description();
-  app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffects(web_app));
+
+  bool paused = paused_apps_.IsPaused(web_app->app_id());
+  app->icon_key =
+      icon_key_factory_.MakeIconKey(GetIconEffects(web_app, paused));
   // app->version is left empty here.
   // TODO(loyso): Populate app->last_launch_time and app->install_time.
 
@@ -602,9 +603,8 @@ apps::mojom::AppPtr WebApps::Convert(const web_app::WebApp* web_app,
   app->is_platform_app = apps::mojom::OptionalBool::kFalse;
   app->recommendable = apps::mojom::OptionalBool::kTrue;
   app->searchable = apps::mojom::OptionalBool::kTrue;
-  app->paused = base::Contains(paused_apps_, web_app->app_id())
-                    ? apps::mojom::OptionalBool::kTrue
-                    : apps::mojom::OptionalBool::kFalse;
+  app->paused = paused ? apps::mojom::OptionalBool::kTrue
+                       : apps::mojom::OptionalBool::kFalse;
   SetShowInFields(app, web_app);
 
   // Get the intent filters for PWAs.
@@ -634,7 +634,8 @@ void WebApps::StartPublishingWebApps(
   subscribers_.Add(std::move(subscriber));
 }
 
-IconEffects WebApps::GetIconEffects(const web_app::WebApp* web_app) {
+IconEffects WebApps::GetIconEffects(const web_app::WebApp* web_app,
+                                    bool paused) {
   IconEffects icon_effects = IconEffects::kNone;
 #if defined(OS_CHROMEOS)
   icon_effects =
@@ -651,7 +652,7 @@ IconEffects WebApps::GetIconEffects(const web_app::WebApp* web_app) {
   }
   icon_effects =
       static_cast<IconEffects>(icon_effects | IconEffects::kRoundCorners);
-  if (base::Contains(paused_apps_, web_app->app_id())) {
+  if (paused) {
     icon_effects =
         static_cast<IconEffects>(icon_effects | IconEffects::kPaused);
   }
@@ -678,7 +679,8 @@ void WebApps::SetIconEffect(const std::string& app_id) {
   apps::mojom::AppPtr app = apps::mojom::App::New();
   app->app_type = apps::mojom::AppType::kWeb;
   app->app_id = app_id;
-  app->icon_key = icon_key_factory_.MakeIconKey(GetIconEffects(web_app));
+  app->icon_key = icon_key_factory_.MakeIconKey(
+      GetIconEffects(web_app, paused_apps_.IsPaused(app_id)));
   Publish(std::move(app));
 }
 

chrome/browser/apps/app_service/web_apps.h

@@ -5,13 +5,13 @@
 #ifndef CHROME_BROWSER_APPS_APP_SERVICE_WEB_APPS_H_
 #define CHROME_BROWSER_APPS_APP_SERVICE_WEB_APPS_H_
 
-#include <set>
 #include <string>
 
 #include "base/memory/weak_ptr.h"
 #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/apps/app_service/paused_apps.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/app_registrar_observer.h"
@@ -127,7 +127,7 @@ class WebApps : public apps::mojom::Publisher,
   void StartPublishingWebApps(
       mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote);
 
-  IconEffects GetIconEffects(const web_app::WebApp* web_app);
+  IconEffects GetIconEffects(const web_app::WebApp* web_app, bool paused);
 
   // 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
@@ -152,7 +152,7 @@ class WebApps : public apps::mojom::Publisher,
 
   apps_util::IncrementingIconKeyFactory icon_key_factory_;
 
-  std::set<std::string> paused_apps_;
+  PausedApps paused_apps_;
 
   web_app::WebAppProvider* provider_ = nullptr;