Add test for StopApp and make PauseApp use same "close app" logic.

- Make PauseApp, UnpauseApp, IsPaused, PausedApps CrOS-only (they were
  used in CrOS only). StopApp is already CrOS-only and is seemingly used
  in both Ash (for SWAs) and Lacros (for PWAs), at least in tests.
- Make PauseApp and StopApp use BrowserAppInstanceTracker if available
  (when CrosApi is enabled) and otherwise just close app windows
  directly using WebAppUiManager. This makes their behaviour
  consistent and can be simplified once CrosApi is always enabled.
- Fix includes in lacros_web_apps_controller_browsertest.cc per IWYU.
- Remove the need for FRIEND_TEST_ALL_PREFIXES by upcasting instead.
- Add a few TestFutures when I touched code nearby.


Change-Id: I1b95e000abec37ed43d40fef2a224979fdb590bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4780368
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1185067}

chrome/browser/apps/app_service/app_service_proxy_lacros.cc

@@ -427,6 +427,12 @@ void AppServiceProxyLacros::SetWebsiteMetricsServiceForTesting(
   metrics_service_ = std::move(website_metrics_service);
 }
 
+void AppServiceProxyLacros::SetBrowserAppInstanceTrackerForTesting(
+    std::unique_ptr<apps::BrowserAppInstanceTracker>
+        browser_app_instance_tracker) {
+  browser_app_instance_tracker_ = std::move(browser_app_instance_tracker);
+}
+
 crosapi::mojom::AppServiceSubscriber*
 AppServiceProxyLacros::AsAppServiceSubscriberForTesting() {
   return this;

chrome/browser/apps/app_service/app_service_proxy_lacros.h

@@ -241,6 +241,10 @@ class AppServiceProxyLacros : public KeyedService,
       std::unique_ptr<apps::WebsiteMetricsServiceLacros>
           website_metrics_service);
 
+  void SetBrowserAppInstanceTrackerForTesting(
+      std::unique_ptr<apps::BrowserAppInstanceTracker>
+          browser_app_instance_tracker);
+
   // Exposes AppServiceSubscriber methods to allow tests to fake calls that
   // would normally come from Ash via the mojo interface.
   crosapi::mojom::AppServiceSubscriber* AsAppServiceSubscriberForTesting();

chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc

@@ -190,6 +190,17 @@ size_t WebAppUiManagerImpl::GetNumWindowsForApp(const AppId& app_id) {
   return it->second;
 }
 
+void WebAppUiManagerImpl::CloseAppWindows(const AppId& app_id) {
+  DCHECK(started_);
+
+  for (auto* browser : *BrowserList::GetInstance()) {
+    const AppBrowserController* app_controller = browser->app_controller();
+    if (app_controller && app_controller->app_id() == app_id) {
+      browser->window()->Close();
+    }
+  }
+}
+
 void WebAppUiManagerImpl::NotifyOnAllAppWindowsClosed(
     const AppId& app_id,
     base::OnceClosure callback) {

chrome/browser/ui/web_applications/web_app_ui_manager_impl.h

@@ -54,6 +54,7 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
   // WebAppUiManager:
   WebAppUiManagerImpl* AsImpl() override;
   size_t GetNumWindowsForApp(const AppId& app_id) override;
+  void CloseAppWindows(const AppId& app_id) override;
   void NotifyOnAllAppWindowsClosed(const AppId& app_id,
                                    base::OnceClosure callback) override;
   bool CanAddAppToQuickLaunchBar() const override;

chrome/browser/web_applications/app_service/lacros_web_apps_controller.h

@@ -59,15 +59,6 @@ class LacrosWebAppsController : public crosapi::mojom::AppController,
   void SetPublisherForTesting(crosapi::mojom::AppPublisher* publisher);
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(LacrosWebAppsControllerBrowserTest,
-                           ExecuteContextMenuCommand);
-  FRIEND_TEST_ALL_PREFIXES(LacrosWebAppsControllerBrowserTest, PauseUnpause);
-  FRIEND_TEST_ALL_PREFIXES(LacrosWebAppsControllerBrowserTest,
-                           OpenNativeSettings);
-  FRIEND_TEST_ALL_PREFIXES(LacrosWebAppsControllerBrowserTest, WindowMode);
-  FRIEND_TEST_ALL_PREFIXES(LacrosWebAppsControllerBrowserTest, Launch);
-  FRIEND_TEST_ALL_PREFIXES(LacrosWebAppsControllerBrowserTest, LaunchWithFiles);
-
   void OnReady();
   void ExecuteContextMenuCommandInternal(
       const std::string& app_id,

chrome/browser/web_applications/app_service/web_app_publisher_helper.cc

@@ -700,7 +700,12 @@ apps::AppPtr WebAppPublisherHelper::CreateWebApp(const WebApp* web_app) {
 #endif
 
   app->allow_uninstall = web_app->CanUserUninstallWebApp();
+
+#if BUILDFLAG(IS_CHROMEOS)
   app->paused = IsPaused(web_app->app_id());
+#else
+  app->paused = false;
+#endif
 
   // Add the intent filters for PWAs.
   base::Extend(app->intent_filters,
@@ -821,38 +826,51 @@ void WebAppPublisherHelper::SetIconEffect(const std::string& app_id) {
   delegate_->PublishWebApp(std::move(app));
 }
 
+#if BUILDFLAG(IS_CHROMEOS)
 void WebAppPublisherHelper::PauseApp(const std::string& app_id) {
+  if (IsShuttingDown()) {
+    return;
+  }
+
   if (paused_apps_.MaybeAddApp(app_id)) {
     SetIconEffect(app_id);
   }
 
-  constexpr bool kPaused = true;
-  delegate_->PublishWebApp(
-      paused_apps_.CreateAppWithPauseStatus(app_type(), app_id, kPaused));
+  if (!IsWebAppsCrosapiEnabled()) {
+    provider_->ui_manager().CloseAppWindows(app_id);
+  } else {
+    CHECK(apps::AppServiceProxyFactory::IsAppServiceAvailableForProfile(
+        profile_));
 
-  for (auto* browser : *BrowserList::GetInstance()) {
-    if (!browser->is_type_app()) {
-      continue;
-    }
-    if (GetAppIdFromApplicationName(browser->app_name()) == app_id) {
-      browser->tab_strip_model()->CloseAllTabs();
-    }
+    apps::BrowserAppInstanceTracker* instance_tracker =
+        apps::AppServiceProxyFactory::GetForProfile(profile_)
+            ->BrowserAppInstanceTracker();
+    CHECK(instance_tracker);
+
+    instance_tracker->StopInstancesOfApp(app_id);
   }
+
+  delegate_->PublishWebApp(paused_apps_.CreateAppWithPauseStatus(
+      app_type(), app_id, /*paused=*/true));
 }
 
 void WebAppPublisherHelper::UnpauseApp(const std::string& app_id) {
+  if (IsShuttingDown()) {
+    return;
+  }
+
   if (paused_apps_.MaybeRemoveApp(app_id)) {
     SetIconEffect(app_id);
   }
 
-  constexpr bool kPaused = false;
-  delegate_->PublishWebApp(
-      paused_apps_.CreateAppWithPauseStatus(app_type(), app_id, kPaused));
+  delegate_->PublishWebApp(paused_apps_.CreateAppWithPauseStatus(
+      app_type(), app_id, /*paused=*/false));
 }
 
 bool WebAppPublisherHelper::IsPaused(const std::string& app_id) {
   return paused_apps_.IsPaused(app_id);
 }
+#endif  // BUILDFLAG(IS_CHROMEOS)
 
 void WebAppPublisherHelper::LoadIcon(const std::string& app_id,
                                      apps::IconType icon_type,
@@ -1142,16 +1160,21 @@ void WebAppPublisherHelper::StopApp(const std::string& app_id) {
   }
 
   if (!IsWebAppsCrosapiEnabled()) {
+    provider_->ui_manager().CloseAppWindows(app_id);
     return;
   }
 
+  CHECK(
+      apps::AppServiceProxyFactory::IsAppServiceAvailableForProfile(profile_));
+
   apps::BrowserAppInstanceTracker* instance_tracker =
       apps::AppServiceProxyFactory::GetForProfile(profile_)
           ->BrowserAppInstanceTracker();
+  CHECK(instance_tracker);
 
   instance_tracker->StopInstancesOfApp(app_id);
 }
-#endif
+#endif  // BUILDFLAG(IS_CHROMEOS)
 
 void WebAppPublisherHelper::OpenNativeSettings(const std::string& app_id) {
   if (IsShuttingDown()) {
@@ -1352,9 +1375,10 @@ void WebAppPublisherHelper::OnWebAppManifestUpdated(
 void WebAppPublisherHelper::OnWebAppUninstalled(
     const AppId& app_id,
     webapps::WebappUninstallSource uninstall_source) {
-  paused_apps_.MaybeRemoveApp(app_id);
 
 #if BUILDFLAG(IS_CHROMEOS)
+  paused_apps_.MaybeRemoveApp(app_id);
+
   app_notifications_.RemoveNotificationsForApp(app_id);
 
   auto result = media_requests_.RemoveRequests(app_id);
@@ -1590,9 +1614,11 @@ IconEffects WebAppPublisherHelper::GetIconEffects(const WebApp* web_app) {
   icon_effects |= web_app->is_generated_icon() ? IconEffects::kCrOsStandardMask
                                                : IconEffects::kCrOsStandardIcon;
 
+#if BUILDFLAG(IS_CHROMEOS)
   if (IsPaused(web_app->app_id())) {
     icon_effects |= IconEffects::kPaused;
   }
+#endif
 
   bool is_disabled = false;
   if (web_app->chromeos_data().has_value()) {

chrome/browser/web_applications/app_service/web_app_publisher_helper.h

@@ -22,7 +22,6 @@
 #include "chrome/browser/apps/app_service/app_icon/icon_effects.h"
 #include "chrome/browser/apps/app_service/app_icon/icon_key_util.h"
 #include "chrome/browser/apps/app_service/launch_result_type.h"
-#include "chrome/browser/apps/app_service/paused_apps.h"
 #include "chrome/browser/web_applications/mojom/user_display_mode.mojom.h"
 #include "chrome/browser/web_applications/web_app_constants.h"
 #include "chrome/browser/web_applications/web_app_id.h"
@@ -50,6 +49,7 @@
 #if BUILDFLAG(IS_CHROMEOS)
 #include "chrome/browser/apps/app_service/app_notifications.h"
 #include "chrome/browser/apps/app_service/media_requests.h"
+#include "chrome/browser/apps/app_service/paused_apps.h"
 #include "chrome/browser/badging/badge_manager_delegate.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/notifications/notification_common.h"
@@ -183,11 +183,13 @@ class WebAppPublisherHelper : public WebAppRegistrarObserver,
 
   void SetIconEffect(const std::string& app_id);
 
+#if BUILDFLAG(IS_CHROMEOS)
   void PauseApp(const std::string& app_id);
 
   void UnpauseApp(const std::string& app_id);
 
   bool IsPaused(const std::string& app_id);
+#endif  // BUILDFLAG(IS_CHROMEOS)
 
   void LoadIcon(const std::string& app_id,
                 apps::IconType icon_type,
@@ -437,9 +439,9 @@ class WebAppPublisherHelper : public WebAppRegistrarObserver,
 
   apps_util::IncrementingIconKeyFactory icon_key_factory_;
 
+#if BUILDFLAG(IS_CHROMEOS)
   apps::PausedApps paused_apps_;
 
-#if BUILDFLAG(IS_CHROMEOS)
   base::ScopedObservation<NotificationDisplayService,
                           NotificationDisplayService::Observer>
       notification_display_service_{this};
@@ -453,7 +455,7 @@ class WebAppPublisherHelper : public WebAppRegistrarObserver,
       media_indicator_observation_{this};
 
   apps::MediaRequests media_requests_;
-#endif
+#endif  // BUILDFLAG(IS_CHROMEOS)
 
   std::map<std::string, WebAppShortcutsMenuItemInfo> shortcut_id_map_;
   ShortcutId::Generator shortcut_id_generator_;

chrome/browser/web_applications/test/fake_web_app_ui_manager.h

@@ -40,6 +40,7 @@ class FakeWebAppUiManager : public WebAppUiManager {
   // WebAppUiManager:
   WebAppUiManagerImpl* AsImpl() override;
   size_t GetNumWindowsForApp(const AppId& app_id) override;
+  void CloseAppWindows(const AppId& app_id) override {}
   void NotifyOnAllAppWindowsClosed(const AppId& app_id,
                                    base::OnceClosure callback) override;
   bool CanAddAppToQuickLaunchBar() const override;

chrome/browser/web_applications/web_app_ui_manager.h

@@ -110,6 +110,9 @@ class WebAppUiManager {
 
   virtual size_t GetNumWindowsForApp(const AppId& app_id) = 0;
 
+  // Close app windows. Does not affect tabs in a non-app browser.
+  virtual void CloseAppWindows(const AppId& app_id) = 0;
+
   virtual void NotifyOnAllAppWindowsClosed(const AppId& app_id,
                                            base::OnceClosure callback) = 0;