desktop-pwas: Move uninstallation tests to BookmarkAppUninstaller unit tests

Introduce a TestBookmarkAppUninstaller and use that in
PendingBookmarkAppManager unit tests. This allows us to get rid of
almost all the extensions code from PendingBookmarkAppManager unit tests
(ExtensionIdsMap is the only extensions-related concept in tests).

Also adds unit tests for BookmarkAppUninstaller that were previously
part of PendingBookmarkAppManager tests.

Bug: 916381
Change-Id: Idab844a6213e27bc5a0018a035df57ad1a5904f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1522049
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642272}

chrome/browser/web_applications/extensions/BUILD.gn

@@ -48,6 +48,7 @@ source_set("unit_tests") {
   sources = [
     "bookmark_app_install_finalizer_unittest.cc",
     "bookmark_app_installation_task_unittest.cc",
+    "bookmark_app_uninstaller_unittest.cc",
     "bookmark_app_util_unittest.cc",
     "pending_bookmark_app_manager_unittest.cc",
     "web_app_extension_ids_map_unittest.cc",

chrome/browser/web_applications/extensions/bookmark_app_uninstaller.h

@@ -19,12 +19,12 @@ namespace extensions {
 class BookmarkAppUninstaller {
  public:
   BookmarkAppUninstaller(Profile* profile, web_app::AppRegistrar* registrar);
-  ~BookmarkAppUninstaller();
+  virtual ~BookmarkAppUninstaller();
 
   // Returns true if the app with |app_url| was successfully uninstalled.
   // Returns false if the app doesn't not exist, or the app failed to be
   // uninstalled.
-  bool UninstallApp(const GURL& app_url);
+  virtual bool UninstallApp(const GURL& app_url);
 
  private:
   Profile* profile_;

chrome/browser/web_applications/extensions/bookmark_app_uninstaller_unittest.cc

+// Copyright 2018 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 "chrome/browser/web_applications/extensions/bookmark_app_uninstaller.h"
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/scoped_observer.h"
+#include "chrome/browser/extensions/test_extension_system.h"
+#include "chrome/browser/web_applications/components/web_app_constants.h"
+#include "chrome/browser/web_applications/extensions/bookmark_app_registrar.h"
+#include "chrome/browser/web_applications/test/test_app_registrar.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/crx_file/id_util.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_registry_observer.h"
+#include "extensions/common/extension_builder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+namespace {
+
+const char kFooWebAppUrl[] = "https://foo.example";
+const char kBarWebAppUrl[] = "https://bar.example";
+
+class TestExtensionRegistryObserver : public ExtensionRegistryObserver {
+ public:
+  explicit TestExtensionRegistryObserver(ExtensionRegistry* registry) {
+    extension_registry_observer_.Add(registry);
+  }
+
+  ~TestExtensionRegistryObserver() override = default;
+
+  const std::vector<std::string>& uninstalled_extension_ids() {
+    return uninstalled_extension_ids_;
+  }
+
+  void ResetResults() { uninstalled_extension_ids_.clear(); }
+
+  // ExtensionRegistryObserver
+  void OnExtensionUninstalled(content::BrowserContext* browser_context,
+                              const Extension* extension,
+                              UninstallReason reason) override {
+    uninstalled_extension_ids_.push_back(extension->id());
+  }
+
+ private:
+  std::vector<std::string> uninstalled_extension_ids_;
+
+  ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>
+      extension_registry_observer_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(TestExtensionRegistryObserver);
+};
+
+std::string GenerateFakeAppId(const GURL& url) {
+  return crx_file::id_util::GenerateId("fake_app_id_for:" + url.spec());
+}
+
+}  // namespace
+
+class BookmarkAppUninstallerTest : public ChromeRenderViewHostTestHarness {
+ public:
+  BookmarkAppUninstallerTest() = default;
+  ~BookmarkAppUninstallerTest() override = default;
+
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+
+    TestExtensionSystem* test_system =
+        static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile()));
+    test_system->CreateExtensionService(base::CommandLine::ForCurrentProcess(),
+                                        profile()->GetPath(),
+                                        false /* autoupdate_enabled */);
+    test_extension_registry_observer_ =
+        std::make_unique<TestExtensionRegistryObserver>(
+            ExtensionRegistry::Get(profile()));
+
+    extension_ids_map_ =
+        std::make_unique<web_app::ExtensionIdsMap>(profile()->GetPrefs());
+    registrar_ = std::make_unique<BookmarkAppRegistrar>(profile());
+
+    uninstaller_ =
+        std::make_unique<BookmarkAppUninstaller>(profile(), registrar_.get());
+  }
+
+  void TearDown() override {
+    // Delete the observer before ExtensionRegistry is deleted.
+    test_extension_registry_observer_.reset();
+    ChromeRenderViewHostTestHarness::TearDown();
+  }
+
+  std::string SimulateInstalledApp(const GURL& app_url) {
+    std::string app_id = GenerateFakeAppId(app_url);
+
+    auto extension = ExtensionBuilder("FooBar")
+                         .SetLocation(Manifest::EXTERNAL_POLICY)
+                         .SetID(app_id)
+                         .Build();
+    ExtensionRegistry::Get(profile())->AddEnabled(extension);
+    extension_ids_map_->Insert(app_url, app_id,
+                               web_app::InstallSource::kExternalPolicy);
+    return app_id;
+  }
+
+  void SimulateExternalAppUninstalledByUser(const GURL& app_url) {
+    const std::string app_id = GenerateFakeAppId(app_url);
+    ExtensionRegistry::Get(profile())->RemoveEnabled(app_id);
+    ExtensionPrefs::Get(profile())->OnExtensionUninstalled(
+        app_id, Manifest::EXTERNAL_POLICY, false /* external_uninstall */);
+  }
+
+  void ResetResults() { test_extension_registry_observer_->ResetResults(); }
+
+  BookmarkAppUninstaller& uninstaller() { return *uninstaller_; }
+
+  const std::vector<std::string>& uninstalled_extension_ids() {
+    return test_extension_registry_observer_->uninstalled_extension_ids();
+  }
+
+  const ExtensionSet& enabled_extensions() {
+    return ExtensionRegistry::Get(profile())->enabled_extensions();
+  }
+
+ private:
+  std::unique_ptr<TestExtensionRegistryObserver>
+      test_extension_registry_observer_;
+
+  std::unique_ptr<web_app::ExtensionIdsMap> extension_ids_map_;
+  std::unique_ptr<BookmarkAppRegistrar> registrar_;
+  std::unique_ptr<BookmarkAppUninstaller> uninstaller_;
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkAppUninstallerTest);
+};
+
+TEST_F(BookmarkAppUninstallerTest, Uninstall_Successful) {
+  SimulateInstalledApp(GURL(kFooWebAppUrl));
+  ASSERT_EQ(1u, enabled_extensions().size());
+
+  EXPECT_TRUE(uninstaller().UninstallApp(GURL(kFooWebAppUrl)));
+  content::RunAllTasksUntilIdle();
+
+  EXPECT_EQ(1u, uninstalled_extension_ids().size());
+  EXPECT_EQ(0u, enabled_extensions().size());
+}
+
+TEST_F(BookmarkAppUninstallerTest, Uninstall_Multiple) {
+  auto foo_app_id = SimulateInstalledApp(GURL(kFooWebAppUrl));
+  auto bar_app_id = SimulateInstalledApp(GURL(kBarWebAppUrl));
+  ASSERT_EQ(2u, enabled_extensions().size());
+
+  EXPECT_TRUE(uninstaller().UninstallApp(GURL(kBarWebAppUrl)));
+  content::RunAllTasksUntilIdle();
+
+  EXPECT_EQ(1u, uninstalled_extension_ids().size());
+  EXPECT_EQ(1u, enabled_extensions().size());
+  EXPECT_FALSE(enabled_extensions().Contains(bar_app_id));
+  EXPECT_TRUE(enabled_extensions().Contains(foo_app_id));
+
+  ResetResults();
+
+  EXPECT_TRUE(uninstaller().UninstallApp(GURL(kFooWebAppUrl)));
+  content::RunAllTasksUntilIdle();
+
+  EXPECT_EQ(1u, uninstalled_extension_ids().size());
+  EXPECT_TRUE(enabled_extensions().is_empty());
+}
+
+TEST_F(BookmarkAppUninstallerTest, Uninstall_UninstalledExternalApp) {
+  SimulateInstalledApp(GURL(kFooWebAppUrl));
+  SimulateExternalAppUninstalledByUser(GURL(kFooWebAppUrl));
+
+  EXPECT_FALSE(uninstaller().UninstallApp(GURL(kFooWebAppUrl)));
+}
+
+// Tests trying to uninstall an app that was never installed.
+TEST_F(BookmarkAppUninstallerTest, Uninstall_FailsNeverInstalled) {
+  EXPECT_FALSE(uninstaller().UninstallApp(GURL(kFooWebAppUrl)));
+}
+
+// Tests trying to uninstall an app that was previously uninstalled.
+TEST_F(BookmarkAppUninstallerTest, Uninstall_FailsAlreadyUninstalled) {
+  SimulateInstalledApp(GURL(kFooWebAppUrl));
+
+  EXPECT_TRUE(uninstaller().UninstallApp(GURL(kFooWebAppUrl)));
+  content::RunAllTasksUntilIdle();
+
+  EXPECT_EQ(1u, uninstalled_extension_ids().size());
+  EXPECT_TRUE(enabled_extensions().is_empty());
+
+  ResetResults();
+
+  EXPECT_FALSE(uninstaller().UninstallApp(GURL(kFooWebAppUrl)));
+  content::RunAllTasksUntilIdle();
+
+  EXPECT_EQ(0u, uninstalled_extension_ids().size());
+}
+
+}  // namespace extensions

chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc

@@ -55,7 +55,8 @@ PendingBookmarkAppManager::PendingBookmarkAppManager(
     web_app::AppRegistrar* registrar)
     : profile_(profile),
       registrar_(registrar),
-      uninstaller_(profile_, registrar_),
+      uninstaller_(
+          std::make_unique<BookmarkAppUninstaller>(profile_, registrar_)),
       extension_ids_map_(profile->GetPrefs()),
       web_contents_factory_(base::BindRepeating(&WebContentsCreateWrapper)),
       task_factory_(base::BindRepeating(&InstallationTaskCreateWrapper)),
@@ -93,7 +94,8 @@ void PendingBookmarkAppManager::UninstallApps(
     std::vector<GURL> apps_to_uninstall,
     const UninstallCallback& callback) {
   for (auto& app_to_uninstall : apps_to_uninstall) {
-    callback.Run(app_to_uninstall, uninstaller_.UninstallApp(app_to_uninstall));
+    callback.Run(app_to_uninstall,
+                 uninstaller_->UninstallApp(app_to_uninstall));
   }
 }
 
@@ -115,6 +117,11 @@ void PendingBookmarkAppManager::SetFactoriesForTesting(
   task_factory_ = std::move(task_factory);
 }
 
+void PendingBookmarkAppManager::SetUninstallerForTesting(
+    std::unique_ptr<BookmarkAppUninstaller> uninstaller) {
+  uninstaller_ = std::move(uninstaller);
+}
+
 void PendingBookmarkAppManager::SetTimerForTesting(
     std::unique_ptr<base::OneShotTimer> timer) {
   timer_ = std::move(timer);

chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h

@@ -64,6 +64,8 @@ class PendingBookmarkAppManager final : public web_app::PendingAppManager,
 
   void SetFactoriesForTesting(WebContentsFactory web_contents_factory,
                               TaskFactory task_factory);
+  void SetUninstallerForTesting(
+      std::unique_ptr<BookmarkAppUninstaller> uninstaller);
   void SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer);
 
  private:
@@ -98,7 +100,7 @@ class PendingBookmarkAppManager final : public web_app::PendingAppManager,
 
   Profile* profile_;
   web_app::AppRegistrar* registrar_;
-  BookmarkAppUninstaller uninstaller_;
+  std::unique_ptr<BookmarkAppUninstaller> uninstaller_;
   web_app::ExtensionIdsMap extension_ids_map_;
 
   WebContentsFactory web_contents_factory_;