DPWA: Allow OS hooks to be uninstalled individually

This CL updates the OsIntegrationManager::UninstallOsHooks() method to
accept an additional |os_hooks| parameter to specify which OS Hooks to
uninstall.

This CL is in preparation to allow Run on OS Login to be enabled/disabled
by the user after the web app is installed, via the
OsIntegrationManager::UninstallOsHooks() API. Currently there is no way
to uninstall an OS Hook (i.e. Run on OS Login) individually.

This CL consists of the following changes:

1. Update OsIntegrationManager::UninstallOsHooks() to honor the passed
    |os_hooks| and only uninstall the specified OS hooks.

2. Add OsIntegrationManager::UninstallAllOsHooks() API to be replaced in
    places that currently call UninstallOsHooks() with the intention to
    effectively uninstall all OS hooks.


Bug: 1121372
Change-Id: Ie294f61b9518e997bf4192ac1bdcb7db5bf819fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372819
Commit-Queue: Carlos Frias <carlos.frias@microsoft.com>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807652}

chrome/browser/web_applications/components/os_integration_manager.cc

@@ -136,7 +136,16 @@ void OsIntegrationManager::InstallOsHooks(
   }
 }
 
+void OsIntegrationManager::UninstallAllOsHooks(
+    const AppId& app_id,
+    UninstallOsHooksCallback callback) {
+  OsHooksResults os_hooks;
+  os_hooks.set();
+  UninstallOsHooks(app_id, os_hooks, std::move(callback));
+}
+
 void OsIntegrationManager::UninstallOsHooks(const AppId& app_id,
+                                            const OsHooksResults& os_hooks,
                                             UninstallOsHooksCallback callback) {
   DCHECK(shortcut_manager_);
 
@@ -148,30 +157,41 @@ void OsIntegrationManager::UninstallOsHooks(const AppId& app_id,
           &OsHooksBarrierInfo::Run,
           base::Owned(new OsHooksBarrierInfo(std::move(callback))));
 
-  if (ShouldRegisterShortcutsMenuWithOs()) {
+  if (os_hooks[OsHookType::kShortcutsMenu] &&
+      ShouldRegisterShortcutsMenuWithOs()) {
     barrier.Run(OsHookType::kShortcutsMenu,
                 UnregisterShortcutsMenuWithOs(app_id, profile_->GetPath()));
   } else {
     barrier.Run(OsHookType::kShortcutsMenu, /*completed=*/true);
   }
 
+  if (os_hooks[OsHookType::kShortcuts] || os_hooks[OsHookType::kRunOnOsLogin]) {
     std::unique_ptr<ShortcutInfo> shortcut_info =
         shortcut_manager_->BuildShortcutInfo(app_id);
     base::FilePath shortcut_data_dir =
         internals::GetShortcutDataDir(*shortcut_info);
 
-  if (base::FeatureList::IsEnabled(features::kDesktopPWAsRunOnOsLogin)) {
+    if (os_hooks[OsHookType::kRunOnOsLogin] &&
+        base::FeatureList::IsEnabled(features::kDesktopPWAsRunOnOsLogin)) {
       ScheduleUnregisterRunOnOsLogin(
           shortcut_info->profile_path, shortcut_info->title,
           base::BindOnce(barrier, OsHookType::kRunOnOsLogin));
+    } else {
+      barrier.Run(OsHookType::kRunOnOsLogin, /*completed=*/true);
     }
 
+    if (os_hooks[OsHookType::kShortcuts]) {
       internals::ScheduleDeletePlatformShortcuts(
           shortcut_data_dir, std::move(shortcut_info),
           base::BindOnce(barrier, OsHookType::kShortcuts));
+    } else {
+      barrier.Run(OsHookType::kShortcuts, /*completed=*/true);
+    }
+  }
 
   // TODO(https://crbug.com/1108109) we should return the result of file handler
   // unregistration and record errors during unregistration.
+  if (os_hooks[OsHookType::kFileHandlers])
     file_handler_manager_->DisableAndUnregisterOsFileHandlers(app_id);
   barrier.Run(OsHookType::kFileHandlers, /*completed=*/true);
 

chrome/browser/web_applications/components/os_integration_manager.h

@@ -79,10 +79,19 @@ class OsIntegrationManager {
                               std::unique_ptr<WebApplicationInfo> web_app_info,
                               InstallOsHooksOptions options);
 
-  // Uninstall all OS hooks for the web app.
+  // Uninstall specific OS hooks for the web app.
+  // Used when removing specific hooks resulting from an app setting change.
+  // Example: Running on OS login.
   // TODO(https://crbug.com/1108109) we should record uninstall result and allow
   // callback. virtual for testing
   virtual void UninstallOsHooks(const AppId& app_id,
+                                const OsHooksResults& os_hooks,
+                                UninstallOsHooksCallback callback);
+
+  // Uninstall all OS hooks for the web app.
+  // Used when uninstalling a web app.
+  // virtual for testing
+  virtual void UninstallAllOsHooks(const AppId& app_id,
                                    UninstallOsHooksCallback callback);
 
   // Update all needed OS hooks for the web app.

chrome/browser/web_applications/extensions/bookmark_app_registrar.cc

@@ -81,7 +81,7 @@ void BookmarkAppRegistrar::OnExtensionUninstalled(
     NotifyWebAppUninstalled(extension->id());
     web_app::WebAppProviderBase::GetProviderBase(profile())
         ->os_integration_manager()
-        .UninstallOsHooks(extension->id(), base::DoNothing());
+        .UninstallAllOsHooks(extension->id(), base::DoNothing());
 
     bookmark_app_being_observed_ = nullptr;
   }
@@ -104,7 +104,7 @@ void BookmarkAppRegistrar::OnExtensionUnloaded(
     NotifyWebAppProfileWillBeDeleted(extension->id());
     web_app::WebAppProviderBase::GetProviderBase(profile())
         ->os_integration_manager()
-        .UninstallOsHooks(extension->id(), base::DoNothing());
+        .UninstallAllOsHooks(extension->id(), base::DoNothing());
   }
 
   bookmark_app_being_observed_ = nullptr;

chrome/browser/web_applications/test/test_os_integration_manager.cc

@@ -73,6 +73,13 @@ void TestOsIntegrationManager::InstallOsHooks(
 }
 
 void TestOsIntegrationManager::UninstallOsHooks(
+    const AppId& app_id,
+    const OsHooksResults& os_hooks,
+    UninstallOsHooksCallback callback) {
+  NOTIMPLEMENTED();
+}
+
+void TestOsIntegrationManager::UninstallAllOsHooks(
     const AppId& app_id,
     UninstallOsHooksCallback callback) {
   NOTIMPLEMENTED();

chrome/browser/web_applications/test/test_os_integration_manager.h

@@ -30,6 +30,9 @@ class TestOsIntegrationManager : public OsIntegrationManager {
                       std::unique_ptr<WebApplicationInfo> web_app_info,
                       InstallOsHooksOptions options) override;
   void UninstallOsHooks(const AppId& app_id,
+                        const OsHooksResults& os_hooks,
+                        UninstallOsHooksCallback callback) override;
+  void UninstallAllOsHooks(const AppId& app_id,
                            UninstallOsHooksCallback callback) override;
   void UpdateOsHooks(const AppId& app_id,
                      base::StringPiece old_name,

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -330,7 +330,7 @@ void WebAppInstallFinalizer::UninstallWebApp(const AppId& app_id,
   registrar().NotifyWebAppUninstalled(app_id);
   WebAppProviderBase::GetProviderBase(profile_)
       ->os_integration_manager()
-      .UninstallOsHooks(app_id, base::DoNothing());
+      .UninstallAllOsHooks(app_id, base::DoNothing());
 
   ScopedRegistryUpdate update(registry_controller().AsWebAppSyncBridge());
   update->DeleteApp(app_id);

chrome/browser/web_applications/web_app_registrar.cc

@@ -197,7 +197,7 @@ void WebAppRegistrar::OnProfileMarkedForPermanentDeletion(
     NotifyWebAppProfileWillBeDeleted(app.app_id());
     WebAppProviderBase::GetProviderBase(profile())
         ->os_integration_manager()
-        .UninstallOsHooks(app.app_id(), base::DoNothing());
+        .UninstallAllOsHooks(app.app_id(), base::DoNothing());
   }
   // 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

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -521,7 +521,7 @@ void WebAppSyncBridge::ApplySyncChangesToRegistrar(
     registrar_->NotifyWebAppUninstalled(app_id);
     WebAppProviderBase::GetProviderBase(profile())
         ->os_integration_manager()
-        .UninstallOsHooks(app_id, base::DoNothing());
+        .UninstallAllOsHooks(app_id, base::DoNothing());
   }
 
   std::vector<WebApp*> apps_to_install;