WebApp: Handle OS shortcut deletion for bookmark apps in web apps land.

Make BookmarkAppShortcutManager responsible for bookmark apps.

Bug: 860581
Change-Id: I683aebfd3bb321b26cf59f026436a5c78e4daac5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2006282Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733834}

chrome/browser/apps/platform_apps/shortcut_manager.cc

@@ -162,6 +162,7 @@ void AppShortcutManager::OnExtensionUninstalled(
     const Extension* extension,
     extensions::UninstallReason reason) {
 #if defined(OS_MACOSX)
+  // TODO(crbug.com/860581): Move this code to BookmarkAppShortcutManager.
   if (UseAppShimRegistry(browser_context, extension)) {
     bool delete_multi_profile_shortcuts =
         AppShimRegistry::Get()->OnAppUninstalledForProfile(extension->id(),
@@ -175,7 +176,10 @@ void AppShortcutManager::OnExtensionUninstalled(
   }
 #endif
 
-  web_app::DeleteAllShortcuts(profile_, extension);
+  // Bookmark apps are handled in
+  // web_app::AppShortcutManager::OnWebAppWillBeUninstalled()
+  if (!extension->from_bookmark())
+    web_app::DeleteAllShortcuts(profile_, extension);
 }
 
 void AppShortcutManager::OnProfileWillBeRemoved(

chrome/browser/apps/platform_apps/shortcut_manager.h

@@ -20,7 +20,11 @@ namespace user_prefs {
 class PrefRegistrySyncable;
 }
 
-// This class manages the installation of shortcuts for platform apps.
+// This class manages the installation of shortcuts for any extension-based apps
+// (Chrome Apps). Bookmark apps OS shortcut management is handled in
+// web_app::AppShortcutManager and its subclasses.
+//
+// Long term, this class must be deleted together with all extension-based apps.
 class AppShortcutManager : public KeyedService,
                            public extensions::ExtensionRegistryObserver,
                            public ProfileAttributesStorage::Observer {

chrome/browser/web_applications/components/app_shortcut_manager.cc

@@ -39,6 +39,16 @@ void AppShortcutManager::RemoveObserver(AppShortcutObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void AppShortcutManager::OnWebAppWillBeUninstalled(const AppId& app_id) {
+  std::unique_ptr<ShortcutInfo> shortcut_info = BuildShortcutInfo(app_id);
+  base::FilePath shortcut_data_dir =
+      internals::GetShortcutDataDir(*shortcut_info);
+
+  internals::PostShortcutIOTask(
+      base::BindOnce(&internals::DeletePlatformShortcuts, shortcut_data_dir),
+      std::move(shortcut_info));
+}
+
 bool AppShortcutManager::CanCreateShortcuts() const {
 #if defined(OS_CHROMEOS)
   return false;

chrome/browser/web_applications/components/app_shortcut_manager.h

@@ -44,6 +44,9 @@ class AppShortcutManager : public AppRegistrarObserver {
   void AddObserver(AppShortcutObserver* observer);
   void RemoveObserver(AppShortcutObserver* observer);
 
+  // AppRegistrarObserver:
+  void OnWebAppWillBeUninstalled(const AppId& app_id) override;
+
   // Tells the AppShortcutManager that no shortcuts should actually be written
   // to the disk.
   void SuppressShortcutsForTesting();
@@ -55,6 +58,10 @@ class AppShortcutManager : public AppRegistrarObserver {
                                bool add_to_desktop,
                                CreateShortcutsCallback callback);
 
+  // Builds initial ShortcutInfo without |ShortcutInfo::favicon| being read.
+  virtual std::unique_ptr<ShortcutInfo> BuildShortcutInfo(
+      const AppId& app_id) = 0;
+
   // The result of a call to GetShortcutInfo.
   using GetShortcutInfoCallback =
       base::OnceCallback<void(std::unique_ptr<ShortcutInfo>)>;

chrome/browser/web_applications/extensions/bookmark_app_shortcut_manager.cc

@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/components/web_app_shortcut.h"
+#include "chrome/browser/web_applications/extensions/bookmark_app_registrar.h"
 #include "chrome/browser/web_applications/extensions/web_app_extension_shortcut.h"
 #include "extensions/browser/extension_registry.h"
 
@@ -17,6 +18,17 @@ BookmarkAppShortcutManager::BookmarkAppShortcutManager(Profile* profile)
 
 BookmarkAppShortcutManager::~BookmarkAppShortcutManager() = default;
 
+std::unique_ptr<web_app::ShortcutInfo>
+BookmarkAppShortcutManager::BuildShortcutInfo(const web_app::AppId& app_id) {
+  BookmarkAppRegistrar* registry = registrar()->AsBookmarkAppRegistrar();
+  DCHECK(registry);
+
+  const Extension* app = registry->FindExtension(app_id);
+  DCHECK(app);
+
+  return web_app::ShortcutInfoForExtensionAndProfile(app, profile());
+}
+
 void BookmarkAppShortcutManager::GetShortcutInfoForApp(
     const web_app::AppId& app_id,
     GetShortcutInfoCallback callback) {

chrome/browser/web_applications/extensions/bookmark_app_shortcut_manager.h

@@ -17,6 +17,9 @@ class BookmarkAppShortcutManager : public web_app::AppShortcutManager {
   explicit BookmarkAppShortcutManager(Profile* profile);
   ~BookmarkAppShortcutManager() override;
 
+  // AppShortcutManager:
+  std::unique_ptr<web_app::ShortcutInfo> BuildShortcutInfo(
+      const web_app::AppId& app_id) override;
   void GetShortcutInfoForApp(const web_app::AppId& app_id,
                              GetShortcutInfoCallback callback) override;
 

chrome/browser/web_applications/test/test_app_shortcut_manager.cc

@@ -44,6 +44,12 @@ void TestAppShortcutManager::CreateShortcuts(const AppId& app_id,
                                 std::move(callback), success));
 }
 
+std::unique_ptr<ShortcutInfo> TestAppShortcutManager::BuildShortcutInfo(
+    const AppId& app_id) {
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
 void TestAppShortcutManager::GetShortcutInfoForApp(
     const AppId& app_id,
     GetShortcutInfoCallback callback) {

chrome/browser/web_applications/test/test_app_shortcut_manager.h

@@ -40,6 +40,7 @@ class TestAppShortcutManager : public AppShortcutManager {
   void CreateShortcuts(const AppId& app_id,
                        bool on_desktop,
                        CreateShortcutsCallback callback) override;
+  std::unique_ptr<ShortcutInfo> BuildShortcutInfo(const AppId& app_id) override;
   void GetShortcutInfoForApp(const AppId& app_id,
                              GetShortcutInfoCallback callback) override;
 

chrome/browser/web_applications/web_app_shortcut_manager.cc

@@ -36,17 +36,11 @@ WebAppShortcutManager::WebAppShortcutManager(
 
 WebAppShortcutManager::~WebAppShortcutManager() = default;
 
-void WebAppShortcutManager::OnWebAppWillBeUninstalled(const AppId& app_id) {
+std::unique_ptr<ShortcutInfo> WebAppShortcutManager::BuildShortcutInfo(
+    const AppId& app_id) {
   const WebApp* app = GetWebAppRegistrar().GetAppById(app_id);
   DCHECK(app);
-
-  std::unique_ptr<ShortcutInfo> shortcut_info = BuildShortcutInfoForWebApp(app);
-  base::FilePath shortcut_data_dir =
-      internals::GetShortcutDataDir(*shortcut_info);
-
-  internals::PostShortcutIOTask(
-      base::BindOnce(&internals::DeletePlatformShortcuts, shortcut_data_dir),
-      std::move(shortcut_info));
+  return BuildShortcutInfoForWebApp(app);
 }
 
 void WebAppShortcutManager::GetShortcutInfoForApp(

chrome/browser/web_applications/web_app_shortcut_manager.h

@@ -32,10 +32,8 @@ class WebAppShortcutManager : public AppShortcutManager {
                         FileHandlerManager* file_handler_manager);
   ~WebAppShortcutManager() override;
 
-  // AppRegistrarObserver:
-  void OnWebAppWillBeUninstalled(const AppId& app_id) override;
-
   // AppShortcutManager:
+  std::unique_ptr<ShortcutInfo> BuildShortcutInfo(const AppId& app_id) override;
   void GetShortcutInfoForApp(const AppId& app_id,
                              GetShortcutInfoCallback callback) override;