Signed settings refactoring: Proper caching and more tests.

chrome/browser/chromeos/device_settings_provider.cc

+// 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.
+
+#include "chrome/browser/chromeos/device_settings_provider.h"
+
+#include "base/base64.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback.h"
+#include "base/hash_tables.h"
+#include "base/logging.h"
+#include "base/memory/singleton.h"
+#include "base/string_util.h"
+#include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/chromeos/cros/network_library.h"
+#include "chrome/browser/chromeos/cros_settings.h"
+#include "chrome/browser/chromeos/cros_settings_names.h"
+#include "chrome/browser/chromeos/login/ownership_service.h"
+#include "chrome/browser/chromeos/login/ownership_status_checker.h"
+#include "chrome/browser/chromeos/login/signed_settings_cache.h"
+#include "chrome/browser/chromeos/login/user_manager.h"
+#include "chrome/browser/policy/proto/chrome_device_policy.pb.h"
+#include "chrome/browser/policy/proto/device_management_backend.pb.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"
+#include "chrome/installer/util/google_update_settings.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_service.h"
+
+using content::BrowserThread;
+using google::protobuf::RepeatedPtrField;
+
+namespace chromeos {
+

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

remove extra blank line.

[pastarmovj, 2011/11/30 17:21:16]

Done.

+
+namespace {
+
+// For all our boolean settings following is applicable:

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

missing "the"

[pastarmovj, 2011/11/30 17:21:16]

Done.

+// true is default permissive value and false is safe prohibitive value.
+// Exception: kSignedDataRoamingEnabled which has default value of false.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

This comment is interesting, but doesn't seem to be helpful at this point. Is
there a better place to put it?

[pastarmovj, 2011/11/30 17:21:16]

Moved to UpdateValuesCache. The place we apply defaults actually.

+const char* kBooleanSettings[] = {
+  kAccountsPrefAllowNewUser,
+  kAccountsPrefAllowGuest,
+  kAccountsPrefShowUserNamesOnSignIn,
+  kSignedDataRoamingEnabled,
+  kStatsReportingPref
+};
+
+const char* kStringSettings[] = {
+  kDeviceOwner,
+  kReleaseChannel,
+  kSettingProxyEverywhere
+};
+
+const char* kListSettings[] = {
+  kAccountsPrefUsers
+};
+
+// upper bound for number of retries to fetch a signed setting.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Upper-case upper :)

[pastarmovj, 2011/11/30 17:21:16]

Done.

+static const int kNumRetriesLimit = 9;
+
+bool IsControlledBooleanSetting(const std::string& pref_path) {
+  return std::find(kBooleanSettings,
+                   kBooleanSettings + arraysize(kBooleanSettings),
+                   pref_path) !=
+      kBooleanSettings + arraysize(kBooleanSettings);

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

how about const char* end = kBooleanSettings + arraysize(kBooleanSettings), and
then use it in the find? If that fits the column limit better, I guess the
result would be more readable.

(also below)

[pastarmovj, 2011/11/30 17:21:16]

Done.

+}
+
+bool IsControlledStringSetting(const std::string& pref_path) {
+  return std::find(kStringSettings,
+                   kStringSettings + arraysize(kStringSettings),
+                   pref_path) !=
+      kStringSettings + arraysize(kStringSettings);
+}
+
+bool IsControlledListSetting(const std::string& pref_path) {
+  return std::find(kListSettings,
+                   kListSettings + arraysize(kListSettings),
+                   pref_path) !=
+      kListSettings + arraysize(kListSettings);
+}
+
+bool IsControlledSetting(const std::string& pref_path) {
+  return (IsControlledBooleanSetting(pref_path) ||
+          IsControlledStringSetting(pref_path) ||
+          IsControlledListSetting(pref_path));
+}
+
+bool HasOldMetricsFile() {
+  // TODO(pastarmovj): Remove this once migration is not needed anymore.
+  // If the value is not set we should try to migrate legacy consent file.
+  // Loading consent file state causes us to do blocking IO on UI thread.
+  // Temporarily allow it until we fix http://crbug.com/62626
+  base::ThreadRestrictions::ScopedAllowIO allow_io;

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Any chance we can clean this up? Read the file asynchronously on Reload()?

[pastarmovj, 2011/11/30 17:21:16]

I changed a bit DeviceSettingsProvider::ApplyMetricsSetting to only go to this
line iof we are really migrating and not every time (which was stupid anyhow) so
I think it is not such a big deal because it should happen only once and only
for people that are upgrading from pre 16 version to new one.

+  return GoogleUpdateSettings::GetCollectStatsConsent();
+}
+
+}  // namespace
+
+// ###########################################################################
+// ###########################################################################
+// ###########################################################################

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

what's this?

[pastarmovj, 2011/11/30 17:21:16]

A bar made of hashes ... i think :S ... removed :)

+
+DeviceSettingsProvider::DeviceSettingsProvider()
+    : ownership_service_(OwnershipService::GetSharedInstance()),
+      migration_helper_(new SignedSettingsMigrationHelper()),
+      retries_left_(kNumRetriesLimit),
+      trusted_(false) {
+  // Register for notification when user logs in, so that we can activate the
+  // new proxy config.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Proxy config? Observe() doesn't appear to do proxy-config-related work.

[pastarmovj, 2011/11/30 17:21:16]

Nope the comment was wrong. Fixed now.

+  registrar_.Add(this, chrome::NOTIFICATION_OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED,
+                 content::NotificationService::AllSources());
+  // Make sure we have at least the cache data immediately.
+  RetrieveCachedData();
+  // Start prefetching preferences.
+  Reload();
+}
+
+void DeviceSettingsProvider::Reload() {
+  // While fetching we can't trust the cache anymore.
+  trusted_ = false;
+  OwnershipService::Status ownership_status =
+      ownership_service_->GetStatus(true);

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

All the GetStatus calls you have here are potentially blocking. Not good. Any
chance to improve this?

[pastarmovj, 2011/11/30 17:21:16]

All are potentially blocking but only one at most will ever block - so I think
the overhead of this is acceptable.

+  if (ownership_status == OwnershipService::OWNERSHIP_NONE) {
+    RetrieveCachedData();
+  } else {
+    // Retrieve the real data.
+    SignedSettingsHelper::Get()->StartRetrievePolicyOp(
+        base::Bind(&DeviceSettingsProvider::OnRetrievePolicyCompleted,
+                   base::Unretained(this)));
+  }
+}
+
+void DeviceSettingsProvider::DoSet(const std::string& path,
+                                   const base::Value& in_value) {
+  OwnershipService::Status ownership_status =
+      ownership_service_->GetStatus(true);
+  if (!UserManager::Get()->current_user_is_owner() &&
+      ownership_status != OwnershipService::OWNERSHIP_NONE) {
+    LOG(WARNING) << "Changing settings from non-owner, setting=" << path;
+
+    // Revert UI change.
+    CrosSettings::Get()->FireObservers(path.c_str());
+    return;
+  }
+
+  if (IsControlledSetting(path))
+    SetInPolicy(path, in_value);
+  else
+    NOTREACHED() << "Try to set unhandled cros setting " << path;
+}
+
+void DeviceSettingsProvider::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  if (type == chrome::NOTIFICATION_OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED &&
+      UserManager::Get()->current_user_is_owner()) {
+    // Store data from the temp storage to the signed settings store but first
+    // we have to load the initially generated blob in the cache.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

s/in/into/. Also, the comment is backwards. Maybe it would be clearer if you say
"Reload the initial policy blob, apply settings from temp storage, and write
back the blob" or somesuch.

[pastarmovj, 2011/11/30 17:21:16]

Done.

+    SignedSettingsHelper::Get()->StartRetrievePolicyOp(
+        base::Bind(&DeviceSettingsProvider::FinalizeTempStorage,
+                   base::Unretained(this)));
+  }
+}
+
+void DeviceSettingsProvider::FinalizeTempStorage(
+    SignedSettings::ReturnCode code,
+    const em::PolicyFetchResponse& policy) {
+  if (code != SignedSettings::SUCCESS) {
+    LOG(ERROR) << "Can't finalize temp store error code:" << code;
+    return;
+  }
+
+  policy_.ParseFromString(policy.policy_data());
+  trusted_ = true;
+  SignedSettingsCache::Finalize(g_browser_process->local_state(), policy);
+  Reload();
+}
+
+const em::PolicyData DeviceSettingsProvider::get_policy() const {
+  return policy_;
+}
+
+void DeviceSettingsProvider::RetrieveCachedData() {
+  // No owner yet use the temp storage.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Make a proper sentence? "If there is no owner yet, query temp storage". Also, I
don't quite understand the comment, since it pulls the policy object out of
SignedSettingsCache unconditionally.

Ah, now I understand: This is only called if there is no owner yet. So please
make the comment clearer :)

[pastarmovj, 2011/11/30 17:21:16]

Done.

+  em::PolicyData policy;
+  if (!SignedSettingsCache::Retrieve(&policy,
+                                     g_browser_process->local_state())) {
+    LOG(WARNING) << "Can't retrieve temp store possibly not created yet";

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Missing period, full sentence? Also, if this is a condition that can happen in
regular operation, we shouldn't log a warning.

[pastarmovj, 2011/11/30 17:21:16]

Done.

+    // prepare empty data for the case we don't have temp cache yet.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

s/prepare/Prepare/

[pastarmovj, 2011/11/30 17:21:16]

Done.

+    policy.set_policy_type(kDevicePolicyType);
+    em::ChromeDeviceSettingsProto pol;
+    policy.set_policy_value(pol.SerializeAsString());
+  }
+
+  policy_ = policy;
+}
+
+void DeviceSettingsProvider::SetInPolicy(const std::string& prop,
+                                         const base::Value& value) {
+  if (prop == kDeviceOwner) {
+    // Just store it in the memory cache no trusted checks no persisting.

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Full sentence? Also, I don't quite understand why we need to have this branch at
all. Doesn't that mean people can call Set() to clobber our understanding of who
the owner is?

[pastarmovj, 2011/11/30 17:21:16]

I made this branch set the trusted flag to false but I have to keep it because
we want to have the new owner name in the time while the key is generated
available already so we store it in the cache and can serve it from there else
we will have some time while the new owner will not be able to use his owner
powers after first boot.

+    std::string owner;
+    if (value.GetAsString(&owner))
+      policy_.set_username(owner);
+    else
+      NOTREACHED();
+    return;
+  }
+
+  if (!RequestTrustedEntity()) {
+    // Otherwise we should first reload and apply on top of that.
+    SignedSettingsHelper::Get()->StartRetrievePolicyOp(

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

Isn't this redundant? I mean, if RequestTrustedEntity() returns false don't we
have a retrieval op already running?

[pastarmovj, 2011/11/30 17:21:16]

Not necessarily. It might be a failed read so retrying here might be a good
idea. And the most important reason is that this ensures proper queuing of
writes so that we can call SetInPolicy while another SetInPolicy is still in
flow and this will ensure the second will wait for the async parts of the first
one to finish.

+            base::Bind(&DeviceSettingsProvider::FinishSetInPolicy,
+                       base::Unretained(this),
+                       prop, base::Owned(value.DeepCopy())));
+    return;
+  }
+
+  trusted_ = false;
+  em::PolicyData data = get_policy();
+  em::ChromeDeviceSettingsProto pol;
+  pol.ParseFromString(data.policy_value());
+  if (prop == kAccountsPrefAllowNewUser) {
+    em::AllowNewUsersProto* allow = pol.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 = pol.mutable_guest_mode_enabled();
+    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();
+    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();
+    bool roaming_value = false;
+    if (value.GetAsBoolean(&roaming_value))
+      roam->set_data_roaming_enabled(roaming_value);
+    else
+      NOTREACHED();
+    ApplyRoamingSetting(roaming_value);
+  } else if (prop == kSettingProxyEverywhere) {
+    // TODO(cmasone): NOTIMPLEMENTED() once http://crosbug.com/13052 is fixed.
+    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();
+    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();
+    bool metrics_value = false;
+    if (value.GetAsBoolean(&metrics_value))
+      metrics->set_metrics_enabled(metrics_value);
+    else
+      NOTREACHED();
+    ApplyMetricsSetting(false, metrics_value);
+  } 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();
+  }
+  data.set_policy_value(pol.SerializeAsString());
+  // Set the cache to the updated value.
+  policy_ = data;
+
+  if (!SignedSettingsCache::Store(data, g_browser_process->local_state()))
+    LOG(ERROR) << "Couldn't store to the temp storage.";
+
+  OwnershipService::Status ownership_status =
+      ownership_service_->GetStatus(true);
+  if (ownership_status == OwnershipService::OWNERSHIP_TAKEN) {
+    em::PolicyFetchResponse policy_envelope;
+    policy_envelope.set_policy_data(policy_.SerializeAsString());
+    SignedSettingsHelper::Get()->StartStorePolicyOp(
+        policy_envelope, base::Bind(

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

I think an additional line break after , would help here.

[pastarmovj, 2011/11/30 17:21:16]

Done.

+            &DeviceSettingsProvider::OnStorePolicyCompleted,
+            base::Unretained(this)));
+  }
+}
+
+void DeviceSettingsProvider::FinishSetInPolicy(
+    const std::string& prop,
+    const base::Value* value,
+    SignedSettings::ReturnCode code,
+    const em::PolicyFetchResponse& policy) {
+  if (code != SignedSettings::SUCCESS) {
+    LOG(ERROR) << "Can't serialize to policy error code: " << code;
+    Reload();
+    return;
+  }
+  SetInPolicy(prop, *value);
+}
+

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

remove extra blank line.

[pastarmovj, 2011/11/30 17:21:16]

Done.

+
+base::Value* DeviceSettingsProvider::LookUpInPolicy(
+    const std::string& prop) const {
+  const em::PolicyData data = get_policy();
+  if (prop == kDeviceOwner) {
+    if (data.has_username() && !data.has_request_token())
+      return base::Value::CreateStringValue(data.username());
+  }
+  VLOG(2) << "Looking up " << prop;
+  em::ChromeDeviceSettingsProto pol;
+  pol.ParseFromString(data.policy_value());
+  if (prop == kAccountsPrefAllowNewUser) {
+    if (pol.has_allow_new_users() &&
+        pol.allow_new_users().has_allow_new_users() &&
+        pol.allow_new_users().allow_new_users()) {
+      // 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 we default to allowing new users.
+    if (!pol.has_user_whitelist())
+      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()) {
+      // Default to allowing guests;
+      return base::Value::CreateBooleanValue(true);
+    }
+    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()) {
+      // Default to showing pods on the login screen;
+      return base::Value::CreateBooleanValue(true);
+    }
+    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()) {
+      // Default to disabling cellular data roaming;
+      return base::Value::CreateBooleanValue(false);
+    }
+    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 base::Value::CreateStringValue(serialized);
+    }
+
+  } else if (prop == kReleaseChannel) {
+    if (!pol.has_release_channel() ||
+        !pol.release_channel().has_release_channel()) {
+      // Default to an invalid channel (will be ignored).
+      return base::Value::CreateStringValue("");
+    }
+    return base::Value::CreateStringValue(
+               pol.release_channel().release_channel());
+
+  } else if (prop == kStatsReportingPref) {
+    if (pol.has_metrics_enabled()) {
+      return base::Value::CreateBooleanValue(
+                 pol.metrics_enabled().metrics_enabled());
+    } else {
+      return base::Value::CreateBooleanValue(HasOldMetricsFile());
+    }
+  } else if (prop == kAccountsPrefUsers) {
+    base::ListValue* list = new base::ListValue();
+    const em::UserWhitelistProto& whitelist_proto = pol.user_whitelist();
+    const RepeatedPtrField<std::string>& whitelist =
+        whitelist_proto.user_whitelist();
+    for (RepeatedPtrField<std::string>::const_iterator it = whitelist.begin();
+         it != whitelist.end(); ++it) {
+      list->Append(base::Value::CreateStringValue(*it));
+    }
+    return list;
+  }
+  return NULL;
+}
+
+void DeviceSettingsProvider::ApplyMetricsSetting(bool use_file,
+                                                 bool new_value) const {
+  // TODO(pastarmovj): Remove this once migration is not needed anymore.
+  // If the value is not set we should try to migrate legacy consent file.
+  bool stats_consent = HasOldMetricsFile();
+  if (use_file) {
+    new_value = stats_consent;
+    // Make sure the values will get eventually written to the policy file.
+    migration_helper_->AddMigrationValue(
+        kStatsReportingPref, base::Value::CreateBooleanValue(new_value));
+    migration_helper_->MigrateValues();
+    LOG(WARNING) << "No metrics policy set will revert to checking "
+                 << "consent file which is "
+                 << (new_value ? "on." : "off.");

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

We always print this when you initially claim a device, correct? If so, this
should probably not be a WARNING.

[pastarmovj, 2011/11/30 17:21:16]

Made INFO out of it.

+  }
+  VLOG(1) << "Metrics policy is being set to : " << stats_consent
+          << "(use file : " << use_file << ")";
+  // TODO(pastarmovj): Remove this once we don't need to regenerate the
+  // consent file for the GUID anymore.
+  OptionsUtil::ResolveMetricsReportingEnabled(new_value);
+}
+
+void DeviceSettingsProvider::ApplyRoamingSetting(bool new_value) const {
+  NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
+  const NetworkDevice* cellular = cros->FindCellularDevice();
+  if (cellular) {
+    bool device_value = cellular->data_roaming_allowed();
+    if (!device_value && cros->IsCellularAlwaysInRoaming()) {
+      // If operator requires roaming always enabled, ignore supplied value
+      // and set data roaming allowed in true always.
+      cros->SetCellularDataRoamingAllowed(true);
+    } else if (device_value != new_value) {
+      cros->SetCellularDataRoamingAllowed(new_value);
+    }
+  }
+}
+
+void DeviceSettingsProvider::ApplySideEffects() const {
+  const em::PolicyData data = get_policy();
+  em::ChromeDeviceSettingsProto pol;
+  pol.ParseFromString(data.policy_value());
+  // First migrate metrics settings as needed.
+  if (pol.has_metrics_enabled())
+    ApplyMetricsSetting(false, pol.metrics_enabled().metrics_enabled());
+  else
+    ApplyMetricsSetting(true, false);
+  // Next set the roaming setting as needed.
+  ApplyRoamingSetting(pol.has_data_roaming_enabled() ?
+      pol.data_roaming_enabled().data_roaming_enabled() : false);
+}
+
+base::Value* DeviceSettingsProvider::Get(const std::string& path) const {
+  if (IsControlledSetting(path))
+    return LookUpInPolicy(path);
+  else
+    NOTREACHED() << "Trying to get non cros setting.";
+  return NULL;
+}
+
+bool DeviceSettingsProvider::GetTrusted(const std::string& path,
+                                        const base::Closure& callback) {
+  if (!IsControlledSetting(path)) {
+    NOTREACHED();
+    return true;
+  }
+
+  if (RequestTrustedEntity()) {
+    return true;
+  } else {
+    if (!callback.is_null())
+      callbacks_.push_back(callback);
+    return false;
+  }
+}
+
+bool DeviceSettingsProvider::HandlesSetting(const std::string& path) const {
+  return IsControlledSetting(path);
+}
+
+bool DeviceSettingsProvider::RequestTrustedEntity() {
+  OwnershipService::Status ownership_status =
+      ownership_service_->GetStatus(true);
+  if (ownership_status == OwnershipService::OWNERSHIP_NONE)
+    return true;
+  return trusted_;
+}
+
+void DeviceSettingsProvider::OnStorePolicyCompleted(
+    SignedSettings::ReturnCode code) {
+  // In any case reload the policy cache to now.
+  if (code != SignedSettings::SUCCESS)
+    Reload();
+  else
+    trusted_ = true;
+}
+
+void DeviceSettingsProvider::OnRetrievePolicyCompleted(
+    SignedSettings::ReturnCode code,
+    const em::PolicyFetchResponse& policy) {
+  switch (code) {
+    case SignedSettings::SUCCESS: {
+      DCHECK(policy.has_policy_data());
+      policy_.ParseFromString(policy.policy_data());
+      SignedSettingsCache::Store(get_policy(),
+                                 g_browser_process->local_state());
+      trusted_ = true;
+      for (size_t i = 0; i < callbacks_.size(); ++i)
+        MessageLoop::current()->PostTask(FROM_HERE, callbacks_[i]);

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

any reason to go through the message loop? Can't we just do callbacks_[i].Run()?

[pastarmovj, 2011/11/30 17:21:16]

I guess not. This was legacy code using tasks.

+      callbacks_.clear();
+      // TODO(pastarmovj): Make those side effects responsibility of the
+      // respective subsystems.
+      ApplySideEffects();
+      break;
+    }
+    case SignedSettings::NOT_FOUND:
+    case SignedSettings::KEY_UNAVAILABLE: {
+      if (ownership_service_->GetStatus(true) !=
+          OwnershipService::OWNERSHIP_TAKEN) {
+        NOTREACHED() << "No policies present yet will use the temp storage.";

[Mattias Nissler (ping if slow), 2011/11/30 11:22:44]

comma after yet

[pastarmovj, 2011/11/30 17:21:16]

Done.

+      }
+      break;
+    }
+    case SignedSettings::BAD_SIGNATURE:
+    case SignedSettings::OPERATION_FAILED: {
+      LOG(ERROR) << "Failed to retrieve cros policies. Reason:" << code;
+      if (retries_left_ > 0) {
+        retries_left_ -= 1;
+        Reload();
+        return;
+      }
+      LOG(ERROR) << "No retries left";
+      break;
+    }
+  }
+}
+
+}  // namespace chromeos