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

chrome/browser/chromeos/cros_settings.h

Index: chrome/browser/chromeos/cros_settings.h
diff --git a/chrome/browser/chromeos/cros_settings.h b/chrome/browser/chromeos/cros_settings.h
index 90c8dcd428da318b2c516d11eee0a9d1edbd932b..d27611d8b028deedf5ac9875a16629e27d48e03a 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);

[Denis Lagno, 2011/10/04 17:01:50]

AFAIU base::Values are intended to be created via Create functions, so users
normally operate with pointers to Value.  Thus why take reference instead of
pointer?

[pastarmovj, 2011/10/05 08:42:31]

This is true because the usual pref stores take ownership of those values and
take care to destroy them. In signed settings however the values are not stored
internally so I decided to go for an interface that makes sure the caller takes
the responsibility for the lifetime of the Value object itself to avoid memory
leaks. I think this way is much safer and straightforward for the users than
using a pointer and taking ownership of it.

[Denis Lagno, 2011/10/05 17:50:10]

So you use const reference as a signal that ownership is not transferred.  I am
not sure if this style is acknowledged in  chromium codebase.  For instance
Value::Equals uses pointers as arguments and still ownership is not transferred.

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

Actually my interface exactly mimics the interface of the PrefService class
which was my aim from the very beginning so I am not an exception rather a
confirmation for the rule :)

Excerpt from pref_Service.h

  // If the path is valid (i.e., registered), update the pref value in the user
  // prefs.
  // To set the value of dictionary or list values in the pref tree use
  // Set(), but to modify the value of a dictionary or list use either
  // ListPrefUpdate or DictionaryPrefUpdate from scoped_user_pref_update.h.
  void Set(const char* path, const base::Value& value);
  void SetBoolean(const char* path, bool value);
  void SetInteger(const char* path, int value);
  void SetDouble(const char* path, double value);
  void SetString(const char* path, const std::string& value);
  void SetFilePath(const char* path, const FilePath& value);

 
   // Fires system setting change notification.
   // TODO(pastarmovj): Consider to remove this function from the public
@@ -59,6 +58,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.