AppService: Clean up ExtensionApps class. It manages kExtension apps only.

Previous cleanup CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2520476

- app_type() and app_type_ is always kExtension.
- Exclude from_bookmark() extensions.

Bug: 1065748
Change-Id: I39301a2537c2e77633ef85067f6bd81f9adacecc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522831
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827499}

chrome/browser/apps/app_service/app_service_proxy.cc

@@ -175,8 +175,7 @@ void AppServiceProxy::Initialize() {
     }
     crostini_apps_ = std::make_unique<CrostiniApps>(app_service_, profile_);
     extension_apps_ = std::make_unique<ExtensionAppsChromeOs>(
-        app_service_, profile_, apps::mojom::AppType::kExtension,
-        &instance_registry_);
+        app_service_, profile_, &instance_registry_);
     if (!g_omit_plugin_vm_apps_for_testing_) {
       plugin_vm_apps_ = std::make_unique<PluginVmApps>(app_service_, profile_);
     }
@@ -193,8 +192,7 @@ void AppServiceProxy::Initialize() {
                                                   &instance_registry_);
 #else
     web_apps_ = std::make_unique<WebApps>(app_service_, profile_);
-    extension_apps_ = std::make_unique<ExtensionApps>(
-        app_service_, profile_, apps::mojom::AppType::kExtension);
+    extension_apps_ = std::make_unique<ExtensionApps>(app_service_, profile_);
 #endif
 
     // Asynchronously add app icon source, so we don't do too much work in the

chrome/browser/apps/app_service/app_service_proxy.h

@@ -443,8 +443,6 @@ class AppServiceProxy : public KeyedService,
                                     base::UniquePtrComparator>;
   UninstallDialogs uninstall_dialogs_;
 #else
-  // TODO(crbug.com/877898): Erase extension_web_apps_ when BMO is on.
-  std::unique_ptr<ExtensionApps> extension_web_apps_;
   std::unique_ptr<WebApps> web_apps_;
   std::unique_ptr<ExtensionApps> extension_apps_;
 #endif

chrome/browser/apps/app_service/extension_apps.cc

@@ -12,9 +12,8 @@ namespace apps {
 
 ExtensionApps::ExtensionApps(
     const mojo::Remote<apps::mojom::AppService>& app_service,
-    Profile* profile,
-    apps::mojom::AppType app_type)
-    : ExtensionAppsBase(app_service, profile, app_type) {}
+    Profile* profile)
+    : ExtensionAppsBase(app_service, profile) {}
 
 ExtensionApps::~ExtensionApps() = default;
 
@@ -34,15 +33,7 @@ bool ExtensionApps::Accepts(const extensions::Extension* extension) {
     return false;
   }
 
-  switch (app_type()) {
-    case apps::mojom::AppType::kExtension:
   return !extension->from_bookmark();
-    case apps::mojom::AppType::kWeb:
-      return extension->from_bookmark();
-    default:
-      NOTREACHED();
-      return false;
-  }
 }
 
 bool ExtensionApps::ShouldShownInLauncher(

chrome/browser/apps/app_service/extension_apps.h

@@ -32,8 +32,7 @@ namespace apps {
 class ExtensionApps : public apps::ExtensionAppsBase {
  public:
   ExtensionApps(const mojo::Remote<apps::mojom::AppService>& app_service,
-                Profile* profile,
-                apps::mojom::AppType app_type);
+                Profile* profile);
   ~ExtensionApps() override;
 
   ExtensionApps(const ExtensionApps&) = delete;

chrome/browser/apps/app_service/extension_apps_base.h

@@ -14,12 +14,6 @@
 #include "chrome/browser/apps/app_service/app_icon_factory.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/app_service/icon_key_util.h"
-#include "chrome/browser/ui/web_applications/web_app_launch_manager.h"
-#include "chrome/browser/web_applications/components/app_registrar.h"
-#include "chrome/browser/web_applications/components/app_registrar_observer.h"
-#include "chrome/browser/web_applications/components/web_app_id.h"
-#include "components/content_settings/core/browser/content_settings_observer.h"
-#include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/services/app_service/public/cpp/publisher_base.h"
 #include "components/services/app_service/public/mojom/app_service.mojom.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
@@ -53,13 +47,10 @@ class ExtensionAppsEnableFlow;
 // See components/services/app_service/README.md.
 class ExtensionAppsBase : public apps::PublisherBase,
                           public extensions::ExtensionPrefsObserver,
-                          public extensions::ExtensionRegistryObserver,
-                          public content_settings::Observer,
-                          public web_app::AppRegistrarObserver {
+                          public extensions::ExtensionRegistryObserver {
  public:
   ExtensionAppsBase(const mojo::Remote<apps::mojom::AppService>& app_service,
-                    Profile* profile,
-                    apps::mojom::AppType app_type);
+                    Profile* profile);
   ~ExtensionAppsBase() override;
 
   ExtensionAppsBase(const ExtensionAppsBase&) = delete;
@@ -103,8 +94,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
 
   Profile* profile() const { return profile_; }
 
-  apps::mojom::AppType app_type() { return app_type_; }
-
   base::WeakPtr<ExtensionAppsBase> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
@@ -143,19 +132,12 @@ class ExtensionAppsBase : public apps::PublisherBase,
                            apps::mojom::IntentPtr intent,
                            apps::mojom::LaunchSource launch_source,
                            int64_t display_id) override;
-  void SetPermission(const std::string& app_id,
-                     apps::mojom::PermissionPtr permission) override;
   void Uninstall(const std::string& app_id,
                  apps::mojom::UninstallSource uninstall_source,
                  bool clear_site_data,
                  bool report_abuse) override;
   void OpenNativeSettings(const std::string& app_id) override;
 
-  // content_settings::Observer overrides.
-  void OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,
-                               const ContentSettingsPattern& secondary_pattern,
-                               ContentSettingsType content_type) override;
-
   // extensions::ExtensionPrefsObserver overrides.
   void OnExtensionLastLaunchTimeChanged(
       const std::string& app_id,
@@ -163,11 +145,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
   void OnExtensionPrefsWillBeDestroyed(
       extensions::ExtensionPrefs* prefs) override;
 
-  // web_app::AppRegistrarObserver:
-  void OnAppRegistrarDestroyed() override;
-  void OnWebAppLocallyInstalledStateChanged(const web_app::AppId& app_id,
-                                            bool is_locally_installed) override;
-
   // extensions::ExtensionRegistryObserver overrides.
   void OnExtensionLoaded(content::BrowserContext* browser_context,
                          const extensions::Extension* extension) override;
@@ -178,9 +155,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
                             const extensions::Extension* extension,
                             bool is_update) override;
 
-  // Function called when SystemWebAppManager::on_apps_synchronized() runs.
-  void OnSystemWebAppsInstalled();
-
   // Checks if extension is disabled and if enable flow should be started.
   // Returns true if extension enable flow is started or there is already one
   // running, and run |callback| to launch the app.
@@ -194,8 +168,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
   static bool ShouldShow(const extensions::Extension* extension,
                          Profile* profile);
 
-  void PopulatePermissions(const extensions::Extension* extension,
-                           std::vector<mojom::PermissionPtr>* target);
   void PopulateIntentFilters(const base::Optional<GURL>& app_scope,
                              std::vector<mojom::IntentFilterPtr>* target);
   virtual apps::mojom::AppPtr Convert(const extensions::Extension* extension,
@@ -209,8 +181,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
 
   Profile* const profile_;
 
-  const apps::mojom::AppType app_type_;
-
   apps_util::IncrementingIconKeyFactory icon_key_factory_;
 
   ScopedObserver<extensions::ExtensionPrefs, extensions::ExtensionPrefsObserver>
@@ -218,10 +188,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
   ScopedObserver<extensions::ExtensionRegistry,
                  extensions::ExtensionRegistryObserver>
       registry_observer_{this};
-  ScopedObserver<HostContentSettingsMap, content_settings::Observer>
-      content_settings_observer_{this};
-  ScopedObserver<web_app::AppRegistrar, web_app::AppRegistrarObserver>
-      app_registrar_observer_{this};
 
   using EnableFlowPtr = std::unique_ptr<ExtensionAppsEnableFlow>;
   std::map<std::string, EnableFlowPtr> enable_flow_map_;
@@ -229,9 +195,6 @@ class ExtensionAppsBase : public apps::PublisherBase,
   // app_service_ is owned by the object that owns this object.
   apps::mojom::AppService* app_service_;
 
-  // TODO(crbug.com/1061843): Remove web_app_launch_manager_ when BMO launches.
-  std::unique_ptr<web_app::WebAppLaunchManager> web_app_launch_manager_;
-
   base::WeakPtrFactory<ExtensionAppsBase> weak_factory_{this};
 };
 

chrome/browser/apps/app_service/extension_apps_chromeos.cc

@@ -43,13 +43,6 @@
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h"
 #include "chrome/browser/ui/ash/session_controller_client_impl.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_list.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/web_applications/components/web_app_helpers.h"
-#include "chrome/browser/web_applications/system_web_app_manager.h"
-#include "chrome/browser/web_applications/web_app_provider.h"
-#include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_metrics.h"
@@ -95,9 +88,8 @@ namespace apps {
 ExtensionAppsChromeOs::ExtensionAppsChromeOs(
     const mojo::Remote<apps::mojom::AppService>& app_service,
     Profile* profile,
-    apps::mojom::AppType app_type,
     apps::InstanceRegistry* instance_registry)
-    : ExtensionAppsBase(app_service, profile, app_type),
+    : ExtensionAppsBase(app_service, profile),
       instance_registry_(instance_registry) {
   DCHECK(instance_registry_);
   Initialize();
@@ -163,11 +155,6 @@ void ExtensionAppsChromeOs::Initialize() {
   notification_display_service_.Add(
       NotificationDisplayServiceFactory::GetForProfile(profile()));
 
-  // Remaining initialization is only relevant to the kExtension app type.
-  if (app_type() != apps::mojom::AppType::kExtension) {
-    return;
-  }
-
   profile_pref_change_registrar_.Init(profile()->GetPrefs());
   profile_pref_change_registrar_.Add(
       prefs::kHideWebStoreIcon,
@@ -207,33 +194,14 @@ void ExtensionAppsChromeOs::PauseApp(const std::string& app_id) {
   }
 
   constexpr bool kPaused = true;
-  Publish(paused_apps_.GetAppWithPauseStatus(app_type(), app_id, kPaused),
+  Publish(paused_apps_.GetAppWithPauseStatus(apps::mojom::AppType::kExtension,
+                                             app_id, kPaused),
           subscribers());
 
   if (instance_registry_->GetWindows(app_id).empty()) {
     return;
   }
 
-  // For Web apps that are opened in app windows, close all tabs to close the
-  // opened window, otherwise, show pause information in browsers.
-  bool is_web_app = false;
-  for (auto* browser : *BrowserList::GetInstance()) {
-    if (!browser->is_type_app()) {
-      continue;
-    }
-    if (web_app::GetAppIdFromApplicationName(browser->app_name()) == app_id) {
-      TabStripModel* tab_strip = browser->tab_strip_model();
-      tab_strip->CloseAllTabs();
-      is_web_app = true;
-    }
-  }
-
-  // For web apps that are opened in tabs, PauseApp() should be
-  // called with Chrome's app_id to show pause information in browsers.
-  if (is_web_app) {
-    return;
-  }
-
   chromeos::app_time::AppTimeLimitInterface* app_limit =
       chromeos::app_time::AppTimeLimitInterface::Get(profile());
   DCHECK(app_limit);
@@ -246,18 +214,10 @@ void ExtensionAppsChromeOs::UnpauseApps(const std::string& app_id) {
   }
 
   constexpr bool kPaused = false;
-  Publish(paused_apps_.GetAppWithPauseStatus(app_type(), app_id, kPaused),
+  Publish(paused_apps_.GetAppWithPauseStatus(apps::mojom::AppType::kExtension,
+                                             app_id, kPaused),
           subscribers());
 
-  for (auto* browser : *BrowserList::GetInstance()) {
-    if (!browser->is_type_app()) {
-      continue;
-    }
-    if (web_app::GetAppIdFromApplicationName(browser->app_name()) == app_id) {
-      return;
-    }
-  }
-
   chromeos::app_time::AppTimeLimitInterface* app_time =
       chromeos::app_time::AppTimeLimitInterface::Get(profile());
   DCHECK(app_time);
@@ -281,11 +241,7 @@ void ExtensionAppsChromeOs::GetMenuModel(const std::string& app_id,
   }
 
   bool is_platform_app = extension->is_platform_app();
-  bool is_system_web_app = web_app::WebAppProvider::Get(profile())
-                               ->system_web_app_manager()
-                               .IsSystemWebApp(app_id);
-
-  if (!is_platform_app && !is_system_web_app) {
+  if (!is_platform_app) {
     CreateOpenNewSubmenu(
         menu_type,
         extensions::GetLaunchType(extensions::ExtensionPrefs::Get(profile()),
@@ -315,7 +271,7 @@ void ExtensionAppsChromeOs::GetMenuModel(const std::string& app_id,
     AddCommandItem(ash::UNINSTALL, IDS_APP_LIST_UNINSTALL_ITEM, &menu_items);
   }
 
-  if (!is_system_web_app && extension->ShouldDisplayInAppLauncher()) {
+  if (extension->ShouldDisplayInAppLauncher()) {
     AddCommandItem(ash::SHOW_APP_INFO, IDS_APP_CONTEXT_MENU_SHOW_INFO,
                    &menu_items);
   }
@@ -447,7 +403,8 @@ void ExtensionAppsChromeOs::OnNotificationClosed(
   app_notifications_.RemoveNotification(notification_id);
 
   for (const auto& app_id : app_ids) {
-    Publish(app_notifications_.GetAppWithHasBadgeStatus(app_type(), app_id),
+    Publish(app_notifications_.GetAppWithHasBadgeStatus(
+                apps::mojom::AppType::kExtension, app_id),
             subscribers());
   }
 }
@@ -465,7 +422,8 @@ bool ExtensionAppsChromeOs::MaybeAddNotification(
   }
 
   app_notifications_.AddNotification(app_id, notification_id);
-  Publish(app_notifications_.GetAppWithHasBadgeStatus(app_type(), app_id),
+  Publish(app_notifications_.GetAppWithHasBadgeStatus(
+              apps::mojom::AppType::kExtension, app_id),
           subscribers());
   return true;
 }
@@ -478,7 +436,6 @@ void ExtensionAppsChromeOs::MaybeAddWebPageNotifications(
           ? PersistentNotificationMetadata::From(metadata)->service_worker_scope
           : notification.origin_url();
 
-  if (app_type() == apps::mojom::AppType::kExtension) {
   extensions::ExtensionRegistry* registry =
       extensions::ExtensionRegistry::Get(profile());
   DCHECK(registry);
@@ -487,34 +444,6 @@ void ExtensionAppsChromeOs::MaybeAddWebPageNotifications(
   if (extension) {
     MaybeAddNotification(extension->id(), notification.id());
   }
-    return;
-  }
-
-  if (metadata) {
-    // For persistent notifications, find the web app with the scope url.
-    base::Optional<web_app::AppId> app_id =
-        web_app::FindInstalledAppWithUrlInScope(profile(), url,
-                                                /*window_only=*/false);
-    if (app_id.has_value()) {
-      MaybeAddNotification(app_id.value(), notification.id());
-    }
-  } else {
-    // For non-persistent notifications, find all web apps that are installed
-    // under the origin url.
-    auto* web_app_provider = web_app::WebAppProvider::Get(profile());
-    if (!web_app_provider) {
-      return;
-    }
-
-    auto app_ids = web_app_provider->registrar().FindAppsInScope(url);
-    int count = 0;
-    for (const auto& app_id : app_ids) {
-      if (MaybeAddNotification(app_id, notification.id())) {
-        ++count;
-      }
-    }
-    RecordAppsPerNotification(count);
-  }
 }
 
 // static
@@ -546,7 +475,7 @@ void ExtensionAppsChromeOs::UpdateShowInFields(const std::string& app_id) {
   }
 
   apps::mojom::AppPtr app = apps::mojom::App::New();
-  app->app_type = app_type();
+  app->app_type = apps::mojom::AppType::kExtension;
   app->app_id = app_id;
   SetShowInFields(app, extension);
   Publish(std::move(app), subscribers());
@@ -558,10 +487,6 @@ void ExtensionAppsChromeOs::OnHideWebStoreIconPrefChanged() {
 }
 
 void ExtensionAppsChromeOs::OnSystemFeaturesPrefChanged() {
-  if (app_type() != apps::mojom::AppType::kExtension) {
-    return;
-  }
-
   PrefService* const local_state = g_browser_process->local_state();
   if (!local_state || !local_state->FindPreference(
                           policy::policy_prefs::kSystemFeaturesDisableList)) {
@@ -610,21 +535,7 @@ bool ExtensionAppsChromeOs::Accepts(const extensions::Extension* extension) {
     return false;
   }
 
-  switch (app_type()) {
-    case apps::mojom::AppType::kExtension:
   return !extension->from_bookmark();
-    case apps::mojom::AppType::kWeb:
-      // Crostini Terminal System App is handled by Crostini Apps.
-      // TODO(crbug.com/1028898): Register Terminal as a System App rather than
-      // a crostini app.
-      if (extension->id() == crostini::kCrostiniTerminalSystemAppId) {
-        return false;
-      }
-      return extension->from_bookmark();
-    default:
-      NOTREACHED();
-      return false;
-  }
 }
 
 bool ExtensionAppsChromeOs::ShouldShownInLauncher(
@@ -697,7 +608,7 @@ void ExtensionAppsChromeOs::SetIconEffect(const std::string& app_id) {
   }
 
   apps::mojom::AppPtr app = apps::mojom::App::New();
-  app->app_type = app_type();
+  app->app_type = apps::mojom::AppType::kExtension;
   app->app_id = app_id;
   app->icon_key = icon_key_factory().MakeIconKey(
       GetIconEffects(extension, paused_apps_.IsPaused(app_id)));
@@ -715,8 +626,7 @@ bool ExtensionAppsChromeOs::ShouldRecordAppWindowActivity(
 
   // ARC Play Store is not published by this publisher, but the window for Play
   // Store should be able to be added to InstanceRegistry.
-  if (extension->id() == arc::kPlayStoreAppId &&
-      app_type() == apps::mojom::AppType::kExtension) {
+  if (extension->id() == arc::kPlayStoreAppId) {
     return true;
   }
 
@@ -781,28 +691,4 @@ void ExtensionAppsChromeOs::GetMenuModelForChromeBrowserApp(
   std::move(callback).Run(std::move(menu_items));
 }
 
-void ExtensionAppsChromeOs::OnWebAppDisabledStateChanged(
-    const web_app::AppId& app_id,
-    bool is_disabled) {
-  const auto* extension = MaybeGetExtension(app_id);
-  if (!extension) {
-    return;
-  }
-
-  if (base::Contains(disabled_apps_, app_id) == is_disabled) {
-    return;
-  }
-
-  if (is_disabled) {
-    disabled_apps_.insert(app_id);
-  } else {
-    disabled_apps_.erase(app_id);
-  }
-
-  Publish(
-      Convert(extension, is_disabled ? apps::mojom::Readiness::kDisabledByPolicy
-                                     : apps::mojom::Readiness::kReady),
-      subscribers());
-}
-
 }  // namespace apps

chrome/browser/apps/app_service/extension_apps_chromeos.h

@@ -54,7 +54,6 @@ class ExtensionAppsChromeOs : public ExtensionAppsBase,
   ExtensionAppsChromeOs(
       const mojo::Remote<apps::mojom::AppService>& app_service,
       Profile* profile,
-      apps::mojom::AppType app_type,
       apps::InstanceRegistry* instance_registry);
   ~ExtensionAppsChromeOs() override;
 
@@ -149,10 +148,6 @@ class ExtensionAppsChromeOs : public ExtensionAppsBase,
   void GetMenuModelForChromeBrowserApp(apps::mojom::MenuType menu_type,
                                        GetMenuModelCallback callback);
 
-  // web_app::AppRegistrarObserver overrides.
-  void OnWebAppDisabledStateChanged(const std::string& app_id,
-                                    bool is_disabled) override;
-
   apps::InstanceRegistry* instance_registry_;
   ScopedObserver<extensions::AppWindowRegistry,
                  extensions::AppWindowRegistry::Observer>