printing: wait for primary profile in CupsProxyServiceManager

CupsProxyServiceManager spawns CupsProxyService, which depends on the
primary profile existing. Before spawning CupsProxyService, check to
make sure the primary profile exists. If the primary profile does not
exist, CupsProxyServiceManager will continue periodically checking for
the primary profile's existence until it reaches an attempt limit, when
it will then stop trying to spawn CupsProxyService.

Bug: crbug:1424583
Change-Id: I4baecf0387fb066175e7e55c9ae598503293bdd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4401076
Reviewed-by: Benjamin Gordon <bmgordon@chromium.org>
Commit-Queue: Paul Moy <pmoy@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1144186}

chrome/browser/ash/BUILD.gn

@@ -6261,8 +6261,12 @@ source_set("unit_tests") {
   }
 
   if (use_cups) {
-    sources += [ "printing/cups_print_job_manager_utils_unittest.cc" ]
+    sources += [
+      "printing/cups_print_job_manager_utils_unittest.cc",
+      "printing/cups_proxy_service_manager_unittest.cc",
+    ]
     deps += [
+      "//chromeos/ash/components/dbus/cups_proxy:cups_proxy",
       "//printing:printing_base",
       "//printing/backend",
     ]

chrome/browser/ash/printing/cups_proxy_service_manager.cc

@@ -6,31 +6,118 @@
 
 #include <memory>
 
+#include "base/check.h"
 #include "base/feature_list.h"
 #include "chrome/browser/ash/printing/cups_proxy_service_delegate_impl.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/services/cups_proxy/cups_proxy_service.h"
+#include "chromeos/ash/components/browser_context_helper/browser_context_helper.h"
 #include "chromeos/ash/components/dbus/cups_proxy/cups_proxy_client.h"
+#include "components/user_manager/user.h"
+#include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_context.h"
 
 namespace ash {
 
-CupsProxyServiceManager::CupsProxyServiceManager() {
-  // Don't wait for the daemon if the feature is turned off anyway.
-  if (base::FeatureList::IsEnabled(features::kPluginVm)) {
-    CupsProxyClient::Get()->WaitForServiceToBeAvailable(
-        base::BindOnce(&CupsProxyServiceManager::OnDaemonAvailable,
-                       weak_factory_.GetWeakPtr()));
+namespace {
+
+// Returns true iff the primary profile has been created.
+bool IsPrimaryProfileCreated() {
+  if (!user_manager::UserManager::IsInitialized()) {
+    return false;
+  }
+
+  const user_manager::User* primary_user =
+      user_manager::UserManager::Get()->GetPrimaryUser();
+  return primary_user && primary_user->is_profile_created();
+}
+
+}  // namespace
+
+CupsProxyServiceManager::CupsProxyServiceManager()
+    : profile_manager_(g_browser_process->profile_manager()) {
+  // Don't wait for the daemon or subscribe to ProfileManager if the feature is
+  // turned off anyway.
+  if (!base::FeatureList::IsEnabled(features::kPluginVm)) {
+    return;
+  }
+
+  // The primary profile might have been created already. If so, there's no
+  // need to subscribe to ProfileManager.
+  primary_profile_available_ = IsPrimaryProfileCreated();
+
+  if (!primary_profile_available_ && profile_manager_) {
+    profile_manager_->AddObserver(this);
   }
+
+  CupsProxyClient::Get()->WaitForServiceToBeAvailable(base::BindOnce(
+      &CupsProxyServiceManager::OnDaemonAvailable, weak_factory_.GetWeakPtr()));
 }
 
-CupsProxyServiceManager::~CupsProxyServiceManager() = default;
+CupsProxyServiceManager::~CupsProxyServiceManager() {
+  if (profile_manager_) {
+    profile_manager_->RemoveObserver(this);
+  }
+
+  if (cups_proxy::CupsProxyService::GetInstance() != nullptr) {
+    cups_proxy::CupsProxyService::Shutdown();
+  }
+}
 
 void CupsProxyServiceManager::OnDaemonAvailable(bool daemon_available) {
   if (!daemon_available) {
     DVLOG(1) << "CupsProxyDaemon startup error";
     return;
   }
 
+  daemon_available_ = true;
+
+  MaybeSpawnCupsProxyService();
+}
+
+void CupsProxyServiceManager::OnProfileAdded(Profile* profile) {
+  if (!profile) {
+    return;
+  }
+
+  BrowserContextHelper* browser_context_helper = BrowserContextHelper::Get();
+  if (!browser_context_helper) {
+    return;
+  }
+
+  const user_manager::User* user =
+      browser_context_helper->GetUserByBrowserContext(profile);
+  if (!user) {
+    return;
+  }
+
+  DCHECK(user_manager::UserManager::IsInitialized());
+  if (!user_manager::UserManager::Get()->IsPrimaryUser(user)) {
+    return;
+  }
+
+  // Now that we've seen the primary profile, there's no need to keep our
+  // subscription to ProfileManager.
+  profile_manager_->RemoveObserver(this);
+  profile_manager_ = nullptr;
+
+  primary_profile_available_ = true;
+
+  MaybeSpawnCupsProxyService();
+}
+
+void CupsProxyServiceManager::OnProfileManagerDestroying() {
+  if (profile_manager_) {
+    profile_manager_->RemoveObserver(this);
+    profile_manager_ = nullptr;
+  }
+}
+
+void CupsProxyServiceManager::MaybeSpawnCupsProxyService() {
+  if (!primary_profile_available_ || !daemon_available_) {
+    return;
+  }
+
   // Attempt to start the service, which will then bootstrap a connection
   // with the daemon.
   cups_proxy::CupsProxyService::Spawn(

chrome/browser/ash/printing/cups_proxy_service_manager.h

@@ -6,6 +6,9 @@
 #define CHROME_BROWSER_ASH_PRINTING_CUPS_PROXY_SERVICE_MANAGER_H_
 
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
 #include "chrome/services/cups_proxy/cups_proxy_service.h"
 #include "components/keyed_service/core/keyed_service.h"
 
@@ -19,7 +22,8 @@ namespace ash {
 //
 // Note: This manager is not fault-tolerant, i.e. should the service/daemon
 // fail, we do not try to restart.
-class CupsProxyServiceManager : public KeyedService {
+class CupsProxyServiceManager : public KeyedService,
+                                public ProfileManagerObserver {
  public:
   CupsProxyServiceManager();
 
@@ -28,9 +32,24 @@ class CupsProxyServiceManager : public KeyedService {
 
   ~CupsProxyServiceManager() override;
 
+  // ProfileManagerObserver overrides:
+  void OnProfileAdded(Profile* profile) override;
+  void OnProfileManagerDestroying() override;
+
  private:
   void OnDaemonAvailable(bool daemon_available);
 
+  // Spawns CupsProxyService iff the primary profile is available and the
+  // CupsProxyDaemon is available.
+  void MaybeSpawnCupsProxyService();
+
+  // Whether or not the CupsProxyDaemon is available.
+  bool daemon_available_ = false;
+  // Whether or not the primary profile is available.
+  bool primary_profile_available_ = false;
+
+  ProfileManager* profile_manager_ = nullptr;
+
   base::WeakPtrFactory<CupsProxyServiceManager> weak_factory_{this};
 };
 

chrome/browser/ash/printing/cups_proxy_service_manager_unittest.cc

@@ -0,0 +1,123 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ash/printing/cups_proxy_service_manager.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/test/scoped_feature_list.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/services/cups_proxy/cups_proxy_service.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "chromeos/ash/components/dbus/cups_proxy/cups_proxy_client.h"
+#include "components/account_id/account_id.h"
+#include "components/user_manager/fake_user_manager.h"
+#include "components/user_manager/scoped_user_manager.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ash {
+
+namespace {
+
+constexpr char kProfileName[] = "user@example.com";
+
+}  // namespace
+
+class CupsProxyServiceManagerTest : public testing::Test {
+ protected:
+  CupsProxyServiceManagerTest()
+      : testing_profile_manager_(TestingBrowserProcess::GetGlobal()) {}
+  CupsProxyServiceManagerTest(const CupsProxyServiceManagerTest&) = delete;
+  CupsProxyServiceManagerTest& operator=(const CupsProxyServiceManagerTest&) =
+      delete;
+  ~CupsProxyServiceManagerTest() override = default;
+
+  void SetUp() override {
+    ASSERT_TRUE(testing_profile_manager_.SetUp());
+
+    auto fake_user_manager = std::make_unique<user_manager::FakeUserManager>();
+    fake_user_manager_ = fake_user_manager.get();
+    scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
+        std::move(fake_user_manager));
+
+    CupsProxyClient::InitializeFake();
+  }
+
+  void TearDown() override { CupsProxyClient::Shutdown(); }
+
+  void CreatePrimaryProfile() {
+    AccountId account_id = AccountId::FromUserEmail(kProfileName);
+    fake_user_manager_->AddUser(account_id);
+    user_manager::UserManager::Get()->UserLoggedIn(
+        account_id,
+        user_manager::FakeUserManager::GetFakeUsernameHash(account_id),
+        /*browser_restart=*/false,
+        /*is_child=*/false);
+    testing_profile_manager_.CreateTestingProfile(kProfileName,
+                                                  /*is_main_profile=*/true);
+  }
+
+  content::BrowserTaskEnvironment* task_environment() {
+    return &task_environment_;
+  }
+
+  base::test::ScopedFeatureList* scoped_feature_list() {
+    return &scoped_feature_list_;
+  }
+
+  user_manager::FakeUserManager* fake_user_manager() {
+    return fake_user_manager_;
+  }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+  base::test::ScopedFeatureList scoped_feature_list_;
+  TestingProfileManager testing_profile_manager_;
+  // Owned by `scoped_user_manager_`.
+  user_manager::FakeUserManager* fake_user_manager_ = nullptr;
+  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
+};
+
+TEST_F(CupsProxyServiceManagerTest, FeatureNotEnabled) {
+  scoped_feature_list()->InitAndDisableFeature(features::kPluginVm);
+
+  CupsProxyServiceManager manager;
+
+  EXPECT_EQ(nullptr, cups_proxy::CupsProxyService::GetInstance());
+}
+
+TEST_F(CupsProxyServiceManagerTest, PrimaryProfileAlreadyCreated) {
+  scoped_feature_list()->InitAndEnableFeature(features::kPluginVm);
+  CreatePrimaryProfile();
+
+  CupsProxyServiceManager manager;
+
+  task_environment()->RunUntilIdle();
+
+  EXPECT_NE(nullptr, cups_proxy::CupsProxyService::GetInstance());
+}
+
+TEST_F(CupsProxyServiceManagerTest, PrimaryProfileCreatedLater) {
+  scoped_feature_list()->InitAndEnableFeature(features::kPluginVm);
+
+  // Before the primary profile has been created, we don't expect
+  // CupsProxyService to have been spawned.
+  CupsProxyServiceManager manager;
+
+  task_environment()->RunUntilIdle();
+
+  EXPECT_EQ(nullptr, cups_proxy::CupsProxyService::GetInstance());
+
+  CreatePrimaryProfile();
+
+  task_environment()->RunUntilIdle();
+
+  EXPECT_NE(nullptr, cups_proxy::CupsProxyService::GetInstance());
+}
+
+}  // namespace ash

chrome/services/cups_proxy/cups_proxy_service.cc

@@ -8,7 +8,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/no_destructor.h"
 #include "chrome/services/cups_proxy/cups_proxy_service_delegate.h"
 #include "chrome/services/cups_proxy/proxy_manager.h"
 #include "chromeos/ash/components/dbus/cups_proxy/cups_proxy_client.h"
@@ -18,16 +17,15 @@
 
 namespace cups_proxy {
 
+namespace {
+
+CupsProxyService* g_instance = nullptr;
+
+}  // namespace
+
 CupsProxyService::CupsProxyService() = default;
 CupsProxyService::~CupsProxyService() = default;
 
-// static
-void CupsProxyService::Spawn(
-    std::unique_ptr<CupsProxyServiceDelegate> delegate) {
-  static base::NoDestructor<CupsProxyService> service;
-  service->BindToCupsProxyDaemon(std::move(delegate));
-}
-
 void CupsProxyService::BindToCupsProxyDaemon(
     std::unique_ptr<CupsProxyServiceDelegate> delegate) {
   DCHECK(delegate);
@@ -71,4 +69,24 @@ void CupsProxyService::OnBindToCupsProxyDaemon(bool success) {
   DVLOG(1) << "CupsProxyService: bootstrap success!";
 }
 
+// static
+void CupsProxyService::Spawn(
+    std::unique_ptr<CupsProxyServiceDelegate> delegate) {
+  DCHECK(!g_instance);
+  g_instance = new CupsProxyService();
+  g_instance->BindToCupsProxyDaemon(std::move(delegate));
+}
+
+// static
+CupsProxyService* CupsProxyService::GetInstance() {
+  return g_instance;
+}
+
+// static
+void CupsProxyService::Shutdown() {
+  DCHECK(g_instance);
+  delete g_instance;
+  g_instance = nullptr;
+}
+
 }  // namespace cups_proxy

chrome/services/cups_proxy/cups_proxy_service.h

@@ -20,10 +20,6 @@ class ProxyManager;
 // This service lives in the browser process and is managed by the
 // CupsProxyServiceManager. It bootstraps/maintains a mojom connection with the
 // CupsProxyDaemon.
-//
-// Note: There is no method granting a service handle since beyond creation,
-// this service's only client is the daemon, who's connection is managed
-// internally.
 class CupsProxyService {
  public:
   CupsProxyService(const CupsProxyService&) = delete;
@@ -32,8 +28,13 @@ class CupsProxyService {
   // Spawns the global service instance.
   static void Spawn(std::unique_ptr<CupsProxyServiceDelegate> delegate);
 
+  // Gets the global service instance. May be null.
+  static CupsProxyService* GetInstance();
+
+  // Destroys the global service instance.
+  static void Shutdown();
+
  private:
-  friend base::NoDestructor<CupsProxyService>;
   CupsProxyService();
   ~CupsProxyService();