Extension Storage API: make the onChanged event pass in a map of changes rather

than a list.


BUG=
TEST=*ExtensionSetting* (unit_tests, browser_tests)


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

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

5 files changed, 27 insertions, 108 deletions

diff --git a/chrome/browser/extensions/settings/setting_change.cc b/chrome/browser/extensions/settings/setting_change.cc
index ea2063a..5f0521d 100644
--- a/chrome/browser/extensions/settings/setting_change.cc
+++ b/chrome/browser/extensions/settings/setting_change.cc
@@ -11,20 +11,18 @@
 namespace extensions {
 
 /* static */
-std::string SettingChange::GetEventJson(
-    const SettingChangeList& changes) {
-  ListValue changes_value;
+std::string SettingChange::GetEventJson(const SettingChangeList& changes) {
+  DictionaryValue changes_value;
   for (SettingChangeList::const_iterator it = changes.begin();
       it != changes.end(); ++it) {
     DictionaryValue* change_value = new DictionaryValue();
-    change_value->SetString("key", it->key());
     if (it->old_value()) {
       change_value->Set("oldValue", it->old_value()->DeepCopy());
     }
     if (it->new_value()) {
       change_value->Set("newValue", it->new_value()->DeepCopy());
     }
-    changes_value.Append(change_value);
+    changes_value.Set(it->key(), change_value);
   }
   std::string json;
   base::JSONWriter::Write(&changes_value, false, &json);
diff --git a/chrome/browser/extensions/settings/setting_change.h b/chrome/browser/extensions/settings/setting_change.h
index 0e29219..99e2bad 100644
--- a/chrome/browser/extensions/settings/setting_change.h
+++ b/chrome/browser/extensions/settings/setting_change.h
@@ -21,7 +21,7 @@ typedef std::vector<SettingChange> SettingChangeList;
 class SettingChange {
  public:
   // Converts an SettingChangeList into JSON of the form:
-  // [ { "key": "foo", "oldValue": "bar", "newValue": "baz" } ]
+  // { "foo": { "key": "foo", "oldValue": "bar", "newValue": "baz" } }
   static std::string GetEventJson(const SettingChangeList& changes);
 
   // Ownership of |old_value| and |new_value| taken.
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index f6f0b31..1330a98 100644
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -1021,7 +1021,6 @@
         "id": "StorageChange",
         "type": "object",
         "properties": {
-          "key": {"type": "string", "description": "The key of the item which changed."},
           "oldValue": {
             "type": "any",
             "description": "The old value of the item, if there was an old value.",
@@ -1147,13 +1146,15 @@
         "parameters": [
           {
             "name": "changes",
-            "type": "array",
-            "items": {"$ref": "StorageChange"}
+            "type": "object",
+            "properties": {},
+            "additionalProperties": { "$ref": "StorageChange" },
+            "description": "Object mapping each key that changed to its corresponding <a href='#type-StorageChange'>StorageChange</a> for that item."
           },
           {
             "name": "namespace",
             "type": "string",
-            "description": "The namespace (e.g. sync or local) of the storage area the changes are for."
+            "description": "The namespace (\"sync\" or \"local\") of the storage area the changes are for."
           }
         ]
       }
diff --git a/chrome/common/extensions/docs/experimental.storage.html b/chrome/common/extensions/docs/experimental.storage.html
index f288926..d2543c0 100644
--- a/chrome/common/extensions/docs/experimental.storage.html
+++ b/chrome/common/extensions/docs/experimental.storage.html
@@ -652,7 +652,7 @@
 
               <div class="summary">
                 <!-- Note: intentionally longer 80 columns -->
-                <span class="subdued">chrome.experimental.storage.</span><span>onChanged</span><span class="subdued">.addListener</span>(function(<span>array of StorageChange changes, string namespace</span>) <span class="subdued">{...}</span><span></span>);
+                <span class="subdued">chrome.experimental.storage.</span><span>onChanged</span><span class="subdued">.addListener</span>(function(<span>object changes, string namespace</span>) <span class="subdued">{...}</span><span></span>);
               </div>
 
               <div class="description">
@@ -679,21 +679,10 @@
                         <a> Type</a>
                       </span>
                       <span>
-                        <span>
-                          array of <span><span>
-                      <span>
-                        <a href="experimental.storage.html#type-StorageChange">StorageChange</a>
-                      </span>
-                      <span style="display: none; ">
-                        <span>
+                        <span style="display: none; ">
                           array of <span><span></span></span>
                         </span>
-                        <span>paramType</span>
-                        <span></span>
-                      </span>
-                    </span></span>
-                        </span>
-                        <span style="display: none; ">paramType</span>
+                        <span>object</span>
                         <span style="display: none; "></span>
                       </span>
                     </span>
@@ -702,12 +691,10 @@
 
               </em>
           </dt>
-          <dd class="todo">
+          <dd class="todo" style="display: none; ">
             Undocumented.
           </dd>
-          <dd style="display: none; ">
-            Description of this parameter from the json schema.
-          </dd>
+          <dd>Object mapping each key that changed to its corresponding <a href="#type-StorageChange">StorageChange</a> for that item.</dd>
           <dd style="display: none; ">
             This parameter was added in version
             <b><span></span></b>.
@@ -719,9 +706,9 @@
           </dd>
 
           <!-- OBJECT PROPERTIES -->
-          <dd style="display: none; ">
+          <dd>
             <dl>
-              <div>
+              <div style="display: none; ">
                 <div>
                 </div>
               </div>
@@ -775,7 +762,7 @@
           <dd class="todo" style="display: none; ">
             Undocumented.
           </dd>
-          <dd>The namespace (e.g. sync or local) of the storage area the changes are for.</dd>
+          <dd>The namespace ("sync" or "local") of the storage area the changes are for.</dd>
           <dd style="display: none; ">
             This parameter was added in version
             <b><span></span></b>.
@@ -900,74 +887,6 @@
               <div>
                 <div>
           <dt>
-            <var>key</var>
-              <em>
-
-                <!-- TYPE -->
-                <div style="display:inline">
-                  (
-                    <span class="optional" style="display: none; ">optional</span>
-                    <span class="enum" style="display: none; ">enumerated</span>
-                    <span id="typeTemplate">
-                      <span style="display: none; ">
-                        <a> Type</a>
-                      </span>
-                      <span>
-                        <span style="display: none; ">
-                          array of <span><span></span></span>
-                        </span>
-                        <span>string</span>
-                        <span style="display: none; "></span>
-                      </span>
-                    </span>
-                  )
-                </div>
-
-              </em>
-          </dt>
-          <dd class="todo" style="display: none; ">
-            Undocumented.
-          </dd>
-          <dd>The key of the item which changed.</dd>
-          <dd style="display: none; ">
-            This parameter was added in version
-            <b><span></span></b>.
-            You must omit this parameter in earlier versions,
-            and you may omit it in any version.  If you require this
-            parameter, the manifest key
-            <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a>
-            can ensure that your extension won't be run in an earlier browser version.
-          </dd>
-
-          <!-- OBJECT PROPERTIES -->
-          <dd style="display: none; ">
-            <dl>
-              <div>
-                <div>
-                </div>
-              </div>
-            </dl>
-          </dd>
-
-          <!-- OBJECT METHODS -->
-          <dd style="display: none; ">
-            <div></div>
-          </dd>
-
-          <!-- OBJECT EVENT FIELDS -->
-          <dd style="display: none; ">
-            <div></div>
-          </dd>
-
-          <!-- FUNCTION PARAMETERS -->
-          <dd style="display: none; ">
-            <div></div>
-          </dd>
-
-        </div>
-              </div><div>
-                <div>
-          <dt>
             <var>oldValue</var>
               <em>
 
diff --git a/chrome/test/data/extensions/api_test/settings/split_incognito/background.html b/chrome/test/data/extensions/api_test/settings/split_incognito/background.html
index d98b74a..5bc49c0 100644
--- a/chrome/test/data/extensions/api_test/settings/split_incognito/background.html
+++ b/chrome/test/data/extensions/api_test/settings/split_incognito/background.html
@@ -4,11 +4,12 @@ var assertEq = chrome.test.assertEq;
 var inIncognitoContext = chrome.extension.inIncognitoContext;
 
 ['sync', 'local'].forEach(function(namespace) {
-  api[namespace].notifications = [];
+  api[namespace].notifications = {};
   api.onChanged.addListener(function(changes, event_namespace) {
     if (event_namespace == namespace) {
-      changes.forEach(function(change) {
-        api[namespace].notifications.push(change);
+      var notifications = api[namespace].notifications;
+      Object.keys(changes).forEach(function(key) {
+        notifications[key] = changes[key];
       });
     }
   });
@@ -25,13 +26,13 @@ var testActions = {
   },
   assertEmpty: function(callback) {
     this.get(null, function(settings) {
-      chrome.test.assertEq({}, settings);
+      assertEq({}, settings);
       callback();
     });
   },
   assertFoo: function(callback) {
     this.get(null, function(settings) {
-      chrome.test.assertEq({foo: "bar"}, settings);
+      assertEq({foo: "bar"}, settings);
       callback();
     });
   },
@@ -45,19 +46,19 @@ var testActions = {
     this.clear(callback);
   },
   assertNoNotifications: function(callback) {
-    assertEq([], this.notifications);
+    assertEq({}, this.notifications);
     callback();
   },
   clearNotifications: function(callback) {
-    this.notifications.length = 0;
+    this.notifications = {};
     callback();
   },
   assertAddFooNotification: function(callback) {
-    assertEq([{ key: 'foo', newValue: 'bar' }], this.notifications);
+    assertEq({ foo: { newValue: 'bar' } }, this.notifications);
     callback();
   },
   assertDeleteFooNotification: function(callback) {
-    assertEq([{ key: 'foo', oldValue: 'bar' }], this.notifications);
+    assertEq({ foo: { oldValue: 'bar' } }, this.notifications);
     callback();
   }
 };