MacPWAs BMO: Access AppShimRegistry from web_applications

This access was previously living in apps/platform_apps,
and using extension mechanisms.

In a previous patch, I had moved AppShimRegistry into
apps/app_shim. This was a mistake -- AppShimRegistry is
only for web apps (not for all app shims). Move it to
be in web_applications/components, and update it
in web_app::AppShortcutManager.

R=loyso
TBR=avi (chrome/)

Bug: 1057195
Change-Id: I284a56b472fde55f33f30a84c5959a68e4f0a04a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116564
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753483}

chrome/browser/apps/app_shim/BUILD.gn

@@ -14,8 +14,6 @@ source_set("app_shim") {
     "app_shim_listener.mm",
     "app_shim_manager_mac.cc",
     "app_shim_manager_mac.h",
-    "app_shim_registry_mac.cc",
-    "app_shim_registry_mac.h",
     "app_shim_termination_manager.cc",
     "app_shim_termination_manager.h",
     "mach_bootstrap_acceptor.cc",

chrome/browser/apps/app_shim/app_shim_manager_mac.cc

@@ -24,7 +24,6 @@
 #include "chrome/browser/apps/app_shim/app_shim_host_bootstrap_mac.h"
 #include "chrome/browser/apps/app_shim/app_shim_host_mac.h"
 #include "chrome/browser/apps/app_shim/app_shim_listener.h"
-#include "chrome/browser/apps/app_shim/app_shim_registry_mac.h"
 #include "chrome/browser/apps/app_shim/app_shim_termination_manager.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
@@ -37,6 +36,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/web_applications/components/app_shim_registry_mac.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_shortcut_mac.h"
 #include "components/crx_file/id_util.h"

chrome/browser/apps/app_shim/app_shim_manager_mac_unittest.cc

@@ -18,9 +18,9 @@
 #include "base/test/mock_callback.h"
 #include "chrome/browser/apps/app_shim/app_shim_host_bootstrap_mac.h"
 #include "chrome/browser/apps/app_shim/app_shim_host_mac.h"
-#include "chrome/browser/apps/app_shim/app_shim_registry_mac.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/avatar_menu.h"
+#include "chrome/browser/web_applications/components/app_shim_registry_mac.h"
 #include "chrome/common/mac/app_shim.mojom.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/prefs/testing_pref_service.h"

chrome/browser/apps/platform_apps/shortcut_manager.cc

@@ -30,7 +30,6 @@
 #include "extensions/common/extension_set.h"
 
 #if defined(OS_MACOSX)
-#include "chrome/browser/apps/app_shim/app_shim_registry_mac.h"
 #include "chrome/common/mac/app_mode_common.h"
 #endif
 
@@ -38,15 +37,6 @@ using extensions::Extension;
 
 namespace {
 
-#if defined(OS_MACOSX)
-bool UseAppShimRegistry(content::BrowserContext* browser_context,
-                        const Extension* extension) {
-  if (browser_context->IsOffTheRecord())
-    return false;
-  return extension->is_app() && extension->from_bookmark();
-}
-#endif
-
 // This version number is stored in local prefs to check whether app shortcuts
 // need to be recreated. This might happen when we change various aspects of app
 // shortcuts like command-line flags or associated icons, binaries, etc.
@@ -114,23 +104,6 @@ AppShortcutManager::~AppShortcutManager() {
   }
 }
 
-void AppShortcutManager::OnExtensionLoaded(
-    content::BrowserContext* browser_context,
-    const Extension* extension) {
-#if defined(OS_MACOSX)
-  // Register installed apps as soon as their extension is loaded. This happens
-  // 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).
-  if (UseAppShimRegistry(browser_context, extension)) {
-    AppShimRegistry::Get()->OnAppInstalledForProfile(extension->id(),
-                                                     profile_->GetPath());
-  }
-#endif
-}
-
 void AppShortcutManager::OnExtensionWillBeInstalled(
     content::BrowserContext* browser_context,
     const Extension* extension,
@@ -139,13 +112,6 @@ void AppShortcutManager::OnExtensionWillBeInstalled(
   if (!extension->is_app())
     return;
 
-#if defined(OS_MACOSX)
-  if (UseAppShimRegistry(browser_context, extension)) {
-    AppShimRegistry::Get()->OnAppInstalledForProfile(extension->id(),
-                                                     profile_->GetPath());
-  }
-#endif
-
   // If the app is being updated, update any existing shortcuts but do not
   // create new ones. If it is being installed, automatically create a
   // shortcut in the applications menu (e.g., Start Menu).
@@ -161,21 +127,6 @@ void AppShortcutManager::OnExtensionUninstalled(
     content::BrowserContext* browser_context,
     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(),
-                                                           profile_->GetPath());
-    if (delete_multi_profile_shortcuts) {
-      web_app::internals::GetShortcutIOTaskRunner()->PostTask(
-          FROM_HERE,
-          base::BindOnce(&web_app::internals::DeleteMultiProfileShortcutsForApp,
-                         extension->id()));
-    }
-  }
-#endif
-
   // Bookmark apps are handled in
   // web_app::AppShortcutManager::OnWebAppWillBeUninstalled()
   if (!extension->from_bookmark())
@@ -186,24 +137,6 @@ void AppShortcutManager::OnProfileWillBeRemoved(
     const base::FilePath& profile_path) {
   if (profile_path != profile_->GetPath())
     return;
-
-#if defined(OS_MACOSX)
-  // If any multi-profile app shims exist only for this profile, delete them.
-  std::set<std::string> apps_for_profile =
-      AppShimRegistry::Get()->GetInstalledAppsForProfile(profile_path);
-  for (const auto& app_id : apps_for_profile) {
-    bool delete_multi_profile_shortcuts =
-        AppShimRegistry::Get()->OnAppUninstalledForProfile(app_id,
-                                                           profile_path);
-    if (delete_multi_profile_shortcuts) {
-      web_app::internals::GetShortcutIOTaskRunner()->PostTask(
-          FROM_HERE,
-          base::BindOnce(&web_app::internals::DeleteMultiProfileShortcutsForApp,
-                         app_id));
-    }
-  }
-#endif
-
   web_app::internals::GetShortcutIOTaskRunner()->PostTask(
       FROM_HERE,
       base::BindOnce(&web_app::internals::DeleteAllShortcutsForProfile,

chrome/browser/apps/platform_apps/shortcut_manager.h

@@ -40,8 +40,6 @@ class AppShortcutManager : public KeyedService,
   void UpdateShortcutsForAllAppsIfNeeded();
 
   // extensions::ExtensionRegistryObserver.
-  void OnExtensionLoaded(content::BrowserContext* browser_context,
-                         const extensions::Extension* extension) override;
   void OnExtensionWillBeInstalled(content::BrowserContext* browser_context,
                                   const extensions::Extension* extension,
                                   bool is_update,

chrome/browser/prefs/browser_prefs.cc

@@ -354,9 +354,9 @@
 #endif
 
 #if defined(OS_MACOSX)
-#include "chrome/browser/apps/app_shim/app_shim_registry_mac.h"
 #include "chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac.h"
 #include "chrome/browser/ui/cocoa/confirm_quit.h"
+#include "chrome/browser/web_applications/components/app_shim_registry_mac.h"
 #endif
 
 #if defined(OS_WIN)

chrome/browser/web_applications/components/BUILD.gn

@@ -89,6 +89,8 @@ source_set("components") {
 
   if (is_mac) {
     sources += [
+      "app_shim_registry_mac.cc",
+      "app_shim_registry_mac.h",
       "web_app_file_handler_registration_mac.cc",
       "web_app_shortcut_mac.h",
       "web_app_shortcut_mac.mm",
@@ -159,7 +161,10 @@ source_set("unit_tests") {
   }
 
   if (is_mac) {
-    sources += [ "web_app_shortcut_mac_unittest.mm" ]
+    sources += [
+      "app_shim_registry_mac_unittest.cc",
+      "web_app_shortcut_mac_unittest.mm",
+    ]
   }
 
   if (is_desktop_linux) {

chrome/browser/apps/app_shim/app_shim_registry_mac.cc → chrome/browser/web_applications/components/app_shim_registry_mac.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/apps/app_shim/app_shim_registry_mac.h"
+#include "chrome/browser/web_applications/components/app_shim_registry_mac.h"
 
 #include <memory>
 #include <utility>

chrome/browser/apps/app_shim/app_shim_registry_mac.h → chrome/browser/web_applications/components/app_shim_registry_mac.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_APPS_APP_SHIM_APP_SHIM_REGISTRY_MAC_H_
-#define CHROME_BROWSER_APPS_APP_SHIM_APP_SHIM_REGISTRY_MAC_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_SHIM_REGISTRY_MAC_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_SHIM_REGISTRY_MAC_H_
 
 #include <set>
 #include <string>
@@ -96,4 +96,4 @@ class AppShimRegistry {
   base::FilePath override_user_data_dir_;
 };
 
-#endif  // CHROME_BROWSER_APPS_APP_SHIM_APP_SHIM_REGISTRY_MAC_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_SHIM_REGISTRY_MAC_H_

chrome/browser/apps/app_shim/app_shim_registry_mac_unittest.cc → chrome/browser/web_applications/components/app_shim_registry_mac_unittest.cc

@@ -2,9 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/apps/app_shim/app_shim_registry_mac.h"
+#include "chrome/browser/web_applications/components/app_shim_registry_mac.h"
 
-#include "chrome/browser/apps/app_shim/app_shim_manager_mac.h"
 #include "components/prefs/testing_pref_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 

chrome/browser/web_applications/components/app_shortcut_manager.cc

@@ -5,10 +5,16 @@
 #include "chrome/browser/web_applications/components/app_shortcut_manager.h"
 
 #include "base/callback.h"
+#include "build/build_config.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/components/app_shortcut_observer.h"
 #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 {
 
 AppShortcutManager::AppShortcutManager(Profile* profile) : profile_(profile) {}
@@ -25,6 +31,20 @@ void AppShortcutManager::SetSubsystems(AppRegistrar* registrar) {
 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() {
@@ -39,6 +59,12 @@ void AppShortcutManager::RemoveObserver(AppShortcutObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void AppShortcutManager::OnWebAppInstalled(const AppId& app_id) {
+#if defined(OS_MACOSX)
+  AppShimRegistry::Get()->OnAppInstalledForProfile(app_id, profile_->GetPath());
+#endif
+}
+
 void AppShortcutManager::OnWebAppWillBeUninstalled(const AppId& app_id) {
   std::unique_ptr<ShortcutInfo> shortcut_info = BuildShortcutInfo(app_id);
   base::FilePath shortcut_data_dir =
@@ -49,6 +75,28 @@ void AppShortcutManager::OnWebAppWillBeUninstalled(const AppId& app_id) {
       std::move(shortcut_info));
 }
 
+void AppShortcutManager::OnWebAppUninstalled(const AppId& app_id) {
+  DeleteSharedAppShims(app_id);
+}
+
+void AppShortcutManager::OnWebAppProfileWillBeDeleted(const AppId& app_id) {
+  DeleteSharedAppShims(app_id);
+}
+
+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
+}
+
 bool AppShortcutManager::CanCreateShortcuts() const {
 #if defined(OS_CHROMEOS)
   return false;

chrome/browser/web_applications/components/app_shortcut_manager.h

@@ -45,7 +45,10 @@ class AppShortcutManager : public AppRegistrarObserver {
   void RemoveObserver(AppShortcutObserver* observer);
 
   // AppRegistrarObserver:
+  void OnWebAppInstalled(const AppId& app_id) override;
   void OnWebAppWillBeUninstalled(const AppId& app_id) override;
+  void OnWebAppUninstalled(const AppId& app_id) override;
+  void OnWebAppProfileWillBeDeleted(const AppId& app_id) override;
 
   // Tells the AppShortcutManager that no shortcuts should actually be written
   // to the disk.
@@ -71,6 +74,7 @@ class AppShortcutManager : public AppRegistrarObserver {
                                      GetShortcutInfoCallback callback) = 0;
 
  protected:
+  void DeleteSharedAppShims(const AppId& app_id);
   void OnShortcutsCreated(const AppId& app_id,
                           CreateShortcutsCallback callback,
                           bool success);

chrome/test/BUILD.gn

@@ -4486,7 +4486,6 @@ test("unit_tests") {
       "../../tools/json_schema_compiler/test/features_generation_unittest.cc",
       "../browser/apps/app_shim/app_shim_host_mac_unittest.cc",
       "../browser/apps/app_shim/app_shim_manager_mac_unittest.cc",
-      "../browser/apps/app_shim/app_shim_registry_mac_unittest.cc",
       "../browser/autocomplete/keyword_extensions_delegate_impl_unittest.cc",
       "../browser/browsing_data/counters/hosted_apps_counter_unittest.cc",
       "../browser/extensions/active_tab_unittest.cc",