borealis: Move the context manager to the borealis service

We have been advised to avoid proliferating KeyedServiceFactories and
instead use one omni-service. In line with this, we move the context
manager.

Bug: b/172006764, b/170596931
Change-Id: Iebc52681839b1476c0b01f93948c48334c80162f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562799Reviewed-by: Nic Hollingum <hollingum@google.com>
Reviewed-by: Daniel Ng <danielng@google.com>
Commit-Queue: Nic Hollingum <hollingum@google.com>
Cr-Commit-Position: refs/heads/master@{#833174}

chrome/browser/chromeos/BUILD.gn

@@ -870,8 +870,6 @@ source_set("chromeos") {
     "borealis/borealis_context.h",
     "borealis/borealis_context_manager.cc",
     "borealis/borealis_context_manager.h",
-    "borealis/borealis_context_manager_factory.cc",
-    "borealis/borealis_context_manager_factory.h",
     "borealis/borealis_context_manager_impl.cc",
     "borealis/borealis_context_manager_impl.h",
     "borealis/borealis_features.cc",
@@ -3156,6 +3154,8 @@ static_library("test_support") {
     "attestation/mock_enrollment_certificate_uploader.h",
     "attestation/mock_machine_certificate_uploader.cc",
     "attestation/mock_machine_certificate_uploader.h",
+    "borealis/borealis_service_fake.cc",
+    "borealis/borealis_service_fake.h",
     "cert_provisioning/mock_cert_provisioning_scheduler.cc",
     "cert_provisioning/mock_cert_provisioning_scheduler.h",
     "certificate_provider/test_certificate_provider_extension.cc",

chrome/browser/chromeos/borealis/borealis_app_launcher.cc

@@ -7,7 +7,6 @@
 #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_context_manager_factory.h"
 #include "chrome/browser/chromeos/borealis/borealis_features.h"
 #include "chrome/browser/chromeos/borealis/borealis_service.h"
 #include "chrome/browser/chromeos/borealis/borealis_util.h"
@@ -86,7 +85,7 @@ void BorealisAppLauncher::Launch(std::string app_id,
     return;
   }
 
-  BorealisContextManagerFactory::GetForProfile(profile_)->StartBorealis(
+  BorealisService::GetForProfile(profile_)->ContextManager().StartBorealis(
       base::BindOnce(
           [](std::string app_id,
              BorealisAppLauncher::OnLaunchedCallback callback,

chrome/browser/chromeos/borealis/borealis_context_manager_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_context_manager_factory.h"
-
-#include "chrome/browser/chromeos/borealis/borealis_context_manager_impl.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.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
-BorealisContextManager* BorealisContextManagerFactory::GetForProfile(
-    Profile* profile) {
-  return static_cast<BorealisContextManager*>(
-      GetInstance()->GetServiceForBrowserContext(profile, true));
-}
-
-// static
-BorealisContextManagerFactory* BorealisContextManagerFactory::GetInstance() {
-  static base::NoDestructor<BorealisContextManagerFactory> factory;
-  return factory.get();
-}
-
-BorealisContextManagerFactory::BorealisContextManagerFactory()
-    : BrowserContextKeyedServiceFactory(
-          "BorealisContextManager",
-          BrowserContextDependencyManager::GetInstance()) {}
-
-BorealisContextManagerFactory::~BorealisContextManagerFactory() = default;
-
-KeyedService* BorealisContextManagerFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  Profile* profile = Profile::FromBrowserContext(context);
-  if (chromeos::ProfileHelper::IsPrimaryProfile(profile))
-    return new BorealisContextManagerImpl(profile);
-  return nullptr;
-}
-
-content::BrowserContext* BorealisContextManagerFactory::GetBrowserContextToUse(
-    content::BrowserContext* context) const {
-  return chrome::GetBrowserContextRedirectedInIncognito(context);
-}
-
-}  // namespace borealis

chrome/browser/chromeos/borealis/borealis_context_manager_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_CONTEXT_MANAGER_FACTORY_H_
-#define CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_CONTEXT_MANAGER_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 BorealisContextManager;
-
-class BorealisContextManagerFactory : public BrowserContextKeyedServiceFactory {
- public:
-  static BorealisContextManager* GetForProfile(Profile* profile);
-  static BorealisContextManagerFactory* GetInstance();
-
- private:
-  friend base::NoDestructor<BorealisContextManagerFactory>;
-
-  BorealisContextManagerFactory();
-  BorealisContextManagerFactory(const BorealisContextManagerFactory&) = delete;
-  BorealisContextManagerFactory& operator=(
-      const BorealisContextManagerFactory&) = delete;
-  ~BorealisContextManagerFactory() override;
-
-  // BrowserContextKeyedServiceFactory implementation.
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-  content::BrowserContext* GetBrowserContextToUse(
-      content::BrowserContext* context) const override;
-};
-
-}  // namespace borealis
-
-#endif  // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_CONTEXT_MANAGER_FACTORY_H_

chrome/browser/chromeos/borealis/borealis_context_manager_unittest.cc

@@ -12,7 +12,6 @@
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
-#include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h"
 #include "chrome/browser/chromeos/borealis/borealis_metrics.h"
 #include "chrome/browser/chromeos/borealis/borealis_task.h"
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
@@ -310,48 +309,5 @@ TEST_F(BorealisContextManagerTest, TasksCanOutliveCompletion) {
   task_environment_.RunUntilIdle();
 }
 
-class BorealisContextManagerFactoryTest : public testing::Test {
- public:
-  BorealisContextManagerFactoryTest() = default;
-  BorealisContextManagerFactoryTest(const BorealisContextManagerFactoryTest&) =
-      delete;
-  BorealisContextManagerFactoryTest& operator=(
-      const BorealisContextManagerFactoryTest&) = delete;
-  ~BorealisContextManagerFactoryTest() override = default;
-
- protected:
-  void TearDown() override { chromeos::DBusThreadManager::Shutdown(); }
-
-  content::BrowserTaskEnvironment task_environment_;
-};
-
-TEST_F(BorealisContextManagerFactoryTest, ReturnsContextManagerForMainProfile) {
-  TestingProfile profile;
-  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager;
-  auto mock_user_manager =
-      std::make_unique<testing::NiceMock<chromeos::MockUserManager>>();
-  mock_user_manager->AddUser(
-      AccountId::FromUserEmailGaiaId(profile.GetProfileUserName(), "id"));
-  scoped_user_manager = std::make_unique<user_manager::ScopedUserManager>(
-      std::move(mock_user_manager));
-  chromeos::DBusThreadManager::Initialize();
-
-  BorealisContextManager* context_manager =
-      BorealisContextManagerFactory::GetForProfile(&profile);
-  EXPECT_TRUE(context_manager);
-}
-
-TEST_F(BorealisContextManagerFactoryTest,
-       ReturnsNullpointerForSecondaryProfile) {
-  TestingProfile::Builder profile_builder;
-  profile_builder.SetProfileName("defaultprofile");
-  std::unique_ptr<TestingProfile> profile = profile_builder.Build();
-  chromeos::DBusThreadManager::Initialize();
-
-  BorealisContextManager* context_manager =
-      BorealisContextManagerFactory::GetForProfile(profile.get());
-  EXPECT_FALSE(context_manager);
-}
-
 }  // namespace
 }  // namespace borealis

chrome/browser/chromeos/borealis/borealis_service.h

@@ -12,6 +12,7 @@ class Profile;
 namespace borealis {
 
 class BorealisAppLauncher;
+class BorealisContextManager;
 class BorealisFeatures;
 class BorealisWindowManager;
 
@@ -24,6 +25,7 @@ class BorealisService : public KeyedService {
   ~BorealisService() override = default;
 
   virtual BorealisAppLauncher& AppLauncher() = 0;
+  virtual BorealisContextManager& ContextManager() = 0;
   virtual BorealisFeatures& Features() = 0;
   virtual BorealisWindowManager& WindowManager() = 0;
 };

chrome/browser/chromeos/borealis/borealis_service_fake.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_service_fake.h"
+
+#include "chrome/browser/chromeos/borealis/borealis_service_factory.h"
+
+namespace borealis {
+
+// static
+BorealisServiceFake* BorealisServiceFake::UseFakeForTesting(
+    content::BrowserContext* context) {
+  return static_cast<BorealisServiceFake*>(
+      borealis::BorealisServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+          context, base::BindRepeating([](content::BrowserContext* context)
+                                           -> std::unique_ptr<KeyedService> {
+            return std::make_unique<BorealisServiceFake>();
+          })));
+}
+
+BorealisServiceFake::~BorealisServiceFake() = default;
+
+BorealisAppLauncher& BorealisServiceFake::AppLauncher() {
+  DCHECK(app_launcher_);
+  return *app_launcher_;
+}
+
+BorealisContextManager& BorealisServiceFake::ContextManager() {
+  DCHECK(context_manager_);
+  return *context_manager_;
+}
+
+BorealisFeatures& BorealisServiceFake::Features() {
+  DCHECK(features_);
+  return *features_;
+}
+
+BorealisWindowManager& BorealisServiceFake::WindowManager() {
+  DCHECK(window_manager_);
+  return *window_manager_;
+}
+
+void BorealisServiceFake::SetAppLauncherForTesting(
+    BorealisAppLauncher* app_launcher) {
+  app_launcher_ = app_launcher;
+}
+
+void BorealisServiceFake::SetContextManagerForTesting(
+    BorealisContextManager* context_manager) {
+  context_manager_ = context_manager;
+}
+
+void BorealisServiceFake::SetFeaturesForTesting(BorealisFeatures* features) {
+  features_ = features;
+}
+
+void BorealisServiceFake::SetWindowManagerForTesting(
+    BorealisWindowManager* window_manager) {
+  window_manager_ = window_manager;
+}
+
+}  // namespace borealis

chrome/browser/chromeos/borealis/borealis_service_fake.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_SERVICE_FAKE_H_
+#define CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_SERVICE_FAKE_H_
+
+#include "chrome/browser/chromeos/borealis/borealis_service.h"
+
+namespace content {
+class BrowserContext;
+}
+
+namespace borealis {
+
+class BorealisServiceFake : public BorealisService {
+ public:
+  // Causes the service for the given |context| to be a fake in tests. Returns a
+  // handle to the fake, which will be owned by the service factory.
+  static BorealisServiceFake* UseFakeForTesting(
+      content::BrowserContext* context);
+
+  ~BorealisServiceFake() override;
+
+  BorealisAppLauncher& AppLauncher() override;
+  BorealisContextManager& ContextManager() override;
+  BorealisFeatures& Features() override;
+  BorealisWindowManager& WindowManager() override;
+
+  void SetAppLauncherForTesting(BorealisAppLauncher* app_launcher);
+  void SetContextManagerForTesting(BorealisContextManager* context_manager);
+  void SetFeaturesForTesting(BorealisFeatures* features);
+  void SetWindowManagerForTesting(BorealisWindowManager* window_manager);
+
+ private:
+  BorealisAppLauncher* app_launcher_ = nullptr;
+  BorealisContextManager* context_manager_ = nullptr;
+  BorealisFeatures* features_ = nullptr;
+  BorealisWindowManager* window_manager_ = nullptr;
+};
+
+}  // namespace borealis
+
+#endif  // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_SERVICE_FAKE_H_

chrome/browser/chromeos/borealis/borealis_service_impl.cc

@@ -14,6 +14,7 @@ namespace borealis {
 BorealisServiceImpl::BorealisServiceImpl(Profile* profile)
     : profile_(profile),
       app_launcher_(profile_),
+      context_manager_(profile),
       features_(profile_),
       window_manager_(profile_) {}
 
@@ -23,6 +24,10 @@ BorealisAppLauncher& BorealisServiceImpl::AppLauncher() {
   return app_launcher_;
 }
 
+BorealisContextManager& BorealisServiceImpl::ContextManager() {
+  return context_manager_;
+}
+
 BorealisFeatures& BorealisServiceImpl::Features() {
   return features_;
 }

chrome/browser/chromeos/borealis/borealis_service_impl.h

@@ -8,6 +8,7 @@
 #include "chrome/browser/chromeos/borealis/borealis_service.h"
 
 #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_window_manager.h"
 
@@ -22,12 +23,14 @@ class BorealisServiceImpl : public BorealisService {
  private:
   // BorealisService overrides.
   BorealisAppLauncher& AppLauncher() override;
+  BorealisContextManager& ContextManager() override;
   BorealisFeatures& Features() override;
   BorealisWindowManager& WindowManager() override;
 
   Profile* const profile_;
 
   BorealisAppLauncher app_launcher_;
+  BorealisContextManagerImpl context_manager_;
   BorealisFeatures features_;
   BorealisWindowManager window_manager_;
 };

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

@@ -12,9 +12,9 @@
 #include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h"
-#include "chrome/browser/chromeos/borealis/borealis_context_manager_impl.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_service.h"
 #include "chrome/browser/chromeos/borealis/borealis_util.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
@@ -193,8 +193,9 @@ bool BorealisInstallerView::Accept() {
 
   if (state_ == State::kCompleted) {
     // Launch button has been clicked.
-    borealis::BorealisContextManagerFactory::GetForProfile(profile_)
-        ->StartBorealis(base::DoNothing());
+    borealis::BorealisService::GetForProfile(profile_)
+        ->ContextManager()
+        .StartBorealis(base::DoNothing());
     return true;
   }
 

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

@@ -7,9 +7,9 @@
 #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_context_manager_factory.h"
 #include "chrome/browser/chromeos/borealis/borealis_installer_factory.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"
 #include "chrome/browser/chromeos/borealis/borealis_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
@@ -33,11 +33,6 @@ namespace {
 
 class BorealisInstallerMock : public borealis::BorealisInstaller {
  public:
-  BorealisInstallerMock() = default;
-  ~BorealisInstallerMock() = default;
-  BorealisInstallerMock(const BorealisInstallerMock&) = delete;
-  BorealisInstallerMock& operator=(const BorealisInstallerMock&) = delete;
-
   MOCK_METHOD0(IsProcessing, bool());
   MOCK_METHOD0(Start, void());
   MOCK_METHOD0(Cancel, void());
@@ -47,12 +42,6 @@ class BorealisInstallerMock : public borealis::BorealisInstaller {
 
 class BorealisContextManagerMock : public borealis::BorealisContextManager {
  public:
-  BorealisContextManagerMock() = default;
-  ~BorealisContextManagerMock() = default;
-  BorealisContextManagerMock(const BorealisContextManagerMock&) = delete;
-  BorealisContextManagerMock& operator=(const BorealisContextManagerMock&) =
-      delete;
-
   MOCK_METHOD(void,
               StartBorealis,
               (BorealisContextManager::ResultCallback),
@@ -74,17 +63,13 @@ class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
                     browser()->profile(),
                     base::BindRepeating([](content::BrowserContext* context)
                                             -> std::unique_ptr<KeyedService> {
-                      return std::make_unique<BorealisInstallerMock>();
-                    })));
-    mock_context_manager_ =
-        static_cast<::testing::StrictMock<BorealisContextManagerMock>*>(
-            borealis::BorealisContextManagerFactory::GetInstance()
-                ->SetTestingFactoryAndUse(
-                    browser()->profile(),
-                    base::BindRepeating([](content::BrowserContext* context)
-                                            -> std::unique_ptr<KeyedService> {
-                      return std::make_unique<BorealisContextManagerMock>();
+                      return std::make_unique<
+                          testing::StrictMock<BorealisInstallerMock>>();
                     })));
+
+    BorealisServiceFake* fake_service =
+        BorealisServiceFake::UseFakeForTesting(browser()->profile());
+    fake_service->SetContextManagerForTesting(&mock_context_manager_);
   }
 
   void ShowUi(const std::string& name) override {
@@ -160,7 +145,7 @@ class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
   }
 
   ::testing::StrictMock<BorealisInstallerMock>* mock_installer_;
-  ::testing::StrictMock<BorealisContextManagerMock>* mock_context_manager_;
+  ::testing::StrictMock<BorealisContextManagerMock> mock_context_manager_;
   BorealisInstallerView* view_;
   base::string16 app_name_;
 
@@ -175,6 +160,7 @@ 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());
   ShowAndVerifyUi();
 }
 
@@ -185,7 +171,7 @@ IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest, SucessfulInstall) {
   view_->OnInstallationEnded(InstallationResult::kSuccess);
   ExpectInstallationCompletedSucessfully();
 
-  EXPECT_CALL(*mock_context_manager_, StartBorealis(_));
+  EXPECT_CALL(mock_context_manager_, StartBorealis(_));
   EXPECT_CALL(*mock_installer_, RemoveObserver(_));
   view_->AcceptDialog();
 
@@ -227,7 +213,7 @@ IN_PROC_BROWSER_TEST_F(BorealisInstallerViewBrowserTest,
   view_->OnInstallationEnded(InstallationResult::kSuccess);
   ExpectInstallationCompletedSucessfully();
 
-  EXPECT_CALL(*mock_context_manager_, StartBorealis(_));
+  EXPECT_CALL(mock_context_manager_, StartBorealis(_));
   EXPECT_CALL(*mock_installer_, RemoveObserver(_));
   view_->AcceptDialog();