DeviceSettingsProvider's write path uses OwnerSettingsService now.

chrome/browser/chromeos/settings/device_settings_provider.cc

Index: chrome/browser/chromeos/settings/device_settings_provider.cc
diff --git a/chrome/browser/chromeos/settings/device_settings_provider.cc b/chrome/browser/chromeos/settings/device_settings_provider.cc
index 02e16e7a46ecf5f53167def8b5f3a44b2be8f312..0d9697c3d2adb3ef9c4db015a99c18e330707352 100644
--- a/chrome/browser/chromeos/settings/device_settings_provider.cc
+++ b/chrome/browser/chromeos/settings/device_settings_provider.cc
@@ -479,11 +479,37 @@ void DeviceSettingsProvider::DoSet(const std::string& path,
     return;
   }
 
-  // TODO (ygorshenin@, crbug.com/230018)): use OwnerSettingsService::Set()
-  // here.
-  pending_changes_.push_back(PendingQueueElement(path, in_value.DeepCopy()));
-  if (!store_callback_factory_.HasWeakPtrs())
-    SetInPolicy();
+  if (device_settings_service_->HasPrivateOwnerKey()) {
+    // Directly set setting through OwnerSettingsService.
+    ownership::OwnerSettingsService* service =
+        device_settings_service_->GetOwnerSettingsService();
+    if (!service->Set(path, in_value)) {
+      NotifyObservers(path);
+      return;
+    }
+  } else {
+    // Temporary store new setting in
+    // |device_settings_|. |device_settings_| will be stored on a disk
+    // as soon as an ownership of device the will be taken.
+    OwnerSettingsServiceChromeOS::UpdateDeviceSettings(
+        path, in_value, device_settings_);
+    em::PolicyData data;
+    data.set_username(device_settings_service_->GetUsername());
+    CHECK(device_settings_.SerializeToString(data.mutable_policy_value()));
+
+    // Set the cache to the updated value.
+    UpdateValuesCache(data, device_settings_, TEMPORARILY_UNTRUSTED);
+
+    if (!device_settings_cache::Store(data, g_browser_process->local_state())) {
+      LOG(ERROR) << "Couldn't store to the temp storage.";
+      NotifyObservers(path);
+      return;
+    }
+  }
+
+  bool metrics_value;
+  if (path == kStatsReportingPref && in_value.GetAsBoolean(&metrics_value))
+    ApplyMetricsSetting(false, metrics_value);
 }
 
 void DeviceSettingsProvider::OwnershipStatusChanged() {
@@ -513,7 +539,14 @@ void DeviceSettingsProvider::OwnershipStatusChanged() {
       new_settings.MergeFrom(device_settings_);
       device_settings_.Swap(&new_settings);
     }
-    StoreDeviceSettings();
+
+    scoped_ptr<em::PolicyData> policy(new em::PolicyData());

[Mattias Nissler (ping if slow), 2014/10/24 08:08:02]

Assuming that we want to get rid of the Set() code path in
DeviceSettingsProvider entirely, we probably want to break out the code for
caching settings before ownership gets established. Perhaps we can get away with
a static SetTentativeDeviceSetting function in OwnershipSettingsServiceChromeOS
that just updates the cache in local_state?

Let's do that in a subsequent CL though.

[ygorshenin1, 2014/10/24 09:13:28]

Sounds good, I'll implement that in a subsequent CL.

+    policy->set_username(device_settings_service_->GetUsername());
+    CHECK(device_settings_.SerializeToString(policy->mutable_policy_value()));
+    if (!device_settings_service_->GetOwnerSettingsService()->SetPolicy(
+            policy.Pass())) {
+      LOG(ERROR) << "Can't store policy";
+    }
   }
 
   // The owner key might have become available, allowing migration to happen.
@@ -545,50 +578,6 @@ void DeviceSettingsProvider::RetrieveCachedData() {
   UpdateValuesCache(policy_data, device_settings_, trusted_status_);
 }
 
-void DeviceSettingsProvider::SetInPolicy() {
-  if (pending_changes_.empty()) {
-    NOTREACHED();
-    return;
-  }
-
-  if (RequestTrustedEntity() != TRUSTED) {
-    // Re-sync device settings before proceeding.
-    device_settings_service_->Load();
-    return;
-  }
-
-  std::string prop(pending_changes_.front().first);
-  scoped_ptr<base::Value> value(pending_changes_.front().second);
-  pending_changes_.pop_front();
-
-  trusted_status_ = TEMPORARILY_UNTRUSTED;
-  OwnerSettingsServiceChromeOS::UpdateDeviceSettings(
-      prop, *value, device_settings_);
-
-  bool metrics_value;
-  if (prop == kStatsReportingPref && value->GetAsBoolean(&metrics_value))
-    ApplyMetricsSetting(false, metrics_value);
-
-  em::PolicyData data;
-  data.set_username(device_settings_service_->GetUsername());
-  CHECK(device_settings_.SerializeToString(data.mutable_policy_value()));
-
-  // Set the cache to the updated value.
-  UpdateValuesCache(data, device_settings_, trusted_status_);
-
-  if (ownership_status_ == DeviceSettingsService::OWNERSHIP_TAKEN) {
-    StoreDeviceSettings();
-  } else {
-    if (!device_settings_cache::Store(data, g_browser_process->local_state()))
-      LOG(ERROR) << "Couldn't store to the temp storage.";
-
-    // OnStorePolicyCompleted won't get called in this case so proceed with any
-    // pending operations immediately.
-    if (!pending_changes_.empty())
-      SetInPolicy();
-  }
-}
-
 void DeviceSettingsProvider::UpdateValuesCache(
     const em::PolicyData& policy_data,
     const em::ChromeDeviceSettingsProto& settings,
@@ -732,10 +721,6 @@ DeviceSettingsProvider::TrustedStatus
 void DeviceSettingsProvider::UpdateAndProceedStoring() {
   // Re-sync the cache from the service.
   UpdateFromService();
-
-  // Trigger the next change if necessary.
-  if (trusted_status_ == TRUSTED && !pending_changes_.empty())
-    SetInPolicy();
 }
 
 bool DeviceSettingsProvider::UpdateFromService() {
@@ -799,18 +784,6 @@ bool DeviceSettingsProvider::UpdateFromService() {
   return settings_loaded;
 }
 
-void DeviceSettingsProvider::StoreDeviceSettings() {
-  // Mute all previous callbacks to guarantee the |pending_changes_| queue is
-  // processed serially.
-  store_callback_factory_.InvalidateWeakPtrs();
-
-  device_settings_service_->SignAndStore(
-      scoped_ptr<em::ChromeDeviceSettingsProto>(
-          new em::ChromeDeviceSettingsProto(device_settings_)),
-      base::Bind(&DeviceSettingsProvider::UpdateAndProceedStoring,
-                 store_callback_factory_.GetWeakPtr()));
-}
-
 void DeviceSettingsProvider::AttemptMigration() {
   if (device_settings_service_->HasPrivateOwnerKey()) {
     PrefValueMap::const_iterator i;