Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1081)

Unified Diff: chrome/browser/chromeos/settings/device_settings_provider.cc

Issue 666363002: DeviceSettingsProvider's write path uses OwnerSettingsService now. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fixes. Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..41d55d4156c80674ded0dec7ed1202e797321d33 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());
+ policy->set_username(device_settings_service_->GetUsername());
+ CHECK(device_settings_.SerializeToString(policy->mutable_policy_value()));
+ if (!device_settings_service_->GetOwnerSettingsService()
+ ->CommitTentativeDeviceSettings(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;
« no previous file with comments | « chrome/browser/chromeos/settings/device_settings_provider.h ('k') | components/ownership/owner_settings_service.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698