DPWA: move appshim setup to OsIntegrationManager::Start.

This moves AppshortcutManager::DeleteSharedAppShims to OsIntegrationManager
and also removes observing of |OnWebAppProfileWillBeDeleted| from
shortcut manager and filehandler manager, instead makes registrar
call |OsIntegrationManager::UnisntallOsHooks| on profile being deleted.

Bug: 1098027
Change-Id: I629126d7661e2fdbbaff8645a46382d59c93dca9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2320997Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Commit-Queue: Phillis Tang <phillis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792734}

chrome/browser/web_applications/components/app_shortcut_manager.cc

@@ -18,10 +18,6 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 
-#if defined(OS_MACOSX)
-#include "chrome/browser/web_applications/components/app_shim_registry_mac.h"
-#endif
-
 namespace web_app {
 
 namespace {
@@ -46,20 +42,6 @@ void AppShortcutManager::SetSubsystems(AppIconManager* icon_manager,
 void AppShortcutManager::Start() {
   DCHECK(registrar_);
   app_registrar_observer_.Add(registrar_);
-
-#if defined(OS_MACOSX)
-  // Ensure that all installed apps are included in the AppShimRegistry when the
-  // profile is loaded. This is redundant, because apps are registered when they
-  // are installed. It is necessary, however, because app registration was added
-  // long after app installation launched. This should be removed after shipping
-  // for a few versions (whereupon it may be assumed that most applications have
-  // been registered).
-  std::vector<AppId> app_ids = registrar_->GetAppIds();
-  for (const auto& app_id : app_ids) {
-    AppShimRegistry::Get()->OnAppInstalledForProfile(app_id,
-                                                     profile_->GetPath());
-  }
-#endif
 }
 
 void AppShortcutManager::Shutdown() {
@@ -77,10 +59,6 @@ void AppShortcutManager::OnWebAppManifestUpdated(const AppId& app_id,
                   weak_ptr_factory_.GetWeakPtr(), base::UTF8ToUTF16(old_name)));
 }
 
-void AppShortcutManager::OnWebAppProfileWillBeDeleted(const AppId& app_id) {
-  DeleteSharedAppShims(app_id);
-}
-
 void AppShortcutManager::SetShortcutUpdateCallbackForTesting(
     base::OnceCallback<void(const ShortcutInfo*)> callback) {
   GetShortcutUpdateCallbackForTesting() = std::move(callback);
@@ -160,19 +138,6 @@ void AppShortcutManager::UnregisterShortcutsMenuWithOs(const AppId& app_id) {
   web_app::UnregisterShortcutsMenuWithOs(app_id, profile_->GetPath());
 }
 
-void AppShortcutManager::DeleteSharedAppShims(const AppId& app_id) {
-#if defined(OS_MACOSX)
-  bool delete_multi_profile_shortcuts =
-      AppShimRegistry::Get()->OnAppUninstalledForProfile(app_id,
-                                                         profile_->GetPath());
-  if (delete_multi_profile_shortcuts) {
-    web_app::internals::GetShortcutIOTaskRunner()->PostTask(
-        FROM_HERE,
-        base::BindOnce(&web_app::internals::DeleteMultiProfileShortcutsForApp,
-                       app_id));
-  }
-#endif
-}
 
 void AppShortcutManager::OnShortcutsCreated(const AppId& app_id,
                                             CreateShortcutsCallback callback,

chrome/browser/web_applications/components/app_shortcut_manager.h

@@ -48,7 +48,6 @@ class AppShortcutManager : public AppRegistrarObserver {
   // AppRegistrarObserver:
   void OnWebAppManifestUpdated(const web_app::AppId& app_id,
                                base::StringPiece old_name) override;
-  void OnWebAppProfileWillBeDeleted(const AppId& app_id) override;
 
   // Tells the AppShortcutManager that no shortcuts should actually be written
   // to the disk.
@@ -86,8 +85,6 @@ class AppShortcutManager : public AppRegistrarObserver {
 
   void UnregisterShortcutsMenuWithOs(const AppId& app_id);
 
-  void DeleteSharedAppShims(const AppId& app_id);
-
   // Builds initial ShortcutInfo without |ShortcutInfo::favicon| being read.
   virtual std::unique_ptr<ShortcutInfo> BuildShortcutInfo(
       const AppId& app_id) = 0;

chrome/browser/web_applications/components/file_handler_manager.cc

@@ -232,10 +232,6 @@ int FileHandlerManager::CleanupAfterOriginTrials() {
   return cleaned_up_count;
 }
 
-void FileHandlerManager::OnWebAppProfileWillBeDeleted(const AppId& app_id) {
-  DisableAndUnregisterOsFileHandlers(app_id);
-}
-
 void FileHandlerManager::OnAppRegistrarDestroyed() {
   registrar_observer_.RemoveAll();
 }

chrome/browser/web_applications/components/file_handler_manager.h

@@ -135,7 +135,6 @@ class FileHandlerManager : public AppRegistrarObserver {
   int CleanupAfterOriginTrials();
 
   // AppRegistrarObserver:
-  void OnWebAppProfileWillBeDeleted(const AppId& app_id) override;
   void OnAppRegistrarDestroyed() override;
 
   ScopedObserver<AppRegistrar, AppRegistrarObserver> registrar_observer_;

chrome/browser/web_applications/extensions/bookmark_app_registrar.cc

@@ -100,8 +100,12 @@ void BookmarkAppRegistrar::OnExtensionUnloaded(
 
   // If a profile is removed, notify the web app that it is uninstalled, so it
   // can cleanup any state outside the profile dir (e.g., registry settings).
-  if (reason == UnloadedExtensionReason::PROFILE_SHUTDOWN)
+  if (reason == UnloadedExtensionReason::PROFILE_SHUTDOWN) {
     NotifyWebAppProfileWillBeDeleted(extension->id());
+    web_app::WebAppProviderBase::GetProviderBase(profile())
+        ->os_integration_manager()
+        .UninstallOsHooks(extension->id());
+  }
 
   bookmark_app_being_observed_ = nullptr;
 }

chrome/browser/web_applications/os_integration_manager.cc

@@ -53,16 +53,32 @@ OsIntegrationManager::OsIntegrationManager(Profile* profile)
 OsIntegrationManager::~OsIntegrationManager() = default;
 
 void OsIntegrationManager::SetSubsystems(
+    AppRegistrar* registrar,
     AppShortcutManager* shortcut_manager,
     FileHandlerManager* file_handler_manager,
     WebAppUiManager* ui_manager) {
+  registrar_ = registrar;
   shortcut_manager_ = shortcut_manager;
   file_handler_manager_ = file_handler_manager;
   ui_manager_ = ui_manager;
 }
 
-void OsIntegrationManager::SuppressOsHooksForTesting() {
-  suppress_os_hooks_for_testing_ = true;
+void OsIntegrationManager::Start() {
+  DCHECK(registrar_);
+
+#if defined(OS_MACOSX)
+  // Ensure that all installed apps are included in the AppShimRegistry when the
+  // profile is loaded. This is redundant, because apps are registered when they
+  // are installed. It is necessary, however, because app registration was added
+  // long after app installation launched. This should be removed after shipping
+  // for a few versions (whereupon it may be assumed that most applications have
+  // been registered).
+  std::vector<AppId> app_ids = registrar_->GetAppIds();
+  for (const auto& app_id : app_ids) {
+    AppShimRegistry::Get()->OnAppInstalledForProfile(app_id,
+                                                     profile_->GetPath());
+  }
+#endif
 }
 
 void OsIntegrationManager::InstallOsHooks(
@@ -136,7 +152,11 @@ void OsIntegrationManager::UninstallOsHooks(const AppId& app_id) {
 
   file_handler_manager_->DisableAndUnregisterOsFileHandlers(app_id);
 
-  shortcut_manager_->DeleteSharedAppShims(app_id);
+  DeleteSharedAppShims(app_id);
+}
+
+void OsIntegrationManager::SuppressOsHooksForTesting() {
+  suppress_os_hooks_for_testing_ = true;
 }
 
 void OsIntegrationManager::OnShortcutsCreated(
@@ -189,4 +209,17 @@ void OsIntegrationManager::OnShortcutsCreated(
   }
 }
 
+void OsIntegrationManager::DeleteSharedAppShims(const AppId& app_id) {
+#if defined(OS_MACOSX)
+  bool delete_multi_profile_shortcuts =
+      AppShimRegistry::Get()->OnAppUninstalledForProfile(app_id,
+                                                         profile_->GetPath());
+  if (delete_multi_profile_shortcuts) {
+    web_app::internals::GetShortcutIOTaskRunner()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&web_app::internals::DeleteMultiProfileShortcutsForApp,
+                       app_id));
+  }
+#endif
+}
 }  // namespace web_app

chrome/browser/web_applications/os_integration_manager.h

@@ -10,6 +10,7 @@
 
 #include "base/callback_forward.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_id.h"
 #include "chrome/common/web_application_info.h"
@@ -47,10 +48,13 @@ class OsIntegrationManager {
   explicit OsIntegrationManager(Profile* profile);
   virtual ~OsIntegrationManager();
 
-  void SetSubsystems(AppShortcutManager* shortcut_manager,
+  void SetSubsystems(AppRegistrar* registrar,
+                     AppShortcutManager* shortcut_manager,
                      FileHandlerManager* file_handler_manager,
                      WebAppUiManager* ui_manager);
 
+  void Start();
+
   // Install all needed OS hooks for the web app.
   // If provided |web_app_info| is a nullptr, it will read icons data from disk,
   // otherwise it will use (SkBitmaps) from |web_app_info|.
@@ -78,8 +82,11 @@ class OsIntegrationManager {
                                                        bool created)> callback,
                           bool shortcuts_created);
 
+  void DeleteSharedAppShims(const AppId& app_id);
+
   Profile* const profile_;
 
+  AppRegistrar* registrar_ = nullptr;
   AppShortcutManager* shortcut_manager_ = nullptr;
   FileHandlerManager* file_handler_manager_ = nullptr;
   bool suppress_os_hooks_for_testing_ = false;

chrome/browser/web_applications/web_app_provider.cc

@@ -274,7 +274,8 @@ void WebAppProvider::ConnectSubsystems() {
   file_handler_manager_->SetSubsystems(registrar_.get());
   shortcut_manager_->SetSubsystems(icon_manager_.get(), registrar_.get());
   os_integration_manager_->SetSubsystems(
-      shortcut_manager_.get(), file_handler_manager_.get(), ui_manager_.get());
+      registrar_.get(), shortcut_manager_.get(), file_handler_manager_.get(),
+      ui_manager_.get());
 
   connected_ = true;
 }
@@ -295,9 +296,10 @@ void WebAppProvider::OnRegistryControllerReady() {
   external_web_app_manager_->Start();
   web_app_policy_manager_->Start();
   system_web_app_manager_->Start();
-  shortcut_manager_->Start();
   manifest_update_manager_->Start();
+  shortcut_manager_->Start();
   file_handler_manager_->Start();
+  os_integration_manager_->Start();
   ui_manager_->Start();
 
   on_registry_ready_.Signal();

chrome/browser/web_applications/web_app_registrar.cc

@@ -13,6 +13,8 @@
 #include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/web_applications/components/web_app_provider_base.h"
+#include "chrome/browser/web_applications/os_integration_manager.h"
 #include "chrome/browser/web_applications/web_app.h"
 
 namespace web_app {
@@ -185,9 +187,12 @@ void WebAppRegistrar::OnProfileMarkedForPermanentDeletion(
   if (profile() != profile_to_be_deleted)
     return;
 
-  for (const auto& app : AllApps())
+  for (const auto& app : AllApps()) {
     NotifyWebAppProfileWillBeDeleted(app.app_id());
-
+    WebAppProviderBase::GetProviderBase(profile())
+        ->os_integration_manager()
+        .UninstallOsHooks(app.app_id());
+  }
   // We can't do registry_.clear() here because it makes in-memory registry
   // diverged from the sync server registry and from the on-disk registry
   // (WebAppDatabase/LevelDB and "Web Applications" profile directory).