Reland "[WebAppProvider] Test OS hook uninstallation calls & completion logic."

This is a reland of d3cc95bb

Original change's description:
> [WebAppProvider] Test OS hook uninstallation calls & completion logic.
>
> This patch adds the virtual methods to the uninstallation path, and adds
> corresponding tests for those in the unittests file.
>
> R=phillis@chromium.org
>
> Change-Id: I316145e6030d16ff3a28bf7fecb3d2cdbac940d4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545864
> Commit-Queue: Daniel Murphy <dmurph@chromium.org>
> Reviewed-by: Phillis Tang <phillis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#832087}

Change-Id: Ie95af999acb33e2970dd2b9350ff9316cf6f7af5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2567820
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Phillis Tang <phillis@chromium.org>
Auto-Submit: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Phillis Tang <phillis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832547}

chrome/browser/web_applications/components/os_integration_manager.cc

@@ -65,6 +65,7 @@ class OsIntegrationManager::OsHooksBarrier
   }
 
   void AddResult(OsHookType::Type type, bool result) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     results_[type] = result;
   }
 
@@ -178,46 +179,39 @@ void OsIntegrationManager::UninstallOsHooks(const AppId& app_id,
   scoped_refptr<OsHooksBarrier> barrier =
       base::MakeRefCounted<OsHooksBarrier>(os_hooks, std::move(callback));
 
-  if (os_hooks[OsHookType::kShortcutsMenu] &&
-      ShouldRegisterShortcutsMenuWithOs()) {
-    bool success = UnregisterShortcutsMenuWithOs(app_id, profile_->GetPath());
+  if (os_hooks[OsHookType::kShortcutsMenu]) {
+    bool success = UnregisterShortcutsMenu(app_id);
     if (!success)
       barrier->OnError(OsHookType::kShortcutsMenu);
   }
 
   if (os_hooks[OsHookType::kShortcuts] || os_hooks[OsHookType::kRunOnOsLogin]) {
-    std::unique_ptr<ShortcutInfo> shortcut_info =
-        shortcut_manager_->BuildShortcutInfo(app_id);
+    std::unique_ptr<ShortcutInfo> shortcut_info = BuildShortcutInfo(app_id);
     base::FilePath shortcut_data_dir =
         internals::GetShortcutDataDir(*shortcut_info);
 
     if (os_hooks[OsHookType::kRunOnOsLogin] &&
         base::FeatureList::IsEnabled(features::kDesktopPWAsRunOnOsLogin)) {
-      ScheduleUnregisterRunOnOsLogin(
+      UnregisterRunOnOsLogin(
           shortcut_info->profile_path, shortcut_info->title,
           barrier->CreateBarrierCallbackForType(OsHookType::kRunOnOsLogin));
     }
 
     if (os_hooks[OsHookType::kShortcuts]) {
-      internals::ScheduleDeletePlatformShortcuts(
-          shortcut_data_dir, std::move(shortcut_info),
-          base::BindOnce(
-              &OsIntegrationManager::OnShortcutsDeleted,
-              weak_ptr_factory_.GetWeakPtr(), app_id,
-              barrier->CreateBarrierCallbackForType(OsHookType::kShortcuts)));
+      DeleteShortcuts(
+          app_id, shortcut_data_dir, std::move(shortcut_info),
+          barrier->CreateBarrierCallbackForType(OsHookType::kShortcuts));
     }
   }
   // TODO(https://crbug.com/1108109) we should return the result of file handler
   // unregistration and record errors during unregistration.
   if (os_hooks[OsHookType::kFileHandlers])
-    file_handler_manager_->DisableAndUnregisterOsFileHandlers(app_id);
+    UnregisterFileHandlers(app_id);
 
   // There is a chance uninstallation point was created with feature flag
   // enabled so we need to clean it up regardless of feature flag state.
-  if (os_hooks[OsHookType::kUninstallationViaOsSettings] &&
-      ShouldRegisterUninstallationViaOsSettingsWithOs()) {
-    UnegisterUninstallationViaOsSettingsWithOs(app_id, profile_);
-  }
+  if (os_hooks[OsHookType::kUninstallationViaOsSettings])
+    UnregisterWebAppOsUninstallation(app_id);
 }
 
 void OsIntegrationManager::UpdateOsHooks(
@@ -339,7 +333,12 @@ void OsIntegrationManager::RegisterShortcutsMenu(
         shortcuts_menu_item_infos,
     const ShortcutsMenuIconsBitmaps& shortcuts_menu_icons_bitmaps,
     base::OnceCallback<void(bool success)> callback) {
+  if (!ShouldRegisterShortcutsMenuWithOs()) {
+    std::move(callback).Run(true);
+    return;
+  }
 
+  DCHECK(shortcut_manager_);
   shortcut_manager_->RegisterShortcutsMenuWithOs(
       app_id, shortcuts_menu_item_infos, shortcuts_menu_icons_bitmaps);
 
@@ -351,6 +350,11 @@ void OsIntegrationManager::RegisterShortcutsMenu(
 void OsIntegrationManager::ReadAllShortcutsMenuIconsAndRegisterShortcutsMenu(
     const AppId& app_id,
     base::OnceCallback<void(bool success)> callback) {
+  if (!ShouldRegisterShortcutsMenuWithOs()) {
+    std::move(callback).Run(true);
+    return;
+  }
+
   shortcut_manager_->ReadAllShortcutsMenuIconsAndRegisterShortcutsMenu(
       app_id, std::move(callback));
 }
@@ -389,12 +393,56 @@ void OsIntegrationManager::RegisterWebAppOsUninstallation(
   }
 }
 
+bool OsIntegrationManager::UnregisterShortcutsMenu(const AppId& app_id) {
+  if (!ShouldRegisterShortcutsMenuWithOs())
+    return true;
+  return UnregisterShortcutsMenuWithOs(app_id, profile_->GetPath());
+}
+
+void OsIntegrationManager::UnregisterRunOnOsLogin(
+    const base::FilePath& profile_path,
+    const base::string16& shortcut_title,
+    UnregisterRunOnOsLoginCallback callback) {
+  ScheduleUnregisterRunOnOsLogin(profile_path, shortcut_title,
+                                 std::move(callback));
+}
+
+void OsIntegrationManager::DeleteShortcuts(
+    const AppId& app_id,
+    const base::FilePath& shortcuts_data_dir,
+    std::unique_ptr<ShortcutInfo> shortcut_info,
+    DeleteShortcutsCallback callback) {
+  internals::ScheduleDeletePlatformShortcuts(
+      shortcuts_data_dir, std::move(shortcut_info),
+      base::BindOnce(&OsIntegrationManager::OnShortcutsDeleted,
+                     weak_ptr_factory_.GetWeakPtr(), app_id,
+                     std::move(callback)));
+}
+
+void OsIntegrationManager::UnregisterFileHandlers(const AppId& app_id) {
+  file_handler_manager_->DisableAndUnregisterOsFileHandlers(app_id);
+}
+
+void OsIntegrationManager::UnregisterWebAppOsUninstallation(
+    const AppId& app_id) {
+  if (ShouldRegisterUninstallationViaOsSettingsWithOs())
+    UnegisterUninstallationViaOsSettingsWithOs(app_id, profile_);
+}
+
+std::unique_ptr<ShortcutInfo> OsIntegrationManager::BuildShortcutInfo(
+    const AppId& app_id) {
+  DCHECK(shortcut_manager_);
+  return shortcut_manager_->BuildShortcutInfo(app_id);
+}
+
 void OsIntegrationManager::OnShortcutsCreated(
     const AppId& app_id,
     std::unique_ptr<WebApplicationInfo> web_app_info,
     InstallOsHooksOptions options,
     scoped_refptr<OsHooksBarrier> barrier,
     bool shortcuts_created) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(barrier);
 
   bool shortcut_creation_failure =
       !shortcuts_created && options.os_hooks[OsHookType::kShortcuts];

chrome/browser/web_applications/components/os_integration_manager.h

@@ -160,6 +160,7 @@ class OsIntegrationManager {
                                bool add_to_desktop,
                                CreateShortcutsCallback callback);
 
+  // Installation:
   virtual void RegisterFileHandlers(
       const AppId& app_id,
       base::OnceCallback<void(bool success)> callback);
@@ -179,6 +180,21 @@ class OsIntegrationManager {
   virtual void RegisterWebAppOsUninstallation(const AppId& app_id,
                                               const std::string& name);
 
+  // Uninstallation:
+  virtual bool UnregisterShortcutsMenu(const AppId& app_id);
+  virtual void UnregisterRunOnOsLogin(const base::FilePath& profile_path,
+                                      const base::string16& shortcut_title,
+                                      UnregisterRunOnOsLoginCallback callback);
+  virtual void DeleteShortcuts(const AppId& app_id,
+                               const base::FilePath& shortcuts_data_dir,
+                               std::unique_ptr<ShortcutInfo> shortcut_info,
+                               DeleteShortcutsCallback callback);
+  virtual void UnregisterFileHandlers(const AppId& app_id);
+  virtual void UnregisterWebAppOsUninstallation(const AppId& app_id);
+
+  // Utility mathods:
+  virtual std::unique_ptr<ShortcutInfo> BuildShortcutInfo(const AppId& app_id);
+
  private:
   class OsHooksBarrier;
 

chrome/browser/web_applications/components/os_integration_manager_unittest.cc

@@ -7,23 +7,29 @@
 #include <memory>
 
 #include "base/bind.h"
+#include "base/files/file_path.h"
 #include "base/run_loop.h"
 #include "base/test/bind.h"
 #include "base/test/gmock_callback_support.h"
-#include "base/test/task_environment.h"
+#include "build/build_config.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "chrome/common/chrome_constants.h"
+#include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/features.h"
 
 namespace web_app {
+namespace {
+
 class MockOsIntegrationManager : public OsIntegrationManager {
  public:
   MockOsIntegrationManager()
       : OsIntegrationManager(nullptr, nullptr, nullptr, nullptr) {}
   ~MockOsIntegrationManager() override = default;
 
+  // Installation:
   MOCK_METHOD(void,
               CreateShortcuts,
               (const AppId& app_id,
@@ -68,12 +74,64 @@ class MockOsIntegrationManager : public OsIntegrationManager {
               RegisterWebAppOsUninstallation,
               (const AppId& app_id, const std::string& name),
               (override));
+
+  // Uninstallation:
+  MOCK_METHOD(bool, UnregisterShortcutsMenu, (const AppId& app_id), (override));
+  MOCK_METHOD(void,
+              UnregisterRunOnOsLogin,
+              (const base::FilePath& profile_path,
+               const base::string16& shortcut_title,
+               UnregisterRunOnOsLoginCallback callback),
+              (override));
+  MOCK_METHOD(void,
+              DeleteShortcuts,
+              (const AppId& app_id,
+               const base::FilePath& shortcuts_data_dir,
+               std::unique_ptr<ShortcutInfo> shortcut_info,
+               DeleteShortcutsCallback callback),
+              (override));
+  MOCK_METHOD(void, UnregisterFileHandlers, (const AppId& app_id), (override));
+  MOCK_METHOD(void,
+              UnregisterWebAppOsUninstallation,
+              (const AppId& app_id),
+              (override));
+
+  // Utility methods:
+  MOCK_METHOD(std::unique_ptr<ShortcutInfo>,
+              BuildShortcutInfo,
+              (const AppId& app_id),
+              (override));
 };
 
-TEST(OsIntegrationManagerTest, InstallOsHooksOnlyShortcuts) {
-  base::test::TaskEnvironment task_environment_{
-      base::test::TaskEnvironment::MainThreadType::UI};
+#if defined(OS_WIN)
+const base::FilePath::CharType kFakeProfilePath[] =
+    FILE_PATH_LITERAL("\\profile\\path");
+#else
+const base::FilePath::CharType kFakeProfilePath[] =
+    FILE_PATH_LITERAL("/profile/path");
+#endif  // defined(OS_WIN)
+
+const char kFakeAppUrl[] = "https://fake.com";
+
+std::unique_ptr<ShortcutInfo> CreateTestShorcutInfo(
+    const web_app::AppId& app_id) {
+  auto shortcut_info = std::make_unique<ShortcutInfo>();
+  shortcut_info->profile_path = base::FilePath(kFakeProfilePath);
+  shortcut_info->extension_id = app_id;
+  shortcut_info->url = GURL(kFakeAppUrl);
+  return shortcut_info;
+}
+
+class OsIntegrationManagerTest : public testing::Test {
+ public:
+  OsIntegrationManagerTest() = default;
+  ~OsIntegrationManagerTest() override = default;
 
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+};
+
+TEST_F(OsIntegrationManagerTest, InstallOsHooksOnlyShortcuts) {
   base::RunLoop run_loop;
 
   OsHooksResults install_results;
@@ -98,10 +156,7 @@ TEST(OsIntegrationManagerTest, InstallOsHooksOnlyShortcuts) {
   EXPECT_TRUE(install_results[OsHookType::kShortcuts]);
 }
 
-TEST(OsIntegrationManagerTest, InstallOsHooksEverything) {
-  base::test::TaskEnvironment task_environment_{
-      base::test::TaskEnvironment::MainThreadType::UI};
-
+TEST_F(OsIntegrationManagerTest, InstallOsHooksEverything) {
   base::RunLoop run_loop;
 
   OsHooksResults install_results;
@@ -123,7 +178,7 @@ TEST(OsIntegrationManagerTest, InstallOsHooksEverything) {
   EXPECT_CALL(manager, AddAppToQuickLaunchBar(app_id)).Times(1);
   EXPECT_CALL(manager, ReadAllShortcutsMenuIconsAndRegisterShortcutsMenu(
                            app_id, testing::_))
-      .Times(1);
+      .WillOnce(base::test::RunOnceCallback<1>(true));
 
   InstallOsHooksOptions options;
   options.add_to_desktop = true;
@@ -141,4 +196,44 @@ TEST(OsIntegrationManagerTest, InstallOsHooksEverything) {
   EXPECT_TRUE(install_results[OsHookType::kShortcutsMenu]);
   EXPECT_TRUE(install_results[OsHookType::kUninstallationViaOsSettings]);
 }
+
+TEST_F(OsIntegrationManagerTest, UninstallOsHooksEverything) {
+  base::RunLoop run_loop;
+
+  OsHooksResults uninstall_results;
+  UninstallOsHooksCallback callback =
+      base::BindLambdaForTesting([&](OsHooksResults results) {
+        uninstall_results = results;
+        run_loop.Quit();
+      });
+
+  const AppId app_id = "test";
+
+  const base::FilePath kExpectedShortcutPath =
+      base::FilePath(kFakeProfilePath)
+          .Append(chrome::kWebAppDirname)
+          .AppendASCII("_crx_test");
+
+  testing::StrictMock<MockOsIntegrationManager> manager;
+  EXPECT_CALL(manager, BuildShortcutInfo(app_id))
+      .WillOnce(
+          testing::Return(testing::ByMove(CreateTestShorcutInfo(app_id))));
+  EXPECT_CALL(manager, DeleteShortcuts(app_id, kExpectedShortcutPath,
+                                       testing::_, testing::_))
+      .WillOnce(base::test::RunOnceCallback<3>(true));
+  EXPECT_CALL(manager, UnregisterFileHandlers(app_id)).Times(1);
+  EXPECT_CALL(manager, UnregisterWebAppOsUninstallation(app_id)).Times(1);
+  EXPECT_CALL(manager, UnregisterShortcutsMenu(app_id))
+      .WillOnce(testing::Return(true));
+
+  manager.UninstallAllOsHooks(app_id, std::move(callback));
+  run_loop.Run();
+  EXPECT_TRUE(uninstall_results[OsHookType::kShortcuts]);
+  EXPECT_TRUE(uninstall_results[OsHookType::kFileHandlers]);
+  EXPECT_TRUE(uninstall_results[OsHookType::kRunOnOsLogin]);
+  EXPECT_TRUE(uninstall_results[OsHookType::kShortcutsMenu]);
+  EXPECT_TRUE(uninstall_results[OsHookType::kUninstallationViaOsSettings]);
+}
+
+}  // namespace
 }  // namespace web_app