Refactor of moving shortcut deletion to AppShortcutManager

OsIntegrationManager is a proxy class that doesn't have to know
its detailed OS calls instead it delegates a work to the target
class if it exists.

AppShortcutManager handles shortcut creation (see CreateShortcuts
in OsIntegrationManager) that includes proper telemetry message
at the end of it. However the current DeleteShortcuts is simply
handled by the OsItengrationManager instead of AppShortcutManager.

The change is to follow pattern of CreateShortcuts for the shortcut
deletion that seems more consistent in terms of OsItegrationManager
and AppShortcutManager relationship.

Bug: 1151796
Change-Id: I01e80b0a13ef1169a238d65a2ffc5acb6cca292f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2606680Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841612}

chrome/browser/web_applications/components/app_shortcut_manager.cc

@@ -27,6 +27,10 @@ namespace {
 constexpr const char* kCreationResultMetric =
     "WebApp.Shortcuts.Creation.Result";
 
+// UMA metric name for shortcuts deletion result.
+constexpr const char* kDeletionResultMetric =
+    "WebApp.Shortcuts.Deletion.Success";
+
 // Result of shortcuts creation process.
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
@@ -96,6 +100,21 @@ void AppShortcutManager::CreateShortcuts(const AppId& app_id,
                                  std::move(callback))));
 }
 
+void AppShortcutManager::DeleteShortcuts(
+    const AppId& app_id,
+    const base::FilePath& shortcuts_data_dir,
+    std::unique_ptr<ShortcutInfo> shortcut_info,
+    DeleteShortcutsCallback callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(CanCreateShortcuts());
+
+  internals::ScheduleDeletePlatformShortcuts(
+      shortcuts_data_dir, std::move(shortcut_info),
+      base::BindOnce(&AppShortcutManager::OnShortcutsDeleted,
+                     weak_ptr_factory_.GetWeakPtr(), app_id,
+                     std::move(callback)));
+}
+
 void AppShortcutManager::ReadAllShortcutsMenuIconsAndRegisterShortcutsMenu(
     const AppId& app_id,
     RegisterShortcutsMenuCallback callback) {
@@ -155,6 +174,15 @@ void AppShortcutManager::OnShortcutsCreated(const AppId& app_id,
   std::move(callback).Run(success);
 }
 
+void AppShortcutManager::OnShortcutsDeleted(const AppId& app_id,
+                                            DeleteShortcutsCallback callback,
+                                            bool success) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  UMA_HISTOGRAM_BOOLEAN(kDeletionResultMetric, success);
+
+  std::move(callback).Run(success);
+}
+
 void AppShortcutManager::OnShortcutInfoRetrievedCreateShortcuts(
     bool add_to_desktop,
     CreateShortcutsCallback callback,

chrome/browser/web_applications/components/app_shortcut_manager.h

@@ -52,6 +52,10 @@ class AppShortcutManager {
                        CreateShortcutsCallback callback);
   void UpdateShortcuts(const web_app::AppId& app_id,
                        base::StringPiece old_name);
+  void DeleteShortcuts(const AppId& app_id,
+                       const base::FilePath& shortcuts_data_dir,
+                       std::unique_ptr<ShortcutInfo> shortcut_info,
+                       DeleteShortcutsCallback callback);
 
   // TODO(crbug.com/1098471): Move this into web_app_shortcuts_menu_win.cc when
   // a callback is integrated into the Shortcuts Menu registration flow.
@@ -97,6 +101,9 @@ class AppShortcutManager {
   void OnShortcutsCreated(const AppId& app_id,
                           CreateShortcutsCallback callback,
                           bool success);
+  void OnShortcutsDeleted(const AppId& app_id,
+                          DeleteShortcutsCallback callback,
+                          bool success);
 
   AppRegistrar* registrar() { return registrar_; }
   Profile* profile() { return profile_; }

chrome/browser/web_applications/components/os_integration_manager.cc

@@ -433,11 +433,17 @@ void OsIntegrationManager::DeleteShortcuts(
     const base::FilePath& shortcuts_data_dir,
     std::unique_ptr<ShortcutInfo> shortcut_info,
     DeleteShortcutsCallback callback) {
-  internals::ScheduleDeletePlatformShortcuts(
-      shortcuts_data_dir, std::move(shortcut_info),
-      base::BindOnce(&OsIntegrationManager::OnShortcutsDeleted,
-                     weak_ptr_factory_.GetWeakPtr(), app_id,
-                     std::move(callback)));
+  if (shortcut_manager_->CanCreateShortcuts()) {
+    auto shortcuts_callback = base::BindOnce(
+        &OsIntegrationManager::OnShortcutsDeleted,
+        weak_ptr_factory_.GetWeakPtr(), app_id, std::move(callback));
+
+    shortcut_manager_->DeleteShortcuts(app_id, shortcuts_data_dir,
+                                       std::move(shortcut_info),
+                                       std::move(shortcuts_callback));
+  } else {
+    std::move(callback).Run(false);
+  }
 }
 
 void OsIntegrationManager::UnregisterFileHandlers(const AppId& app_id) {

tools/metrics/histograms/histograms_xml/others/histograms.xml

@@ -17594,6 +17594,18 @@ regressions. -->
   <summary>Records the result of shortcut creation for PWA.</summary>
 </histogram>
 
+<histogram name="WebApp.Shortcuts.Deletion.Success" enum="BooleanSuccess"
+    expires_after="M93">
+  <owner>phillis@chromium.org</owner>
+  <owner>dmurph@chromium.org</owner>
+  <owner>sunggch@microsoft.com</owner>
+  <summary>
+    Records the result of shortcut deletion for a PWA. This occurs when an
+    installed PWA is uninstalled, which can be triggered by user, policy admin,
+    or sync system depending on the situation.
+  </summary>
+</histogram>
+
 <histogram name="Webapp.SyncInitiatedUninstallResult" enum="BooleanSuccess"
     expires_after="2022-01-01">
   <owner>alancutter@chromium.org</owner>