desktop-pwas: Introduce TestAppRegistrar in PendingBookmarkAppManager unit tests

Now that PendingBookmarkAppManager doesn't access ExtensionRegistry and
ExtensionPrefs directly for installations, we can avoid faking some cases
in tests.

PendingBookmarkAppManager still uses ExtensionRegistry for
uninstallations so we still need to fake some calls to it. Once
we abstract uninstallation, we can remove all usage of ExtensionRegistry
from tests.

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

chrome/browser/web_applications/BUILD.gn

@@ -58,6 +58,8 @@ source_set("web_applications_test_support") {
   testonly = true
 
   sources = [
+    "test/test_app_registrar.cc",
+    "test/test_app_registrar.h",
     "test/test_data_retriever.cc",
     "test/test_data_retriever.h",
     "test/test_file_utils.cc",

chrome/browser/web_applications/components/app_registrar.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_REGISTRAR_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_APP_REGISTRAR_H_
 
+#include "base/callback_forward.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 
 namespace web_app {

chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/extensions/bookmark_app_installation_task.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"
@@ -118,15 +119,15 @@ class TestExtensionRegistryObserver : public ExtensionRegistryObserver {
   DISALLOW_COPY_AND_ASSIGN(TestExtensionRegistryObserver);
 };
 
-}  // namespace
-
 class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask {
  public:
   TestBookmarkAppInstallationTask(Profile* profile,
+                                  web_app::TestAppRegistrar* registrar,
                                   web_app::PendingAppManager::AppInfo app_info,
                                   bool succeeds)
       : BookmarkAppInstallationTask(profile, std::move(app_info)),
         profile_(profile),
+        registrar_(registrar),
         succeeds_(succeeds),
         extension_ids_map_(profile_->GetPrefs()) {}
   ~TestBookmarkAppInstallationTask() override = default;
@@ -142,6 +143,7 @@ class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask {
           CreateDummyExtension(app_id));
       extension_ids_map_.Insert(app_info().url, app_id,
                                 app_info().install_source);
+      registrar_->AddAsInstalled(app_id);
     }
 
     std::move(on_install_called_).Run();
@@ -155,6 +157,7 @@ class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask {
 
  private:
   Profile* profile_;
+  web_app::TestAppRegistrar* registrar_;
   bool succeeds_;
   web_app::ExtensionIdsMap extension_ids_map_;
 
@@ -163,6 +166,8 @@ class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask {
   DISALLOW_COPY_AND_ASSIGN(TestBookmarkAppInstallationTask);
 };
 
+}  // namespace
+
 class PendingBookmarkAppManagerTest : public ChromeRenderViewHostTestHarness {
  public:
   PendingBookmarkAppManagerTest()
@@ -192,7 +197,7 @@ class PendingBookmarkAppManagerTest : public ChromeRenderViewHostTestHarness {
         std::make_unique<TestExtensionRegistryObserver>(
             ExtensionRegistry::Get(profile()));
 
-    registrar_ = std::make_unique<extensions::BookmarkAppRegistrar>(profile());
+    registrar_ = std::make_unique<web_app::TestAppRegistrar>();
   }
 
   void TearDown() override {
@@ -213,7 +218,7 @@ class PendingBookmarkAppManagerTest : public ChromeRenderViewHostTestHarness {
       web_app::PendingAppManager::AppInfo app_info,
       bool succeeds) {
     auto task = std::make_unique<TestBookmarkAppInstallationTask>(
-        profile, std::move(app_info), succeeds);
+        profile, registrar_.get(), std::move(app_info), succeeds);
     auto* task_ptr = task.get();
     task->SetOnInstallCalled(base::BindLambdaForTesting([task_ptr, this]() {
       ++installation_task_run_count_;
@@ -328,6 +333,8 @@ class PendingBookmarkAppManagerTest : public ChromeRenderViewHostTestHarness {
     return test_extension_registry_observer_->uninstalled_extension_ids();
   }
 
+  web_app::TestAppRegistrar* registrar() { return registrar_.get(); }
+
  private:
   content::WebContentsTester* web_contents_tester_ = nullptr;
   base::Optional<GURL> install_callback_url_;
@@ -344,7 +351,7 @@ class PendingBookmarkAppManagerTest : public ChromeRenderViewHostTestHarness {
   PendingBookmarkAppManager::TaskFactory successful_installation_task_creator_;
   PendingBookmarkAppManager::TaskFactory failing_installation_task_creator_;
 
-  std::unique_ptr<web_app::AppRegistrar> registrar_;
+  std::unique_ptr<web_app::TestAppRegistrar> registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(PendingBookmarkAppManagerTest);
 };
@@ -942,7 +949,7 @@ TEST_F(PendingBookmarkAppManagerTest, ExtensionUninstalled) {
 
   // Simulate the extension for the app getting uninstalled.
   const std::string app_id = GenerateFakeAppId(GURL(kFooWebAppUrl));
-  ExtensionRegistry::Get(profile())->RemoveEnabled(app_id);
+  registrar()->RemoveAsInstalled(app_id);
 
   // Trying to uninstall the app should fail and have no effect.
   pending_app_manager->UninstallApps(
@@ -985,9 +992,8 @@ TEST_F(PendingBookmarkAppManagerTest, ExternalExtensionUninstalled) {
 
   // Simulate external extension for the app getting uninstalled by the user.
   const std::string app_id = GenerateFakeAppId(GURL(kFooWebAppUrl));
-  ExtensionRegistry::Get(profile())->RemoveEnabled(app_id);
-  ExtensionPrefs::Get(profile())->OnExtensionUninstalled(
-      app_id, Manifest::EXTERNAL_POLICY, false /* external_uninstall */);
+  registrar()->AddAsExternalAppUninstalledByUser(app_id);
+  registrar()->RemoveAsInstalled(app_id);
 
   // Trying to uninstall the app should fail and have no effect.
   pending_app_manager->UninstallApps(

chrome/browser/web_applications/test/test_app_registrar.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 "chrome/browser/web_applications/test/test_app_registrar.h"
+
+#include "base/callback.h"
+#include "base/stl_util.h"
+
+namespace web_app {
+
+TestAppRegistrar::TestAppRegistrar() = default;
+
+TestAppRegistrar::~TestAppRegistrar() = default;
+
+void TestAppRegistrar::AddAsInstalled(const AppId& app_id) {
+  installed_apps_.insert(app_id);
+}
+
+void TestAppRegistrar::RemoveAsInstalled(const AppId& app_id) {
+  DCHECK(base::ContainsKey(installed_apps_, app_id));
+  installed_apps_.erase(app_id);
+}
+
+void TestAppRegistrar::AddAsExternalAppUninstalledByUser(const AppId& app_id) {
+  DCHECK(!base::ContainsKey(uninstalled_external_apps_, app_id));
+  uninstalled_external_apps_.insert(app_id);
+}
+
+void TestAppRegistrar::Init(base::OnceClosure callback) {}
+
+bool TestAppRegistrar::IsInstalled(const AppId& app_id) const {
+  return base::ContainsKey(installed_apps_, app_id);
+}
+
+bool TestAppRegistrar::WasExternalAppUninstalledByUser(
+    const AppId& app_id) const {
+  return base::ContainsKey(uninstalled_external_apps_, app_id);
+}
+
+}  // namespace web_app

chrome/browser/web_applications/test/test_app_registrar.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_TEST_TEST_APP_REGISTRAR_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_APP_REGISTRAR_H_
+
+#include <set>
+
+#include "chrome/browser/web_applications/components/app_registrar.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+
+namespace web_app {
+
+class TestAppRegistrar : public AppRegistrar {
+ public:
+  TestAppRegistrar();
+  ~TestAppRegistrar() override;
+
+  // Adds |app_id| to the map of installed apps.
+  void AddAsInstalled(const AppId& app_id);
+
+  // Removes |app_id| from the map of installed apps.
+  void RemoveAsInstalled(const AppId& app_id);
+
+  // Adds |app_id| to the map of external extensions uninstalled by the user.
+  void AddAsExternalAppUninstalledByUser(const AppId& app_id);
+
+  // AppRegistrar
+  void Init(base::OnceClosure callback) override;
+  bool IsInstalled(const AppId& app_id) const override;
+  bool WasExternalAppUninstalledByUser(const AppId& app_id) const override;
+
+ private:
+  std::set<AppId> installed_apps_;
+  std::set<AppId> uninstalled_external_apps_;
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_APP_REGISTRAR_H_