PART2: Make SignedSettings use proper Value types instead of string all around the place.

chrome/browser/chromeos/user_cros_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/user_cros_settings_provider.h"
 
 #include <map>
 #include <set>
 
 #include "base/hash_tables.h"
@@ skipping 56 matching lines @@
  public:
   explicit MigrationHelper() : callback_(NULL) {
     registrar_.Add(this, chrome::NOTIFICATION_OWNERSHIP_CHECKED,
                    NotificationService::AllSources());
   }
 
   void set_callback(SignedSettingsHelper::Callback* callback) {
     callback_ = callback;
   }
 
-  void AddMigrationValue(const std::string& path, const std::string& value) {
+  void AddMigrationValue(const std::string& path, base::Value* value) {
     migration_values_[path] = value;

[Mattias Nissler (ping if slow), 2011/10/07 11:02:57]

this may leak btw. if called twice for the same value. Consider using
PrefValueMap.

[pastarmovj, 2011/10/13 11:25:06]

Done.

   }
 
   void MigrateValues(void) {
     ownership_checker_.reset(new OwnershipStatusChecker(NewCallback(
         this, &MigrationHelper::DoMigrateValues)));
   }
 
   // NotificationObserver overrides:
   virtual void Observe(int type,
                        const NotificationSource& source,
                        const NotificationDetails& details) OVERRIDE {
     if (type == chrome::NOTIFICATION_OWNERSHIP_CHECKED)
       MigrateValues();
   }
 
  private:
   void DoMigrateValues(OwnershipService::Status status,
                        bool current_user_is_owner) {
     ownership_checker_.reset(NULL);
 
     // We can call StartStorePropertyOp in two cases - either if the owner is
     // currently logged in and the policy can be updated immediately or if there
     // is no owner yet in which case the value will be temporarily stored in the
     // 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;
+      std::map<std::string, base::Value*>::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_);
+        // We have to take care of the Value ourselves.
+        delete i->second;
       }
       migration_values_.clear();
     }
   }
 
   NotificationRegistrar registrar_;
   scoped_ptr<OwnershipStatusChecker> ownership_checker_;
   SignedSettingsHelper::Callback* callback_;
 
-  std::map<std::string, std::string> migration_values_;
+  std::map<std::string, base::Value*> migration_values_;
 
   DISALLOW_COPY_AND_ASSIGN(MigrationHelper);
 };
 
+// A task that exports the Op completion to a Closure callback.
+class TrustedEntryFetcher : public Task {
+ public:
+  explicit TrustedEntryFetcher(const base::Closure& callback)
+      : callback_(callback) {
+  }
+
+  void Run() {
+    callback_.Run();
+  }
+
+ private:
+  base::Closure callback_;
+};
+
 bool IsControlledBooleanSetting(const std::string& pref_path) {
   // TODO(nkostylev): Using std::find for 4 value array generates this warning
   // in chroot stl_algo.h:231: error: array subscript is above array bounds.
   // GCC 4.4.3
   return (pref_path == kAccountsPrefAllowNewUser) ||
          (pref_path == kAccountsPrefAllowGuest) ||
          (pref_path == kAccountsPrefShowUserNamesOnSignIn) ||
          (pref_path == kSignedDataRoamingEnabled) ||
          (pref_path == kStatsReportingPref);
 }
 
 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));
+}
+
 void RegisterSetting(PrefService* local_state, const std::string& pref_path) {
   local_state->RegisterBooleanPref((pref_path + kTrustedSuffix).c_str(),
                                    false,
                                    PrefService::UNSYNCABLE_PREF);
   if (IsControlledBooleanSetting(pref_path)) {
     if (pref_path == kSignedDataRoamingEnabled ||
         pref_path == kStatsReportingPref)
       local_state->RegisterBooleanPref(pref_path.c_str(),
                                        false,
                                        PrefService::UNSYNCABLE_PREF);
@@ skipping 10 matching lines @@
     local_state->RegisterListPref(pref_path.c_str(),
                                   PrefService::UNSYNCABLE_PREF);
   }
 }
 
 enum UseValue {
   USE_VALUE_SUPPLIED,
   USE_VALUE_DEFAULT
 };
 
-void UpdateCacheBool(const std::string& name,
-                     bool 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->SetBoolean(name.c_str(), value);
+    prefs->Set(name.c_str(), value);
   prefs->ScheduleSavePersistentPrefs();
 }
 
-void UpdateCacheString(const std::string& name,
-                       const std::string& 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->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() {
     return Singleton<UserCrosSettingsTrust>::get();
   }
 
   // Working horse for UserCrosSettingsProvider::RequestTrusted* family.
   bool RequestTrustedEntity(const std::string& name) {
     OwnershipService::Status ownership_status =
         ownership_service_->GetStatus(false);
@@ skipping 21 matching lines @@
         callbacks_[name].push_back(callback);
       return false;
     }
   }
 
   void Reload() {
     for (size_t i = 0; i < arraysize(kBooleanSettings); ++i)
       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()));
 
     if (!UserManager::Get()->current_user_is_owner()) {
       LOG(WARNING) << "Changing settings from non-owner, setting=" << path;
 
       // Revert UI change.
       CrosSettings::Get()->FireObservers(path.c_str());
       return;
     }
+    if (IsControlledSetting(path)) {
+      if (IsControlledBooleanSetting(path)) {
+        bool bool_value = false;
+        if (in_value.GetAsBoolean(&bool_value)) {
+          OnBooleanPropertyChange(path, bool_value);
+        }
+      }
+      SignedSettingsHelper::Get()->StartStorePropertyOp(
+          path, in_value, this);
+      UpdateCache(path, in_value, USE_VALUE_SUPPLIED);
 
-    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.";
-      }
-    } 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.";
+      LOG(ERROR) << "Set cros setting " << path;
     } else {
       LOG(WARNING) << "Try to set unhandled cros setting " << path;
     }
   }
 
  private:
   // upper bound for number of retries to fetch a signed setting.
   static const int kNumRetriesLimit = 9;
 
   UserCrosSettingsTrust()
@@ skipping 26 matching lines @@
       }
       cros->SetCellularDataRoamingAllowed(new_value);
     } else if (path == kStatsReportingPref) {
       // TODO(pastarmovj): Remove this once we don't need to regenerate the
       // consent file for the GUID anymore.
       OptionsUtil::ResolveMetricsReportingEnabled(new_value);
     }
   }
 
   // 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) {
       if (!CrosLibrary::Get()->EnsureLoaded())
         return;
 
       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 {
-          bool new_value = (use_value == USE_VALUE_SUPPLIED) ? value : false;
+          bool prop_val;
+          value.GetAsBoolean(&prop_val);
+          bool new_value = (use_value == USE_VALUE_SUPPLIED) ? prop_val : false;
           if (device_value != new_value)
             cros->SetCellularDataRoamingAllowed(new_value);
         }
       }
     } else if (path == kStatsReportingPref) {
-      bool stats_consent = (use_value == USE_VALUE_SUPPLIED) ? value : false;
+      bool prop_val;
+      value.GetAsBoolean(&prop_val);
+      bool stats_consent = (use_value == USE_VALUE_SUPPLIED) ? prop_val : false;
       // 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) {
         // 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;
         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.");
       }
       // TODO(pastarmovj): Remove this once we don't need to regenerate the
       // consent file for the GUID anymore.
       VLOG(1) << "Metrics policy is being set to : " << stats_consent
               << "(reason : " << use_value << ")";
       OptionsUtil::ResolveMetricsReportingEnabled(stats_consent);
     }
   }
 
   void StartFetchingSetting(const std::string& name) {
     DCHECK(g_browser_process);
     PrefService* prefs = g_browser_process->local_state();
     if (!prefs)
       return;
     // Do not trust before fetching complete.
     prefs->ClearPref((name + kTrustedSuffix).c_str());
     prefs->ScheduleSavePersistentPrefs();
     if (CrosLibrary::Get()->EnsureLoaded()) {
       SignedSettingsHelper::Get()->StartRetrieveProperty(name, this);
     }
   }
 
   // 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;
     }
 
     bool is_owned = ownership_service_->GetStatus(true) ==
         OwnershipService::OWNERSHIP_TAKEN;
     PrefService* prefs = g_browser_process->local_state();
     switch (code) {
       case SignedSettings::SUCCESS:
       case SignedSettings::NOT_FOUND:
       case SignedSettings::KEY_UNAVAILABLE: {
         bool fallback_to_default = !is_owned
             || (code == SignedSettings::NOT_FOUND);
         DCHECK(fallback_to_default || code == SignedSettings::SUCCESS);
         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:
       default: {
         DCHECK(code == SignedSettings::OPERATION_FAILED);
         DCHECK(is_owned);
         LOG(ERROR) << "On owned device: failed to retrieve cros "
                       "setting, name=" << name;
         if (retries_left_ > 0) {
           retries_left_ -= 1;
           StartFetchingSetting(name);
           return;
         }
         LOG(ERROR) << "No retries left";
         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, USE_VALUE_SUPPLIED);
+          UpdateCache(name, *false_value, USE_VALUE_SUPPLIED);
         } else {
           prefs->ClearPref((name + kTrustedSuffix).c_str());
           return;
         }
         break;
       }
     }
     prefs->SetBoolean((name + kTrustedSuffix).c_str(), true);
     {
       std::vector<Task*>& callbacks_vector = callbacks_[name];
       for (size_t i = 0; i < callbacks_vector.size(); ++i)
         MessageLoop::current()->PostTask(FROM_HERE, callbacks_vector[i]);
       callbacks_vector.clear();
     }
     if (code == SignedSettings::SUCCESS)
       CrosSettings::Get()->FireObservers(name.c_str());
   }
 
   // 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)
       SignedSettingsHelper::Get()->StartRetrieveProperty(name, this);
   }
 
   // Implementation of SignedSettingsHelper::Callback.
   virtual void OnWhitelistCompleted(SignedSettings::ReturnCode code,
                                     const std::string& email) {
     VLOG(1) << "Add " << email << " to whitelist, code=" << code;
@@ skipping 52 matching lines @@
     RegisterSetting(local_state, kStringSettings[i]);
   for (size_t i = 0; i < arraysize(kListSettings); ++i)
     RegisterSetting(local_state, kListSettings[i]);
 }
 
 void UserCrosSettingsProvider::Reload() {
   UserCrosSettingsTrust::GetInstance()->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();
   }
   return NULL;
 }
 
-class TrustedEntryFetcher : public Task {
- public:
-  explicit TrustedEntryFetcher(const base::Closure& callback)
-      : callback_(callback) {
-  }
-
-  void Run() {
-    callback_.Run();
-  }
-
- private:
-  base::Closure callback_;
-};
-
 bool UserCrosSettingsProvider::GetTrusted(const std::string& path,
                                           const base::Closure& callback) const {
   return UserCrosSettingsTrust::GetInstance()->RequestTrustedEntity(
       path, new TrustedEntryFetcher(callback));
 }
 
 bool UserCrosSettingsProvider::HandlesSetting(const std::string& path) const {
   return ::StartsWithASCII(path, "cros.accounts.", true) ||
       ::StartsWithASCII(path, "cros.signed.", true) ||
       ::StartsWithASCII(path, "cros.metrics.", true) ||
@@ skipping 18 matching lines @@
 
   PrefService* prefs = g_browser_process->local_state();
   ListPrefUpdate cached_whitelist_update(prefs, kAccountsPrefUsers);
   StringValue email_value(email);
   if (cached_whitelist_update->Remove(email_value, NULL))
     prefs->ScheduleSavePersistentPrefs();
 }
 
 // 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