Replace c/b/nss_context by a KeyedService.

chrome/browser/net/cert_database_service_factory.cc

+// Copyright 2014 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/net/cert_database_service_factory.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/singleton.h"
+#include "chrome/browser/profiles/incognito_helpers.h"
+#include "components/cert_database/public/cert_database_service.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "content/public/browser/browser_thread.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part.h"
+#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chromeos/cert_loader.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/tpm_token_loader.h"
+#include "components/cert_database/public/chromeos/cert_database_service_io_part_chromeos.h"
+#include "components/policy/core/common/cloud/cloud_policy_constants.h"
+#include "components/user_manager/user.h"
+#include "components/user_manager/user_manager.h"
+#else
+#include "components/cert_database/public/linux/cert_database_service_io_part_linux.h"
+#endif
+
+namespace cert_database {

[mattm, 2014/10/18 00:45:15]

Is it normal to use the same namespace in chrome/ and in the components/ dir?

[pneubeck (no reviews), 2014/10/21 09:22:09]

Not sure, I find examples for both, even for the same component.
E.g.
  chrome/browser/history/history_service.h
  chrome/browser/history/history_database.h

Bookmarks also reuses the namespace
  chrome/browser/ui/cocoa/bookmarks/bookmark_bar_constants.h

If we had a chrome namespace, is would prefer chrome::cert_database.
Since we don't, I think the current choice is reasonable.

+
+namespace {
+
+#if defined(OS_CHROMEOS)
+void WrapReadyCallbackForUI(

[mattm, 2014/10/18 00:45:16]

Maybe "RunReadyCallbackOnIOThread" ?

[pneubeck (no reviews), 2014/10/21 09:22:09]

Done.

+    const CertDatabaseServiceIOPartChromeOS::SystemTPMTokenReadyCallback&
+        callback,
+    bool system_tpm_token_enabled) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(callback, system_tpm_token_enabled));
+}
+#endif
+
+}  // namespace
+
+// static
+CertDatabaseService* CertDatabaseServiceFactory::GetForBrowserContext(
+    content::BrowserContext* context) {
+  return static_cast<CertDatabaseService*>(
+      GetInstance()->GetServiceForBrowserContext(context, true));
+}
+
+// static
+CertDatabaseServiceFactory* CertDatabaseServiceFactory::GetInstance() {
+  return Singleton<CertDatabaseServiceFactory>::get();
+}
+
+CertDatabaseServiceFactory::CertDatabaseServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+          "CertDatabaseService",
+          BrowserContextDependencyManager::GetInstance()) {
+}
+
+CertDatabaseServiceFactory::~CertDatabaseServiceFactory() {
+}
+
+content::BrowserContext* CertDatabaseServiceFactory::GetBrowserContextToUse(
+    content::BrowserContext* context) const {
+  // TODO(pneubeck): Once CertLoader is not a separate singleton anymore, we can
+  // create the CertDatabaseService on demand.

[mattm, 2014/10/18 00:45:16]

Is this comment in the right place?

[pneubeck (no reviews), 2014/10/21 09:22:09]

Done.

+  return chrome::GetBrowserContextRedirectedInIncognito(context);
+}
+
+bool CertDatabaseServiceFactory::ServiceIsCreatedWithBrowserContext() const {
+  return true;
+}
+
+KeyedService* CertDatabaseServiceFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+#if defined(OS_CHROMEOS)

[mattm, 2014/10/18 00:45:16]

Can this be changed to use platform specific files for
CertDatabaseServiceFactory::BuildServiceInstanceFor instead of ifdefs?

+  Profile* profile = static_cast<Profile*>(context);

[mattm, 2014/10/18 00:45:15]

Profile::FromBrowserContext(context)

[pneubeck (no reviews), 2014/10/21 09:22:09]

Done.

+
+  // No cert database for the sign-in profile.
+  if (chromeos::ProfileHelper::IsSigninProfile(profile))
+    return NULL;
+
+  user_manager::User* user =
+      chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
+
+  // Use the device-wide system key slot only if the user is of the same
+  // domain as the device is registered to.
+  policy::BrowserPolicyConnectorChromeOS* connector =
+      g_browser_process->platform_part()->browser_policy_connector_chromeos();
+  bool use_system_key_slot = connector->GetUserAffiliation(user->email()) ==
+                             policy::USER_AFFILIATION_MANAGED;
+  VLOG(1) << "Use system key slot " << use_system_key_slot;
+
+  scoped_ptr<CertDatabaseServiceIOPartChromeOS> io_part(
+      new CertDatabaseServiceIOPartChromeOS(
+          user->email(),
+          user->username_hash(),
+          use_system_key_slot,
+          profile->GetPath(),
+          content::BrowserThread::GetMessageLoopProxyForThread(
+              content::BrowserThread::UI),  // Thread for DBus calls
+          chromeos::DBusThreadManager::Get()->GetCryptohomeClient()));
+
+  // This callback must be called on IO.
+  CertDatabaseServiceIOPartChromeOS::SystemTPMTokenReadyCallback
+      callback_on_io = io_part->GetSystemTPMTokenReadyCallback();
+
+  // Wrap it to be callable from the UI thread.
+  base::Callback<void(bool enabled)> callback_on_ui =
+      base::Bind(&WrapReadyCallbackForUI, callback_on_io);
+#else
+  scoped_ptr<CertDatabaseServiceIOPart> io_part(
+      new CertDatabaseServiceIOPartLinux());
+#endif
+
+  scoped_ptr<CertDatabaseService> service(new CertDatabaseService(
+      content::BrowserThread::GetMessageLoopProxyForThread(
+          content::BrowserThread::IO)));
+
+  // After this point, the IOPart must only be accessed from the IO thread!
+  service->SetIOPart(io_part.PassAs<CertDatabaseServiceIOPart>());

[mattm, 2014/10/18 00:45:16]

I think you can use Pass instead of PassAs now

[pneubeck (no reviews), 2014/10/21 09:22:09]

Awesome! Thanks for pointing that out.

+
+#if defined(OS_CHROMEOS)
+  chromeos::TPMTokenLoader::TPMTokenStatus tpm_token_status =
+      chromeos::TPMTokenLoader::Get()->IsTPMTokenEnabled(callback_on_ui);
+  if (tpm_token_status !=
+      chromeos::TPMTokenLoader::TPM_TOKEN_STATUS_UNDETERMINED) {
+    callback_on_ui.Run(tpm_token_status ==
+                       chromeos::TPMTokenLoader::TPM_TOKEN_STATUS_ENABLED);
+  }
+
+  // TODO(pneubeck): Integrate CertLoader into the CertDatabaseService so that
+  // it can be used per user and not only for the primary user.
+  user_manager::UserManager* user_manager = user_manager::UserManager::Get();
+  bool is_primary_user = user_manager && user == user_manager->GetPrimaryUser();
+  if (is_primary_user) {
+    service->GetNSSCertDatabase(
+        base::Bind(&chromeos::CertLoader::StartWithNSSDB,
+                   base::Unretained(chromeos::CertLoader::Get())));
+  }
+#endif
+
+  return service.release();
+}
+
+}  // namespace cert_database