WebApps: Adds a AppShortcutObserver, for observing shortcut creation.

Some actions (such as file handler registration on Linux) can only be
run after shortcuts have been created. This CL provides a way for those
actions to be run at the appropriate time.

Bug: 860581
Change-Id: Iea6a14c15c15c4ab39e8b0bbb71ac996dd9c0e81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868566
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708084}

chrome/browser/web_applications/components/BUILD.gn

@@ -13,6 +13,7 @@ source_set("components") {
     "app_registry_controller.h",
     "app_shortcut_manager.cc",
     "app_shortcut_manager.h",
+    "app_shortcut_observer.h",
     "external_install_options.cc",
     "external_install_options.h",
     "externally_installed_web_app_prefs.cc",
@@ -111,6 +112,7 @@ source_set("unit_tests") {
   testonly = true
 
   sources = [
+    "app_shortcut_manager_unittest.cc",
     "file_handler_manager_unittest.cc",
     "pending_app_manager_unittest.cc",
     "web_app_data_retriever_unittest.cc",

chrome/browser/web_applications/components/app_shortcut_manager.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/web_applications/components/app_shortcut_manager.h"
 
 #include "base/callback.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"
 
@@ -37,6 +38,14 @@ void AppShortcutManager::SetSubsystems(AppRegistrar* registrar) {
   registrar_ = registrar;
 }
 
+void AppShortcutManager::AddObserver(AppShortcutObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void AppShortcutManager::RemoveObserver(AppShortcutObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 bool AppShortcutManager::CanCreateShortcuts() const {
 #if defined(OS_CHROMEOS)
   return false;
@@ -48,10 +57,25 @@ bool AppShortcutManager::CanCreateShortcuts() const {
 void AppShortcutManager::CreateShortcuts(const AppId& app_id,
                                          bool add_to_desktop,
                                          CreateShortcutsCallback callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(CanCreateShortcuts());
-  GetShortcutInfoForApp(app_id,
-                        base::BindOnce(&OnShortcutInfoRetrievedCreateShortcuts,
-                                       add_to_desktop, std::move(callback)));
+  GetShortcutInfoForApp(
+      app_id,
+      base::BindOnce(&OnShortcutInfoRetrievedCreateShortcuts, add_to_desktop,
+                     base::BindOnce(&AppShortcutManager::OnShortcutsCreated,
+                                    weak_ptr_factory_.GetWeakPtr(), app_id,
+                                    std::move(callback))));
+}
+
+void AppShortcutManager::OnShortcutsCreated(const AppId& app_id,
+                                            CreateShortcutsCallback callback,
+                                            bool success) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (success) {
+    for (auto& observer : observers_)
+      observer.OnShortcutsCreated(app_id);
+  }
+  std::move(callback).Run(success);
 }
 
 }  // namespace web_app

chrome/browser/web_applications/components/app_shortcut_manager.h

@@ -9,6 +9,8 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_shortcut.h"
 
@@ -17,6 +19,7 @@ class Profile;
 namespace web_app {
 
 class AppRegistrar;
+class AppShortcutObserver;
 struct ShortcutInfo;
 
 // TODO(crbug.com/860581): Migrate functions from
@@ -31,6 +34,9 @@ class AppShortcutManager {
 
   void SetSubsystems(AppRegistrar* registrar);
 
+  void AddObserver(AppShortcutObserver* observer);
+  void RemoveObserver(AppShortcutObserver* observer);
+
   // virtual for testing.
   virtual bool CanCreateShortcuts() const;
 
@@ -48,6 +54,10 @@ class AppShortcutManager {
                                      GetShortcutInfoCallback callback) = 0;
 
  protected:
+  void OnShortcutsCreated(const AppId& app_id,
+                          CreateShortcutsCallback callback,
+                          bool success);
+
   AppRegistrar* registrar() { return registrar_; }
   Profile* profile() { return profile_; }
 
@@ -55,6 +65,10 @@ class AppShortcutManager {
   AppRegistrar* registrar_ = nullptr;
   Profile* const profile_;
 
+  base::ObserverList<AppShortcutObserver, /*check_empty=*/true> observers_;
+
+  base::WeakPtrFactory<AppShortcutManager> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(AppShortcutManager);
 };
 

chrome/browser/web_applications/components/app_shortcut_manager_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/web_applications/components/app_shortcut_observer.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/test/test_app_shortcut_manager.h"
+#include "chrome/browser/web_applications/test/web_app_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace web_app {
+
+namespace {
+
+class TestShortcutObserver : public AppShortcutObserver {
+ public:
+  TestShortcutObserver() {}
+
+  void OnShortcutsCreated(const AppId& app_id) override {
+    on_shortcuts_created_calls_++;
+  }
+
+  size_t on_shortcuts_created_calls() const {
+    return on_shortcuts_created_calls_;
+  }
+
+ private:
+  size_t on_shortcuts_created_calls_ = 0;
+};
+
+}  // namespace
+
+constexpr char kAppId[] = "app-id";
+
+class AppShortcutManagerTest : public WebAppTest {
+ protected:
+  AppShortcutManagerTest() {}
+
+  void SetUp() override {
+    WebAppTest::SetUp();
+
+    shortcut_manager_ = std::make_unique<TestAppShortcutManager>(profile());
+    shortcut_observer_ = std::make_unique<TestShortcutObserver>();
+
+    shortcut_manager_->AddObserver(shortcut_observer_.get());
+  }
+
+  void TearDown() override {
+    WebAppTest::TearDown();
+
+    shortcut_manager_->RemoveObserver(shortcut_observer_.get());
+  }
+
+  TestAppShortcutManager& shortcut_manager() {
+    return *shortcut_manager_.get();
+  }
+
+  TestShortcutObserver& shortcut_observer() {
+    return *shortcut_observer_.get();
+  }
+
+  void CreateShortcuts(const AppId& app_id, bool add_to_desktop) {
+    base::RunLoop loop;
+    shortcut_manager().CreateShortcuts(
+        app_id, add_to_desktop,
+        base::BindLambdaForTesting([&loop](bool /*success*/) { loop.Quit(); }));
+    loop.Run();
+  }
+
+ private:
+  std::unique_ptr<TestAppShortcutManager> shortcut_manager_;
+  std::unique_ptr<TestShortcutObserver> shortcut_observer_;
+};
+
+TEST_F(AppShortcutManagerTest, OnShortcutsCreatedFiresOnSuccess) {
+  shortcut_manager().SetNextCreateShortcutsResult(kAppId, /*success=*/true);
+
+  CreateShortcuts(kAppId, true);
+
+  EXPECT_EQ(1u, shortcut_manager().num_create_shortcuts_calls());
+  EXPECT_EQ(1u, shortcut_observer().on_shortcuts_created_calls());
+}
+
+TEST_F(AppShortcutManagerTest, OnShortcutsCreatedDoesNotFireOnFailure) {
+  shortcut_manager().SetNextCreateShortcutsResult(kAppId, /*success=*/false);
+
+  CreateShortcuts(kAppId, true);
+
+  EXPECT_EQ(1u, shortcut_manager().num_create_shortcuts_calls());
+  EXPECT_EQ(0u, shortcut_observer().on_shortcuts_created_calls());
+}
+
+}  // namespace web_app

chrome/browser/web_applications/components/app_shortcut_observer.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_SHORTCUT_OBSERVER_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_SHORTCUT_OBSERVER_H_
+
+#include "base/observer_list_types.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+
+namespace web_app {
+
+class AppShortcutObserver : public base::CheckedObserver {
+ public:
+  virtual void OnShortcutsCreated(const AppId& app_id) {}
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_SHORTCUT_OBSERVER_H_

chrome/browser/web_applications/test/test_app_shortcut_manager.cc

@@ -39,7 +39,9 @@ void TestAppShortcutManager::CreateShortcuts(const AppId& app_id,
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(std::move(callback), success));
+      FROM_HERE, base::BindOnce(&TestAppShortcutManager::OnShortcutsCreated,
+                                weak_ptr_factory_.GetWeakPtr(), app_id,
+                                std::move(callback), success));
 }
 
 void TestAppShortcutManager::GetShortcutInfoForApp(

chrome/browser/web_applications/test/test_app_shortcut_manager.h

@@ -7,6 +7,7 @@
 
 #include <map>
 
+#include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "chrome/browser/web_applications/components/app_shortcut_manager.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
@@ -48,6 +49,8 @@ class TestAppShortcutManager : public AppShortcutManager {
 
   bool can_create_shortcuts_ = true;
   std::map<AppId, bool> next_create_shortcut_results_;
+
+  base::WeakPtrFactory<TestAppShortcutManager> weak_ptr_factory_{this};
 };
 
 }  // namespace web_app