Add onChanged events to the extension settings API, both from sync and between

chrome/browser/extensions/extension_settings_storage.h

Index: chrome/browser/extensions/extension_settings_storage.h
diff --git a/chrome/browser/extensions/extension_settings_storage.h b/chrome/browser/extensions/extension_settings_storage.h
index afdff804f116d66d9aae65f1206884a9be68ef13..738adf587df7c1c14f4efb6c87eafabeed163815 100644
--- a/chrome/browser/extensions/extension_settings_storage.h
+++ b/chrome/browser/extensions/extension_settings_storage.h
@@ -23,26 +23,39 @@ class ExtensionSettingsStorage {
   // Supports lightweight copying.
   class Result {
    public:
-    // Ownership of |settings| and |changed_keys| taken.
+    // Ownership of all arguments are taken.
     // |settings| may be NULL when the result is for an operation which
     // generates no setting values (e.g. Remove(), Clear()).
-    // |changed_keys| may be NULL when the result is for an operation which
-    // cannot change settings (e.g. Get()).
-    Result(DictionaryValue* settings, std::set<std::string>* changed_keys);
+    // |changed_keys| and |old_settings| may be NULL when the result is for an
+    // operation which cannot change settings (e.g. Get()).
+    Result(
+        DictionaryValue* settings,
+        DictionaryValue* old_settings,
+        std::set<std::string>* changed_keys);
     explicit Result(const std::string& error);
     ~Result();
 
     // The dictionary result of the computation.  NULL does not imply invalid;
     // HasError() should be used to test this.
-    // Ownership remains with with the Result.
+    // Ownership remains with this.
     DictionaryValue* GetSettings() const;

[akalin, 2011/10/12 22:26:30]

return const pointer?

[not at google - send to devlin, 2011/10/13 06:40:43]

Done.

 
-    // Gets the list of setting keys which changed as a result of the
+    // Gets the set of setting keys which changed as a result of the
     // computation.  This includes all settings that existed and removed, all
-    // settings which changed when set, and all setting keys cleared.
-    // May be NULL for operations which cannot change settings, such as Get().
+    // settings which changed when set, and all setting keys cleared.  May be
+    // NULL for operations which cannot change settings, such as Get().
     std::set<std::string>* GetChangedKeys() const;

[akalin, 2011/10/12 22:26:30]

const pointer?

[not at google - send to devlin, 2011/10/13 06:40:43]

Done.

 
+    // Gets the value of a setting prior to the operation which generated
+    // this Result, or NULL if the setting had no original value.
+    // Ownerships remains with this.
+    // Returns false if this Result was from an operation which didn't modify
+    // the settings (such as Get()), in which case old_value will be untouched.
+    bool GetOldValue(const std::string& key, Value** old_value) const;

[akalin, 2011/10/12 22:26:30]

the user of this class doesn't distinguish between this returning false and
old_value being NULL.  So maybe it should just return a const Value*.

[not at google - send to devlin, 2011/10/13 06:40:43]

Done.

+
+    // Like GetOldValue, but gets the new value.
+    bool GetNewValue(const std::string& key, Value** new_value) const;

[akalin, 2011/10/12 22:26:30]

here too

[not at google - send to devlin, 2011/10/13 06:40:43]

Done.

+
     // Whether there was an error in the computation.  If so, the results of
     // GetSettings and ReleaseSettings are not valid.
     bool HasError() const;
@@ -55,11 +68,13 @@ class ExtensionSettingsStorage {
       // Empty error implies no error.
       Inner(
           DictionaryValue* settings,
+          DictionaryValue* old_settings,
           std::set<std::string>* changed_keys,
           const std::string& error);
       ~Inner();
 
       const scoped_ptr<DictionaryValue> settings_;
+      const scoped_ptr<DictionaryValue> old_settings_;
       const scoped_ptr<std::set<std::string> > changed_keys_;
       const std::string error_;
     };