diff options
author | pastarmovj@chromium.org <pastarmovj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-22 13:10:36 +0000 |
---|---|---|
committer | pastarmovj@chromium.org <pastarmovj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-22 13:10:36 +0000 |
commit | 6c2b810e1d54b0a461afb2c089667258c1a61707 (patch) | |
tree | ed3414178390e2280d8191738c50e6b9e793f17c | |
parent | 291890fe382d85cd43041a6da7cb634f39b0f36a (diff) | |
download | chromium_src-6c2b810e1d54b0a461afb2c089667258c1a61707.zip chromium_src-6c2b810e1d54b0a461afb2c089667258c1a61707.tar.gz chromium_src-6c2b810e1d54b0a461afb2c089667258c1a61707.tar.bz2 |
Make SignedSettings use proper Value types instead of string all around the place.
Extra: Add ListValue support to the bunch and Userlist handling through Retrieve/StorePropertyOp.
BUG=chromium-os:14054
TEST=unit_tests:*Signed*
Review URL: http://codereview.chromium.org/8091002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111141 0039d316-1c4b-4281-b951-d872f2087c98
25 files changed, 382 insertions, 297 deletions
diff --git a/chrome/browser/chromeos/cros_settings.cc b/chrome/browser/chromeos/cros_settings.cc index 16f70a2..bde3c31 100644 --- a/chrome/browser/chromeos/cros_settings.cc +++ b/chrome/browser/chromeos/cros_settings.cc @@ -46,7 +46,7 @@ void CrosSettings::FireObservers(const char* path) { } } -void CrosSettings::Set(const std::string& path, Value* in_value) { +void CrosSettings::Set(const std::string& path, const base::Value& in_value) { DCHECK(CalledOnValidThread()); CrosSettingsProvider* provider; provider = GetProvider(path); @@ -57,23 +57,66 @@ void CrosSettings::Set(const std::string& path, Value* in_value) { void CrosSettings::SetBoolean(const std::string& path, bool in_value) { DCHECK(CalledOnValidThread()); - Set(path, Value::CreateBooleanValue(in_value)); + base::FundamentalValue value(in_value); + Set(path, value); } void CrosSettings::SetInteger(const std::string& path, int in_value) { DCHECK(CalledOnValidThread()); - Set(path, Value::CreateIntegerValue(in_value)); + base::FundamentalValue value(in_value); + Set(path, value); } void CrosSettings::SetDouble(const std::string& path, double in_value) { DCHECK(CalledOnValidThread()); - Set(path, Value::CreateDoubleValue(in_value)); + base::FundamentalValue value(in_value); + Set(path, value); } void CrosSettings::SetString(const std::string& path, const std::string& in_value) { DCHECK(CalledOnValidThread()); - Set(path, Value::CreateStringValue(in_value)); + base::StringValue value(in_value); + Set(path, value); +} + +void CrosSettings::AppendToList(const std::string& path, + const base::Value* value) { + DCHECK(CalledOnValidThread()); + const base::Value* old_value = GetPref(path); + scoped_ptr<base::Value> new_value( + old_value ? old_value->DeepCopy() : new base::ListValue()); + static_cast<base::ListValue*>(new_value.get())->Append(value->DeepCopy()); + Set(path, *new_value); +} + +void CrosSettings::RemoveFromList(const std::string& path, + const base::Value* value) { + DCHECK(CalledOnValidThread()); + const base::Value* old_value = GetPref(path); + scoped_ptr<base::Value> new_value( + old_value ? old_value->DeepCopy() : new base::ListValue()); + static_cast<base::ListValue*>(new_value.get())->Remove(*value, NULL); + Set(path, *new_value); +} + +bool CrosSettings::FindEmailInList(const std::string& path, + const std::string& email) const { + DCHECK(CalledOnValidThread()); + base::StringValue email_value(email); + const base::ListValue* value = + static_cast<const base::ListValue*>(GetPref(path)); + if (value) { + if (value->Find(email_value) != value->end()) + return true; + std::string::size_type at_pos = email.find('@'); + if (at_pos != std::string::npos) { + base::StringValue wildcarded_value( + std::string("*").append(email.substr(at_pos))); + return value->Find(wildcarded_value) != value->end(); + } + } + return false; } bool CrosSettings::AddSettingsProvider(CrosSettingsProvider* provider) { diff --git a/chrome/browser/chromeos/cros_settings.h b/chrome/browser/chromeos/cros_settings.h index 563ee99..c7f66d2 100644 --- a/chrome/browser/chromeos/cros_settings.h +++ b/chrome/browser/chromeos/cros_settings.h @@ -35,8 +35,7 @@ class CrosSettings : public base::NonThreadSafe { static bool IsCrosSettings(const std::string& path); // Sets |in_value| to given |path| in cros settings. - // Note that this takes ownership of |in_value|. - void Set(const std::string& path, base::Value* in_value); + void Set(const std::string& path, const base::Value& in_value); // Fires system setting change notification. // TODO(pastarmovj): Consider to remove this function from the public @@ -60,6 +59,10 @@ class CrosSettings : public base::NonThreadSafe { void SetDouble(const std::string& path, double in_value); void SetString(const std::string& path, const std::string& in_value); + // Convenience functions for manipulating lists. + void AppendToList(const std::string& path, const base::Value* value); + void RemoveFromList(const std::string& path, const base::Value* value); + // These are convenience forms of Get(). The value will be retrieved // and the return value will be true if the path is valid and the value at // the end of the path can be returned in the form specified. @@ -70,6 +73,11 @@ class CrosSettings : public base::NonThreadSafe { bool GetList(const std::string& path, const base::ListValue** out_value) const; + // Helper function for the whitelist op. Implemented here because we will need + // this in a few places. The functions searches for |email| in the pref |path| + // It respects whitelists so foo@bar.baz will match *@bar.baz too. + bool FindEmailInList(const std::string& path, const std::string& email) const; + // adding/removing of providers bool AddSettingsProvider(CrosSettingsProvider* provider); bool RemoveSettingsProvider(CrosSettingsProvider* provider); diff --git a/chrome/browser/chromeos/cros_settings_provider.cc b/chrome/browser/chromeos/cros_settings_provider.cc index 95b59c0b..1eeaf5d 100644 --- a/chrome/browser/chromeos/cros_settings_provider.cc +++ b/chrome/browser/chromeos/cros_settings_provider.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -12,7 +12,8 @@ namespace chromeos { -void CrosSettingsProvider::Set(const std::string& path, Value* value) { +void CrosSettingsProvider::Set(const std::string& path, + const base::Value& value) { // We don't allow changing any of the cros settings without prefix // "cros.session." in the guest mode. // It should not reach here from UI in the guest mode, but just in case. diff --git a/chrome/browser/chromeos/cros_settings_provider.h b/chrome/browser/chromeos/cros_settings_provider.h index e5012c3..c050092 100644 --- a/chrome/browser/chromeos/cros_settings_provider.h +++ b/chrome/browser/chromeos/cros_settings_provider.h @@ -20,14 +20,15 @@ class CrosSettingsProvider { virtual ~CrosSettingsProvider() {} // Sets |in_value| to given |path| in cros settings. - // Note that this takes ownership of |in_value|. - void Set(const std::string& path, base::Value* in_value); + void Set(const std::string& path, const base::Value& in_value); // Gets settings value of given |path| to |out_value|. virtual const base::Value* Get(const std::string& path) const = 0; - // Starts a fetch from the trusted store for the value of |path|. It will - // call the |callback| function upon completion. + // Starts a fetch from the trusted store for the value of |path| if not loaded + // yet. It will call the |callback| function upon completion if a new fetch + // was needed in which case the return value is false. Else it will return + // true and won't call the |callback|. virtual bool GetTrusted(const std::string& path, const base::Closure& callback) const = 0; @@ -36,7 +37,8 @@ class CrosSettingsProvider { private: // Does the real job for Set(). - virtual void DoSet(const std::string& path, base::Value* in_value) = 0; + virtual void DoSet(const std::string& path, + const base::Value& in_value) = 0; }; } // namespace chromeos diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc index 6e53b27..74ab5af 100644 --- a/chrome/browser/chromeos/login/existing_user_controller.cc +++ b/chrome/browser/chromeos/login/existing_user_controller.cc @@ -102,14 +102,12 @@ void ExistingUserController::Init(const UserList& users) { &show_users_on_signin); if (show_users_on_signin) { bool allow_new_user = false; - const base::ListValue *user_list; cros_settings_->GetBoolean(kAccountsPrefAllowNewUser, &allow_new_user); - cros_settings_->GetList(kAccountsPrefUsers, &user_list); for (UserList::const_iterator it = users.begin(); it != users.end(); ++it) { - base::StringValue email((*it)->email()); // TODO(xiyuan): Clean user profile whose email is not in whitelist. if (allow_new_user || - user_list->Find(email) != user_list->end()) { + cros_settings_->FindEmailInList(kAccountsPrefUsers, + (*it)->email())) { filtered_users.push_back(*it); } } diff --git a/chrome/browser/chromeos/login/login_performer.cc b/chrome/browser/chromeos/login/login_performer.cc index 3d2b715..910cdb2 100644 --- a/chrome/browser/chromeos/login/login_performer.cc +++ b/chrome/browser/chromeos/login/login_performer.cc @@ -276,10 +276,7 @@ void LoginPerformer::CompleteLogin(const std::string& username, StartLoginCompletion(); } else { // Otherwise, do whitelist check first. - const base::ListValue *user_list; - base::StringValue username_value(username); - if (cros_settings->GetList(kAccountsPrefUsers, &user_list) && - user_list->Find(username_value) != user_list->end()) { + if (cros_settings->FindEmailInList(kAccountsPrefUsers, username)) { StartLoginCompletion(); } else { if (delegate_) @@ -321,10 +318,7 @@ void LoginPerformer::Login(const std::string& username, // Starts authentication if guest login is allowed or online auth pending. StartAuthentication(); } else { - const base::ListValue *user_list; - base::StringValue username_value(username); - if (cros_settings->GetList(kAccountsPrefUsers, &user_list) && - user_list->Find(username_value) != user_list->end()) { + if (cros_settings->FindEmailInList(kAccountsPrefUsers, username)) { StartAuthentication(); } else { if (delegate_) diff --git a/chrome/browser/chromeos/login/mock_signed_settings_helper.h b/chrome/browser/chromeos/login/mock_signed_settings_helper.h index 5376818..5a5b551 100644 --- a/chrome/browser/chromeos/login/mock_signed_settings_helper.h +++ b/chrome/browser/chromeos/login/mock_signed_settings_helper.h @@ -22,7 +22,7 @@ class MockSignedSettingsHelper : public chromeos::SignedSettingsHelper { MOCK_METHOD3(StartWhitelistOp, void(const std::string&, bool, Callback*)); MOCK_METHOD3(StartStorePropertyOp, - void(const std::string&, const std::string&, Callback*)); + void(const std::string&, const base::Value&, Callback*)); MOCK_METHOD2(StartRetrieveProperty, void(const std::string&, Callback*)); MOCK_METHOD2(StartStorePolicyOp, diff --git a/chrome/browser/chromeos/login/signed_settings.cc b/chrome/browser/chromeos/login/signed_settings.cc index 9f56391..6fa426c 100644 --- a/chrome/browser/chromeos/login/signed_settings.cc +++ b/chrome/browser/chromeos/login/signed_settings.cc @@ -11,6 +11,7 @@ #include "base/memory/ref_counted.h" #include "base/stringprintf.h" #include "base/threading/thread_restrictions.h" +#include "base/values.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chromeos/cros/cros_library.h" #include "chrome/browser/chromeos/cros_settings_names.h" @@ -163,7 +164,7 @@ class StorePropertyOp : public SignedSettings, public SignedSettings::Delegate<bool> { public: StorePropertyOp(const std::string& name, - const std::string& value, + const base::Value& value, SignedSettings::Delegate<bool>* d); virtual ~StorePropertyOp(); void Execute(); @@ -177,7 +178,7 @@ class StorePropertyOp : public SignedSettings, private: void SetInPolicy(const std::string& prop, - const std::string& value, + const base::Value& value, em::PolicyData* poldata); // Always call d_->OnSettingOpCompleted() via this call. // It guarantees that the callback will not be triggered until _after_ @@ -186,7 +187,7 @@ class StorePropertyOp : public SignedSettings, void PerformCallback(SignedSettings::ReturnCode code, bool value); std::string name_; - std::string value_; + scoped_ptr<base::Value> value_; SignedSettings::Delegate<bool>* d_; em::PolicyFetchResponse to_store_; scoped_refptr<SignedSettings> store_op_; @@ -195,29 +196,26 @@ class StorePropertyOp : public SignedSettings, class RetrievePropertyOp : public SignedSettings { public: RetrievePropertyOp(const std::string& name, - SignedSettings::Delegate<std::string>* d); + SignedSettings::Delegate<const base::Value*>* d); virtual ~RetrievePropertyOp(); void Execute(); void Fail(SignedSettings::ReturnCode code); - void Succeed(const std::string& value); + void Succeed(const base::Value* value); // Implementation of OwnerManager::Delegate::OnKeyOpComplete() void OnKeyOpComplete(const OwnerManager::KeyOpCode return_code, const std::vector<uint8>& payload); private: - static const char* kVeritas[]; - - std::string LookUpInPolicy(const std::string& prop); + base::Value* LookUpInPolicy(const std::string& prop); // Always call d_->OnSettingOpCompleted() via this call. // It guarantees that the callback will not be triggered until _after_ // Execute() returns, which is implicitly assumed by SignedSettingsHelper // in some cases. void PerformCallback(SignedSettings::ReturnCode code, - const std::string& value); + const base::Value* value); std::string name_; - std::string value_; - SignedSettings::Delegate<std::string>* d_; + SignedSettings::Delegate<const base::Value*>* d_; }; class StorePolicyOp : public SignedSettings { @@ -295,7 +293,7 @@ SignedSettings* SignedSettings::CreateWhitelistOp( // static SignedSettings* SignedSettings::CreateStorePropertyOp( const std::string& name, - const std::string& value, + const base::Value& value, SignedSettings::Delegate<bool>* d) { DCHECK(d != NULL); return new StorePropertyOp(name, value, d); @@ -304,7 +302,7 @@ SignedSettings* SignedSettings::CreateStorePropertyOp( // static SignedSettings* SignedSettings::CreateRetrievePropertyOp( const std::string& name, - SignedSettings::Delegate<std::string>* d) { + SignedSettings::Delegate<const base::Value*>* d) { DCHECK(d != NULL); return new RetrievePropertyOp(name, d); } @@ -520,10 +518,10 @@ void WhitelistOp::PerformCallback(SignedSettings::ReturnCode code, bool value) { } StorePropertyOp::StorePropertyOp(const std::string& name, - const std::string& value, + const base::Value& value, SignedSettings::Delegate<bool>* d) : name_(name), - value_(value), + value_(value.DeepCopy()), d_(d), store_op_(NULL) { } @@ -534,7 +532,7 @@ void StorePropertyOp::Execute() { if (service_->GetStatus(true) != OwnershipService::OWNERSHIP_TAKEN) { if (g_browser_process && g_browser_process->local_state() && - SignedSettingsTempStorage::Store(name_, value_, + SignedSettingsTempStorage::Store(name_, *value_, g_browser_process->local_state())) { Succeed(true); return; @@ -547,7 +545,7 @@ void StorePropertyOp::Execute() { // Posts a task to the FILE thread to sign policy. em::PolicyData to_sign; to_sign.CheckTypeAndMergeFrom(service_->cached_policy()); - SetInPolicy(name_, value_, &to_sign); + SetInPolicy(name_, *value_, &to_sign); to_store_.set_policy_data(to_sign.SerializeAsString()); service_->StartSigningAttempt(to_store_.policy_data(), this); } @@ -598,39 +596,72 @@ void StorePropertyOp::OnSettingsOpCompleted(ReturnCode code, bool value) { } void StorePropertyOp::SetInPolicy(const std::string& prop, - const std::string& value, + const base::Value& value, em::PolicyData* poldata) { em::ChromeDeviceSettingsProto pol; pol.ParseFromString(poldata->policy_value()); if (prop == kAccountsPrefAllowNewUser) { em::AllowNewUsersProto* allow = pol.mutable_allow_new_users(); - allow->set_allow_new_users(value == "true"); - + bool allow_value; + if (value.GetAsBoolean(&allow_value)) + allow->set_allow_new_users(allow_value); + else + NOTREACHED(); } else if (prop == kAccountsPrefAllowGuest) { em::GuestModeEnabledProto* guest = pol.mutable_guest_mode_enabled(); - guest->set_guest_mode_enabled(value == "true"); - + bool guest_value; + if (value.GetAsBoolean(&guest_value)) + guest->set_guest_mode_enabled(guest_value); + else + NOTREACHED(); } else if (prop == kAccountsPrefShowUserNamesOnSignIn) { em::ShowUserNamesOnSigninProto* show = pol.mutable_show_user_names(); - show->set_show_user_names(value == "true"); - + bool show_value; + if (value.GetAsBoolean(&show_value)) + show->set_show_user_names(show_value); + else + NOTREACHED(); } else if (prop == kSignedDataRoamingEnabled) { em::DataRoamingEnabledProto* roam = pol.mutable_data_roaming_enabled(); - roam->set_data_roaming_enabled(value == "true"); - + bool roaming_value; + if (value.GetAsBoolean(&roaming_value)) + roam->set_data_roaming_enabled(roaming_value); + else + NOTREACHED(); } else if (prop == kSettingProxyEverywhere) { // TODO(cmasone): NOTIMPLEMENTED() once http://crosbug.com/13052 is fixed. - bool success = pol.mutable_device_proxy_settings()->ParseFromString(value); - DCHECK(success); - + std::string proxy_value; + if (value.GetAsString(&proxy_value)) { + bool success = + pol.mutable_device_proxy_settings()->ParseFromString(proxy_value); + DCHECK(success); + } else { + NOTREACHED(); + } } else if (prop == kReleaseChannel) { em::ReleaseChannelProto* release_channel = pol.mutable_release_channel(); - release_channel->set_release_channel(value); - + 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 = pol.mutable_metrics_enabled(); - metrics->set_metrics_enabled(value == "true"); - + bool metrics_value; + if (value.GetAsBoolean(&metrics_value)) + metrics->set_metrics_enabled(metrics_value); + else + NOTREACHED(); + } else if (prop == kAccountsPrefUsers) { + em::UserWhitelistProto* whitelist_proto = pol.mutable_user_whitelist(); + whitelist_proto->clear_user_whitelist(); + const base::ListValue& users = static_cast<const base::ListValue&>(value); + 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.c_str()); + } } else { NOTREACHED(); } @@ -642,11 +673,9 @@ void StorePropertyOp::PerformCallback(SignedSettings::ReturnCode code, d_->OnSettingsOpCompleted(code, value); } -// static -const char* RetrievePropertyOp::kVeritas[] = { "false", "true" }; - -RetrievePropertyOp::RetrievePropertyOp(const std::string& name, - SignedSettings::Delegate<std::string>* d) +RetrievePropertyOp::RetrievePropertyOp( + const std::string& name, + SignedSettings::Delegate<const base::Value*>* d) : name_(name), d_(d) { } @@ -654,6 +683,7 @@ RetrievePropertyOp::RetrievePropertyOp(const std::string& name, RetrievePropertyOp::~RetrievePropertyOp() {} void RetrievePropertyOp::Execute() { + base::Value* value; // TODO(dilmah): Fix the race: // At the moment when device becomes owned there is lapse of time after // device has been owned and before temp_storage settings are finally @@ -663,8 +693,8 @@ void RetrievePropertyOp::Execute() { if (g_browser_process && g_browser_process->local_state() && SignedSettingsTempStorage::Retrieve( - name_, &value_, g_browser_process->local_state())) { - Succeed(value_); + name_, &value, g_browser_process->local_state())) { + Succeed(value->DeepCopy()); return; } } @@ -673,8 +703,8 @@ void RetrievePropertyOp::Execute() { TryToFetchPolicyAndCallBack(); return; } - std::string value = LookUpInPolicy(name_); - if (value.empty()) + value = LookUpInPolicy(name_); + if (!value) Fail(NOT_FOUND); else Succeed(value); @@ -683,14 +713,15 @@ void RetrievePropertyOp::Execute() { void RetrievePropertyOp::Fail(SignedSettings::ReturnCode code) { BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, - base::Bind(&RetrievePropertyOp::PerformCallback, this, code, - std::string())); + base::Bind(&RetrievePropertyOp::PerformCallback, this, + code, static_cast<const base::Value*>(NULL))); } -void RetrievePropertyOp::Succeed(const std::string& value) { +void RetrievePropertyOp::Succeed(const base::Value* value) { BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, - base::Bind(&RetrievePropertyOp::PerformCallback, this, SUCCESS, value)); + base::Bind(&RetrievePropertyOp::PerformCallback, this, + SUCCESS, base::Owned(value))); } // DEPRECATED. @@ -700,12 +731,11 @@ void RetrievePropertyOp::OnKeyOpComplete( NOTREACHED(); } -std::string RetrievePropertyOp::LookUpInPolicy(const std::string& prop) { +base::Value* RetrievePropertyOp::LookUpInPolicy(const std::string& prop) { if (prop == kDeviceOwner) { const em::PolicyData& data = service_->cached_policy(); if (data.has_username() && !data.has_request_token()) - return data.username(); - return ""; + return base::Value::CreateStringValue(data.username()); } VLOG(2) << "Looking up " << prop; em::ChromeDeviceSettingsProto pol; @@ -714,63 +744,83 @@ std::string RetrievePropertyOp::LookUpInPolicy(const std::string& prop) { if (pol.has_allow_new_users() && pol.allow_new_users().has_allow_new_users() && pol.allow_new_users().allow_new_users()) { - return kVeritas[1]; // New users allowed, user_whitelist() ignored. + // New users allowed, user_whitelist() ignored. + return base::Value::CreateBooleanValue(true); } // If we have the allow_new_users bool, and it is true, we honor that above. // In all other cases (don't have it, have it and it is set to false, etc), // We will honor the user_whitelist() if it is there and populated. - // Otherwise, fail open (to do otherwise could render the device unusable). + // Otherwise we default to allowing new users. if (!pol.has_user_whitelist()) - return kVeritas[1]; // Default to allowing new users. - return kVeritas[pol.user_whitelist().user_whitelist_size() == 0]; + return base::Value::CreateBooleanValue(true); + return base::Value::CreateBooleanValue( + pol.user_whitelist().user_whitelist_size() == 0); } else if (prop == kAccountsPrefAllowGuest) { if (!pol.has_guest_mode_enabled() || !pol.guest_mode_enabled().has_guest_mode_enabled()) { - return kVeritas[1]; // Default to allowing guests; + // Default to allowing guests; + return base::Value::CreateBooleanValue(true); } - return kVeritas[pol.guest_mode_enabled().guest_mode_enabled()]; + return base::Value::CreateBooleanValue( + pol.guest_mode_enabled().guest_mode_enabled()); } else if (prop == kAccountsPrefShowUserNamesOnSignIn) { if (!pol.has_show_user_names() || !pol.show_user_names().has_show_user_names()) { - return kVeritas[1]; // Default to showing pods on the login screen; + // Default to showing pods on the login screen; + return base::Value::CreateBooleanValue(true); } - return kVeritas[pol.show_user_names().show_user_names()]; + return base::Value::CreateBooleanValue( + pol.show_user_names().show_user_names()); } else if (prop == kSignedDataRoamingEnabled) { if (!pol.has_data_roaming_enabled() || !pol.data_roaming_enabled().has_data_roaming_enabled()) { - return kVeritas[0]; // Default to disabling cellular data roaming; + // Default to disabling cellular data roaming; + return base::Value::CreateBooleanValue(false); } - return kVeritas[pol.data_roaming_enabled().data_roaming_enabled()]; + return base::Value::CreateBooleanValue( + pol.data_roaming_enabled().data_roaming_enabled()); } else if (prop == kSettingProxyEverywhere) { // TODO(cmasone): NOTIMPLEMENTED() once http://crosbug.com/13052 is fixed. std::string serialized; - if (!pol.has_device_proxy_settings() || - !pol.device_proxy_settings().SerializeToString(&serialized)) { - return ""; // Default to invalid proxy config (will be ignored). + if (pol.has_device_proxy_settings() && + pol.device_proxy_settings().SerializeToString(&serialized)) { + return base::Value::CreateStringValue(serialized); } - return serialized; } else if (prop == kReleaseChannel) { if (!pol.has_release_channel() || !pol.release_channel().has_release_channel()) { - return ""; // Default to an invalid channel (will be ignored). + // Default to an invalid channel (will be ignored). + return base::Value::CreateStringValue(""); } - return pol.release_channel().release_channel(); + return base::Value::CreateStringValue( + pol.release_channel().release_channel()); } else if (prop == kStatsReportingPref) { if (pol.has_metrics_enabled()) { - return kVeritas[pol.metrics_enabled().metrics_enabled()]; + return base::Value::CreateBooleanValue( + pol.metrics_enabled().metrics_enabled()); + } + } else if (prop == kAccountsPrefUsers) { + base::ListValue* list = new base::ListValue(); + const em::UserWhitelistProto& whitelist_proto = pol.user_whitelist(); + const RepeatedPtrField<string>& whitelist = + whitelist_proto.user_whitelist(); + for (RepeatedPtrField<string>::const_iterator it = whitelist.begin(); + it != whitelist.end(); ++it) { + list->Append(base::Value::CreateStringValue(*it)); } + return list; } - return std::string(); + return NULL; } void RetrievePropertyOp::PerformCallback(SignedSettings::ReturnCode code, - const std::string& value) { + const base::Value* value) { d_->OnSettingsOpCompleted(code, value); } diff --git a/chrome/browser/chromeos/login/signed_settings.h b/chrome/browser/chromeos/login/signed_settings.h index a423aae..a483962 100644 --- a/chrome/browser/chromeos/login/signed_settings.h +++ b/chrome/browser/chromeos/login/signed_settings.h @@ -30,6 +30,10 @@ // and then call the appropriate method of the Delegate you passed in // -- again, on the UI thread. +namespace base { +class Value; +} // namespace base + namespace enterprise_management { class PolicyFetchResponse; class PolicyData; @@ -74,12 +78,12 @@ class SignedSettings : public base::RefCountedThreadSafe<SignedSettings>, // one type can be in flight at a time. static SignedSettings* CreateStorePropertyOp( const std::string& name, - const std::string& value, + const base::Value& value, SignedSettings::Delegate<bool>* d); static SignedSettings* CreateRetrievePropertyOp( const std::string& name, - SignedSettings::Delegate<std::string>* d); + SignedSettings::Delegate<const base::Value*>* d); // These are both "policy" operations, and only one instance of // one type can be in flight at a time. diff --git a/chrome/browser/chromeos/login/signed_settings_helper.cc b/chrome/browser/chromeos/login/signed_settings_helper.cc index 2c2309d..e5ea6fb 100644 --- a/chrome/browser/chromeos/login/signed_settings_helper.cc +++ b/chrome/browser/chromeos/login/signed_settings_helper.cc @@ -10,6 +10,7 @@ #include "base/lazy_instance.h" #include "base/logging.h" #include "base/memory/ref_counted.h" +#include "base/values.h" #include "chrome/browser/chromeos/login/signed_settings.h" #include "chrome/browser/policy/proto/device_management_backend.pb.h" #include "content/public/browser/browser_thread.h" @@ -170,12 +171,12 @@ class StorePropertyOpContext : public SignedSettings::Delegate<bool>, public OpContext { public: StorePropertyOpContext(const std::string& name, - const std::string& value, + const base::Value& value, SignedSettingsHelper::Callback* callback, OpContext::Delegate* delegate) : OpContext(callback, delegate), name_(name), - value_(value) { + value_(value.DeepCopy()) { } // chromeos::SignedSettings::Delegate implementation @@ -183,25 +184,25 @@ class StorePropertyOpContext : public SignedSettings::Delegate<bool>, bool unused) OVERRIDE { VLOG(2) << "OnSettingsOpCompleted, code = " << code; if (callback_) - callback_->OnStorePropertyCompleted(code, name_, value_); + callback_->OnStorePropertyCompleted(code, name_, *value_); OnOpCompleted(); } protected: // OpContext implemenetation virtual void CreateOp() OVERRIDE { - op_ = SignedSettings::CreateStorePropertyOp(name_, value_, this); + op_ = SignedSettings::CreateStorePropertyOp(name_, *value_, this); } private: std::string name_; - std::string value_; + scoped_ptr<base::Value> value_; DISALLOW_COPY_AND_ASSIGN(StorePropertyOpContext); }; class RetrievePropertyOpContext - : public SignedSettings::Delegate<std::string>, + : public SignedSettings::Delegate<const base::Value*>, public OpContext { public: RetrievePropertyOpContext(const std::string& name, @@ -213,7 +214,7 @@ class RetrievePropertyOpContext // chromeos::SignedSettings::Delegate implementation virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code, - std::string value) OVERRIDE { + const base::Value* value) OVERRIDE { if (callback_) callback_->OnRetrievePropertyCompleted(code, name_, value); @@ -303,7 +304,7 @@ class SignedSettingsHelperImpl : public SignedSettingsHelper, bool add_to_whitelist, Callback* callback) OVERRIDE; virtual void StartStorePropertyOp(const std::string& name, - const std::string& value, + const base::Value& value, Callback* callback) OVERRIDE; virtual void StartRetrieveProperty(const std::string& name, Callback* callback) OVERRIDE; @@ -367,7 +368,7 @@ void SignedSettingsHelperImpl::StartWhitelistOp( void SignedSettingsHelperImpl::StartStorePropertyOp( const std::string& name, - const std::string& value, + const base::Value& value, SignedSettingsHelper::Callback* callback) { AddOpContext(new StorePropertyOpContext( name, diff --git a/chrome/browser/chromeos/login/signed_settings_helper.h b/chrome/browser/chromeos/login/signed_settings_helper.h index 406ba7b..1d1411c 100644 --- a/chrome/browser/chromeos/login/signed_settings_helper.h +++ b/chrome/browser/chromeos/login/signed_settings_helper.h @@ -10,6 +10,10 @@ #include "chrome/browser/chromeos/login/signed_settings.h" +namespace base { +class Value; +} // namespace base + namespace enterprise_management { class PolicyFetchResponse; } // namespace enterprise_management @@ -42,13 +46,13 @@ class SignedSettingsHelper { virtual void OnStorePropertyCompleted( SignedSettings::ReturnCode code, const std::string& name, - const std::string& value) {} + const base::Value& value) {} // Callback of RetrievePropertyOp. virtual void OnRetrievePropertyCompleted( SignedSettings::ReturnCode code, const std::string& name, - const std::string& value) {} + const base::Value* value) {} // Callback of StorePolicyOp. virtual void OnStorePolicyCompleted( @@ -70,7 +74,7 @@ class SignedSettingsHelper { bool add_to_whitelist, Callback* callback) = 0; virtual void StartStorePropertyOp(const std::string& name, - const std::string& value, + const base::Value& value, Callback* callback) = 0; virtual void StartRetrieveProperty(const std::string& name, Callback* callback) = 0; diff --git a/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc b/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc index 7f19868..7b5ad5b 100644 --- a/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc +++ b/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc @@ -41,11 +41,11 @@ class MockSignedSettingsHelperCallback : public SignedSettingsHelper::Callback { MOCK_METHOD3(OnStorePropertyCompleted, void( SignedSettings::ReturnCode code, const std::string& name, - const std::string& value)); + const base::Value& value)); MOCK_METHOD3(OnRetrievePropertyCompleted, void( SignedSettings::ReturnCode code, const std::string& name, - const std::string& value)); + const base::Value* value)); }; class SignedSettingsHelperTest : public testing::Test, @@ -53,7 +53,7 @@ class SignedSettingsHelperTest : public testing::Test, public: SignedSettingsHelperTest() : fake_email_("fakey@example.com"), - fake_prop_(kAccountsPrefAllowGuest), + fake_prop_(kReleaseChannel), fake_value_("false"), message_loop_(MessageLoop::TYPE_UI), ui_thread_(BrowserThread::UI, &message_loop_), @@ -100,7 +100,7 @@ class SignedSettingsHelperTest : public testing::Test, const std::string fake_email_; const std::string fake_prop_; - const std::string fake_value_; + const base::StringValue fake_value_; MockOwnershipService m_; MessageLoop message_loop_; diff --git a/chrome/browser/chromeos/login/signed_settings_temp_storage.cc b/chrome/browser/chromeos/login/signed_settings_temp_storage.cc index aa4767e..4435b99 100644 --- a/chrome/browser/chromeos/login/signed_settings_temp_storage.cc +++ b/chrome/browser/chromeos/login/signed_settings_temp_storage.cc @@ -22,13 +22,13 @@ void SignedSettingsTempStorage::RegisterPrefs(PrefService* local_state) { // static bool SignedSettingsTempStorage::Store(const std::string& name, - const std::string& value, + const base::Value& value, PrefService* local_state) { if (local_state) { DictionaryPrefUpdate temp_storage_update( local_state, prefs::kSignedSettingsTempStorage); temp_storage_update->SetWithoutPathExpansion( - name, Value::CreateStringValue(value)); + name, value.DeepCopy()); return true; } return false; @@ -36,13 +36,13 @@ bool SignedSettingsTempStorage::Store(const std::string& name, // static bool SignedSettingsTempStorage::Retrieve(const std::string& name, - std::string* value, + base::Value** value, PrefService* local_state) { if (local_state) { const DictionaryValue* temp_storage = local_state->GetDictionary(prefs::kSignedSettingsTempStorage); if (temp_storage && temp_storage->HasKey(name)) { - temp_storage->GetStringWithoutPathExpansion(name, value); + temp_storage->GetWithoutPathExpansion(name, value); return true; } } @@ -63,9 +63,11 @@ void SignedSettingsTempStorage::Finalize(PrefService* local_state) { for (DictionaryValue::key_iterator it = temp_storage->begin_keys(); it != temp_storage->end_keys(); ++it) { - std::string value; - temp_storage->GetStringWithoutPathExpansion(*it, &value); - SignedSettingsHelper::Get()->StartStorePropertyOp(*it, value, NULL); + base::Value* value = NULL; + bool get_result = temp_storage->GetWithoutPathExpansion(*it, &value); + DCHECK(value && get_result); + if (value) + SignedSettingsHelper::Get()->StartStorePropertyOp(*it, *value, NULL); } local_state->ClearPref(prefs::kSignedSettingsTempStorage); } diff --git a/chrome/browser/chromeos/login/signed_settings_temp_storage.h b/chrome/browser/chromeos/login/signed_settings_temp_storage.h index f7ae0f5..6633c99 100644 --- a/chrome/browser/chromeos/login/signed_settings_temp_storage.h +++ b/chrome/browser/chromeos/login/signed_settings_temp_storage.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -9,6 +9,7 @@ #include <string> #include "base/basictypes.h" +#include "base/values.h" class PrefService; @@ -24,10 +25,10 @@ class SignedSettingsTempStorage { static void RegisterPrefs(PrefService* local_state); static bool Store(const std::string& name, - const std::string& value, + const base::Value& value, PrefService* local_state); static bool Retrieve(const std::string& name, - std::string* value, + base::Value** value, PrefService* local_state); // Call this after owner has been assigned to persist settings diff --git a/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc b/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc index c6e4131..ac1a9a3 100644 --- a/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc +++ b/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc @@ -21,32 +21,34 @@ namespace chromeos { class SignedSettingsTempStorageTest : public testing::Test { protected: virtual void SetUp() { - ref_map_["some_stuff"] = "a=35;code=64"; - ref_map_["another_stuff"] = ""; - ref_map_["name"] = "value"; - ref_map_["2bc6aa16-e0ea-11df-b13d-18a90520e2e5"] = "512"; + ref_map_.Set("some_stuff", base::Value::CreateStringValue("a=35;code=64")); + ref_map_.Set("another_stuff", base::Value::CreateBooleanValue(false)); + ref_map_.Set("name", base::Value::CreateStringValue("value")); + ref_map_.Set("2bc6aa16-e0ea-11df-b13d-18a90520e2e5", + base::Value::CreateIntegerValue(512)); SignedSettingsTempStorage::RegisterPrefs(&local_state_); } - std::map<std::string, std::string> ref_map_; + base::DictionaryValue ref_map_; TestingPrefService local_state_; }; TEST_F(SignedSettingsTempStorageTest, Basic) { - typedef std::map<std::string, std::string>::iterator It; - for (It it = ref_map_.begin(); it != ref_map_.end(); ++it) { - EXPECT_TRUE(SignedSettingsTempStorage::Store(it->first, - it->second, + typedef base::DictionaryValue::key_iterator It; + base::Value* value; + for (It it = ref_map_.begin_keys(); it != ref_map_.end_keys(); ++it) { + ref_map_.Get(*it, &value); + EXPECT_TRUE(SignedSettingsTempStorage::Store(*it, *value, &local_state_)); } - for (It it = ref_map_.begin(); it != ref_map_.end(); ++it) { - std::string value; - EXPECT_TRUE(SignedSettingsTempStorage::Retrieve(it->first, &value, + for (It it = ref_map_.begin_keys(); it != ref_map_.end_keys(); ++it) { + EXPECT_TRUE(SignedSettingsTempStorage::Retrieve(*it, &value, &local_state_)); - EXPECT_EQ(it->second, value); + base::Value* orignal_value; + ref_map_.Get(*it, &orignal_value); + EXPECT_TRUE(orignal_value->Equals(value)); } - std::string value; EXPECT_FALSE(SignedSettingsTempStorage::Retrieve("non-existent tv-series", &value, &local_state_)); diff --git a/chrome/browser/chromeos/login/signed_settings_unittest.cc b/chrome/browser/chromeos/login/signed_settings_unittest.cc index 7dc197b..e6ca1f1 100644 --- a/chrome/browser/chromeos/login/signed_settings_unittest.cc +++ b/chrome/browser/chromeos/login/signed_settings_unittest.cc @@ -78,7 +78,21 @@ class NormalDelegate : public DummyDelegate<T> { virtual ~NormalDelegate() {} protected: virtual void compare_expected(T to_compare) { - EXPECT_EQ(this->expected_, to_compare); // without this-> this won't build. + // without this-> this won't build. + EXPECT_EQ(this->expected_, to_compare); + } +}; + +// Specialized version for Value objects because these compare differently. +class PolicyDelegate : public DummyDelegate<const base::Value*> { + public: + explicit PolicyDelegate(const base::Value* to_expect) + : DummyDelegate<const base::Value*>(to_expect) {} + virtual ~PolicyDelegate() {} + protected: + virtual void compare_expected(const base::Value* to_compare) { + // without this-> this won't build. + EXPECT_TRUE(this->expected_->Equals(to_compare)); } }; @@ -105,7 +119,11 @@ class SignedSettingsTest : public testing::Test { : fake_email_("fakey@example.com"), fake_domain_("*@example.com"), fake_prop_(kAccountsPrefAllowGuest), - fake_value_("false"), + fake_signature_("false"), + fake_value_(false), + fake_value_signature_( + fake_signature_.c_str(), + fake_signature_.c_str() + fake_signature_.length()), message_loop_(MessageLoop::TYPE_UI), ui_thread_(BrowserThread::UI, &message_loop_), file_thread_(BrowserThread::FILE), @@ -248,9 +266,9 @@ class SignedSettingsTest : public testing::Test { } void DoRetrieveProperty(const std::string& name, - const std::string& value, + const base::Value* value, em::PolicyData* fake_pol) { - NormalDelegate<std::string> d(value); + PolicyDelegate d(value); d.expect_success(); scoped_refptr<SignedSettings> s( SignedSettings::CreateRetrievePropertyOp(name, &d)); @@ -270,7 +288,9 @@ class SignedSettingsTest : public testing::Test { const std::string fake_email_; const std::string fake_domain_; const std::string fake_prop_; - const std::string fake_value_; + const std::string fake_signature_; + const base::FundamentalValue fake_value_; + const std::vector<uint8> fake_value_signature_; MockOwnershipService m_; ScopedTempDir tmpdir_; @@ -497,42 +517,48 @@ TEST_F(SignedSettingsTest, StorePropertyFailed) { TEST_F(SignedSettingsTest, RetrieveProperty) { em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>()); - DoRetrieveProperty(fake_prop_, fake_value_, &fake_pol); + base::FundamentalValue fake_value(false); + DoRetrieveProperty(fake_prop_, &fake_value, &fake_pol); } TEST_F(SignedSettingsTest, RetrieveOwnerProperty) { em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>()); fake_pol.set_username(fake_email_); - DoRetrieveProperty(kDeviceOwner, fake_email_, &fake_pol); + base::StringValue fake_value(fake_email_); + DoRetrieveProperty(kDeviceOwner, &fake_value, &fake_pol); } TEST_F(SignedSettingsTest, ExplicitlyAllowNewUsers) { em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>()); SetAllowNewUsers(true, &fake_pol); - DoRetrieveProperty(kAccountsPrefAllowNewUser, "true", &fake_pol); + base::FundamentalValue fake_value(true); + DoRetrieveProperty(kAccountsPrefAllowNewUser, &fake_value, &fake_pol); } TEST_F(SignedSettingsTest, ExplicitlyDisallowNewUsers) { std::vector<std::string> whitelist(1, fake_email_ + "m"); em::PolicyData fake_pol = BuildPolicyData(whitelist); SetAllowNewUsers(false, &fake_pol); - DoRetrieveProperty(kAccountsPrefAllowNewUser, "false", &fake_pol); + base::FundamentalValue fake_value(false); + DoRetrieveProperty(kAccountsPrefAllowNewUser, &fake_value, &fake_pol); } TEST_F(SignedSettingsTest, ImplicitlyDisallowNewUsers) { std::vector<std::string> whitelist(1, fake_email_ + "m"); em::PolicyData fake_pol = BuildPolicyData(whitelist); - DoRetrieveProperty(kAccountsPrefAllowNewUser, "false", &fake_pol); + base::FundamentalValue fake_value(false); + DoRetrieveProperty(kAccountsPrefAllowNewUser, &fake_value, &fake_pol); } TEST_F(SignedSettingsTest, AccidentallyDisallowNewUsers) { em::PolicyData fake_pol = BuildPolicyData(std::vector<std::string>()); SetAllowNewUsers(false, &fake_pol); - DoRetrieveProperty(kAccountsPrefAllowNewUser, "true", &fake_pol); + base::FundamentalValue fake_value(true); + DoRetrieveProperty(kAccountsPrefAllowNewUser, &fake_value, &fake_pol); } TEST_F(SignedSettingsTest, RetrievePropertyNotFound) { - NormalDelegate<std::string> d(fake_value_); + PolicyDelegate d(&fake_value_); d.expect_failure(SignedSettings::NOT_FOUND); scoped_refptr<SignedSettings> s( SignedSettings::CreateRetrievePropertyOp("unknown_prop", &d)); @@ -551,7 +577,8 @@ TEST_F(SignedSettingsTest, RetrievePropertyNotFound) { } TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) { - NormalDelegate<std::string> d(fake_value_); + base::FundamentalValue fake_value(false); + PolicyDelegate d(&fake_value); d.expect_success(); scoped_refptr<SignedSettings> s( SignedSettings::CreateRetrievePropertyOp(fake_prop_, &d)); @@ -560,7 +587,7 @@ TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) { std::string data = fake_pol.SerializeAsString(); std::string signed_serialized; em::PolicyFetchResponse signed_policy = BuildProto(data, - fake_value_, + fake_signature_, &signed_serialized); MockSessionManagerClient* client = mock_dbus_thread_manager_->mock_session_manager_client(); @@ -582,10 +609,8 @@ TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) { EXPECT_CALL(m_, cached_policy()) .WillOnce(ReturnRef(out_pol)); - std::vector<uint8> fake_sig(fake_value_.c_str(), - fake_value_.c_str() + fake_value_.length()); - EXPECT_CALL(m_, StartVerifyAttempt(data, fake_sig, _)) - .WillOnce(FinishKeyOp(fake_sig)) + EXPECT_CALL(m_, StartVerifyAttempt(data, fake_value_signature_, _)) + .WillOnce(FinishKeyOp(fake_value_signature_)) .RetiresOnSaturation(); s->Execute(); @@ -619,17 +644,14 @@ TEST_F(SignedSettingsTest, SignAndStorePolicy) { // Fake out a successful signing. std::string signed_serialized; em::PolicyFetchResponse signed_policy = BuildProto(data_serialized, - fake_value_, + fake_signature_, &signed_serialized); - std::vector<uint8> fake_sig(fake_value_.c_str(), - fake_value_.c_str() + fake_value_.length()); - MockSessionManagerClient* client = mock_dbus_thread_manager_->mock_session_manager_client(); EXPECT_CALL(*client, StorePolicy(signed_serialized, _)) .WillOnce(Store(true)) .RetiresOnSaturation(); - s->OnKeyOpComplete(OwnerManager::SUCCESS, fake_sig); + s->OnKeyOpComplete(OwnerManager::SUCCESS, fake_value_signature_); message_loop_.RunAllPending(); } @@ -641,7 +663,7 @@ TEST_F(SignedSettingsTest, StoreSignedPolicy) { std::string serialized = in_pol.SerializeAsString(); std::string signed_serialized; em::PolicyFetchResponse signed_policy = BuildProto(serialized, - fake_value_, + fake_signature_, &signed_serialized); scoped_refptr<SignedSettings> s( SignedSettings::CreateStorePolicyOp(&signed_policy, &d)); @@ -688,7 +710,7 @@ TEST_F(SignedSettingsTest, RetrievePolicy) { std::string serialized = in_pol.SerializeAsString(); std::string signed_serialized; em::PolicyFetchResponse signed_policy = BuildProto(serialized, - fake_value_, + fake_signature_, &signed_serialized); ProtoDelegate d(signed_policy); d.expect_success(); @@ -701,9 +723,7 @@ TEST_F(SignedSettingsTest, RetrievePolicy) { .RetiresOnSaturation(); mock_service(s.get(), &m_); - std::vector<uint8> fake_sig(fake_value_.c_str(), - fake_value_.c_str() + fake_value_.length()); - EXPECT_CALL(m_, StartVerifyAttempt(serialized, fake_sig, _)) + EXPECT_CALL(m_, StartVerifyAttempt(serialized, fake_value_signature_, _)) .Times(1); em::PolicyData out_pol; EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>())) @@ -771,7 +791,7 @@ TEST_F(SignedSettingsTest, RetrieveUnsignedPolicy) { TEST_F(SignedSettingsTest, RetrieveMalsignedPolicy) { std::string signed_serialized; em::PolicyFetchResponse signed_policy = BuildProto(fake_prop_, - fake_value_, + fake_signature_, &signed_serialized); ProtoDelegate d(signed_policy); d.expect_failure(SignedSettings::BAD_SIGNATURE); @@ -784,9 +804,7 @@ TEST_F(SignedSettingsTest, RetrieveMalsignedPolicy) { .RetiresOnSaturation(); mock_service(s.get(), &m_); - std::vector<uint8> fake_sig(fake_value_.c_str(), - fake_value_.c_str() + fake_value_.length()); - EXPECT_CALL(m_, StartVerifyAttempt(fake_prop_, fake_sig, _)) + EXPECT_CALL(m_, StartVerifyAttempt(fake_prop_, fake_value_signature_, _)) .Times(1); s->Execute(); diff --git a/chrome/browser/chromeos/login/wizard_controller.cc b/chrome/browser/chromeos/login/wizard_controller.cc index 86fed61..fde6176 100644 --- a/chrome/browser/chromeos/login/wizard_controller.cc +++ b/chrome/browser/chromeos/login/wizard_controller.cc @@ -382,9 +382,10 @@ void WizardController::OnEulaAccepted() { // But we can at least create the consent file and Chrome would port that // if the device is owned by a local user. In case of enterprise enrolled // device the setting will be respected only until the policy is not set. + base::FundamentalValue value(usage_statistics_reporting_); SignedSettingsTempStorage::Store( kStatsReportingPref, - (usage_statistics_reporting_ ? "true" : "false"), + value, g_browser_process->local_state()); bool enabled = OptionsUtil::ResolveMetricsReportingEnabled(usage_statistics_reporting_); diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc index 97b6aef..bd37a46 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl.cc +++ b/chrome/browser/chromeos/proxy_config_service_impl.cc @@ -526,17 +526,23 @@ void ProxyConfigServiceImpl::OnProxyConfigChanged( void ProxyConfigServiceImpl::OnSettingsOpCompleted( SignedSettings::ReturnCode code, - std::string value) { + const base::Value* value) { + // We assume ownership here to make sure this gets deleted no matter where + // this function ends. + scoped_ptr<const base::Value> own_value(value); + retrieve_property_op_ = NULL; if (code != SignedSettings::SUCCESS) { LOG(WARNING) << this << ": Error retrieving proxy setting from device"; device_config_.clear(); return; } - VLOG(1) << this << ": Retrieved proxy setting from device, value=[" - << value << "]"; + std::string policy_value; + value->GetAsString(&policy_value); + VLOG(1) << "Retrieved proxy setting from device, value=[" + << policy_value << "]"; ProxyConfig device_config; - if (!device_config.DeserializeForDevice(value) || + if (!device_config.DeserializeForDevice(policy_value) || !device_config.SerializeForNetwork(&device_config_)) { LOG(WARNING) << "Can't deserialize device setting or serialize for network"; device_config_.clear(); diff --git a/chrome/browser/chromeos/proxy_config_service_impl.h b/chrome/browser/chromeos/proxy_config_service_impl.h index b059080..4cb5b87 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl.h +++ b/chrome/browser/chromeos/proxy_config_service_impl.h @@ -35,7 +35,7 @@ namespace chromeos { // user profile class ProxyConfigServiceImpl : public PrefProxyConfigTrackerImpl, - public SignedSettings::Delegate<std::string>, + public SignedSettings::Delegate<const base::Value*>, public NetworkLibrary::NetworkManagerObserver, public NetworkLibrary::NetworkObserver { public: @@ -193,7 +193,7 @@ class ProxyConfigServiceImpl // Implementation for SignedSettings::Delegate virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code, - std::string value) OVERRIDE; + const base::Value* value) OVERRIDE; // NetworkLibrary::NetworkManagerObserver implementation. virtual void OnNetworkManagerChanged(NetworkLibrary* cros) OVERRIDE; diff --git a/chrome/browser/chromeos/user_cros_settings_provider.cc b/chrome/browser/chromeos/user_cros_settings_provider.cc index 97f61bb..fff561f 100644 --- a/chrome/browser/chromeos/user_cros_settings_provider.cc +++ b/chrome/browser/chromeos/user_cros_settings_provider.cc @@ -4,9 +4,6 @@ #include "chrome/browser/chromeos/user_cros_settings_provider.h" -#include <map> -#include <set> - #include "base/bind.h" #include "base/bind_helpers.h" #include "base/callback.h" @@ -24,6 +21,7 @@ #include "chrome/browser/chromeos/login/ownership_status_checker.h" #include "chrome/browser/chromeos/login/user_manager.h" #include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/prefs/pref_value_map.h" #include "chrome/browser/prefs/scoped_user_pref_update.h" #include "chrome/browser/ui/options/options_util.h" #include "chrome/common/chrome_notification_types.h" @@ -78,8 +76,8 @@ class MigrationHelper : public content::NotificationObserver { callback_ = callback; } - void AddMigrationValue(const std::string& path, const std::string& value) { - migration_values_[path] = value; + void AddMigrationValue(const std::string& path, base::Value* value) { + migration_values_.SetValue(path, value); } void MigrateValues(void) { @@ -106,21 +104,20 @@ class MigrationHelper : public content::NotificationObserver { // SignedSettingsTempStorage until the device is owned. If none of these // cases is met then we will wait for user change notification and retry. if (current_user_is_owner || status != OwnershipService::OWNERSHIP_TAKEN) { - std::map<std::string, std::string>::const_iterator i; + PrefValueMap::const_iterator i; for (i = migration_values_.begin(); i != migration_values_.end(); ++i) { // Queue all values for storing. - SignedSettingsHelper::Get()->StartStorePropertyOp(i->first, i->second, + SignedSettingsHelper::Get()->StartStorePropertyOp(i->first, *i->second, callback_); } - migration_values_.clear(); + migration_values_.Clear(); } } content::NotificationRegistrar registrar_; scoped_ptr<OwnershipStatusChecker> ownership_checker_; SignedSettingsHelper::Callback* callback_; - - std::map<std::string, std::string> migration_values_; + PrefValueMap migration_values_; DISALLOW_COPY_AND_ASSIGN(MigrationHelper); }; @@ -150,6 +147,12 @@ bool IsControlledListSetting(const std::string& pref_path) { kListSettings + arraysize(kListSettings); } +bool IsControlledSetting(const std::string& pref_path) { + return (IsControlledBooleanSetting(pref_path) || + IsControlledStringSetting(pref_path) || + IsControlledListSetting(pref_path)); +} + void RegisterSetting(PrefService* local_state, const std::string& pref_path) { local_state->RegisterBooleanPref((pref_path + kTrustedSuffix).c_str(), false, @@ -180,52 +183,17 @@ enum UseValue { USE_VALUE_DEFAULT }; -void UpdateCacheBool(const std::string& name, - bool value, - UseValue use_value) { - PrefService* prefs = g_browser_process->local_state(); - if (use_value == USE_VALUE_DEFAULT) - prefs->ClearPref(name.c_str()); - else - prefs->SetBoolean(name.c_str(), value); - prefs->ScheduleSavePersistentPrefs(); -} - -void UpdateCacheString(const std::string& name, - const std::string& value, - UseValue use_value) { +void UpdateCache(const std::string& name, + const base::Value* value, + UseValue use_value) { PrefService* prefs = g_browser_process->local_state(); if (use_value == USE_VALUE_DEFAULT) prefs->ClearPref(name.c_str()); else - prefs->SetString(name.c_str(), value); + prefs->Set(name.c_str(), *value); prefs->ScheduleSavePersistentPrefs(); } -// Helper function to parse the whitelist from the policy cache into the local -// state. -// TODO(pastarmovj): This function will disappear in step two of the refactoring -// as per design doc. (Contact pastarmovj@chromium.org for a link to it.) -bool GetUserWhitelist(ListValue* user_list) { - PrefService* prefs = g_browser_process->local_state(); - DCHECK(!prefs->IsManagedPreference(kAccountsPrefUsers)); - - std::vector<std::string> whitelist; - if (!SignedSettings::EnumerateWhitelist(&whitelist)) { - LOG(WARNING) << "Failed to retrieve user whitelist."; - return false; - } - - ListPrefUpdate cached_whitelist_update(prefs, kAccountsPrefUsers); - cached_whitelist_update->Clear(); - - for (size_t i = 0; i < whitelist.size(); ++i) - cached_whitelist_update->Append(Value::CreateStringValue(whitelist[i])); - - prefs->ScheduleSavePersistentPrefs(); - return true; -} - class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { public: static UserCrosSettingsTrust* GetInstance() { @@ -267,9 +235,11 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { StartFetchingSetting(kBooleanSettings[i]); for (size_t i = 0; i < arraysize(kStringSettings); ++i) StartFetchingSetting(kStringSettings[i]); + for (size_t i = 0; i < arraysize(kListSettings); ++i) + StartFetchingSetting(kListSettings[i]); } - void Set(const std::string& path, Value* in_value) { + void Set(const std::string& path, const base::Value& in_value) { PrefService* prefs = g_browser_process->local_state(); DCHECK(!prefs->IsManagedPreference(path.c_str())); @@ -280,33 +250,18 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { CrosSettings::Get()->FireObservers(path.c_str()); return; } - - if (IsControlledBooleanSetting(path)) { - bool bool_value = false; - if (in_value->GetAsBoolean(&bool_value)) { - OnBooleanPropertyChange(path, bool_value); - std::string value = bool_value ? kTrueIncantation : kFalseIncantation; - SignedSettingsHelper::Get()->StartStorePropertyOp(path, value, this); - UpdateCacheBool(path, bool_value, USE_VALUE_SUPPLIED); - - VLOG(1) << "Set cros setting " << path << "=" << value; - } - } else if (IsControlledStringSetting(path)) { - std::string value; - if (in_value->GetAsString(&value)) { - SignedSettingsHelper::Get()->StartStorePropertyOp(path, value, this); - UpdateCacheString(path, value, USE_VALUE_SUPPLIED); - - VLOG(1) << "Set cros setting " << path << "=" << value; - } else { - NOTREACHED() << "Unable to convert string value."; + if (IsControlledSetting(path)) { + if (IsControlledBooleanSetting(path)) { + bool bool_value = false; + if (in_value.GetAsBoolean(&bool_value)) { + OnBooleanPropertyChange(path, bool_value); + } } - } else if (path == kDeviceOwner) { - VLOG(1) << "Setting owner is not supported. Please use " - "'UpdateCachedOwner' instead."; - } else if (path == kAccountsPrefUsers) { - VLOG(1) << "Setting user whitelist is not implemented. Please use " - "whitelist/unwhitelist instead."; + SignedSettingsHelper::Get()->StartStorePropertyOp( + path, in_value, this); + UpdateCache(path, &in_value, USE_VALUE_SUPPLIED); + + LOG(ERROR) << "Set cros setting " << path; } else { LOG(WARNING) << "Try to set unhandled cros setting " << path; } @@ -349,9 +304,9 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { } // Called right after signed value was checked. - void OnBooleanPropertyRetrieve(const std::string& path, - bool value, - UseValue use_value) { + void OnPropertyRetrieve(const std::string& path, + const base::Value* value, + UseValue use_value) { if (path == kSignedDataRoamingEnabled) { NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary(); const NetworkDevice* cellular = cros->FindCellularDevice(); @@ -362,13 +317,17 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { // and set data roaming allowed in true always. cros->SetCellularDataRoamingAllowed(true); } else { - bool new_value = (use_value == USE_VALUE_SUPPLIED) ? value : false; + bool new_value = false; + if (use_value == USE_VALUE_SUPPLIED) + value->GetAsBoolean(&new_value); if (device_value != new_value) cros->SetCellularDataRoamingAllowed(new_value); } } } else if (path == kStatsReportingPref) { - bool stats_consent = (use_value == USE_VALUE_SUPPLIED) ? value : false; + bool stats_consent = false; + if (use_value == USE_VALUE_SUPPLIED) + value->GetAsBoolean(&stats_consent); // TODO(pastarmovj): Remove this once migration is not needed anymore. // If the value is not set we should try to migrate legacy consent file. if (use_value == USE_VALUE_DEFAULT) { @@ -378,9 +337,10 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { stats_consent = GoogleUpdateSettings::GetCollectStatsConsent(); // Make sure the values will get eventually written to the policy file. migration_helper_.AddMigrationValue( - path, stats_consent ? "true" : "false"); + path, base::Value::CreateBooleanValue(stats_consent)); migration_helper_.MigrateValues(); - UpdateCacheBool(path, stats_consent, USE_VALUE_SUPPLIED); + base::FundamentalValue base_value(stats_consent); + UpdateCache(path, &base_value, USE_VALUE_SUPPLIED); LOG(WARNING) << "No metrics policy set will revert to checking " << "consent file which is " << (stats_consent ? "on." : "off."); @@ -407,8 +367,8 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { // Implementation of SignedSettingsHelper::Callback. virtual void OnRetrievePropertyCompleted(SignedSettings::ReturnCode code, const std::string& name, - const std::string& value) { - if (!IsControlledBooleanSetting(name) && !IsControlledStringSetting(name)) { + const base::Value* value) { + if (!IsControlledSetting(name)) { NOTREACHED(); return; } @@ -426,16 +386,13 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { if (fallback_to_default) VLOG(1) << "Going default for cros setting " << name; else - VLOG(1) << "Retrieved cros setting " << name << "=" << value; - if (IsControlledBooleanSetting(name)) { - UpdateCacheBool(name, (value == kTrueIncantation), - fallback_to_default ? USE_VALUE_DEFAULT : USE_VALUE_SUPPLIED); - OnBooleanPropertyRetrieve(name, (value == kTrueIncantation), - fallback_to_default ? USE_VALUE_DEFAULT : USE_VALUE_SUPPLIED); - } else if (IsControlledStringSetting(name)) { - UpdateCacheString(name, value, - fallback_to_default ? USE_VALUE_DEFAULT : USE_VALUE_SUPPLIED); - } + VLOG(1) << "Retrieved cros setting " << name; + UpdateCache( + name, value, + fallback_to_default ? USE_VALUE_DEFAULT : USE_VALUE_SUPPLIED); + OnPropertyRetrieve( + name, value, + fallback_to_default ? USE_VALUE_DEFAULT : USE_VALUE_SUPPLIED); break; } case SignedSettings::OPERATION_FAILED: @@ -453,8 +410,10 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { if (IsControlledBooleanSetting(name)) { // For boolean settings we can just set safe (false) values // and continue as trusted. - OnBooleanPropertyRetrieve(name, false, USE_VALUE_SUPPLIED); - UpdateCacheBool(name, false, USE_VALUE_SUPPLIED); + scoped_ptr<base::Value> false_value( + base::Value::CreateBooleanValue(false)); + OnPropertyRetrieve(name, false_value.get(), USE_VALUE_SUPPLIED); + UpdateCache(name, false_value.get(), USE_VALUE_SUPPLIED); } else { prefs->ClearPref((name + kTrustedSuffix).c_str()); return; @@ -476,9 +435,8 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback { // Implementation of SignedSettingsHelper::Callback. virtual void OnStorePropertyCompleted(SignedSettings::ReturnCode code, const std::string& name, - const std::string& value) { - VLOG(1) << "Store cros setting " << name << "=" << value << ", code=" - << code; + const base::Value& value) { + VLOG(1) << "Store cros setting " << name << ", code=" << code; // Reload the setting if store op fails. if (code != SignedSettings::SUCCESS) @@ -547,20 +505,14 @@ void UserCrosSettingsProvider::Reload() { } void UserCrosSettingsProvider::DoSet(const std::string& path, - Value* in_value) { + const base::Value& in_value) { UserCrosSettingsTrust::GetInstance()->Set(path, in_value); } const base::Value* UserCrosSettingsProvider::Get( const std::string& path) const { if (HandlesSetting(path)) { - PrefService* prefs = g_browser_process->local_state(); - // TODO(pastarmovj): Temporary hack until we refactor the user whitelisting - // handling to completely coincide with the rest of the settings. - if (path == kAccountsPrefUsers && - prefs->GetList(kAccountsPrefUsers) == NULL) { - GetUserWhitelist(NULL); - } + const PrefService* prefs = g_browser_process->local_state(); const PrefService::Preference* pref = prefs->FindPreference(path.c_str()); return pref->GetValue(); } @@ -605,7 +557,8 @@ void UserCrosSettingsProvider::UnwhitelistUser(const std::string& email) { // static void UserCrosSettingsProvider::UpdateCachedOwner(const std::string& email) { - UpdateCacheString(kDeviceOwner, email, USE_VALUE_SUPPLIED); + base::StringValue email_value(email); + UpdateCache(kDeviceOwner, &email_value, USE_VALUE_SUPPLIED); } } // namespace chromeos diff --git a/chrome/browser/chromeos/user_cros_settings_provider.h b/chrome/browser/chromeos/user_cros_settings_provider.h index 61158bde..cd4ae1f 100644 --- a/chrome/browser/chromeos/user_cros_settings_provider.h +++ b/chrome/browser/chromeos/user_cros_settings_provider.h @@ -49,7 +49,8 @@ class UserCrosSettingsProvider : public CrosSettingsProvider { private: // CrosSettingsProvider implementation. - virtual void DoSet(const std::string& path, base::Value* value) OVERRIDE; + virtual void DoSet(const std::string& path, + const base::Value& value) OVERRIDE; DISALLOW_COPY_AND_ASSIGN(UserCrosSettingsProvider); }; diff --git a/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc index e609dfa..00cd210 100644 --- a/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc +++ b/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc @@ -101,13 +101,10 @@ void AccountsOptionsHandler::UnwhitelistUser(const base::ListValue* args) { void AccountsOptionsHandler::WhitelistExistingUsers( const base::ListValue* args) { base::ListValue whitelist_users; - const base::ListValue *user_list; - CrosSettings::Get()->GetList(kAccountsPrefUsers, &user_list); const UserList& users = UserManager::Get()->GetUsers(); for (UserList::const_iterator it = users.begin(); it < users.end(); ++it) { const std::string& email = (*it)->email(); - base::StringValue email_value(email); - if (user_list->Find(email_value) == user_list->end()) { + if (!CrosSettings::Get()->FindEmailInList(kAccountsPrefUsers, email)) { base::DictionaryValue* user_dict = new DictionaryValue; user_dict->SetString("name", (*it)->GetDisplayName()); user_dict->SetString("email", email); diff --git a/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc index 6af116a..5a8807e 100644 --- a/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc +++ b/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc @@ -158,9 +158,7 @@ void CoreChromeOSOptionsHandler::SetPref(const std::string& pref_name, if (!CrosSettings::IsCrosSettings(pref_name)) return ::CoreOptionsHandler::SetPref(pref_name, value, metric); handling_change_ = true; - // CrosSettings takes ownership of its value so we need to copy it. - base::Value* pref_value = value->DeepCopy(); - CrosSettings::Get()->Set(pref_name, pref_value); + CrosSettings::Get()->Set(pref_name, *value); handling_change_ = false; ProcessUserMetric(value, metric); diff --git a/chrome/browser/ui/webui/options/chromeos/system_settings_provider.cc b/chrome/browser/ui/webui/options/chromeos/system_settings_provider.cc index 359a193..1d62956 100644 --- a/chrome/browser/ui/webui/options/chromeos/system_settings_provider.cc +++ b/chrome/browser/ui/webui/options/chromeos/system_settings_provider.cc @@ -197,15 +197,15 @@ SystemSettingsProvider::~SystemSettingsProvider() { STLDeleteElements(&timezones_); } -void SystemSettingsProvider::DoSet(const std::string& path, Value* in_value) { +void SystemSettingsProvider::DoSet(const std::string& path, + const base::Value& in_value) { // Non-guest users can change the time zone. if (UserManager::Get()->IsLoggedInAsGuest()) return; if (path == kSystemTimezone) { string16 value; - if (!in_value || !in_value->IsType(Value::TYPE_STRING) || - !in_value->GetAsString(&value)) + if (!in_value.IsType(Value::TYPE_STRING) || !in_value.GetAsString(&value)) return; const icu::TimeZone* timezone = GetTimezone(value); if (!timezone) diff --git a/chrome/browser/ui/webui/options/chromeos/system_settings_provider.h b/chrome/browser/ui/webui/options/chromeos/system_settings_provider.h index e3f95b0a..4ac725a 100644 --- a/chrome/browser/ui/webui/options/chromeos/system_settings_provider.h +++ b/chrome/browser/ui/webui/options/chromeos/system_settings_provider.h @@ -41,7 +41,8 @@ class SystemSettingsProvider : public CrosSettingsProvider, private: // CrosSettingsProvider overrides. - virtual void DoSet(const std::string& path, base::Value* in_value) OVERRIDE; + virtual void DoSet(const std::string& path, + const base::Value& in_value) OVERRIDE; // Gets timezone name. static string16 GetTimezoneName(const icu::TimeZone& timezone); |