MacPWAs: Add callback for shortcut creation

This callback will be needed to determine if we should reparent web
contents into an app window on macOS. For the moment, leave the
callback unused in all instances.

Bug: 915571
Change-Id: Ib86415a7a3d1981db817c39dfe7bb3a8b23ed571
Reviewed-on: https://chromium-review.googlesource.com/c/1487917Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635648}

chrome/browser/apps/platform_apps/shortcut_manager.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/apps/platform_apps/shortcut_manager.h"
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/strings/string16.h"
@@ -54,7 +55,7 @@ void CreateShortcutsForApp(Profile* profile, const Extension* app) {
       web_app::APP_MENU_LOCATION_SUBDIR_CHROMEAPPS;
 
   web_app::CreateShortcuts(web_app::SHORTCUT_CREATION_AUTOMATED,
-                           creation_locations, profile, app);
+                           creation_locations, profile, app, base::DoNothing());
 }
 
 }  // namespace

chrome/browser/extensions/bookmark_app_extension_util.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/extensions/bookmark_app_extension_util.h"
 
+#include <utility>
+
+#include "base/callback.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
@@ -25,8 +28,10 @@
 
 namespace extensions {
 
-void BookmarkAppCreateOsShortcuts(Profile* profile,
-                                  const Extension* extension) {
+void BookmarkAppCreateOsShortcuts(
+    Profile* profile,
+    const Extension* extension,
+    base::OnceCallback<void(bool created_shortcuts)> callback) {
 #if !defined(OS_CHROMEOS)
   web_app::ShortcutLocations creation_locations;
 #if defined(OS_LINUX) || defined(OS_WIN)
@@ -40,7 +45,8 @@ void BookmarkAppCreateOsShortcuts(Profile* profile,
 
   Profile* current_profile = profile->GetOriginalProfile();
   web_app::CreateShortcuts(web_app::SHORTCUT_CREATION_BY_USER,
-                           creation_locations, current_profile, extension);
+                           creation_locations, current_profile, extension,
+                           std::move(callback));
 #else
   // ChromeLauncherController does not exist in unit tests.
   if (ChromeLauncherController::instance()) {

chrome/browser/extensions/bookmark_app_extension_util.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_BOOKMARK_APP_EXTENSION_UTIL_H_
 #define CHROME_BROWSER_EXTENSIONS_BOOKMARK_APP_EXTENSION_UTIL_H_
 
+#include "base/callback_forward.h"
+
 class Profile;
 
 namespace content {
@@ -15,7 +17,10 @@ namespace extensions {
 
 class Extension;
 
-void BookmarkAppCreateOsShortcuts(Profile* profile, const Extension* extension);
+void BookmarkAppCreateOsShortcuts(
+    Profile* profile,
+    const Extension* extension,
+    base::OnceCallback<void(bool created_shortcuts)> callback);
 
 void BookmarkAppReparentTab(content::WebContents* contents,
                             const Extension* extension);

chrome/browser/extensions/bookmark_app_helper.cc

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/macros.h"
@@ -471,7 +472,7 @@ void BookmarkAppHelper::FinishInstallation(const Extension* extension) {
   web_app::RecordAppBanner(contents_, web_app_info_.app_url);
 
   if (create_shortcuts_)
-    BookmarkAppCreateOsShortcuts(profile_, extension);
+    BookmarkAppCreateOsShortcuts(profile_, extension, base::DoNothing());
 
   // If there is a browser, it means that the app is being installed in the
   // foreground: window reparenting needed.

chrome/browser/ui/views/create_application_shortcut_view.cc

@@ -210,7 +210,7 @@ bool CreateChromeApplicationShortcutView::Accept() {
 #endif
 
   web_app::CreateShortcutsWithInfo(web_app::SHORTCUT_CREATION_BY_USER,
-                                   creation_locations,
+                                   creation_locations, base::DoNothing(),
                                    std::move(shortcut_info_));
   return true;
 }

chrome/browser/web_applications/components/install_finalizer.h

@@ -27,12 +27,15 @@ class InstallFinalizer {
  public:
   using InstallFinalizedCallback =
       base::OnceCallback<void(const AppId& app_id, InstallResultCode code)>;
+  using CreateOsShortcutsCallback =
+      base::OnceCallback<void(bool shortcuts_created)>;
 
   // Write the WebApp data to disk and register the app.
   virtual void FinalizeInstall(const WebApplicationInfo& web_app_info,
                                InstallFinalizedCallback callback) = 0;
 
-  virtual void CreateOsShortcuts(const AppId& app_id) = 0;
+  virtual void CreateOsShortcuts(const AppId& app_id,
+                                 CreateOsShortcutsCallback callback) = 0;
   virtual void ReparentTab(const AppId& app_id,
                            content::WebContents* web_contents) = 0;
   virtual bool CanRevealAppShim() const = 0;

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.cc

@@ -96,9 +96,10 @@ void BookmarkAppInstallFinalizer::OnExtensionInstalled(
 }
 
 void BookmarkAppInstallFinalizer::CreateOsShortcuts(
-    const web_app::AppId& app_id) {
+    const web_app::AppId& app_id,
+    CreateOsShortcutsCallback callback) {
   const Extension* app = GetExtensionById(profile_, app_id);
-  BookmarkAppCreateOsShortcuts(profile_, app);
+  BookmarkAppCreateOsShortcuts(profile_, app, std::move(callback));
 }
 
 void BookmarkAppInstallFinalizer::ReparentTab(

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_INSTALL_FINALIZER_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_INSTALL_FINALIZER_H_
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ -35,7 +36,8 @@ class BookmarkAppInstallFinalizer : public web_app::InstallFinalizer {
   // InstallFinalizer:
   void FinalizeInstall(const WebApplicationInfo& web_app_info,
                        InstallFinalizedCallback callback) override;
-  void CreateOsShortcuts(const web_app::AppId& app_id) override;
+  void CreateOsShortcuts(const web_app::AppId& app_id,
+                         CreateOsShortcutsCallback callback) override;
   void ReparentTab(const web_app::AppId& app_id,
                    content::WebContents* web_contents) override;
   bool CanRevealAppShim() const override;

chrome/browser/web_applications/extensions/web_app_extension_shortcut.cc

@@ -10,6 +10,8 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_ui_util.h"
@@ -19,6 +21,7 @@
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
+#include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/image_loader.h"
@@ -97,15 +100,29 @@ void UpdateAllShortcutsForShortcutInfo(
       std::move(shortcut_info), std::move(callback));
 }
 
+void CreatePlatformShortcutsAndPostCallback(
+    const base::FilePath& shortcut_data_path,
+    const ShortcutLocations& creation_locations,
+    ShortcutCreationReason creation_reason,
+    CreateShortcutsCallback callback,
+    const ShortcutInfo& shortcut_info) {
+  bool shortcut_created = internals::CreatePlatformShortcuts(
+      shortcut_data_path, creation_locations, creation_reason, shortcut_info);
+  base::PostTaskWithTraits(
+      FROM_HERE, {BrowserThread::UI},
+      base::BindOnce(std::move(callback), shortcut_created));
+}
+
 void ScheduleCreatePlatformShortcut(
     ShortcutCreationReason reason,
     const ShortcutLocations& locations,
+    CreateShortcutsCallback callback,
     std::unique_ptr<ShortcutInfo> shortcut_info) {
   base::FilePath shortcut_data_dir =
       internals::GetShortcutDataDir(*shortcut_info);
   internals::PostShortcutIOTask(
-      base::BindOnce(base::IgnoreResult(&internals::CreatePlatformShortcuts),
-                     shortcut_data_dir, locations, reason),
+      base::BindOnce(&CreatePlatformShortcutsAndPostCallback, shortcut_data_dir,
+                     locations, reason, std::move(callback)),
       std::move(shortcut_info));
 }
 
@@ -113,6 +130,7 @@ void ScheduleCreatePlatformShortcut(
 
 void CreateShortcutsWithInfo(ShortcutCreationReason reason,
                              const ShortcutLocations& locations,
+                             CreateShortcutsCallback callback,
                              std::unique_ptr<ShortcutInfo> shortcut_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -120,26 +138,33 @@ void CreateShortcutsWithInfo(ShortcutCreationReason reason,
   // flow disabled, there will be no corresponding extension.
   if (!shortcut_info->extension_id.empty()) {
     // The profile manager does not exist in some unit tests.
-    if (!g_browser_process->profile_manager())
+    if (!g_browser_process->profile_manager()) {
+      std::move(callback).Run(false /* created_shortcut */);
       return;
+    }
 
     // It's possible for the extension to be deleted before we get here.
     // For example, creating a hosted app from a website. Double check that
     // it still exists.
     Profile* profile = g_browser_process->profile_manager()->GetProfileByPath(
         shortcut_info->profile_path);
-    if (!profile)
+    if (!profile) {
+      std::move(callback).Run(false /* created_shortcut */);
       return;
+    }
 
     extensions::ExtensionRegistry* registry =
         extensions::ExtensionRegistry::Get(profile);
     const extensions::Extension* extension = registry->GetExtensionById(
         shortcut_info->extension_id, extensions::ExtensionRegistry::EVERYTHING);
-    if (!extension)
+    if (!extension) {
+      std::move(callback).Run(false /* created_shortcut */);
       return;
+    }
   }
 
-  ScheduleCreatePlatformShortcut(reason, locations, std::move(shortcut_info));
+  ScheduleCreatePlatformShortcut(reason, locations, std::move(callback),
+                                 std::move(shortcut_info));
 }
 
 void GetShortcutInfoForApp(const extensions::Extension* extension,
@@ -250,15 +275,18 @@ base::FilePath GetWebAppDataDirectory(const base::FilePath& profile_path,
 void CreateShortcuts(ShortcutCreationReason reason,
                      const ShortcutLocations& locations,
                      Profile* profile,
-                     const extensions::Extension* app) {
+                     const extensions::Extension* app,
+                     CreateShortcutsCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (!ShouldCreateShortcutFor(reason, profile, app))
+  if (!ShouldCreateShortcutFor(reason, profile, app)) {
+    std::move(callback).Run(false /* created_shortcut */);
     return;
+  }
 
-  GetShortcutInfoForApp(
-      app, profile,
-      base::BindOnce(&CreateShortcutsWithInfo, reason, locations));
+  GetShortcutInfoForApp(app, profile,
+                        base::BindOnce(&CreateShortcutsWithInfo, reason,
+                                       locations, std::move(callback)));
 }
 
 void DeleteAllShortcuts(Profile* profile, const extensions::Extension* app) {

chrome/browser/web_applications/extensions/web_app_extension_shortcut.h

@@ -21,15 +21,22 @@ class Extension;
 
 namespace web_app {
 
+// Callback made when CreateShortcuts has finished trying to create the
+// platform shortcuts indicating whether or not they were successfully created.
+using CreateShortcutsCallback = base::OnceCallback<void(bool shortcut_created)>;
+
 // Called by GetShortcutInfoForApp after fetching the ShortcutInfo.
-typedef base::OnceCallback<void(std::unique_ptr<ShortcutInfo>)>
-    ShortcutInfoCallback;
+using ShortcutInfoCallback =
+    base::OnceCallback<void(std::unique_ptr<ShortcutInfo>)>;
 
 // Create shortcuts for web application based on given shortcut data.
-// |shortcut_info| contains information about the shortcuts to create, and
-// |locations| contains information about where to create them.
+// |shortcut_info| contains information about the shortcuts to create,
+// |locations| contains information about where to create them, and
+// |callback| is a callback that is made when completed, indicating success
+// or failure of the operation.
 void CreateShortcutsWithInfo(ShortcutCreationReason reason,
                              const ShortcutLocations& locations,
+                             CreateShortcutsCallback callback,
                              std::unique_ptr<ShortcutInfo> shortcut_info);
 
 // Populates a ShortcutInfo for the given |extension| in |profile| and passes
@@ -58,7 +65,8 @@ base::FilePath GetWebAppDataDirectory(const base::FilePath& profile_path,
 void CreateShortcuts(ShortcutCreationReason reason,
                      const ShortcutLocations& locations,
                      Profile* profile,
-                     const extensions::Extension* app);
+                     const extensions::Extension* app,
+                     CreateShortcutsCallback callback);
 
 // Delete all shortcuts that have been created for the given profile and
 // extension.

chrome/browser/web_applications/extensions/web_app_extension_shortcut_mac.mm

@@ -158,7 +158,8 @@ void ShowCreateChromeAppShortcutsDialog(
   // On Mac, the Applications folder is the only option, so don't bother asking
   // the user anything. Just create shortcuts.
   CreateShortcuts(web_app::SHORTCUT_CREATION_BY_USER,
-                  web_app::ShortcutLocations(), profile, app);
+                  web_app::ShortcutLocations(), profile, app,
+                  base::DoNothing());
   if (!close_callback.is_null())
     close_callback.Run(true);
 }

chrome/browser/web_applications/test/test_install_finalizer.cc

@@ -40,8 +40,13 @@ void TestInstallFinalizer::FinalizeInstall(
       FROM_HERE, base::BindOnce(std::move(callback), app_id, code));
 }
 
-void TestInstallFinalizer::CreateOsShortcuts(const AppId& app_id) {
+void TestInstallFinalizer::CreateOsShortcuts(
+    const AppId& app_id,
+    CreateOsShortcutsCallback callback) {
   ++num_create_os_shortcuts_calls_;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(std::move(callback), true /* shortcuts_created */));
 }
 
 void TestInstallFinalizer::ReparentTab(const AppId& app_id,

chrome/browser/web_applications/test/test_install_finalizer.h

@@ -23,7 +23,8 @@ class TestInstallFinalizer final : public InstallFinalizer {
   // InstallFinalizer:
   void FinalizeInstall(const WebApplicationInfo& web_app_info,
                        InstallFinalizedCallback callback) override;
-  void CreateOsShortcuts(const AppId& app_id) override;
+  void CreateOsShortcuts(const AppId& app_id,
+                         CreateOsShortcutsCallback callback) override;
   void ReparentTab(const AppId& app_id,
                    content::WebContents* web_contents) override;
   bool CanRevealAppShim() const override;

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -96,9 +96,14 @@ void WebAppInstallFinalizer::OnDataWritten(InstallFinalizedCallback callback,
   std::move(callback).Run(std::move(app_id), InstallResultCode::kSuccess);
 }
 
-void WebAppInstallFinalizer::CreateOsShortcuts(const AppId& app_id) {
+void WebAppInstallFinalizer::CreateOsShortcuts(
+    const AppId& app_id,
+    CreateOsShortcutsCallback callback) {
   // TODO(loyso): Implement it.
   NOTIMPLEMENTED();
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(std::move(callback), false /* shortcuts_created */));
 }
 
 void WebAppInstallFinalizer::ReparentTab(const AppId& app_id,

chrome/browser/web_applications/web_app_install_finalizer.h

@@ -28,7 +28,8 @@ class WebAppInstallFinalizer final : public InstallFinalizer {
   // InstallFinalizer:
   void FinalizeInstall(const WebApplicationInfo& web_app_info,
                        InstallFinalizedCallback callback) override;
-  void CreateOsShortcuts(const AppId& app_id) override;
+  void CreateOsShortcuts(const AppId& app_id,
+                         CreateOsShortcutsCallback callback) override;
   void ReparentTab(const AppId& app_id,
                    content::WebContents* web_contents) override;
   bool CanRevealAppShim() const override;

chrome/browser/web_applications/web_app_install_manager.cc

@@ -218,7 +218,9 @@ void WebAppInstallManager::OnInstallFinalized(
     RecordAppBanner(web_contents(), web_app_info->app_url);
 
     // TODO(loyso): Implement |create_shortcuts| to skip OS shortcuts creation.
-    install_finalizer_->CreateOsShortcuts(app_id);
+    // TODO(https://crbug.com/915571): Only re-parent tabs upon successful
+    // shortcut creation (at least on macOS).
+    install_finalizer_->CreateOsShortcuts(app_id, base::DoNothing());
     // TODO(loyso): Implement |reparent_tab| to skip tab reparenting logic.
     if (web_app_info->open_as_window)
       install_finalizer_->ReparentTab(app_id, web_contents());