borealis: Move BorealisInstaller to the BorealisService

Similar to crrev.com/c/2562799, we remove the installer factory and move
it to the omni-service.

Bug: b/170596931
Change-Id: I182816dba38fd66d116a1d066d414d69b1503c7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2567276
Commit-Queue: Nic Hollingum <hollingum@google.com>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833523}

chrome/browser/chromeos/BUILD.gn

@@ -877,8 +877,6 @@ source_set("chromeos") {
     "borealis/borealis_features.h",
     "borealis/borealis_installer.cc",
     "borealis/borealis_installer.h",
-    "borealis/borealis_installer_factory.cc",
-    "borealis/borealis_installer_factory.h",
     "borealis/borealis_installer_impl.cc",
     "borealis/borealis_installer_impl.h",
     "borealis/borealis_launch_watcher.cc",

chrome/browser/chromeos/borealis/borealis_installer.h

@@ -33,6 +33,7 @@ class BorealisInstaller : public KeyedService {
   };
 
   BorealisInstaller();
+  ~BorealisInstaller() override;
 
   static std::string GetInstallingStateName(InstallingState state);
 
@@ -45,14 +46,6 @@ class BorealisInstaller : public KeyedService {
 
   virtual void AddObserver(Observer* observer) = 0;
   virtual void RemoveObserver(Observer* observer) = 0;
-
- protected:
-  ~BorealisInstaller() override;
-
-  base::ObserverList<Observer> observers_;
-
- private:
-  base::WeakPtrFactory<BorealisInstaller> weak_ptr_factory_{this};
 };
 
 }  // namespace borealis

chrome/browser/chromeos/borealis/borealis_installer_factory.cc

-// Copyright 2020 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/chromeos/borealis/borealis_installer_factory.h"
-
-#include "chrome/browser/chromeos/borealis/borealis_installer_impl.h"
-#include "chrome/browser/download/download_service_factory.h"
-#include "chrome/browser/profiles/incognito_helpers.h"
-#include "chrome/browser/profiles/profile.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
-
-namespace borealis {
-
-// static
-BorealisInstaller* BorealisInstallerFactory::GetForProfile(Profile* profile) {
-  return static_cast<BorealisInstaller*>(
-      GetInstance()->GetServiceForBrowserContext(profile, true));
-}
-
-// static
-BorealisInstallerFactory* BorealisInstallerFactory::GetInstance() {
-  static base::NoDestructor<BorealisInstallerFactory> factory;
-  return factory.get();
-}
-
-BorealisInstallerFactory::BorealisInstallerFactory()
-    : BrowserContextKeyedServiceFactory(
-          "BorealisInstaller",
-          BrowserContextDependencyManager::GetInstance()) {
-  DependsOn(DownloadServiceFactory::GetInstance());
-}
-
-BorealisInstallerFactory::~BorealisInstallerFactory() = default;
-
-KeyedService* BorealisInstallerFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  return new BorealisInstallerImpl(Profile::FromBrowserContext(context));
-}
-
-content::BrowserContext* BorealisInstallerFactory::GetBrowserContextToUse(
-    content::BrowserContext* context) const {
-  return chrome::GetBrowserContextRedirectedInIncognito(context);
-}
-
-}  // namespace borealis

chrome/browser/chromeos/borealis/borealis_installer_factory.h

-// Copyright 2020 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_CHROMEOS_BOREALIS_BOREALIS_INSTALLER_FACTORY_H_
-#define CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_INSTALLER_FACTORY_H_
-
-#include "base/macros.h"
-#include "base/no_destructor.h"
-#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
-
-namespace content {
-class BrowserContext;
-}  // namespace content
-
-class Profile;
-
-namespace borealis {
-
-class BorealisInstaller;
-
-class BorealisInstallerFactory : public BrowserContextKeyedServiceFactory {
- public:
-  static BorealisInstaller* GetForProfile(Profile* profile);
-  static BorealisInstallerFactory* GetInstance();
-
- private:
-  friend base::NoDestructor<BorealisInstallerFactory>;
-
-  BorealisInstallerFactory();
-  ~BorealisInstallerFactory() override;
-
-  // BrowserContextKeyedServiceFactory implementation.
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-  content::BrowserContext* GetBrowserContextToUse(
-      content::BrowserContext* context) const override;
-
-  DISALLOW_COPY_AND_ASSIGN(BorealisInstallerFactory);
-};
-
-}  // namespace borealis
-
-#endif  // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_INSTALLER_FACTORY_H_

chrome/browser/chromeos/borealis/borealis_installer_impl.h

@@ -19,6 +19,7 @@ namespace borealis {
 class BorealisInstallerImpl : public BorealisInstaller {
  public:
   explicit BorealisInstallerImpl(Profile* profile);
+  ~BorealisInstallerImpl() override;
 
   // Disallow copy and assign.
   BorealisInstallerImpl(const BorealisInstallerImpl&) = delete;
@@ -41,8 +42,6 @@ class BorealisInstallerImpl : public BorealisInstaller {
     kCancelling,
   };
 
-  ~BorealisInstallerImpl() override;
-
   void StartDlcInstallation();
   void InstallationEnded(BorealisInstallResult result);
 
@@ -58,6 +57,7 @@ class BorealisInstallerImpl : public BorealisInstaller {
   double progress_;
   base::TimeTicks installation_start_tick_;
   Profile* profile_;
+  base::ObserverList<Observer> observers_;
 
   base::WeakPtrFactory<BorealisInstallerImpl> weak_ptr_factory_;
 };

chrome/browser/chromeos/borealis/borealis_installer_unittest.cc

@@ -9,7 +9,6 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/chromeos/borealis/borealis_features.h"
-#include "chrome/browser/chromeos/borealis/borealis_installer_factory.h"
 #include "chrome/browser/chromeos/borealis/borealis_metrics.h"
 #include "chrome/browser/chromeos/borealis/borealis_prefs.h"
 #include "chrome/browser/chromeos/borealis/borealis_service.h"
@@ -53,7 +52,8 @@ class BorealisInstallerTest : public testing::Test {
     histogram_tester_ = std::make_unique<base::HistogramTester>();
     CreateProfile();
 
-    installer_ = BorealisInstallerFactory::GetForProfile(profile_.get());
+    installer_impl_ = std::make_unique<BorealisInstallerImpl>(profile_.get());
+    installer_ = installer_impl_.get();
     observer_ = std::make_unique<StrictMock<MockObserver>>();
     installer_->AddObserver(observer_.get());
 
@@ -110,6 +110,7 @@ class BorealisInstallerTest : public testing::Test {
 
   std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<base::HistogramTester> histogram_tester_;
+  std::unique_ptr<BorealisInstallerImpl> installer_impl_;
   BorealisInstaller* installer_;
   std::unique_ptr<MockObserver> observer_;
   content::BrowserTaskEnvironment task_environment_;

chrome/browser/chromeos/borealis/borealis_service.h

@@ -14,6 +14,7 @@ namespace borealis {
 class BorealisAppLauncher;
 class BorealisContextManager;
 class BorealisFeatures;
+class BorealisInstaller;
 class BorealisWindowManager;
 
 // A common location for all the interdependant components of borealis.
@@ -27,6 +28,7 @@ class BorealisService : public KeyedService {
   virtual BorealisAppLauncher& AppLauncher() = 0;
   virtual BorealisContextManager& ContextManager() = 0;
   virtual BorealisFeatures& Features() = 0;
+  virtual BorealisInstaller& Installer() = 0;
   virtual BorealisWindowManager& WindowManager() = 0;
 };
 

chrome/browser/chromeos/borealis/borealis_service_fake.cc

@@ -36,6 +36,11 @@ BorealisFeatures& BorealisServiceFake::Features() {
   return *features_;
 }
 
+BorealisInstaller& BorealisServiceFake::Installer() {
+  DCHECK(installer_);
+  return *installer_;
+}
+
 BorealisWindowManager& BorealisServiceFake::WindowManager() {
   DCHECK(window_manager_);
   return *window_manager_;
@@ -55,6 +60,10 @@ void BorealisServiceFake::SetFeaturesForTesting(BorealisFeatures* features) {
   features_ = features;
 }
 
+void BorealisServiceFake::SetInstallerForTesting(BorealisInstaller* installer) {
+  installer_ = installer;
+}
+
 void BorealisServiceFake::SetWindowManagerForTesting(
     BorealisWindowManager* window_manager) {
   window_manager_ = window_manager;

chrome/browser/chromeos/borealis/borealis_service_fake.h

@@ -25,17 +25,20 @@ class BorealisServiceFake : public BorealisService {
   BorealisAppLauncher& AppLauncher() override;
   BorealisContextManager& ContextManager() override;
   BorealisFeatures& Features() override;
+  BorealisInstaller& Installer() override;
   BorealisWindowManager& WindowManager() override;
 
   void SetAppLauncherForTesting(BorealisAppLauncher* app_launcher);
   void SetContextManagerForTesting(BorealisContextManager* context_manager);
   void SetFeaturesForTesting(BorealisFeatures* features);
+  void SetInstallerForTesting(BorealisInstaller* installer);
   void SetWindowManagerForTesting(BorealisWindowManager* window_manager);
 
  private:
   BorealisAppLauncher* app_launcher_ = nullptr;
   BorealisContextManager* context_manager_ = nullptr;
   BorealisFeatures* features_ = nullptr;
+  BorealisInstaller* installer_ = nullptr;
   BorealisWindowManager* window_manager_ = nullptr;
 };
 

chrome/browser/chromeos/borealis/borealis_service_impl.cc

@@ -4,11 +4,6 @@
 
 #include "chrome/browser/chromeos/borealis/borealis_service_impl.h"
 
-#include "chrome/browser/chromeos/borealis/borealis_app_launcher.h"
-#include "chrome/browser/chromeos/borealis/borealis_features.h"
-#include "chrome/browser/chromeos/borealis/borealis_window_manager.h"
-#include "chrome/browser/profiles/profile.h"
-
 namespace borealis {
 
 BorealisServiceImpl::BorealisServiceImpl(Profile* profile)
@@ -16,6 +11,7 @@ BorealisServiceImpl::BorealisServiceImpl(Profile* profile)
       app_launcher_(profile_),
       context_manager_(profile),
       features_(profile_),
+      installer_(profile_),
       window_manager_(profile_) {}
 
 BorealisServiceImpl::~BorealisServiceImpl() = default;
@@ -32,6 +28,10 @@ BorealisFeatures& BorealisServiceImpl::Features() {
   return features_;
 }
 
+BorealisInstaller& BorealisServiceImpl::Installer() {
+  return installer_;
+}
+
 BorealisWindowManager& BorealisServiceImpl::WindowManager() {
   return window_manager_;
 }

chrome/browser/chromeos/borealis/borealis_service_impl.h

@@ -10,6 +10,7 @@
 #include "chrome/browser/chromeos/borealis/borealis_app_launcher.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager_impl.h"
 #include "chrome/browser/chromeos/borealis/borealis_features.h"
+#include "chrome/browser/chromeos/borealis/borealis_installer_impl.h"
 #include "chrome/browser/chromeos/borealis/borealis_window_manager.h"
 
 namespace borealis {
@@ -25,6 +26,7 @@ class BorealisServiceImpl : public BorealisService {
   BorealisAppLauncher& AppLauncher() override;
   BorealisContextManager& ContextManager() override;
   BorealisFeatures& Features() override;
+  BorealisInstaller& Installer() override;
   BorealisWindowManager& WindowManager() override;
 
   Profile* const profile_;
@@ -32,6 +34,7 @@ class BorealisServiceImpl : public BorealisService {
   BorealisAppLauncher app_launcher_;
   BorealisContextManagerImpl context_manager_;
   BorealisFeatures features_;
+  BorealisInstallerImpl installer_;
   BorealisWindowManager window_manager_;
 };
 

chrome/browser/ui/views/borealis/borealis_installer_view.cc

@@ -13,7 +13,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
-#include "chrome/browser/chromeos/borealis/borealis_installer_factory.h"
+#include "chrome/browser/chromeos/borealis/borealis_installer.h"
 #include "chrome/browser/chromeos/borealis/borealis_service.h"
 #include "chrome/browser/chromeos/borealis/borealis_util.h"
 #include "chrome/browser/ui/browser_navigator.h"
@@ -79,9 +79,7 @@ class BorealisInstallerView::TitleLabel : public views::Label {
 // Currently using the UI specs that the Plugin VM installer use.
 BorealisInstallerView::BorealisInstallerView(Profile* profile)
     : app_name_(l10n_util::GetStringUTF16(IDS_BOREALIS_APP_NAME)),
-      profile_(profile),
-      borealis_installer_(
-          borealis::BorealisInstallerFactory::GetForProfile(profile_)) {
+      profile_(profile) {
   // Layout constants from the spec used for the plugin vm installer.
   gfx::Insets kDialogInsets(60, 64, 0, 64);
   const int kPrimaryMessageHeight = views::style::GetLineHeight(
@@ -165,9 +163,11 @@ BorealisInstallerView::BorealisInstallerView(Profile* profile)
 }
 
 BorealisInstallerView::~BorealisInstallerView() {
-  borealis_installer_->RemoveObserver(this);
+  borealis::BorealisInstaller& installer =
+      borealis::BorealisService::GetForProfile(profile_)->Installer();
+  installer.RemoveObserver(this);
   if (state_ == State::kConfirmInstall || state_ == State::kInstalling) {
-    borealis_installer_->Cancel();
+    installer.Cancel();
   }
   g_borealis_installer_view = nullptr;
 }
@@ -448,6 +448,8 @@ void BorealisInstallerView::StartInstallation() {
   progress_bar_->SetValue(0);
   OnStateUpdated();
 
-  borealis_installer_->AddObserver(this);
-  borealis_installer_->Start();
+  borealis::BorealisInstaller& installer =
+      borealis::BorealisService::GetForProfile(profile_)->Installer();
+  installer.AddObserver(this);
+  installer.Start();
 }

chrome/browser/ui/views/borealis/borealis_installer_view.h

@@ -7,7 +7,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "chrome/browser/chromeos/borealis/borealis_installer_impl.h"
+#include "chrome/browser/chromeos/borealis/borealis_installer.h"
 #include "chrome/browser/chromeos/borealis/borealis_metrics.h"
 #include "ui/views/window/dialog_delegate.h"
 
@@ -93,7 +93,6 @@ class BorealisInstallerView : public views::DialogDelegateView,
   views::BoxLayout* lower_container_layout_ = nullptr;
   views::ImageView* big_image_ = nullptr;
 
-  borealis::BorealisInstaller* borealis_installer_ = nullptr;
   State state_ = State::kConfirmInstall;
   InstallingState installing_state_ = InstallingState::kInactive;
   base::Optional<borealis::BorealisInstallResult> result_;

chrome/browser/ui/views/borealis/borealis_installer_view_browsertest.cc

@@ -7,7 +7,7 @@
 #include "base/bind.h"
 #include "chrome/browser/chromeos/borealis/borealis_context.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
-#include "chrome/browser/chromeos/borealis/borealis_installer_factory.h"
+#include "chrome/browser/chromeos/borealis/borealis_installer.h"
 #include "chrome/browser/chromeos/borealis/borealis_metrics.h"
 #include "chrome/browser/chromeos/borealis/borealis_service_fake.h"
 #include "chrome/browser/chromeos/borealis/borealis_task.h"
@@ -56,20 +56,11 @@ class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
   // DialogBrowserTest:
   void SetUpOnMainThread() override {
     app_name_ = l10n_util::GetStringUTF16(IDS_BOREALIS_APP_NAME);
-    mock_installer_ =
-        static_cast<::testing::StrictMock<BorealisInstallerMock>*>(
-            borealis::BorealisInstallerFactory::GetInstance()
-                ->SetTestingFactoryAndUse(
-                    browser()->profile(),
-                    base::BindRepeating([](content::BrowserContext* context)
-                                            -> std::unique_ptr<KeyedService> {
-                      return std::make_unique<
-                          testing::StrictMock<BorealisInstallerMock>>();
-                    })));
 
     BorealisServiceFake* fake_service =
         BorealisServiceFake::UseFakeForTesting(browser()->profile());
     fake_service->SetContextManagerForTesting(&mock_context_manager_);
+    fake_service->SetInstallerForTesting(&mock_installer_);
   }
 
   void ShowUi(const std::string& name) override {
@@ -129,8 +120,8 @@ class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
   }
 
   void AcceptInstallation() {
-    EXPECT_CALL(*mock_installer_, AddObserver(_));
-    EXPECT_CALL(*mock_installer_, Start());
+    EXPECT_CALL(mock_installer_, AddObserver(_));
+    EXPECT_CALL(mock_installer_, Start());
     view_->AcceptDialog();
     view_->SetInstallingStateForTesting(
         borealis::BorealisInstaller::InstallingState::kInstallingDlc);
@@ -138,13 +129,13 @@ class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
   }
 
   void ClickCancel() {
-    EXPECT_CALL(*mock_installer_, RemoveObserver(_));
+    EXPECT_CALL(mock_installer_, RemoveObserver(_));
     view_->CancelDialog();
 
     EXPECT_TRUE(view_->GetWidget()->IsClosed());
   }
 
-  ::testing::StrictMock<BorealisInstallerMock>* mock_installer_;
+  ::testing::StrictMock<BorealisInstallerMock> mock_installer_;
   ::testing::StrictMock<BorealisContextManagerMock> mock_context_manager_;
   BorealisInstallerView* view_;
   base::string16 app_name_;
@@ -159,8 +150,8 @@ class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
 
 // Test that the dialog can be launched.
 IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest, InvokeUi_default) {
-  EXPECT_CALL(*mock_installer_, RemoveObserver(_));
-  EXPECT_CALL(*mock_installer_, Cancel());
+  EXPECT_CALL(mock_installer_, RemoveObserver(_));
+  EXPECT_CALL(mock_installer_, Cancel());
   ShowAndVerifyUi();
 }
 
@@ -172,7 +163,7 @@ IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest, SucessfulInstall) {
   ExpectInstallationCompletedSucessfully();
 
   EXPECT_CALL(mock_context_manager_, StartBorealis(_));
-  EXPECT_CALL(*mock_installer_, RemoveObserver(_));
+  EXPECT_CALL(mock_installer_, RemoveObserver(_));
   view_->AcceptDialog();
 
   EXPECT_TRUE(view_->GetWidget()->IsClosed());
@@ -182,7 +173,7 @@ IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest,
                        ConfirmationCancelled) {
   ShowUi("default");
 
-  EXPECT_CALL(*mock_installer_, Cancel());
+  EXPECT_CALL(mock_installer_, Cancel());
   ClickCancel();
 }
 
@@ -191,7 +182,7 @@ IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest,
   ShowUi("default");
   AcceptInstallation();
 
-  EXPECT_CALL(*mock_installer_, Cancel());
+  EXPECT_CALL(mock_installer_, Cancel());
   ClickCancel();
 }
 
@@ -214,7 +205,7 @@ IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest,
   ExpectInstallationCompletedSucessfully();
 
   EXPECT_CALL(mock_context_manager_, StartBorealis(_));
-  EXPECT_CALL(*mock_installer_, RemoveObserver(_));
+  EXPECT_CALL(mock_installer_, RemoveObserver(_));
   view_->AcceptDialog();
 
   EXPECT_TRUE(view_->GetWidget()->IsClosed());