dwpa: Modified PendingAppInstallTask to use a constructor provided...

dwpa: Modified PendingAppInstallTask to use a constructor provided InstallManager to support it as a test fixture.

Previously PendingAppInstallTask::ContinueWebAppInstall fetched the InstallManager via the WebAppProvider via the profile. This change passes the InstallManager via the constructor and avoids this lookup.

auto* provider = WebAppProviderBase::GetProviderBase(profile_);
provider->install_manager().InstallWebAppWithParams(

There will be a follow up CL where the system_web_app_manager_unittest.cc tests are refactored. This change to the pending_app_install_task is needed for the refactor.

Bug: 1082880
Change-Id: I038f0005325c77742d15ef3d5e1673903fdf7490
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229013Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#774928}

chrome/browser/web_applications/components/pending_app_manager.cc

@@ -40,12 +40,14 @@ void PendingAppManager::SetSubsystems(AppRegistrar* registrar,
                                       AppShortcutManager* shortcut_manager,
                                       FileHandlerManager* file_handler_manager,
                                       WebAppUiManager* ui_manager,
-                                      InstallFinalizer* finalizer) {
+                                      InstallFinalizer* finalizer,
+                                      InstallManager* install_manager) {
   registrar_ = registrar;
   shortcut_manager_ = shortcut_manager;
   file_handler_manager_ = file_handler_manager;
   ui_manager_ = ui_manager;
   finalizer_ = finalizer;
+  install_manager_ = install_manager;
 }
 
 void PendingAppManager::SynchronizeInstalledApps(

chrome/browser/web_applications/components/pending_app_manager.h

@@ -27,6 +27,7 @@ class AppRegistrar;
 class AppShortcutManager;
 class FileHandlerManager;
 class InstallFinalizer;
+class InstallManager;
 class WebAppUiManager;
 
 enum class RegistrationResultCode { kSuccess, kAlreadyRegistered, kTimeout };
@@ -60,7 +61,8 @@ class PendingAppManager {
                      AppShortcutManager* shortcut_manager,
                      FileHandlerManager* file_handler_manager,
                      WebAppUiManager* ui_manager,
-                     InstallFinalizer* finalizer);
+                     InstallFinalizer* finalizer,
+                     InstallManager* install_manager);
 
   // Queues an installation operation with the highest priority. Essentially
   // installing the app immediately if there are no ongoing operations or
@@ -121,6 +123,7 @@ class PendingAppManager {
   FileHandlerManager* file_handler_manager() { return file_handler_manager_; }
   WebAppUiManager* ui_manager() { return ui_manager_; }
   InstallFinalizer* finalizer() { return finalizer_; }
+  InstallManager* install_manager() { return install_manager_; }
 
   virtual void OnRegistrationFinished(const GURL& launch_url,
                                       RegistrationResultCode result);
@@ -155,6 +158,7 @@ class PendingAppManager {
   FileHandlerManager* file_handler_manager_ = nullptr;
   WebAppUiManager* ui_manager_ = nullptr;
   InstallFinalizer* finalizer_ = nullptr;
+  InstallManager* install_manager_ = nullptr;
 
   base::flat_map<ExternalInstallSource, SynchronizeRequest>
       synchronize_requests_;

chrome/browser/web_applications/extensions/pending_app_install_task_unittest.cc

@@ -330,6 +330,7 @@ class PendingAppInstallTaskTest : public ChromeRenderViewHostTestHarness {
   TestWebAppUiManager* ui_manager() { return ui_manager_; }
   TestAppRegistrar* registrar() { return registrar_; }
   TestPendingAppInstallFinalizer* finalizer() { return install_finalizer_; }
+  WebAppInstallManager* install_manager() { return install_manager_; }
   TestAppShortcutManager* shortcut_manager() { return shortcut_manager_; }
   TestFileHandlerManager* file_handler_manager() {
     return file_handler_manager_;
@@ -372,7 +373,7 @@ class PendingAppInstallTaskTest : public ChromeRenderViewHostTestHarness {
 
     auto task = std::make_unique<PendingAppInstallTask>(
         profile(), registrar_, shortcut_manager_, file_handler_manager_,
-        ui_manager_, install_finalizer_, std::move(options));
+        ui_manager_, install_finalizer_, install_manager_, std::move(options));
     return task;
   }
 
@@ -902,7 +903,7 @@ TEST_F(PendingAppInstallTaskTest, InstallURLLoadFailed) {
         ExternalInstallSource::kInternalDefault);
     PendingAppInstallTask install_task(
         profile(), registrar(), shortcut_manager(), file_handler_manager(),
-        ui_manager(), finalizer(), install_options);
+        ui_manager(), finalizer(), install_manager(), install_options);
 
     install_task.Install(
         web_contents(), result_pair.loader_result,
@@ -919,9 +920,9 @@ TEST_F(PendingAppInstallTaskTest, FailedWebContentsDestroyed) {
   ExternalInstallOptions install_options(
       GURL(), DisplayMode::kStandalone,
       ExternalInstallSource::kInternalDefault);
-  PendingAppInstallTask install_task(profile(), registrar(), shortcut_manager(),
-                                     file_handler_manager(), ui_manager(),
-                                     finalizer(), install_options);
+  PendingAppInstallTask install_task(
+      profile(), registrar(), shortcut_manager(), file_handler_manager(),
+      ui_manager(), finalizer(), install_manager(), install_options);
 
   install_task.Install(
       web_contents(), WebAppUrlLoader::Result::kFailedWebContentsDestroyed,

chrome/browser/web_applications/pending_app_install_task.cc

@@ -22,7 +22,6 @@
 #include "chrome/browser/web_applications/components/install_finalizer.h"
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
-#include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/components/web_app_ui_manager.h"
 #include "chrome/common/web_application_info.h"
 #include "components/performance_manager/embedder/performance_manager_registry.h"
@@ -59,12 +58,14 @@ PendingAppInstallTask::PendingAppInstallTask(
     FileHandlerManager* file_handler_manager,
     WebAppUiManager* ui_manager,
     InstallFinalizer* install_finalizer,
+    InstallManager* install_manager,
     ExternalInstallOptions install_options)
     : profile_(profile),
       registrar_(registrar),
       shortcut_manager_(shortcut_manager),
       file_handler_manager_(file_handler_manager),
       install_finalizer_(install_finalizer),
+      install_manager_(install_manager),
       ui_manager_(ui_manager),
       externally_installed_app_prefs_(profile_->GetPrefs()),
       install_options_(std::move(install_options)) {}
@@ -169,14 +170,12 @@ void PendingAppInstallTask::OnPlaceholderUninstalled(
 void PendingAppInstallTask::ContinueWebAppInstall(
     content::WebContents* web_contents,
     ResultCallback result_callback) {
-  auto* provider = WebAppProviderBase::GetProviderBase(profile_);
-  DCHECK(provider);
 
   auto install_params = ConvertExternalInstallOptionsToParams(install_options_);
   auto install_source = ConvertExternalInstallSourceToInstallSource(
       install_options_.install_source);
 
-  provider->install_manager().InstallWebAppWithParams(
+  install_manager_->InstallWebAppWithParams(
       web_contents, install_params, install_source,
       base::BindOnce(&PendingAppInstallTask::OnWebAppInstalled,
                      weak_ptr_factory_.GetWeakPtr(), false /* is_placeholder */,

chrome/browser/web_applications/pending_app_install_task.h

@@ -28,6 +28,7 @@ namespace web_app {
 class AppShortcutManager;
 class FileHandlerManager;
 class InstallFinalizer;
+class InstallManager;
 class WebAppUiManager;
 enum class InstallResultCode;
 
@@ -62,6 +63,7 @@ class PendingAppInstallTask {
                                  FileHandlerManager* file_handler_manager,
                                  WebAppUiManager* ui_manager,
                                  InstallFinalizer* install_finalizer,
+                                 InstallManager* install_manager,
                                  ExternalInstallOptions install_options);
 
   virtual ~PendingAppInstallTask();
@@ -95,6 +97,7 @@ class PendingAppInstallTask {
   AppShortcutManager* const shortcut_manager_;
   FileHandlerManager* const file_handler_manager_;
   InstallFinalizer* const install_finalizer_;
+  InstallManager* const install_manager_;
   WebAppUiManager* const ui_manager_;
 
   ExternallyInstalledWebAppPrefs externally_installed_app_prefs_;

chrome/browser/web_applications/pending_app_manager_impl.cc

@@ -101,7 +101,7 @@ PendingAppManagerImpl::CreateInstallationTask(
     ExternalInstallOptions install_options) {
   return std::make_unique<PendingAppInstallTask>(
       profile_, registrar(), shortcut_manager(), file_handler_manager(),
-      ui_manager(), finalizer(), std::move(install_options));
+      ui_manager(), finalizer(), install_manager(), std::move(install_options));
 }
 
 std::unique_ptr<PendingAppRegistrationTaskBase>

chrome/browser/web_applications/pending_app_manager_impl_unittest.cc

@@ -218,6 +218,7 @@ class TestPendingAppManagerImpl : public PendingAppManagerImpl {
               pending_app_manager_impl->file_handler_manager(),
               pending_app_manager_impl->ui_manager(),
               pending_app_manager_impl->finalizer(),
+              pending_app_manager_impl->install_manager(),
               install_options),
           pending_app_manager_impl_(pending_app_manager_impl),
           externally_installed_app_prefs_(profile->GetPrefs()) {}

chrome/browser/web_applications/test/test_pending_app_manager.cc

@@ -24,7 +24,7 @@ TestPendingAppManager::TestPendingAppManager(TestAppRegistrar* registrar)
       deduped_uninstall_count_(0),
       registrar_(registrar) {
   // TODO(crbug.com/973324): Wire this up to a TestInstallFinalizer.
-  SetSubsystems(registrar, nullptr, nullptr, nullptr, nullptr);
+  SetSubsystems(registrar, nullptr, nullptr, nullptr, nullptr, nullptr);
 }
 
 TestPendingAppManager::~TestPendingAppManager() = default;

chrome/browser/web_applications/web_app_provider.cc

@@ -258,7 +258,7 @@ void WebAppProvider::ConnectSubsystems() {
       install_manager_.get(), system_web_app_manager_.get());
   pending_app_manager_->SetSubsystems(
       registrar_.get(), shortcut_manager_.get(), file_handler_manager_.get(),
-      ui_manager_.get(), install_finalizer_.get());
+      ui_manager_.get(), install_finalizer_.get(), install_manager_.get());
   external_web_app_manager_->SetSubsystems(pending_app_manager_.get());
   system_web_app_manager_->SetSubsystems(
       pending_app_manager_.get(), registrar_.get(), registry_controller_.get(),