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

chrome/browser/extensions/extension_settings_storage.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_STORAGE_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_STORAGE_H_
 #pragma once
 
 #include <set>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
 
 // Interface for extension settings storage classes.
 //
 // All methods *must* be run on the FILE thread, inclusing construction and
 // destructions.
 class ExtensionSettingsStorage {
  public:
   // The result of an operation.
   //
   // 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;
 
     // Gets the error message, if any.
     const std::string& GetError() const;
 
    private:
     struct Inner : public base::RefCountedThreadSafe<Inner> {
       // 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_;
     };
 
     const scoped_refptr<Inner> inner_;
   };
 
   virtual ~ExtensionSettingsStorage() {}
 
   // Gets a single value from storage.
@@ skipping 23 matching lines @@
   // Removes multiple keys from the storage.
   // If successful, result value is NULL.
   virtual Result Remove(const std::vector<std::string>& keys) = 0;
 
   // Clears the storage.
   // If successful, result value is NULL.
   virtual Result Clear() = 0;
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_STORAGE_H_