WebApp: Extract PinAppToShelf util and pure virtual function.

Refactor CreateOsShortcuts util.

CreateOsShortcuts and PinAppToShelf are unrelated steps.

Bug: 915043
Change-Id: I67cb8e2685e96149ec88fb3cd20c9383e4e35d6c
Reviewed-on: https://chromium-review.googlesource.com/c/1491052Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635884}

chrome/browser/extensions/bookmark_app_extension_util.cc

@@ -28,10 +28,19 @@
 
 namespace extensions {
 
+bool CanBookmarkAppCreateOsShortcuts() {
+#if defined(OS_CHROMEOS)
+  return false;
+#else
+  return true;
+#endif
+}
+
 void BookmarkAppCreateOsShortcuts(
     Profile* profile,
     const Extension* extension,
     base::OnceCallback<void(bool created_shortcuts)> callback) {
+  DCHECK(CanBookmarkAppCreateOsShortcuts());
 #if !defined(OS_CHROMEOS)
   web_app::ShortcutLocations creation_locations;
 #if defined(OS_LINUX) || defined(OS_WIN)
@@ -47,13 +56,26 @@ void BookmarkAppCreateOsShortcuts(
   web_app::CreateShortcuts(web_app::SHORTCUT_CREATION_BY_USER,
                            creation_locations, current_profile, extension,
                            std::move(callback));
+#endif  // !defined(OS_CHROMEOS)
+}
+
+bool CanBookmarkAppBePinnedToShelf() {
+#if defined(OS_CHROMEOS)
+  return true;
 #else
+  return false;
+#endif
+}
+
+void BookmarkAppPinToShelf(const Extension* extension) {
+  DCHECK(CanBookmarkAppBePinnedToShelf());
+#if defined(OS_CHROMEOS)
   // ChromeLauncherController does not exist in unit tests.
   if (ChromeLauncherController::instance()) {
     ChromeLauncherController::instance()->shelf_model()->PinAppWithID(
         extension->id());
   }
-#endif  // !defined(OS_CHROMEOS)
+#endif  // defined(OS_CHROMEOS)
 }
 
 void BookmarkAppReparentTab(content::WebContents* contents,

chrome/browser/extensions/bookmark_app_extension_util.h

@@ -17,17 +17,19 @@ namespace extensions {
 
 class Extension;
 
+bool CanBookmarkAppCreateOsShortcuts();
 void BookmarkAppCreateOsShortcuts(
     Profile* profile,
     const Extension* extension,
     base::OnceCallback<void(bool created_shortcuts)> callback);
 
+bool CanBookmarkAppBePinnedToShelf();
+void BookmarkAppPinToShelf(const Extension* extension);
+
 void BookmarkAppReparentTab(content::WebContents* contents,
                             const Extension* extension);
 
-// Returns true if OS supports AppShim revealing,
 bool CanBookmarkAppRevealAppShim();
-// Reveals AppShim in Finder for a given app,
 void BookmarkAppRevealAppShim(Profile* profile, const Extension* extension);
 
 }  // namespace extensions

chrome/browser/extensions/bookmark_app_helper.cc

@@ -471,9 +471,12 @@ void BookmarkAppHelper::FinishInstallation(const Extension* extension) {
 
   web_app::RecordAppBanner(contents_, web_app_info_.app_url);
 
-  if (create_shortcuts_)
+  if (create_shortcuts_ && CanBookmarkAppCreateOsShortcuts())
     BookmarkAppCreateOsShortcuts(profile_, extension, base::DoNothing());
 
+  if (create_shortcuts_ && CanBookmarkAppBePinnedToShelf())
+    BookmarkAppPinToShelf(extension);
+
   // If there is a browser, it means that the app is being installed in the
   // foreground: window reparenting needed.
   const bool reparent_tab =

chrome/browser/web_applications/components/install_finalizer.h

@@ -34,10 +34,16 @@ class InstallFinalizer {
   virtual void FinalizeInstall(const WebApplicationInfo& web_app_info,
                                InstallFinalizedCallback callback) = 0;
 
+  virtual bool CanCreateOsShortcuts() const = 0;
   virtual void CreateOsShortcuts(const AppId& app_id,
                                  CreateOsShortcutsCallback callback) = 0;
+
+  virtual bool CanPinAppToShelf() const = 0;
+  virtual void PinAppToShelf(const AppId& app_id) = 0;
+
   virtual void ReparentTab(const AppId& app_id,
                            content::WebContents* web_contents) = 0;
+
   virtual bool CanRevealAppShim() const = 0;
   virtual void RevealAppShim(const AppId& app_id) = 0;
 

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.cc

@@ -95,6 +95,10 @@ void BookmarkAppInstallFinalizer::OnExtensionInstalled(
                           web_app::InstallResultCode::kSuccess);
 }
 
+bool BookmarkAppInstallFinalizer::CanCreateOsShortcuts() const {
+  return CanBookmarkAppCreateOsShortcuts();
+}
+
 void BookmarkAppInstallFinalizer::CreateOsShortcuts(
     const web_app::AppId& app_id,
     CreateOsShortcutsCallback callback) {
@@ -102,6 +106,15 @@ void BookmarkAppInstallFinalizer::CreateOsShortcuts(
   BookmarkAppCreateOsShortcuts(profile_, app, std::move(callback));
 }
 
+bool BookmarkAppInstallFinalizer::CanPinAppToShelf() const {
+  return CanBookmarkAppBePinnedToShelf();
+}
+
+void BookmarkAppInstallFinalizer::PinAppToShelf(const web_app::AppId& app_id) {
+  const Extension* app = GetExtensionById(profile_, app_id);
+  BookmarkAppPinToShelf(app);
+}
+
 void BookmarkAppInstallFinalizer::ReparentTab(
     const web_app::AppId& app_id,
     content::WebContents* web_contents) {

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.h

@@ -36,8 +36,11 @@ class BookmarkAppInstallFinalizer : public web_app::InstallFinalizer {
   // InstallFinalizer:
   void FinalizeInstall(const WebApplicationInfo& web_app_info,
                        InstallFinalizedCallback callback) override;
+  bool CanCreateOsShortcuts() const override;
   void CreateOsShortcuts(const web_app::AppId& app_id,
                          CreateOsShortcutsCallback callback) override;
+  bool CanPinAppToShelf() const override;
+  void PinAppToShelf(const web_app::AppId& app_id) override;
   void ReparentTab(const web_app::AppId& app_id,
                    content::WebContents* web_contents) override;
   bool CanRevealAppShim() const override;

chrome/browser/web_applications/test/test_install_finalizer.cc

@@ -40,6 +40,10 @@ void TestInstallFinalizer::FinalizeInstall(
       FROM_HERE, base::BindOnce(std::move(callback), app_id, code));
 }
 
+bool TestInstallFinalizer::CanCreateOsShortcuts() const {
+  return true;
+}
+
 void TestInstallFinalizer::CreateOsShortcuts(
     const AppId& app_id,
     CreateOsShortcutsCallback callback) {
@@ -49,6 +53,14 @@ void TestInstallFinalizer::CreateOsShortcuts(
       base::BindOnce(std::move(callback), true /* shortcuts_created */));
 }
 
+bool TestInstallFinalizer::CanPinAppToShelf() const {
+  return true;
+}
+
+void TestInstallFinalizer::PinAppToShelf(const AppId& app_id) {
+  ++num_pin_app_to_shelf_calls_;
+}
+
 void TestInstallFinalizer::ReparentTab(const AppId& app_id,
                                        content::WebContents* web_contents) {
   ++num_reparent_tab_calls_;

chrome/browser/web_applications/test/test_install_finalizer.h

@@ -23,8 +23,11 @@ class TestInstallFinalizer final : public InstallFinalizer {
   // InstallFinalizer:
   void FinalizeInstall(const WebApplicationInfo& web_app_info,
                        InstallFinalizedCallback callback) override;
+  bool CanCreateOsShortcuts() const override;
   void CreateOsShortcuts(const AppId& app_id,
                          CreateOsShortcutsCallback callback) override;
+  bool CanPinAppToShelf() const override;
+  void PinAppToShelf(const AppId& app_id) override;
   void ReparentTab(const AppId& app_id,
                    content::WebContents* web_contents) override;
   bool CanRevealAppShim() const override;
@@ -40,6 +43,7 @@ class TestInstallFinalizer final : public InstallFinalizer {
   int num_create_os_shortcuts_calls() { return num_create_os_shortcuts_calls_; }
   int num_reparent_tab_calls() { return num_reparent_tab_calls_; }
   int num_reveal_appshim_calls() { return num_reveal_appshim_calls_; }
+  int num_pin_app_to_shelf_calls() { return num_pin_app_to_shelf_calls_; }
 
  private:
   std::unique_ptr<WebApplicationInfo> web_app_info_copy_;
@@ -50,6 +54,7 @@ class TestInstallFinalizer final : public InstallFinalizer {
   int num_create_os_shortcuts_calls_ = 0;
   int num_reparent_tab_calls_ = 0;
   int num_reveal_appshim_calls_ = 0;
+  int num_pin_app_to_shelf_calls_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(TestInstallFinalizer);
 };

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -96,6 +96,12 @@ void WebAppInstallFinalizer::OnDataWritten(InstallFinalizedCallback callback,
   std::move(callback).Run(std::move(app_id), InstallResultCode::kSuccess);
 }
 
+bool WebAppInstallFinalizer::CanCreateOsShortcuts() const {
+  // TODO(loyso): Implement it.
+  NOTIMPLEMENTED();
+  return false;
+}
+
 void WebAppInstallFinalizer::CreateOsShortcuts(
     const AppId& app_id,
     CreateOsShortcutsCallback callback) {
@@ -106,6 +112,17 @@ void WebAppInstallFinalizer::CreateOsShortcuts(
       base::BindOnce(std::move(callback), false /* shortcuts_created */));
 }
 
+bool WebAppInstallFinalizer::CanPinAppToShelf() const {
+  // TODO(loyso): Implement it.
+  NOTIMPLEMENTED();
+  return false;
+}
+
+void WebAppInstallFinalizer::PinAppToShelf(const AppId& app_id) {
+  // TODO(loyso): Implement it.
+  NOTIMPLEMENTED();
+}
+
 void WebAppInstallFinalizer::ReparentTab(const AppId& app_id,
                                          content::WebContents* web_contents) {
   // TODO(loyso): Implement it.

chrome/browser/web_applications/web_app_install_finalizer.h

@@ -28,8 +28,11 @@ class WebAppInstallFinalizer final : public InstallFinalizer {
   // InstallFinalizer:
   void FinalizeInstall(const WebApplicationInfo& web_app_info,
                        InstallFinalizedCallback callback) override;
+  bool CanCreateOsShortcuts() const override;
   void CreateOsShortcuts(const AppId& app_id,
                          CreateOsShortcutsCallback callback) override;
+  bool CanPinAppToShelf() const override;
+  void PinAppToShelf(const AppId& app_id) 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

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

chrome/browser/web_applications/web_app_install_manager_unittest.cc

@@ -616,6 +616,7 @@ TEST_F(WebAppInstallManagerTest, FinalizerMethodsCalled) {
   EXPECT_EQ(1, install_finalizer_->num_create_os_shortcuts_calls());
   EXPECT_EQ(1, install_finalizer_->num_reparent_tab_calls());
   EXPECT_EQ(1, install_finalizer_->num_reveal_appshim_calls());
+  EXPECT_EQ(1, install_finalizer_->num_pin_app_to_shelf_calls());
 }
 
 TEST_F(WebAppInstallManagerTest, FinalizerMethodsNotCalled) {
@@ -631,6 +632,7 @@ TEST_F(WebAppInstallManagerTest, FinalizerMethodsNotCalled) {
   EXPECT_EQ(0, install_finalizer_->num_create_os_shortcuts_calls());
   EXPECT_EQ(0, install_finalizer_->num_reparent_tab_calls());
   EXPECT_EQ(0, install_finalizer_->num_reveal_appshim_calls());
+  EXPECT_EQ(0, install_finalizer_->num_pin_app_to_shelf_calls());
 }
 
 // TODO(loyso): Convert more tests from bookmark_app_helper_unittest.cc