Implemented OwnerSettingsService::Set() method.

chrome/browser/chromeos/settings/device_settings_provider.cc

 // Copyright (c) 2012 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/settings/device_settings_provider.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/logging.h"
@@ skipping 441 matching lines @@
   device_settings_service_->RemoveObserver(this);
 }
 
 // static
 bool DeviceSettingsProvider::IsDeviceSetting(const std::string& name) {
   const char** end = kKnownSettings + arraysize(kKnownSettings);
   return std::find(kKnownSettings, end, name) != end;
 }
 
 void DeviceSettingsProvider::DoSet(const std::string& path,
                                    const base::Value& in_value) {

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

Can we perhaps re-wire this function to go through
OwnershipSettingsServiceChromeOS already in this CL? Doing so would allow you to
get rid of most of the complications in DeviceSettingsService, in particular the
need to stop the processing loop. I think you can get the owner user via
DeviceSettingsService->GetUsername and then locate the right profile via
ProfileHelper. If it's not there, just bail. 

[ygorshenin1, 2014/10/20 11:36:10]

Good idea, but I have a question to you: what is the best way for owner's
instance of OwnerSettingsServiceChromeOS to notify DeviceSettingsProvider about
tentative changes in policy?

[Mattias Nissler (ping if slow), 2014/10/20 12:41:30]

Hm, I'm trying to remember why we need that actually. I guess one reason is the
temporary storage of settings when there's no owner yet and get them persisted
to the signed settings cache in local state? If that is still a thing, then the
best way would probably be to add a public interface that lets one call
UpdateValuesCache(policy_data, settings, TEMPORARILY_UNTRUSTED).

[ygorshenin1, 2014/10/22 09:20:11]

I'd prefer to make the change you're proposing in a separate CL, if you don't
mind, because current one is already quite complex.

   // Make sure that either the current user is the device owner or the
   // device doesn't have an owner yet.
   if (!(device_settings_service_->HasPrivateOwnerKey() ||
         ownership_status_ == DeviceSettingsService::OWNERSHIP_NONE)) {
     LOG(WARNING) << "Changing settings from non-owner, setting=" << path;
 
     // Revert UI change.
     NotifyObservers(path);
     return;
   }
@@ skipping 66 matching lines @@
     // 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;
-  if (prop == kAccountsPrefAllowNewUser) {
-    em::AllowNewUsersProto* allow =
-        device_settings_.mutable_allow_new_users();
-    bool allow_value;
-    if (value->GetAsBoolean(&allow_value))
-      allow->set_allow_new_users(allow_value);
-    else
-      NOTREACHED();
-  } else if (prop == kAccountsPrefAllowGuest) {
-    em::GuestModeEnabledProto* guest =
-        device_settings_.mutable_guest_mode_enabled();
-    bool guest_value;
-    if (value->GetAsBoolean(&guest_value))
-      guest->set_guest_mode_enabled(guest_value);
-    else
-      NOTREACHED();
-  } else if (prop == kAccountsPrefSupervisedUsersEnabled) {
-    em::SupervisedUsersSettingsProto* supervised =
-        device_settings_.mutable_supervised_users_settings();
-    bool supervised_value;
-    if (value->GetAsBoolean(&supervised_value))
-      supervised->set_supervised_users_enabled(supervised_value);
-    else
-      NOTREACHED();
-  } else if (prop == kAccountsPrefShowUserNamesOnSignIn) {
-    em::ShowUserNamesOnSigninProto* show =
-        device_settings_.mutable_show_user_names();
-    bool show_value;
-    if (value->GetAsBoolean(&show_value))
-      show->set_show_user_names(show_value);
-    else
-      NOTREACHED();
-  } else if (prop == kAccountsPrefDeviceLocalAccounts) {
-    em::DeviceLocalAccountsProto* device_local_accounts =
-        device_settings_.mutable_device_local_accounts();
-    device_local_accounts->clear_account();
-    const base::ListValue* accounts_list = NULL;
-    if (value->GetAsList(&accounts_list)) {
-      for (base::ListValue::const_iterator entry(accounts_list->begin());
-           entry != accounts_list->end(); ++entry) {
-        const base::DictionaryValue* entry_dict = NULL;
-        if ((*entry)->GetAsDictionary(&entry_dict)) {
-          em::DeviceLocalAccountInfoProto* account =
-              device_local_accounts->add_account();
-          std::string account_id;
-          if (entry_dict->GetStringWithoutPathExpansion(
-                  kAccountsPrefDeviceLocalAccountsKeyId, &account_id)) {
-            account->set_account_id(account_id);
-          }
-          int type;
-          if (entry_dict->GetIntegerWithoutPathExpansion(
-                  kAccountsPrefDeviceLocalAccountsKeyType, &type)) {
-            account->set_type(
-                static_cast<em::DeviceLocalAccountInfoProto::AccountType>(
-                    type));
-          }
-          std::string kiosk_app_id;
-          if (entry_dict->GetStringWithoutPathExpansion(
-                  kAccountsPrefDeviceLocalAccountsKeyKioskAppId,
-                  &kiosk_app_id)) {
-            account->mutable_kiosk_app()->set_app_id(kiosk_app_id);
-          }
-        } else {
-          NOTREACHED();
-        }
-      }
-    } else {
-      NOTREACHED();
-    }
-  } else if (prop == kAccountsPrefDeviceLocalAccountAutoLoginId) {
-    em::DeviceLocalAccountsProto* device_local_accounts =
-        device_settings_.mutable_device_local_accounts();
-    std::string id;
-    if (value->GetAsString(&id))
-      device_local_accounts->set_auto_login_id(id);
-    else
-      NOTREACHED();
-  } else if (prop == kAccountsPrefDeviceLocalAccountAutoLoginDelay) {
-    em::DeviceLocalAccountsProto* device_local_accounts =
-        device_settings_.mutable_device_local_accounts();
-    int delay;
-    if (value->GetAsInteger(&delay))
-      device_local_accounts->set_auto_login_delay(delay);
-    else
-      NOTREACHED();
-  } else if (prop == kAccountsPrefDeviceLocalAccountAutoLoginBailoutEnabled) {
-    em::DeviceLocalAccountsProto* device_local_accounts =
-        device_settings_.mutable_device_local_accounts();
-    bool enabled;
-    if (value->GetAsBoolean(&enabled))
-      device_local_accounts->set_enable_auto_login_bailout(enabled);
-    else
-      NOTREACHED();
-  } else if (prop ==
-             kAccountsPrefDeviceLocalAccountPromptForNetworkWhenOffline) {
-    em::DeviceLocalAccountsProto* device_local_accounts =
-        device_settings_.mutable_device_local_accounts();
-    bool should_prompt;
-    if (value->GetAsBoolean(&should_prompt))
-      device_local_accounts->set_prompt_for_network_when_offline(should_prompt);
-    else
-      NOTREACHED();
-  } else if (prop == kSignedDataRoamingEnabled) {
-    em::DataRoamingEnabledProto* roam =
-        device_settings_.mutable_data_roaming_enabled();
-    bool roaming_value = false;
-    if (value->GetAsBoolean(&roaming_value))
-      roam->set_data_roaming_enabled(roaming_value);
-    else
-      NOTREACHED();
-  } else if (prop == kReleaseChannel) {
-    em::ReleaseChannelProto* release_channel =
-        device_settings_.mutable_release_channel();
-    std::string channel_value;
-    if (value->GetAsString(&channel_value))
-      release_channel->set_release_channel(channel_value);
-    else
-      NOTREACHED();
-  } else if (prop == kStatsReportingPref) {
-    em::MetricsEnabledProto* metrics =
-        device_settings_.mutable_metrics_enabled();
-    bool metrics_value = false;
-    if (value->GetAsBoolean(&metrics_value))
-      metrics->set_metrics_enabled(metrics_value);
-    else
-      NOTREACHED();
+  DeviceSettingsService::UpdateDeviceSettings(prop, *value, device_settings_);

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

We should probably put a comment somewhere in this file or in the header saying
that the write path is in the process of being migrated to
OwnerSettingsServiceChromeOS

[ygorshenin1, 2014/10/20 11:36:10]

Done.

+
+  bool metrics_value;
+  if (prop == kStatsReportingPref && value->GetAsBoolean(&metrics_value))
     ApplyMetricsSetting(false, metrics_value);
-  } else if (prop == kAccountsPrefUsers) {
-    em::UserWhitelistProto* whitelist_proto =
-        device_settings_.mutable_user_whitelist();
-    whitelist_proto->clear_user_whitelist();
-    const base::ListValue* users;
-    if (value->GetAsList(&users)) {
-      for (base::ListValue::const_iterator i = users->begin();
-           i != users->end(); ++i) {
-        std::string email;
-        if ((*i)->GetAsString(&email))
-          whitelist_proto->add_user_whitelist(email);
-      }
-    }
-  } else if (prop == kAccountsPrefEphemeralUsersEnabled) {
-    em::EphemeralUsersEnabledProto* ephemeral_users_enabled =
-        device_settings_.mutable_ephemeral_users_enabled();
-    bool ephemeral_users_enabled_value = false;
-    if (value->GetAsBoolean(&ephemeral_users_enabled_value)) {
-      ephemeral_users_enabled->set_ephemeral_users_enabled(
-          ephemeral_users_enabled_value);
-    } else {
-      NOTREACHED();
-    }
-  } else if (prop == kAllowRedeemChromeOsRegistrationOffers) {
-    em::AllowRedeemChromeOsRegistrationOffersProto* allow_redeem_offers =
-        device_settings_.mutable_allow_redeem_offers();
-    bool allow_redeem_offers_value;
-    if (value->GetAsBoolean(&allow_redeem_offers_value)) {
-      allow_redeem_offers->set_allow_redeem_offers(
-          allow_redeem_offers_value);
-    } else {
-      NOTREACHED();
-    }
-  } else if (prop == kStartUpFlags) {
-    em::StartUpFlagsProto* flags_proto =
-        device_settings_.mutable_start_up_flags();
-    flags_proto->Clear();
-    const base::ListValue* flags;
-    if (value->GetAsList(&flags)) {
-      for (base::ListValue::const_iterator i = flags->begin();
-           i != flags->end(); ++i) {
-        std::string flag;
-        if ((*i)->GetAsString(&flag))
-          flags_proto->add_flags(flag);
-      }
-    }
-  } else if (prop == kSystemUse24HourClock) {
-    em::SystemUse24HourClockProto* use_24hour_clock_proto =
-        device_settings_.mutable_use_24hour_clock();
-    use_24hour_clock_proto->Clear();
-    bool use_24hour_clock_value;
-    if (value->GetAsBoolean(&use_24hour_clock_value)) {
-      use_24hour_clock_proto->set_use_24hour_clock(use_24hour_clock_value);
-    } else {
-      NOTREACHED();
-    }
-  } else if (prop == kAttestationForContentProtectionEnabled) {
-    em::AttestationSettingsProto* attestation_settings =
-        device_settings_.mutable_attestation_settings();
-    bool setting_enabled;
-    if (value->GetAsBoolean(&setting_enabled)) {
-      attestation_settings->set_content_protection_enabled(setting_enabled);
-    } else {
-      NOTREACHED();
-    }
-  } else {
-    // The remaining settings don't support Set(), since they are not
-    // intended to be customizable by the user:
-    //   kAccountsPrefTransferSAMLCookies
-    //   kAppPack
-    //   kDeviceAttestationEnabled
-    //   kDeviceOwner
-    //   kIdleLogoutTimeout
-    //   kIdleLogoutWarningDuration
-    //   kReleaseChannelDelegated
-    //   kReportDeviceActivityTimes
-    //   kReportDeviceBootMode
-    //   kReportDeviceLocation
-    //   kReportDeviceVersionInfo
-    //   kReportDeviceNetworkInterfaces
-    //   kReportDeviceUsers
-    //   kScreenSaverExtensionId
-    //   kScreenSaverTimeout
-    //   kServiceAccountIdentity
-    //   kStartUpUrls
-    //   kSystemTimezonePolicy
-    //   kVariationsRestrictParameter
-
-    LOG(FATAL) << "Device setting " << prop << " is read-only.";
-  }
 
   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();
@@ skipping 233 matching lines @@
 void DeviceSettingsProvider::AttemptMigration() {
   if (device_settings_service_->HasPrivateOwnerKey()) {
     PrefValueMap::const_iterator i;
     for (i = migration_values_.begin(); i != migration_values_.end(); ++i)
       DoSet(i->first, *i->second);
     migration_values_.Clear();
   }
 }
 
 }  // namespace chromeos