Add DeviceADPolicyManager to provide AD policy.

chrome/browser/chromeos/settings/device_settings_service.cc

Index: chrome/browser/chromeos/settings/device_settings_service.cc
diff --git a/chrome/browser/chromeos/settings/device_settings_service.cc b/chrome/browser/chromeos/settings/device_settings_service.cc
index 586fa8dce2fb489b7af8bce8f6e8e023e7c2756b..08f9dc7a59b2273295a0db65b436621876404424 100644
--- a/chrome/browser/chromeos/settings/device_settings_service.cc
+++ b/chrome/browser/chromeos/settings/device_settings_service.cc
@@ -81,9 +81,9 @@ DeviceSettingsService* DeviceSettingsService::Get() {
 DeviceSettingsService::DeviceSettingsService()
     : session_manager_client_(NULL),
       store_status_(STORE_SUCCESS),
+      device_mode_(policy::DEVICE_MODE_PENDING),

[emaxx, 2016/11/11 15:25:09]

nit: Move initializers to the class definition?

[Thiemo Nagel, 2016/11/16 19:11:01]

Done.

       load_retries_left_(kMaxLoadRetries),
-      weak_factory_(this) {
-}
+      weak_factory_(this) {}
 
 DeviceSettingsService::~DeviceSettingsService() {
   DCHECK(pending_operations_.empty());
@@ -116,6 +116,11 @@ void DeviceSettingsService::UnsetSessionManager() {
   owner_key_util_ = NULL;
 }
 
+void DeviceSettingsService::SetDeviceMode(policy::DeviceMode device_mode) {
+  device_mode_ = device_mode;

[emaxx, 2016/11/11 15:25:09]

Can we add some DCHECK/CHECK regarding allowed device mode transitions?
For instance, that the device mode can only change when it was
DEVICE_MODE_PENDING.

[Thiemo Nagel, 2016/11/16 19:11:01]

Sounds like a good idea.  Done.

+  RunPendingOwnershipStatusCallbacks();
+}
+
 scoped_refptr<PublicKey> DeviceSettingsService::GetPublicKey() {
   return public_key_;
 }
@@ -137,13 +142,15 @@ DeviceSettingsService::OwnershipStatus
     DeviceSettingsService::GetOwnershipStatus() {
   if (public_key_.get())
     return public_key_->is_loaded() ? OWNERSHIP_TAKEN : OWNERSHIP_NONE;
+  if (device_mode_ == policy::DEVICE_MODE_ENTERPRISE_AD)
+    return OWNERSHIP_TAKEN;
   return OWNERSHIP_UNKNOWN;
 }
 
 void DeviceSettingsService::GetOwnershipStatusAsync(
     const OwnershipStatusCallback& callback) {
-  if (public_key_.get()) {
-    // If there is a key, report status immediately.
+  if (GetOwnershipStatus() != OWNERSHIP_UNKNOWN) {
+    // Report status immediately.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, GetOwnershipStatus()));
   } else {
@@ -218,11 +225,15 @@ void DeviceSettingsService::Enqueue(
 }
 
 void DeviceSettingsService::EnqueueLoad(bool force_key_load) {
+  bool verify_signature = true;
+  if (device_mode_ == policy::DEVICE_MODE_ENTERPRISE_AD) {
+    force_key_load = false;

[emaxx, 2016/11/11 15:25:08]

That's a bit unexpected that the "force" value is actually not respected in some
cases.
I'd suggest either to move this condition to the calling place, or to rename the
method argument.

[Thiemo Nagel, 2016/11/16 19:11:01]

I've renamed to "request_key_load" since there are multiple call sites and I
wouldn't want to have the condition at all of them.

+    verify_signature = false;
+  }
   linked_ptr<SessionManagerOperation> operation(new LoadSettingsOperation(
+      force_key_load, verify_signature,
       base::Bind(&DeviceSettingsService::HandleCompletedOperation,
-                 weak_factory_.GetWeakPtr(),
-                 base::Closure())));
-  operation->set_force_key_load(force_key_load);
+                 weak_factory_.GetWeakPtr(), base::Closure())));
   Enqueue(operation);
 }
 
@@ -248,14 +259,7 @@ void DeviceSettingsService::HandleCompletedOperation(
   DCHECK_EQ(operation, pending_operations_.front().get());
   store_status_ = status;
 
-  OwnershipStatus ownership_status = OWNERSHIP_UNKNOWN;
   scoped_refptr<PublicKey> new_key(operation->public_key());
-  if (new_key.get()) {

[emaxx, 2016/11/11 15:25:08]

Are these checks for new_key.get() and new_key->is_loaded() not applicable
anymore?

[Thiemo Nagel, 2016/11/16 19:11:01]

RunPendingOwnershipStatusCallbacks() below calls GetOwnershipStatus() which does
the same check on public_key_ which has the same value as new_key.

But I've re-added the NOTREACHED(), it's now a DCHECK().

-    ownership_status = new_key->is_loaded() ? OWNERSHIP_TAKEN : OWNERSHIP_NONE;
-  } else {
-    NOTREACHED() << "Failed to determine key status.";
-  }
-
   bool new_owner_key = false;
   if (public_key_.get() != new_key.get()) {
     public_key_ = new_key;
@@ -299,12 +303,7 @@ void DeviceSettingsService::HandleCompletedOperation(
   for (auto& observer : observers_)
     observer.DeviceSettingsUpdated();
 
-  std::vector<OwnershipStatusCallback> callbacks;
-  callbacks.swap(pending_ownership_status_callbacks_);
-  for (std::vector<OwnershipStatusCallback>::iterator iter(callbacks.begin());
-       iter != callbacks.end(); ++iter) {
-    iter->Run(ownership_status);
-  }
+  RunPendingOwnershipStatusCallbacks();
 
   // The completion callback happens after the notification so clients can
   // filter self-triggered updates.
@@ -333,6 +332,14 @@ void DeviceSettingsService::HandleError(Status status,
     callback.Run();
 }
 
+void DeviceSettingsService::RunPendingOwnershipStatusCallbacks() {
+  std::vector<OwnershipStatusCallback> callbacks;
+  callbacks.swap(pending_ownership_status_callbacks_);
+  for (auto& callback : callbacks) {

[emaxx, 2016/11/11 15:25:09]

const auto& should be probably enough.

[Thiemo Nagel, 2016/11/16 19:11:01]

Done.

+    callback.Run(GetOwnershipStatus());
+  }
+}
+
 ScopedTestDeviceSettingsService::ScopedTestDeviceSettingsService() {
   DeviceSettingsService::Initialize();
 }