Refactoring of KerberosCredentialsManager initialization.

Added KerberosCredentialsManagerFactory and promoted
KerberosCredentialsManager to KeyedService to provide a way
to control initialization order with other keyed services
that may depend on it.

Bug: 1027087
Change-Id: I157c5aba8e1efc604ee4978d382469650a62a1dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928790
Commit-Queue: Tomasz Dobrowolski <tomdobro@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Auto-Submit: Tomasz Dobrowolski <tomdobro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720202}

chrome/browser/browser_process_platform_part_chromeos.cc

@@ -13,7 +13,6 @@
 #include "base/time/default_tick_clock.h"
 #include "base/time/tick_clock.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h"
 #include "chrome/browser/chromeos/login/saml/in_session_password_change_manager.h"
 #include "chrome/browser/chromeos/login/session/chrome_session_manager.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager_impl.h"
@@ -153,11 +152,6 @@ void BrowserProcessPlatformPart::InitializePrimaryProfileServices(
     Profile* primary_profile) {
   DCHECK(primary_profile);
 
-  DCHECK(!kerberos_credentials_manager_);
-  kerberos_credentials_manager_ =
-      std::make_unique<chromeos::KerberosCredentialsManager>(
-          g_browser_process->local_state(), primary_profile);
-
   DCHECK(!in_session_password_change_manager_);
   in_session_password_change_manager_ =
       chromeos::InSessionPasswordChangeManager::CreateIfEnabled(
@@ -172,7 +166,6 @@ void BrowserProcessPlatformPart::InitializePrimaryProfileServices(
 }
 
 void BrowserProcessPlatformPart::ShutdownPrimaryProfileServices() {
-  kerberos_credentials_manager_.reset();
   in_session_password_change_manager_.reset();
 }
 

chrome/browser/browser_process_platform_part_chromeos.h

@@ -21,7 +21,6 @@ namespace chromeos {
 class AccountManagerFactory;
 class ChromeSessionManager;
 class ChromeUserManager;
-class KerberosCredentialsManager;
 class InSessionPasswordChangeManager;
 class ProfileHelper;
 class TimeZoneResolver;
@@ -167,9 +166,6 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
 
   std::unique_ptr<chromeos::AccountManagerFactory> account_manager_factory_;
 
-  std::unique_ptr<chromeos::KerberosCredentialsManager>
-      kerberos_credentials_manager_;
-
   std::unique_ptr<chromeos::InSessionPasswordChangeManager>
       in_session_password_change_manager_;
 

chrome/browser/chromeos/BUILD.gn

@@ -1201,6 +1201,8 @@ source_set("chromeos") {
     "input_method/native_input_method_engine.h",
     "kerberos/kerberos_credentials_manager.cc",
     "kerberos/kerberos_credentials_manager.h",
+    "kerberos/kerberos_credentials_manager_factory.cc",
+    "kerberos/kerberos_credentials_manager_factory.h",
     "kerberos/kerberos_ticket_expiry_notification.cc",
     "kerberos/kerberos_ticket_expiry_notification.h",
     "language_preferences.cc",

chrome/browser/chromeos/browser_context_keyed_service_factories.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/chromeos/extensions/media_player_api.h"
 #include "chrome/browser/chromeos/extensions/printing/printing_api_handler.h"
 #include "chrome/browser/chromeos/extensions/printing_metrics/print_job_finished_event_dispatcher.h"
+#include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager_factory.h"
 #include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_factory.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_engagement_metrics_service.h"
@@ -63,6 +64,7 @@ void EnsureBrowserContextKeyedServiceFactoriesBuilt() {
   extensions::VirtualKeyboardAPI::GetFactoryInstance();
   extensions::WebcamPrivateAPI::GetFactoryInstance();
   file_manager::EventRouterFactory::GetInstance();
+  KerberosCredentialsManagerFactory::GetInstance();
   OwnerSettingsServiceChromeOSFactory::GetInstance();
   plugin_vm::PluginVmEngagementMetricsService::Factory::GetInstance();
   policy::PolicyCertServiceFactory::GetInstance();

chrome/browser/chromeos/kerberos/kerberos_credentials_manager.cc

@@ -36,8 +36,6 @@ namespace chromeos {
 
 namespace {
 
-KerberosCredentialsManager* g_instance = nullptr;
-
 // Account keys for the kerberos.accounts pref.
 constexpr char kPrincipal[] = "principal";
 constexpr char kPassword[] = "password";
@@ -281,9 +279,6 @@ KerberosCredentialsManager::KerberosCredentialsManager(PrefService* local_state,
       kerberos_files_handler_(std::make_unique<KerberosFilesHandler>(
           base::BindRepeating(&KerberosCredentialsManager::GetKerberosFiles,
                               base::Unretained(this)))) {
-  DCHECK(!g_instance);
-  g_instance = this;
-
   DCHECK(primary_profile_);
   const user_manager::User* primary_user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(primary_profile);
@@ -355,14 +350,6 @@ KerberosCredentialsManager::KerberosCredentialsManager(PrefService* local_state,
 
 KerberosCredentialsManager::~KerberosCredentialsManager() {
   policy_service_->RemoveObserver(policy::POLICY_DOMAIN_CHROME, this);
-  DCHECK(g_instance);
-  g_instance = nullptr;
-}
-
-// static
-KerberosCredentialsManager& KerberosCredentialsManager::Get() {
-  DCHECK(g_instance);
-  return *g_instance;
 }
 
 // static

chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h

@@ -15,6 +15,7 @@
 #include "base/optional.h"
 #include "chrome/browser/chromeos/authpolicy/kerberos_files_handler.h"
 #include "chromeos/dbus/kerberos/kerberos_service.pb.h"
+#include "components/keyed_service/core/keyed_service.h"
 #include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/policy_service.h"
 
@@ -32,7 +33,8 @@ namespace chromeos {
 class KerberosAddAccountRunner;
 class VariableExpander;
 
-class KerberosCredentialsManager : public policy::PolicyService::Observer {
+class KerberosCredentialsManager : public KeyedService,
+                                   public policy::PolicyService::Observer {
  public:
   using ResultCallback = base::OnceCallback<void(kerberos::ErrorType)>;
   using ListAccountsCallback =
@@ -57,10 +59,6 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
                              Profile* primary_profile);
   ~KerberosCredentialsManager() override;
 
-  // Singleton accessor. Available once the primary profile is available.
-  // DCHECKs if the instance has not been created yet.
-  static KerberosCredentialsManager& Get();
-
   // Registers prefs stored in local state.
   static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
 

chrome/browser/chromeos/kerberos/kerberos_credentials_manager_factory.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/chromeos/kerberos/kerberos_credentials_manager_factory.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/memory/singleton.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+
+namespace chromeos {
+
+namespace {
+
+Profile* GetPrimaryProfileFromContext(content::BrowserContext* context) {
+  if (!user_manager::UserManager::IsInitialized())
+    return nullptr;
+  // Get original profile, so it gets primary profile faster if context is
+  // incognito profile.
+  Profile* profile = Profile::FromBrowserContext(context)->GetOriginalProfile();
+  if (!ProfileHelper::IsPrimaryProfile(profile)) {
+    const auto* primary_user =
+        user_manager::UserManager::Get()->GetPrimaryUser();
+    if (!primary_user)
+      return nullptr;
+    // Get primary profile from primary user. Note that it only gets primary
+    // profile if it is fully created.
+    profile = chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
+  }
+  return profile;
+}
+
+}  // namespace
+
+// static
+KerberosCredentialsManager* KerberosCredentialsManagerFactory::GetExisting(
+    content::BrowserContext* context) {
+  Profile* const primary_profile = GetPrimaryProfileFromContext(context);
+  if (!primary_profile)
+    return nullptr;
+  return static_cast<KerberosCredentialsManager*>(
+      GetInstance()->GetServiceForBrowserContext(primary_profile, false));
+}
+
+// static
+KerberosCredentialsManager* KerberosCredentialsManagerFactory::Get(
+    content::BrowserContext* context) {
+  Profile* const primary_profile = GetPrimaryProfileFromContext(context);
+  if (!primary_profile)
+    return nullptr;
+  return static_cast<KerberosCredentialsManager*>(
+      GetInstance()->GetServiceForBrowserContext(primary_profile, true));
+}
+
+// static
+KerberosCredentialsManagerFactory*
+KerberosCredentialsManagerFactory::GetInstance() {
+  return base::Singleton<KerberosCredentialsManagerFactory>::get();
+}
+
+KerberosCredentialsManagerFactory::KerberosCredentialsManagerFactory()
+    : BrowserContextKeyedServiceFactory(
+          "KerberosCredentialsManager",
+          BrowserContextDependencyManager::GetInstance()),
+      service_instance_created_(false) {}
+
+KerberosCredentialsManagerFactory::~KerberosCredentialsManagerFactory() =
+    default;
+
+bool KerberosCredentialsManagerFactory::ServiceIsCreatedWithBrowserContext()
+    const {
+  return true;
+}
+
+KeyedService* KerberosCredentialsManagerFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* const profile = Profile::FromBrowserContext(context);
+
+  // Verify that UserManager is initialized before calling IsPrimaryProfile.
+  if (!user_manager::UserManager::IsInitialized())
+    return nullptr;
+
+  // Verify that we create instance for a primary profile.
+  if (!ProfileHelper::IsPrimaryProfile(profile))
+    return nullptr;
+
+  // Verify that this is not a testing profile.
+  if (profile->AsTestingProfile())
+    return nullptr;
+
+  // Make sure one and only one instance is ever created.
+  if (service_instance_created_)
+    return nullptr;
+  service_instance_created_ = true;
+
+  PrefService* local_state = g_browser_process->local_state();
+  return new KerberosCredentialsManager(local_state, profile);
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/kerberos/kerberos_credentials_manager_factory.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_CHROMEOS_KERBEROS_KERBEROS_CREDENTIALS_MANAGER_FACTORY_H_
+#define CHROME_BROWSER_CHROMEOS_KERBEROS_KERBEROS_CREDENTIALS_MANAGER_FACTORY_H_
+
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;
+}  // namespace base
+
+namespace chromeos {
+
+class KerberosCredentialsManager;
+
+// Singleton that creates and owns one KerberosCredentialsManager instance
+// associated with primary profile. Note that KerberosCredentialsManager holds
+// non-owning pointer to primary profile, so its life-time depends on the
+// life-time of the primary profile.
+class KerberosCredentialsManagerFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  // Get existing service instance associated with the primary profile.
+  // Note that the interface still expects the context in case primary profile
+  // creation is not finalized. It returns nullptr if primary profile doesn't
+  // exist or primary profile has changed.
+  static KerberosCredentialsManager* GetExisting(
+      content::BrowserContext* context);
+
+  // Get existing service instance or create a service instance associated with
+  // the primary profile.
+  // Note that the interface still expects the context in case primary profile
+  // creation is not finalized. It returns nullptr if primary profile doesn't
+  // exist or primary profile has changed.
+  static KerberosCredentialsManager* Get(content::BrowserContext* context);
+
+  static KerberosCredentialsManagerFactory* GetInstance();
+
+ private:
+  friend struct base::DefaultSingletonTraits<KerberosCredentialsManagerFactory>;
+
+  KerberosCredentialsManagerFactory();
+  ~KerberosCredentialsManagerFactory() override;
+
+  // Not copyable.
+  KerberosCredentialsManagerFactory(const KerberosCredentialsManagerFactory&) =
+      delete;
+  KerberosCredentialsManagerFactory& operator=(
+      const KerberosCredentialsManagerFactory&) = delete;
+
+  bool ServiceIsCreatedWithBrowserContext() const override;
+
+  // Returns nullptr in case context is not a primary profile. Otherwise returns
+  // valid KerberosCredentialsManager.
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+
+  // This is workaround to make sure we create only one service (singleton) and
+  // prevent errors when two primary profiles are present (which normally
+  // shouldn't happen, except in tests).
+  // Additional reason to keep this workaround for now is that
+  // KerberosCredentialsManager cannot be restarted at the moment, because it's
+  // tightly coupled with KerberosClient singleton.
+  // Note that it is potential risk for multi-threaded initialization (which is
+  // not supported at the moment).
+  mutable bool service_instance_created_;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_KERBEROS_KERBEROS_CREDENTIALS_MANAGER_FACTORY_H_

chrome/browser/chromeos/kerberos/kerberos_credentials_manager_factory_browsertest.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/chromeos/kerberos/kerberos_credentials_manager_factory.h"
+
+#include <memory>
+
+#include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/constants/chromeos_switches.h"
+
+namespace chromeos {
+
+class KerberosCredentialsManagerFactoryBrowserTest
+    : public InProcessBrowserTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(
+        chromeos::switches::kIgnoreUserProfileMappingForTests);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(KerberosCredentialsManagerFactoryBrowserTest,
+                       GetServiceForPrimaryProfile) {
+  Profile* const profile = browser()->profile();
+  ASSERT_TRUE(ProfileHelper::IsPrimaryProfile(profile));
+
+  KerberosCredentialsManager* manager =
+      KerberosCredentialsManagerFactory::GetExisting(profile);
+  ASSERT_TRUE(manager);
+}
+
+IN_PROC_BROWSER_TEST_F(KerberosCredentialsManagerFactoryBrowserTest,
+                       GetServiceForIncognitoProfile) {
+  Profile* const profile = browser()->profile();
+  Browser* incognito_browser = CreateIncognitoBrowser(profile);
+  ASSERT_TRUE(incognito_browser);
+
+  Profile* incognito_profile = incognito_browser->profile();
+  ASSERT_NE(incognito_profile, profile);
+  ASSERT_EQ(incognito_profile->GetOriginalProfile(), profile);
+
+  // Verify, that Get is not creating a new instance for incognito profile.
+  KerberosCredentialsManager* manager =
+      KerberosCredentialsManagerFactory::GetExisting(profile);
+  ASSERT_TRUE(manager);
+  ASSERT_EQ(KerberosCredentialsManagerFactory::Get(incognito_profile), manager);
+
+  CloseBrowserSynchronously(incognito_browser);
+}
+
+IN_PROC_BROWSER_TEST_F(KerberosCredentialsManagerFactoryBrowserTest,
+                       GetServiceForOtherProfile) {
+  Profile* const profile = browser()->profile();
+  ASSERT_TRUE(ProfileHelper::IsPrimaryProfile(profile));
+
+  Profile* const other_profile = ProfileHelper::GetSigninProfile();
+  ASSERT_NE(other_profile, profile);
+  ASSERT_NE(other_profile->GetOriginalProfile(), profile);
+  ASSERT_TRUE(!ProfileHelper::IsPrimaryProfile(other_profile));
+
+  // Verify, that Get is not creating a new instance for other (non-primary)
+  // profile.
+  KerberosCredentialsManager* manager =
+      KerberosCredentialsManagerFactory::GetExisting(profile);
+  ASSERT_TRUE(manager);
+  ASSERT_EQ(KerberosCredentialsManagerFactory::Get(other_profile), manager);
+}
+
+}  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/kerberos_accounts_handler.cc

@@ -7,10 +7,13 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h"
+#include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager_factory.h"
+#include "chrome/browser/profiles/profile.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/l10n/time_format.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -21,9 +24,26 @@
 namespace chromeos {
 namespace settings {
 
-KerberosAccountsHandler::KerberosAccountsHandler() = default;
+// static
+std::unique_ptr<KerberosAccountsHandler>
+KerberosAccountsHandler::CreateIfKerberosEnabled(Profile* profile) {
+  KerberosCredentialsManager* kerberos_credentials_manager =
+      KerberosCredentialsManagerFactory::GetExisting(profile);
+  if (!kerberos_credentials_manager ||
+      !kerberos_credentials_manager->IsKerberosEnabled())
+    return nullptr;
+  return base::WrapUnique(
+      new KerberosAccountsHandler(kerberos_credentials_manager));
+}
+
 KerberosAccountsHandler::~KerberosAccountsHandler() = default;
 
+KerberosAccountsHandler::KerberosAccountsHandler(
+    KerberosCredentialsManager* kerberos_credentials_manager)
+    : kerberos_credentials_manager_(kerberos_credentials_manager) {
+  DCHECK(kerberos_credentials_manager_);
+}
+
 void KerberosAccountsHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "getKerberosAccounts",
@@ -56,12 +76,12 @@ void KerberosAccountsHandler::HandleGetKerberosAccounts(
   CHECK_EQ(1U, args->GetSize());
   const std::string& callback_id = args->GetList()[0].GetString();
 
-  if (!KerberosCredentialsManager::Get().IsKerberosEnabled()) {
+  if (!kerberos_credentials_manager_->IsKerberosEnabled()) {
     ResolveJavascriptCallback(base::Value(callback_id), base::Value());
     return;
   }
 
-  KerberosCredentialsManager::Get().ListAccounts(
+  kerberos_credentials_manager_->ListAccounts(
       base::BindOnce(&KerberosAccountsHandler::OnListAccounts,
                      weak_factory_.GetWeakPtr(), callback_id));
 }
@@ -79,7 +99,7 @@ void KerberosAccountsHandler::OnListAccounts(
       skia_ticket_icon.GetRepresentation(1.0f).GetBitmap());
 
   const std::string& active_principal =
-      KerberosCredentialsManager::Get().GetActiveAccount();
+      kerberos_credentials_manager_->GetActiveAccount();
 
   for (int n = 0; n < response.accounts_size(); ++n) {
     const kerberos::Account& account = response.accounts(n);
@@ -125,13 +145,13 @@ void KerberosAccountsHandler::HandleAddKerberosAccount(
   const std::string& config = args->GetList()[4].GetString();
   const bool allow_existing = args->GetList()[5].GetBool();
 
-  if (!KerberosCredentialsManager::Get().IsKerberosEnabled()) {
+  if (!kerberos_credentials_manager_->IsKerberosEnabled()) {
     ResolveJavascriptCallback(base::Value(callback_id),
                               base::Value(kerberos::ERROR_KERBEROS_DISABLED));
     return;
   }
 
-  KerberosCredentialsManager::Get().AddAccountAndAuthenticate(
+  kerberos_credentials_manager_->AddAccountAndAuthenticate(
       principal_name, false /* is_managed */, password, remember_password,
       config, allow_existing,
       base::BindOnce(&KerberosAccountsHandler::OnAddAccountAndAuthenticate,
@@ -153,13 +173,13 @@ void KerberosAccountsHandler::HandleRemoveKerberosAccount(
   const std::string& callback_id = args->GetList()[0].GetString();
   const std::string& principal_name = args->GetList()[1].GetString();
 
-  if (!KerberosCredentialsManager::Get().IsKerberosEnabled()) {
+  if (!kerberos_credentials_manager_->IsKerberosEnabled()) {
     ResolveJavascriptCallback(base::Value(callback_id),
                               base::Value(kerberos::ERROR_KERBEROS_DISABLED));
     return;
   }
 
-  KerberosCredentialsManager::Get().RemoveAccount(
+  kerberos_credentials_manager_->RemoveAccount(
       principal_name, base::BindOnce(&KerberosAccountsHandler::OnRemoveAccount,
                                      weak_factory_.GetWeakPtr(), callback_id));
 }
@@ -178,13 +198,13 @@ void KerberosAccountsHandler::HandleValidateKerberosConfig(
   const std::string& callback_id = args->GetList()[0].GetString();
   const std::string& krb5conf = args->GetList()[1].GetString();
 
-  if (!KerberosCredentialsManager::Get().IsKerberosEnabled()) {
+  if (!kerberos_credentials_manager_->IsKerberosEnabled()) {
     ResolveJavascriptCallback(base::Value(callback_id),
                               base::Value(kerberos::ERROR_KERBEROS_DISABLED));
     return;
   }
 
-  KerberosCredentialsManager::Get().ValidateConfig(
+  kerberos_credentials_manager_->ValidateConfig(
       krb5conf, base::BindOnce(&KerberosAccountsHandler::OnValidateConfig,
                                weak_factory_.GetWeakPtr(), callback_id));
 }
@@ -212,11 +232,11 @@ void KerberosAccountsHandler::HandleSetAsActiveKerberosAccount(
   CHECK_EQ(1U, args->GetSize());
   const std::string& principal_name = args->GetList()[0].GetString();
 
-  KerberosCredentialsManager::Get().SetActiveAccount(principal_name);
+  kerberos_credentials_manager_->SetActiveAccount(principal_name);
 }
 
 void KerberosAccountsHandler::OnJavascriptAllowed() {
-  credentials_manager_observer_.Add(&KerberosCredentialsManager::Get());
+  credentials_manager_observer_.Add(kerberos_credentials_manager_);
 }
 
 void KerberosAccountsHandler::OnJavascriptDisallowed() {

chrome/browser/ui/webui/settings/chromeos/kerberos_accounts_handler.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_KERBEROS_ACCOUNTS_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_KERBEROS_ACCOUNTS_HANDLER_H_
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -20,13 +21,17 @@ namespace kerberos {
 class ListAccountsResponse;
 }
 
+class Profile;
+
 namespace chromeos {
 namespace settings {
 
 class KerberosAccountsHandler : public ::settings::SettingsPageUIHandler,
                                 public KerberosCredentialsManager::Observer {
  public:
-  KerberosAccountsHandler();
+  static std::unique_ptr<KerberosAccountsHandler> CreateIfKerberosEnabled(
+      Profile* profile);
+
   ~KerberosAccountsHandler() override;
 
   // WebUIMessageHandler:
@@ -38,6 +43,9 @@ class KerberosAccountsHandler : public ::settings::SettingsPageUIHandler,
   void OnAccountsChanged() override;
 
  private:
+  explicit KerberosAccountsHandler(
+      KerberosCredentialsManager* kerberos_credentials_manager);
+
   // WebUI "getKerberosAccounts" message callback.
   void HandleGetKerberosAccounts(const base::ListValue* args);
 
@@ -79,6 +87,9 @@ class KerberosAccountsHandler : public ::settings::SettingsPageUIHandler,
                  KerberosCredentialsManager::Observer>
       credentials_manager_observer_{this};
 
+  // Not owned.
+  KerberosCredentialsManager* kerberos_credentials_manager_;
+
   base::WeakPtrFactory<KerberosAccountsHandler> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(KerberosAccountsHandler);

chrome/browser/ui/webui/settings/settings_ui.cc

@@ -388,11 +388,16 @@ void SettingsUI::InitOSWebUIHandlers(Profile* profile,
       std::make_unique<chromeos::settings::FingerprintHandler>(profile));
   web_ui->AddMessageHandler(
       std::make_unique<chromeos::settings::GoogleAssistantHandler>(profile));
-  if (g_browser_process->local_state()->GetBoolean(prefs::kKerberosEnabled)) {
-    // Note that UI is also dependent on this pref.
-    web_ui->AddMessageHandler(
-        std::make_unique<chromeos::settings::KerberosAccountsHandler>());
+
+  std::unique_ptr<chromeos::settings::KerberosAccountsHandler>
+      kerberos_accounts_handler =
+          chromeos::settings::KerberosAccountsHandler::CreateIfKerberosEnabled(
+              profile);
+  if (kerberos_accounts_handler) {
+    // Note that the UI is enabled only if Kerberos is enabled.
+    web_ui->AddMessageHandler(std::move(kerberos_accounts_handler));
   }
+
   web_ui->AddMessageHandler(
       std::make_unique<chromeos::settings::KeyboardHandler>());
 

chrome/test/BUILD.gn

@@ -2140,6 +2140,7 @@ if (!is_android) {
         "../browser/chromeos/input_method/textinput_surroundingtext_browsertest.cc",
         "../browser/chromeos/input_method/textinput_test_helper.cc",
         "../browser/chromeos/input_method/textinput_test_helper.h",
+        "../browser/chromeos/kerberos/kerberos_credentials_manager_factory_browsertest.cc",
         "../browser/chromeos/lock_screen_apps/note_taking_browsertest.cc",
         "../browser/chromeos/logging_browsertest.cc",
         "../browser/chromeos/login/accessibility_browsertest.cc",