Implemented profile-aware owner key loading.

chrome/browser/chromeos/login/owner_key_reloader_service.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/chromeos/login/owner_key_reloader_service.h"
+
+#include "base/bind.h"
+#include "chrome/browser/chromeos/login/owner_key_reloader_service_factory.h"
+#include "chrome/browser/chromeos/login/user.h"
+#include "chrome/browser/chromeos/login/user_manager.h"
+#include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chrome/browser/net/nss_context.h"
+#include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/resource_context.h"
+#include "crypto/scoped_nss_types.h"
+
+using content::BrowserThread;
+
+namespace chromeos {
+
+std::string OwnerKeyReloaderService::username_;

[Mattias Nissler (ping if slow), 2014/05/14 12:18:10]

Static non-POD members are outlawed (unless something has changed that I have
missed).

[ygorshenin1, 2014/05/14 15:30:07]

Done.

+
+OwnerKeyReloaderService::OwnerKeyReloaderService(Profile* profile)
+    : profile_(profile) {

[Mattias Nissler (ping if slow), 2014/05/14 12:18:10]

I usually advice people against passing a Profile in as a dependency, and
instead just pass the dependencies you actually need. In this case that would be
the resource context (and eventually the username once all signing happens
here).

A couple reasons:
* If you hold a Profile pointer, there's a risk that you're introducing
dependencies that are not declared at the factory dependency level, and that can
lead to cyclic dependencies between Profile-keyed services, which I've seen
causing grief at shutdown time (for example when Observers come into play that
need to be destroyed in the correct order).
* With explicit dependencies it's much easier to refactor the code in the future
work outside of a Profile context  (that's probably not a concern here, but you
never know).
* Writing unit tests is much easier because you don't have to mock the world,
just the dependencies you need. Also, the test is guaranteed to not have any
hidden dependencies, which avoids surprises for people changing Profile stuff.

[ygorshenin1, 2014/05/14 15:30:07]

Unfortunately ResourceContext is not ready at the time of BCKS creation, so I
need to pass a Profile instance here.

[Mattias Nissler (ping if slow), 2014/05/14 16:00:36]

Hm, too bad :-/ Let's go with a Profile pointer then.

+}
+
+OwnerKeyReloaderService::~OwnerKeyReloaderService() {
+}
+
+// static
+void OwnerKeyReloaderService::SetUsername(const std::string& username) {

[Mattias Nissler (ping if slow), 2014/05/14 12:18:10]

Let's check whether we're on the same page here: I think eventually, we don't
want to maintain a global variable for the user name, but instead have each
instance of this service know the username it belongs to. Similarly, the service
instance for each Profile will try to load the owner key from their NSS DB. The
one that succeeds will be able to write owner settings, all others won't.

[ygorshenin1, 2014/05/14 15:30:07]

Done.

+  username_ = username;
+  if (!UserManager::IsInitialized())

[Mattias Nissler (ping if slow), 2014/05/14 12:18:10]

Is this just for unit tests?

[ygorshenin1, 2014/05/14 15:30:07]

Yes, it's just for unit tests.

On 2014/05/14 12:18:10, Mattias Nissler wrote:

+    return;
+  const User* user = UserManager::Get()->FindUser(username);
+  if (!user || !user->is_profile_created())
+    return;
+  Profile* profile = UserManager::Get()->GetProfileByUser(user);
+  if (!profile)
+    return;
+  OwnerKeyReloaderServiceFactory::GetForProfile(profile)->ReloadOwnerKey();
+}
+
+// static
+std::string OwnerKeyReloaderService::GetUsername() {
+  return username_;
+}
+
+void OwnerKeyReloaderService::ReloadOwnerKey() {
+  if (!UserManager::IsInitialized() || !profile_)
+    return;
+  const User* user = UserManager::Get()->GetUserByProfile(profile_);
+  if (!user || user->email() != GetUsername())
+    return;
+  content::ResourceContext* context =
+      profile_ ? profile_->GetResourceContext() : NULL;
+  if (context) {
+    BrowserThread::PostTaskAndReplyWithResult(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&GetPublicNSSKeySlotForResourceContext, context),
+        base::Bind(&DeviceSettingsService::InitOwner,
+                   base::Unretained(DeviceSettingsService::Get()),
+                   GetUsername()));
+  } else {
+    crypto::ScopedPK11Slot slot;
+    DeviceSettingsService::Get()->InitOwner(GetUsername(), slot.Pass());
+  }
+}
+
+}  // namespace chromeos