Replace CrosSettings::GetTrusted() with PrepareTrustedValues().

This makes the API clearer about what values can be regarded as trusted.

BUG=None
TEST=All works as before


Review URL: http://codereview.chromium.org/9731004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127681 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 69 insertions, 103 deletions

diff --git a/chrome/browser/chromeos/cros_settings.cc b/chrome/browser/chromeos/cros_settings.cc
index 1584967..eb20863 100644
--- a/chrome/browser/chromeos/cros_settings.cc
+++ b/chrome/browser/chromeos/cros_settings.cc
@@ -218,14 +218,13 @@ const base::Value* CrosSettings::GetPref(const std::string& path) const {
   return NULL;
 }
 
-bool CrosSettings::GetTrusted(const std::string& path,
-                              const base::Closure& callback) const {
+bool CrosSettings::PrepareTrustedValues(const base::Closure& callback) const {
   DCHECK(CalledOnValidThread());
-  CrosSettingsProvider* provider = GetProvider(path);
-  if (provider)
-    return provider->GetTrusted(path, callback);
-  NOTREACHED() << "CrosSettings::GetTrusted called for unknown pref : " << path;
-  return false;
+  for (size_t i = 0; i < providers_.size(); ++i) {
+    if (!providers_[i]->PrepareTrustedValues(callback))
+      return false;
+  }
+  return true;
 }
 
 bool CrosSettings::GetBoolean(const std::string& path,
diff --git a/chrome/browser/chromeos/cros_settings.h b/chrome/browser/chromeos/cros_settings.h
index 14578df..6d5342f 100644
--- a/chrome/browser/chromeos/cros_settings.h
+++ b/chrome/browser/chromeos/cros_settings.h
@@ -41,12 +41,12 @@ class CrosSettings : public base::NonThreadSafe {
   // Returns setting value for the given |path|.
   const base::Value* GetPref(const std::string& path) const;
 
-  // 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|.
-  bool GetTrusted(const std::string& path,
-                  const base::Closure& callback) const;
+  // Requests all providers to fetch their values from a trusted store, if they
+  // haven't done so yet. Returns true if the cros settings returned by |this|
+  // are trusted during the current loop cycle; otherwise returns false, and
+  // |callback| will be invoked later when trusted values become available.
+  // PrepareTrustedValues() should be tried again in that case.
+  virtual bool PrepareTrustedValues(const base::Closure& callback) const;
 
   // Convenience forms of Set().  These methods will replace any existing
   // value at that |path|, even if it has a different type.
diff --git a/chrome/browser/chromeos/cros_settings_provider.h b/chrome/browser/chromeos/cros_settings_provider.h
index 16bf27f..3cd815a 100644
--- a/chrome/browser/chromeos/cros_settings_provider.h
+++ b/chrome/browser/chromeos/cros_settings_provider.h
@@ -32,12 +32,12 @@ class CrosSettingsProvider {
   // 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| 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) = 0;
+  // Requests the provider to fetch its values from a trusted store, if it
+  // hasn't done so yet. Returns true if the values returned by this provider
+  // are trusted during the current loop cycle; otherwise returns false, and
+  // |callback| will be invoked later when trusted values become available.
+  // PrepareTrustedValues() should be tried again in that case.
+  virtual bool PrepareTrustedValues(const base::Closure& callback) = 0;
 
   // Gets the namespace prefix provided by this provider.
   virtual bool HandlesSetting(const std::string& path) const = 0;
diff --git a/chrome/browser/chromeos/cros_settings_unittest.cc b/chrome/browser/chromeos/cros_settings_unittest.cc
index 8e7064a..103d362 100644
--- a/chrome/browser/chromeos/cros_settings_unittest.cc
+++ b/chrome/browser/chromeos/cros_settings_unittest.cc
@@ -57,7 +57,7 @@ class CrosSettingsTest : public testing::Test {
     if (expected_props_.find(pref) == expected_props_.end())
       return;
 
-    if (CrosSettings::Get()->GetTrusted(pref,
+    if (CrosSettings::Get()->PrepareTrustedValues(
             base::Bind(&CrosSettingsTest::FetchPref,
                        pointer_factory_.GetWeakPtr(), pref))) {
       scoped_ptr<base::Value> expected_value(
diff --git a/chrome/browser/chromeos/device_settings_provider.cc b/chrome/browser/chromeos/device_settings_provider.cc
index 2b297e2..b75ca81 100644
--- a/chrome/browser/chromeos/device_settings_provider.cc
+++ b/chrome/browser/chromeos/device_settings_provider.cc
@@ -650,20 +650,12 @@ const base::Value* DeviceSettingsProvider::Get(const std::string& path) const {
   return NULL;
 }
 
-bool DeviceSettingsProvider::GetTrusted(const std::string& path,
-                                        const base::Closure& callback) {
-  if (!IsControlledSetting(path)) {
-    NOTREACHED();
-    return true;
-  }
-
-  if (RequestTrustedEntity()) {
+bool DeviceSettingsProvider::PrepareTrustedValues(const base::Closure& cb) {
+  if (RequestTrustedEntity())
     return true;
-  } else {
-    if (!callback.is_null())
-      callbacks_.push_back(callback);
-    return false;
-  }
+  if (!cb.is_null())
+    callbacks_.push_back(cb);
+  return false;
 }
 
 bool DeviceSettingsProvider::HandlesSetting(const std::string& path) const {
diff --git a/chrome/browser/chromeos/device_settings_provider.h b/chrome/browser/chromeos/device_settings_provider.h
index 4794b53..bbd5e4c 100644
--- a/chrome/browser/chromeos/device_settings_provider.h
+++ b/chrome/browser/chromeos/device_settings_provider.h
@@ -35,8 +35,7 @@ class DeviceSettingsProvider : public CrosSettingsProvider,
 
   // CrosSettingsProvider implementation.
   virtual const base::Value* Get(const std::string& path) const OVERRIDE;
-  virtual bool GetTrusted(const std::string& path,
-                          const base::Closure& callback) OVERRIDE;
+  virtual bool PrepareTrustedValues(const base::Closure& callback) OVERRIDE;
   virtual bool HandlesSetting(const std::string& path) const OVERRIDE;
   virtual void Reload() OVERRIDE;
 
diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc
index 2dccb45..19daaf9 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller.cc
@@ -399,11 +399,9 @@ void ExistingUserController::LoginAsGuest() {
 
   // Check allow_guest in case this call is fired from key accelerator.
   // Must not proceed without signature verification.
-  bool trusted_setting_available = cros_settings_->GetTrusted(
-      kAccountsPrefAllowGuest,
+  if (!cros_settings_->PrepareTrustedValues(
       base::Bind(&ExistingUserController::LoginAsGuest,
-                 weak_factory_.GetWeakPtr()));
-  if (!trusted_setting_available) {
+                 weak_factory_.GetWeakPtr()))) {
     // Value of AllowGuest setting is still not verified.
     // Another attempt will be invoked again after verification completion.
     return;
@@ -623,12 +621,9 @@ void ExistingUserController::OnOffTheRecordLoginSuccess() {
 
 void ExistingUserController::OnPasswordChangeDetected() {
   // Must not proceed without signature verification.
-  bool trusted_setting_available = cros_settings_->GetTrusted(
-      kDeviceOwner,
+  if (!cros_settings_->PrepareTrustedValues(
       base::Bind(&ExistingUserController::OnPasswordChangeDetected,
-                 weak_factory_.GetWeakPtr()));
-
-  if (!trusted_setting_available) {
+                 weak_factory_.GetWeakPtr()))) {
     // Value of owner email is still not verified.
     // Another attempt will be invoked after verification completion.
     return;
diff --git a/chrome/browser/chromeos/login/login_performer.cc b/chrome/browser/chromeos/login/login_performer.cc
index 20b1de0..d348c18 100644
--- a/chrome/browser/chromeos/login/login_performer.cc
+++ b/chrome/browser/chromeos/login/login_performer.cc
@@ -267,13 +267,10 @@ void LoginPerformer::CompleteLogin(const std::string& username,
   // should not be performed when ScreenLock is active (pending online auth).
   if (!ScreenLocker::default_screen_locker()) {
     // Must not proceed without signature verification or valid user list.
-    bool trusted_settings_available =
-        cros_settings->GetTrusted(
-            kAccountsPrefAllowNewUser,
+    if (!cros_settings->PrepareTrustedValues(
             base::Bind(&LoginPerformer::CompleteLogin,
                        weak_factory_.GetWeakPtr(),
-                       username, password));
-    if (!trusted_settings_available) {
+                       username, password))) {
       // Value of AllowNewUser setting is still not verified.
       // Another attempt will be invoked after verification completion.
       return;
@@ -305,13 +302,10 @@ void LoginPerformer::Login(const std::string& username,
   // should not be performed when ScreenLock is active (pending online auth).
   if (!ScreenLocker::default_screen_locker()) {
     // Must not proceed without signature verification.
-    bool trusted_settings_available =
-        cros_settings->GetTrusted(
-            kAccountsPrefAllowNewUser,
+    if (!cros_settings->PrepareTrustedValues(
             base::Bind(&LoginPerformer::Login,
                        weak_factory_.GetWeakPtr(),
-                       username, password));
-    if (!trusted_settings_available) {
+                       username, password))) {
       // Value of AllowNewUser setting is still not verified.
       // Another attempt will be invoked after verification completion.
       return;
diff --git a/chrome/browser/chromeos/login/user_manager_impl.cc b/chrome/browser/chromeos/login/user_manager_impl.cc
index c546b59..2115010 100644
--- a/chrome/browser/chromeos/login/user_manager_impl.cc
+++ b/chrome/browser/chromeos/login/user_manager_impl.cc
@@ -158,8 +158,7 @@ void RemoveUserInternal(const std::string& user_email,
   CrosSettings* cros_settings = CrosSettings::Get();
 
   // Ensure the value of owner email has been fetched.
-  if (!cros_settings->GetTrusted(
-          kDeviceOwner,
+  if (!cros_settings->PrepareTrustedValues(
           base::Bind(&RemoveUserInternal, user_email, delegate))) {
     // Value of owner email is not fetched yet.  RemoveUserInternal will be
     // called again after fetch completion.
@@ -783,8 +782,7 @@ void UserManagerImpl::RetrieveTrustedDevicePolicies() {
 
   CrosSettings* cros_settings = CrosSettings::Get();
   // Schedule a callback if device policy has not yet been verified.
-  if (!cros_settings->GetTrusted(
-      kAccountsPrefEphemeralUsersEnabled,
+  if (!cros_settings->PrepareTrustedValues(
       base::Bind(&UserManagerImpl::RetrieveTrustedDevicePolicies,
                  base::Unretained(this)))) {
     return;
diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc
index 9fdf48e..832a800 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl.cc
+++ b/chrome/browser/chromeos/proxy_config_service_impl.cc
@@ -390,8 +390,7 @@ ProxyConfigServiceImpl::ProxyConfigServiceImpl(PrefService* pref_service)
   if (pref_service->FindPreference(prefs::kUseSharedProxies))
     use_shared_proxies_.Init(prefs::kUseSharedProxies, pref_service, this);
 
-  if (CrosSettings::Get()->GetTrusted(
-          kSettingProxyEverywhere,
+  if (CrosSettings::Get()->PrepareTrustedValues(
           base::Bind(&ProxyConfigServiceImpl::FetchProxyPolicy,
                      pointer_factory_.GetWeakPtr()))) {
     FetchProxyPolicy();
diff --git a/chrome/browser/chromeos/stub_cros_settings_provider.cc b/chrome/browser/chromeos/stub_cros_settings_provider.cc
index 7b9c424..fd7a7e6 100644
--- a/chrome/browser/chromeos/stub_cros_settings_provider.cc
+++ b/chrome/browser/chromeos/stub_cros_settings_provider.cc
@@ -55,8 +55,7 @@ const base::Value* StubCrosSettingsProvider::Get(
   return NULL;
 }
 
-bool StubCrosSettingsProvider::GetTrusted(const std::string& path,
-                                          const base::Closure& callback) {
+bool StubCrosSettingsProvider::PrepareTrustedValues(const base::Closure& cb) {
   // We don't have a trusted store so all values are available immediately.
   return true;
 }
diff --git a/chrome/browser/chromeos/stub_cros_settings_provider.h b/chrome/browser/chromeos/stub_cros_settings_provider.h
index b0c652f..9862d50 100644
--- a/chrome/browser/chromeos/stub_cros_settings_provider.h
+++ b/chrome/browser/chromeos/stub_cros_settings_provider.h
@@ -24,8 +24,7 @@ class StubCrosSettingsProvider : public CrosSettingsProvider {
 
   // CrosSettingsProvider implementation.
   virtual const base::Value* Get(const std::string& path) const OVERRIDE;
-  virtual bool GetTrusted(const std::string& path,
-                          const base::Closure& callback) OVERRIDE;
+  virtual bool PrepareTrustedValues(const base::Closure& callback) OVERRIDE;
   virtual bool HandlesSetting(const std::string& path) const OVERRIDE;
   virtual void Reload() OVERRIDE;
 
diff --git a/chrome/browser/chromeos/stub_cros_settings_provider_unittest.cc b/chrome/browser/chromeos/stub_cros_settings_provider_unittest.cc
index fe1d25b..18a22cf 100644
--- a/chrome/browser/chromeos/stub_cros_settings_provider_unittest.cc
+++ b/chrome/browser/chromeos/stub_cros_settings_provider_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -16,9 +16,6 @@ namespace chromeos {
 
 namespace {
 
-const Value* kTrueValue = base::Value::CreateBooleanValue(true);
-const Value* kFalseValue = base::Value::CreateBooleanValue(false);
-
 void Fail() {
   // Should never be called.
   FAIL();
@@ -68,9 +65,10 @@ TEST_F(StubCrosSettingsProviderTest, HandlesSettings) {
 
 TEST_F(StubCrosSettingsProviderTest, Defaults) {
   // Verify default values.
-  AssertPref(kAccountsPrefAllowGuest, kTrueValue);
-  AssertPref(kAccountsPrefAllowNewUser, kTrueValue);
-  AssertPref(kAccountsPrefShowUserNamesOnSignIn, kTrueValue);
+  const base::FundamentalValue kTrueValue(true);
+  AssertPref(kAccountsPrefAllowGuest, &kTrueValue);
+  AssertPref(kAccountsPrefAllowNewUser, &kTrueValue);
+  AssertPref(kAccountsPrefShowUserNamesOnSignIn, &kTrueValue);
 }
 
 TEST_F(StubCrosSettingsProviderTest, Set) {
@@ -90,9 +88,9 @@ TEST_F(StubCrosSettingsProviderTest, SetMissing) {
   ExpectObservers(kReleaseChannel, 1);
 }
 
-TEST_F(StubCrosSettingsProviderTest, GetTrusted) {
+TEST_F(StubCrosSettingsProviderTest, PrepareTrustedValues) {
   // Should return immediately without invoking the callback.
-  bool trusted = provider_->GetTrusted(kDeviceOwner, base::Bind(&Fail));
+  bool trusted = provider_->PrepareTrustedValues(base::Bind(&Fail));
   EXPECT_TRUE(trusted);
 }
 
diff --git a/chrome/browser/policy/app_pack_updater.cc b/chrome/browser/policy/app_pack_updater.cc
index 2833ced..7556f3a 100644
--- a/chrome/browser/policy/app_pack_updater.cc
+++ b/chrome/browser/policy/app_pack_updater.cc
@@ -142,9 +142,9 @@ void AppPackUpdater::Observe(int type,
 
 void AppPackUpdater::LoadPolicy() {
   chromeos::CrosSettings* settings = chromeos::CrosSettings::Get();
-  if (!settings->GetTrusted(chromeos::kAppPack,
-                            base::Bind(&AppPackUpdater::LoadPolicy,
-                                       weak_ptr_factory_.GetWeakPtr()))) {
+  if (!settings->PrepareTrustedValues(
+          base::Bind(&AppPackUpdater::LoadPolicy,
+                     weak_ptr_factory_.GetWeakPtr()))) {
     return;
   }
   app_pack_extensions_.clear();
diff --git a/chrome/browser/policy/device_status_collector.cc b/chrome/browser/policy/device_status_collector.cc
index 6fc9072..5a1be4a 100644
--- a/chrome/browser/policy/device_status_collector.cc
+++ b/chrome/browser/policy/device_status_collector.cc
@@ -114,18 +114,17 @@ void DeviceStatusCollector::UpdateReportingSettings() {
   // Attempt to fetch the current value of the reporting settings.
   // If trusted values are not available, register this function to be called
   // back when they are available.
-  bool is_trusted = cros_settings_->GetTrusted(
-      chromeos::kReportDeviceVersionInfo,
+  if (!cros_settings_->PrepareTrustedValues(
       base::Bind(&DeviceStatusCollector::UpdateReportingSettings,
-                 base::Unretained(this)));
-  if (is_trusted) {
-    cros_settings_->GetBoolean(
-        chromeos::kReportDeviceVersionInfo, &report_version_info_);
-    cros_settings_->GetBoolean(
-        chromeos::kReportDeviceActivityTimes, &report_activity_times_);
-    cros_settings_->GetBoolean(
-        chromeos::kReportDeviceBootMode, &report_boot_mode_);
+                 base::Unretained(this)))) {
+    return;
   }
+  cros_settings_->GetBoolean(
+      chromeos::kReportDeviceVersionInfo, &report_version_info_);
+  cros_settings_->GetBoolean(
+      chromeos::kReportDeviceActivityTimes, &report_activity_times_);
+  cros_settings_->GetBoolean(
+      chromeos::kReportDeviceBootMode, &report_boot_mode_);
 }
 
 Time DeviceStatusCollector::GetCurrentTime() {
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 d88c947..5710904 100644
--- a/chrome/browser/ui/webui/options/chromeos/system_settings_provider.cc
+++ b/chrome/browser/ui/webui/options/chromeos/system_settings_provider.cc
@@ -225,8 +225,7 @@ const base::Value* SystemSettingsProvider::Get(const std::string& path) const {
 }
 
 // The timezone is always trusted.
-bool SystemSettingsProvider::GetTrusted(const std::string& path,
-                                        const base::Closure& callback) {
+bool SystemSettingsProvider::PrepareTrustedValues(const base::Closure& cb) {
   return true;
 }
 
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 af2c0c3..b8cf69a7 100644
--- a/chrome/browser/ui/webui/options/chromeos/system_settings_provider.h
+++ b/chrome/browser/ui/webui/options/chromeos/system_settings_provider.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -16,7 +16,6 @@
 namespace base {
 class Value;
 class ListValue;
-class StringValue;
 }
 
 namespace chromeos {
@@ -27,21 +26,20 @@ class SystemSettingsProvider : public CrosSettingsProvider,
   explicit SystemSettingsProvider(const NotifyObserversCallback& notify_cb);
   virtual ~SystemSettingsProvider();
 
-  // CrosSettingsProvider overrides.
+  // CrosSettingsProvider implementation.
   virtual const base::Value* Get(const std::string& path) const OVERRIDE;
-  virtual bool GetTrusted(const std::string& path,
-                          const base::Closure& callback) OVERRIDE;
+  virtual bool PrepareTrustedValues(const base::Closure& callback) OVERRIDE;
   virtual bool HandlesSetting(const std::string& path) const OVERRIDE;
   virtual void Reload() OVERRIDE;
 
-  // Overridden from TimezoneSettings::Observer:
+  // TimezoneSettings::Observer implementation.
   virtual void TimezoneChanged(const icu::TimeZone& timezone) OVERRIDE;
 
   // Creates the map of timezones used by the options page.
   base::ListValue* GetTimezoneList();
 
  private:
-  // CrosSettingsProvider overrides.
+  // CrosSettingsProvider implementation.
   virtual void DoSet(const std::string& path,
                      const base::Value& in_value) OVERRIDE;
 
diff --git a/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.cc b/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.cc
index 3479ec8..189efec 100644
--- a/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.cc
+++ b/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.cc
@@ -226,8 +226,7 @@ const base::Value* SystemSettingsProvider::Get(const std::string& path) const {
 }
 
 // The timezone is always trusted.
-bool SystemSettingsProvider::GetTrusted(const std::string& path,
-                                        const base::Closure& callback) {
+bool SystemSettingsProvider::PrepareTrustedValues(const base::Closure& cb) {
   return true;
 }
 
diff --git a/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.h b/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.h
index f70caa1..84f8755 100644
--- a/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.h
+++ b/chrome/browser/ui/webui/options2/chromeos/system_settings_provider2.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -28,21 +28,20 @@ class SystemSettingsProvider : public CrosSettingsProvider,
   explicit SystemSettingsProvider(const NotifyObserversCallback& notify_cb);
   virtual ~SystemSettingsProvider();
 
-  // CrosSettingsProvider overrides.
+  // CrosSettingsProvider implementation.
   virtual const base::Value* Get(const std::string& path) const OVERRIDE;
-  virtual bool GetTrusted(const std::string& path,
-                          const base::Closure& callback) OVERRIDE;
+  virtual bool PrepareTrustedValues(const base::Closure& callback) OVERRIDE;
   virtual bool HandlesSetting(const std::string& path) const OVERRIDE;
   virtual void Reload() OVERRIDE;
 
-  // Overridden from TimezoneSettings::Observer:
+  // TimezoneSettings::Observer implementation.
   virtual void TimezoneChanged(const icu::TimeZone& timezone) OVERRIDE;
 
   // Creates the map of timezones used by the options page.
   base::ListValue* GetTimezoneList();
 
  private:
-  // CrosSettingsProvider overrides.
+  // CrosSettingsProvider implementation.
   virtual void DoSet(const std::string& path,
                      const base::Value& in_value) OVERRIDE;