Remove a bunch of DeepCopy() calls in the chrome.storage API.

chrome/browser/extensions/api/storage/settings_sync_unittest.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
+#include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 #include "chrome/browser/extensions/api/storage/sync_value_store_cache.h"
 #include "chrome/browser/extensions/api/storage/syncable_settings_storage.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
 #include "extensions/browser/api/storage/leveldb_settings_storage_factory.h"
 #include "extensions/browser/api/storage/settings_storage_factory.h"
@@ skipping 16 matching lines @@
 
 namespace extensions {
 
 namespace util = settings_test_util;
 
 namespace {
 
 // To save typing ValueStore::DEFAULTS everywhere.
 const ValueStore::WriteOptions DEFAULTS = ValueStore::DEFAULTS;
 
+// More saving typing. Maps extension IDs to a list of sync changes for that
+// extension.
+using SettingSyncDataMultimap =
+    std::map<std::string, linked_ptr<SettingSyncDataList>>;
+
 // Gets the pretty-printed JSON for a value.
 static std::string GetJson(const base::Value& value) {
   std::string json;
   base::JSONWriter::WriteWithOptions(&value,
                                      base::JSONWriter::OPTIONS_PRETTY_PRINT,
                                      &json);
   return json;
 }
 
 // Returns whether two Values are equal.
@@ skipping 45 matching lines @@
       const syncer::SyncChangeList& change_list) override {
     if (fail_all_requests_) {
       return syncer::SyncError(
           FROM_HERE,
           syncer::SyncError::DATATYPE_ERROR,
           "MockSyncChangeProcessor: configured to fail",
           change_list[0].sync_data().GetDataType());
     }
     for (syncer::SyncChangeList::const_iterator it = change_list.begin();
         it != change_list.end(); ++it) {
-      changes_.push_back(SettingSyncData(*it));
+      changes_.push_back(new SettingSyncData(*it));
     }
     return syncer::SyncError();
   }
 
   syncer::SyncDataList GetAllSyncData(syncer::ModelType type) const override {
     return syncer::SyncDataList();
   }
 
   // Mock methods.
 
   const SettingSyncDataList& changes() { return changes_; }
 
   void ClearChanges() {
     changes_.clear();
   }
 
   void set_fail_all_requests(bool fail_all_requests) {
     fail_all_requests_ = fail_all_requests;
   }
 
   // Returns the only change for a given extension setting.  If there is not
   // exactly 1 change for that key, a test assertion will fail.
-  SettingSyncData GetOnlyChange(
-      const std::string& extension_id, const std::string& key) {
-    SettingSyncDataList matching_changes;
+  SettingSyncData* GetOnlyChange(const std::string& extension_id,
+                                 const std::string& key) {
+    std::vector<SettingSyncData*> matching_changes;
     for (SettingSyncDataList::iterator it = changes_.begin();
         it != changes_.end(); ++it) {
-      if (it->extension_id() == extension_id && it->key() == key) {
+      if ((*it)->extension_id() == extension_id && (*it)->key() == key) {
         matching_changes.push_back(*it);
       }
     }
     if (matching_changes.empty()) {
       ADD_FAILURE() << "No matching changes for " << extension_id << "/" <<
           key << " (out of " << changes_.size() << ")";
-      return SettingSyncData(syncer::SyncChange::ACTION_INVALID,
-                             std::string(),
-                             std::string(),
-                             scoped_ptr<base::Value>(
-                                 new base::DictionaryValue()));
+      return nullptr;
     }
     if (matching_changes.size() != 1u) {
       ADD_FAILURE() << matching_changes.size() << " matching changes for " <<
            extension_id << "/" << key << " (out of " << changes_.size() << ")";
     }
     return matching_changes[0];
   }
 
  private:
   SettingSyncDataList changes_;
@@ skipping 77 matching lines @@
   // Gets the syncer::SyncableService for the given sync type.
   syncer::SyncableService* GetSyncableService(syncer::ModelType model_type) {
     base::MessageLoop::current()->RunUntilIdle();
     SyncValueStoreCache* sync_cache = static_cast<SyncValueStoreCache*>(
         frontend_->GetValueStoreCache(settings_namespace::SYNC));
     return sync_cache->GetSyncableService(model_type);
   }
 
   // Gets all the sync data from the SyncableService for a sync type as a map
   // from extension id to its sync data.
-  std::map<std::string, SettingSyncDataList> GetAllSyncData(
-      syncer::ModelType model_type) {
+  SettingSyncDataMultimap GetAllSyncData(syncer::ModelType model_type) {
     syncer::SyncDataList as_list =
         GetSyncableService(model_type)->GetAllSyncData(model_type);
-    std::map<std::string, SettingSyncDataList> as_map;
+    SettingSyncDataMultimap as_map;
     for (syncer::SyncDataList::iterator it = as_list.begin();
         it != as_list.end(); ++it) {
-      SettingSyncData sync_data(*it);
-      as_map[sync_data.extension_id()].push_back(sync_data);
+      SettingSyncData* sync_data = new SettingSyncData(*it);
+      linked_ptr<SettingSyncDataList>& list_for_extension =
+          as_map[sync_data->extension_id()];
+      if (!list_for_extension.get())
+        list_for_extension.reset(new SettingSyncDataList());
+      list_for_extension->push_back(sync_data);
     }
     return as_map;
   }
 
   // Need these so that the DCHECKs for running on FILE or UI threads pass.
   base::MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
 
   base::ScopedTempDir temp_dir_;
@@ skipping 40 matching lines @@
   base::StringValue value1("fooValue");
   base::ListValue value2;
   value2.Append(new base::StringValue("barValue"));
 
   ValueStore* storage1 = AddExtensionAndGetStorage("s1", type);
   ValueStore* storage2 = AddExtensionAndGetStorage("s2", type);
 
   storage1->Set(DEFAULTS, "foo", value1);
   storage2->Set(DEFAULTS, "bar", value2);
 
-  std::map<std::string, SettingSyncDataList> all_sync_data =
-      GetAllSyncData(model_type);
+  SettingSyncDataMultimap all_sync_data = GetAllSyncData(model_type);
   EXPECT_EQ(2u, all_sync_data.size());
-  EXPECT_EQ(1u, all_sync_data["s1"].size());
-  EXPECT_PRED_FORMAT2(ValuesEq, &value1, &all_sync_data["s1"][0].value());
-  EXPECT_EQ(1u, all_sync_data["s2"].size());
-  EXPECT_PRED_FORMAT2(ValuesEq, &value2, &all_sync_data["s2"][0].value());
+  EXPECT_EQ(1u, all_sync_data["s1"]->size());
+  EXPECT_PRED_FORMAT2(ValuesEq, &value1, &(*all_sync_data["s1"])[0]->value());
+  EXPECT_EQ(1u, all_sync_data["s2"]->size());
+  EXPECT_PRED_FORMAT2(ValuesEq, &value2, &(*all_sync_data["s2"])[0]->value());
 
   syncer::SyncDataList sync_data;
   sync_data.push_back(settings_sync_util::CreateData(
       "s1", "foo", value1, model_type));
   sync_data.push_back(settings_sync_util::CreateData(
       "s2", "bar", value2, model_type));
 
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           sync_data,
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
   // Already in sync, so no changes.
   EXPECT_EQ(0u, sync_processor_->changes().size());
 
   // Regression test: not-changing the synced value shouldn't result in a sync
   // change, and changing the synced value should result in an update.
   storage1->Set(DEFAULTS, "foo", value1);
   EXPECT_EQ(0u, sync_processor_->changes().size());
 
   storage1->Set(DEFAULTS, "foo", value2);
   EXPECT_EQ(1u, sync_processor_->changes().size());
-  SettingSyncData change = sync_processor_->GetOnlyChange("s1", "foo");
-  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
-  EXPECT_TRUE(value2.Equals(&change.value()));
+  SettingSyncData* change = sync_processor_->GetOnlyChange("s1", "foo");
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change->change_type());
+  EXPECT_TRUE(value2.Equals(&change->value()));
 
   GetSyncableService(model_type)->StopSyncing(model_type);
 }
 
 TEST_F(ExtensionSettingsSyncTest, LocalDataWithNoSyncDataIsPushedToSync) {
   syncer::ModelType model_type = syncer::EXTENSION_SETTINGS;
   Manifest::Type type = Manifest::TYPE_EXTENSION;
 
   base::StringValue value1("fooValue");
   base::ListValue value2;
   value2.Append(new base::StringValue("barValue"));
 
   ValueStore* storage1 = AddExtensionAndGetStorage("s1", type);
   ValueStore* storage2 = AddExtensionAndGetStorage("s2", type);
 
   storage1->Set(DEFAULTS, "foo", value1);
   storage2->Set(DEFAULTS, "bar", value2);
 
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
   // All settings should have been pushed to sync.
   EXPECT_EQ(2u, sync_processor_->changes().size());
-  SettingSyncData change = sync_processor_->GetOnlyChange("s1", "foo");
-  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change.change_type());
-  EXPECT_TRUE(value1.Equals(&change.value()));
+  SettingSyncData* change = sync_processor_->GetOnlyChange("s1", "foo");
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change->change_type());
+  EXPECT_TRUE(value1.Equals(&change->value()));
   change = sync_processor_->GetOnlyChange("s2", "bar");
-  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change.change_type());
-  EXPECT_TRUE(value2.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change->change_type());
+  EXPECT_TRUE(value2.Equals(&change->value()));
 
   GetSyncableService(model_type)->StopSyncing(model_type);
 }
 
 TEST_F(ExtensionSettingsSyncTest, AnySyncDataOverwritesLocalData) {
   syncer::ModelType model_type = syncer::APP_SETTINGS;
   Manifest::Type type = Manifest::TYPE_LEGACY_PACKAGED_APP;
 
   base::StringValue value1("fooValue");
   base::ListValue value2;
@@ skipping 138 matching lines @@
           sync_data,
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
   // Add something locally.
   storage1->Set(DEFAULTS, "bar", value2);
   storage2->Set(DEFAULTS, "bar", value2);
   storage3->Set(DEFAULTS, "foo", value1);
   storage4->Set(DEFAULTS, "foo", value1);
 
-  SettingSyncData change = sync_processor_->GetOnlyChange("s1", "bar");
-    EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change.change_type());
-    EXPECT_TRUE(value2.Equals(&change.value()));
+  SettingSyncData* change = sync_processor_->GetOnlyChange("s1", "bar");
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change->change_type());
+  EXPECT_TRUE(value2.Equals(&change->value()));
   sync_processor_->GetOnlyChange("s2", "bar");
-    EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change.change_type());
-    EXPECT_TRUE(value2.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change->change_type());
+  EXPECT_TRUE(value2.Equals(&change->value()));
   change = sync_processor_->GetOnlyChange("s3", "foo");
-    EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change.change_type());
-    EXPECT_TRUE(value1.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change->change_type());
+  EXPECT_TRUE(value1.Equals(&change->value()));
   change = sync_processor_->GetOnlyChange("s4", "foo");
-    EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change.change_type());
-    EXPECT_TRUE(value1.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, change->change_type());
+  EXPECT_TRUE(value1.Equals(&change->value()));
 
   // Change something locally, storage1/3 the new setting and storage2/4 the
   // initial setting, for all combinations of local vs sync intialisation and
   // new vs initial.
   sync_processor_->ClearChanges();
   storage1->Set(DEFAULTS, "bar", value1);
   storage2->Set(DEFAULTS, "foo", value2);
   storage3->Set(DEFAULTS, "bar", value1);
   storage4->Set(DEFAULTS, "foo", value2);
 
   change = sync_processor_->GetOnlyChange("s1", "bar");
-    EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
-    EXPECT_TRUE(value1.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change->change_type());
+  EXPECT_TRUE(value1.Equals(&change->value()));
   change = sync_processor_->GetOnlyChange("s2", "foo");
-    EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
-    EXPECT_TRUE(value2.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change->change_type());
+  EXPECT_TRUE(value2.Equals(&change->value()));
   change = sync_processor_->GetOnlyChange("s3", "bar");
-    EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
-    EXPECT_TRUE(value1.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change->change_type());
+  EXPECT_TRUE(value1.Equals(&change->value()));
   change = sync_processor_->GetOnlyChange("s4", "foo");
-    EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
-    EXPECT_TRUE(value2.Equals(&change.value()));
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change->change_type());
+  EXPECT_TRUE(value2.Equals(&change->value()));
 
   // Remove something locally, storage1/3 the new setting and storage2/4 the
   // initial setting, for all combinations of local vs sync intialisation and
   // new vs initial.
   sync_processor_->ClearChanges();
   storage1->Remove("foo");
   storage2->Remove("bar");
   storage3->Remove("foo");
   storage4->Remove("bar");
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s1", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s2", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s3", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s4", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s1", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s2", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s3", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s4", "bar")->change_type());
 
   // Remove some nonexistent settings.
   sync_processor_->ClearChanges();
   storage1->Remove("foo");
   storage2->Remove("bar");
   storage3->Remove("foo");
   storage4->Remove("bar");
 
   EXPECT_EQ(0u, sync_processor_->changes().size());
 
   // Clear the rest of the settings.  Add the removed ones back first so that
   // more than one setting is cleared.
   storage1->Set(DEFAULTS, "foo", value1);
   storage2->Set(DEFAULTS, "bar", value2);
   storage3->Set(DEFAULTS, "foo", value1);
   storage4->Set(DEFAULTS, "bar", value2);
 
   sync_processor_->ClearChanges();
   storage1->Clear();
   storage2->Clear();
   storage3->Clear();
   storage4->Clear();
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s1", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s1", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s2", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s2", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s3", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s3", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s4", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_DELETE,
-      sync_processor_->GetOnlyChange("s4", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s1", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s1", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s2", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s2", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s3", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s3", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s4", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
+            sync_processor_->GetOnlyChange("s4", "bar")->change_type());
 
   GetSyncableService(model_type)->StopSyncing(model_type);
 }
 
 TEST_F(ExtensionSettingsSyncTest, ExtensionAndAppSettingsSyncSeparately) {
   base::StringValue value1("fooValue");
   base::ListValue value2;
   value2.Append(new base::StringValue("barValue"));
 
   // storage1 is an extension, storage2 is an app.
   ValueStore* storage1 = AddExtensionAndGetStorage(
       "s1", Manifest::TYPE_EXTENSION);
   ValueStore* storage2 = AddExtensionAndGetStorage(
       "s2", Manifest::TYPE_LEGACY_PACKAGED_APP);
 
   storage1->Set(DEFAULTS, "foo", value1);
   storage2->Set(DEFAULTS, "bar", value2);
 
-  std::map<std::string, SettingSyncDataList> extension_sync_data =
+  SettingSyncDataMultimap extension_sync_data =
       GetAllSyncData(syncer::EXTENSION_SETTINGS);
   EXPECT_EQ(1u, extension_sync_data.size());
-  EXPECT_EQ(1u, extension_sync_data["s1"].size());
-  EXPECT_PRED_FORMAT2(ValuesEq, &value1, &extension_sync_data["s1"][0].value());
+  EXPECT_EQ(1u, extension_sync_data["s1"]->size());
+  EXPECT_PRED_FORMAT2(ValuesEq, &value1,
+                      &(*extension_sync_data["s1"])[0]->value());
 
-  std::map<std::string, SettingSyncDataList> app_sync_data =
-      GetAllSyncData(syncer::APP_SETTINGS);
+  SettingSyncDataMultimap app_sync_data = GetAllSyncData(syncer::APP_SETTINGS);
   EXPECT_EQ(1u, app_sync_data.size());
-  EXPECT_EQ(1u, app_sync_data["s2"].size());
-  EXPECT_PRED_FORMAT2(ValuesEq, &value2, &app_sync_data["s2"][0].value());
+  EXPECT_EQ(1u, app_sync_data["s2"]->size());
+  EXPECT_PRED_FORMAT2(ValuesEq, &value2, &(*app_sync_data["s2"])[0]->value());
 
   // Stop each separately, there should be no changes either time.
   syncer::SyncDataList sync_data;
   sync_data.push_back(settings_sync_util::CreateData(
       "s1", "foo", value1, syncer::EXTENSION_SETTINGS));
 
   GetSyncableService(syncer::EXTENSION_SETTINGS)
       ->MergeDataAndStartSyncing(
           syncer::EXTENSION_SETTINGS,
           sync_data,
@@ skipping 61 matching lines @@
   {
     base::DictionaryValue dict;
     EXPECT_PRED_FORMAT2(SettingsEq, dict, bad->Get());
   }
 
   // Changes made to good should be sent to sync, changes from bad shouldn't.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "bar", barValue);
   bad->Set(DEFAULTS, "bar", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   {
     base::DictionaryValue dict;
     dict.Set("foo", fooValue.DeepCopy());
     dict.Set("bar", barValue.DeepCopy());
     EXPECT_PRED_FORMAT2(SettingsEq, dict, good->Get());
   }
   {
     base::DictionaryValue dict;
@@ skipping 25 matching lines @@
     dict.Set("bar", barValue.DeepCopy());
     EXPECT_PRED_FORMAT2(SettingsEq, dict, bad->Get());
   }
 
   // Changes made to bad still shouldn't go to sync, even though it didn't fail
   // last time.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "bar", fooValue);
   bad->Set(DEFAULTS, "bar", fooValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   {
     base::DictionaryValue dict;
     dict.Set("foo", barValue.DeepCopy());
     dict.Set("bar", fooValue.DeepCopy());
     EXPECT_PRED_FORMAT2(SettingsEq, dict, good->Get());
   }
   {
     base::DictionaryValue dict;
@@ skipping 33 matching lines @@
       new syncer::SyncChangeProcessorWrapperForTest(sync_processor_.get()));
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
   // Local settings will have been pushed to sync, since it's empty (in this
   // test; presumably it wouldn't be live, since we've been getting changes).
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "bar")->change_type());
   EXPECT_EQ(3u, sync_processor_->changes().size());
 
   // Live local changes now get pushed, too.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "bar", barValue);
   bad->Set(DEFAULTS, "bar", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("bad", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("bad", "bar")->change_type());
   EXPECT_EQ(2u, sync_processor_->changes().size());
 
   // And ProcessSyncChanges work, too.
   {
     syncer::SyncChangeList change_list;
     change_list.push_back(settings_sync_util::CreateUpdate(
           "good", "bar", fooValue, model_type));
     change_list.push_back(settings_sync_util::CreateUpdate(
           "bad", "bar", fooValue, model_type));
     GetSyncableService(model_type)->ProcessSyncChanges(FROM_HERE, change_list);
@@ skipping 77 matching lines @@
     base::DictionaryValue dict;
     dict.Set("foo", fooValue.DeepCopy());
     EXPECT_PRED_FORMAT2(SettingsEq, dict, bad->Get());
   }
 
   // No more changes sent to sync for bad.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "foo", barValue);
   bad->Set(DEFAULTS, "foo", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   // No more changes received from sync should go to bad.
   {
     syncer::SyncChangeList change_list;
     change_list.push_back(settings_sync_util::CreateAdd(
           "good", "foo", fooValue, model_type));
     change_list.push_back(settings_sync_util::CreateAdd(
           "bad", "foo", fooValue, model_type));
     GetSyncableService(model_type)->ProcessSyncChanges(FROM_HERE, change_list);
@@ skipping 40 matching lines @@
   testing_factory->GetExisting("bad")->set_error_code(ValueStore::OK);
 
   // Sync shouldn't be disabled for good (nor bad -- but this is unimportant).
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "foo")->change_type());
   EXPECT_EQ(2u, sync_processor_->changes().size());
 
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "bar", barValue);
   bad->Set(DEFAULTS, "bar", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "bar")->change_type());
   EXPECT_EQ(2u, sync_processor_->changes().size());
 }
 
 TEST_F(ExtensionSettingsSyncTest, FailureToReadChangesToPushDisablesSync) {
   syncer::ModelType model_type = syncer::APP_SETTINGS;
   Manifest::Type type = Manifest::TYPE_LEGACY_PACKAGED_APP;
 
   base::StringValue fooValue("fooValue");
   base::StringValue barValue("barValue");
 
@@ skipping 10 matching lines @@
   // get them so won't.
   testing_factory->GetExisting("bad")->set_error_code(ValueStore::CORRUPTION);
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
   testing_factory->GetExisting("bad")->set_error_code(ValueStore::OK);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   // bad should now be disabled for sync.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "bar", barValue);
   bad->Set(DEFAULTS, "bar", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   {
     syncer::SyncChangeList change_list;
     change_list.push_back(settings_sync_util::CreateUpdate(
           "good", "foo", barValue, model_type));
     // (Sending ADD here even though it's updating, since that's what the state
     // of sync is.  In any case, it won't work.)
     change_list.push_back(settings_sync_util::CreateAdd(
           "bad", "foo", barValue, model_type));
@@ skipping 19 matching lines @@
   GetSyncableService(model_type)->StopSyncing(model_type);
   sync_processor_wrapper_.reset(
       new syncer::SyncChangeProcessorWrapperForTest(sync_processor_.get()));
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "bar")->change_type());
   EXPECT_EQ(4u, sync_processor_->changes().size());
 
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "bar", fooValue);
   bad->Set(DEFAULTS, "bar", fooValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
   EXPECT_EQ(2u, sync_processor_->changes().size());
 }
 
 TEST_F(ExtensionSettingsSyncTest, FailureToPushLocalStateDisablesSync) {
   syncer::ModelType model_type = syncer::EXTENSION_SETTINGS;
   Manifest::Type type = Manifest::TYPE_EXTENSION;
 
   base::StringValue fooValue("fooValue");
   base::StringValue barValue("barValue");
 
@@ skipping 13 matching lines @@
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
   sync_processor_->set_fail_all_requests(false);
 
   // Changes from good will be send to sync, changes from bad won't.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "foo", barValue);
   bad->Set(DEFAULTS, "foo", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   // Changes from sync will be sent to good, not to bad.
   {
     syncer::SyncChangeList change_list;
     change_list.push_back(settings_sync_util::CreateAdd(
           "good", "bar", barValue, model_type));
     change_list.push_back(settings_sync_util::CreateAdd(
           "bad", "bar", barValue, model_type));
     GetSyncableService(model_type)->ProcessSyncChanges(FROM_HERE, change_list);
@@ skipping 16 matching lines @@
   GetSyncableService(model_type)->StopSyncing(model_type);
   sync_processor_wrapper_.reset(
       new syncer::SyncChangeProcessorWrapperForTest(sync_processor_.get()));
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "foo")->change_type());
   EXPECT_EQ(3u, sync_processor_->changes().size());
 
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "foo", fooValue);
   bad->Set(DEFAULTS, "foo", fooValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(2u, sync_processor_->changes().size());
 }
 
 TEST_F(ExtensionSettingsSyncTest, FailureToPushLocalChangeDisablesSync) {
   syncer::ModelType model_type = syncer::EXTENSION_SETTINGS;
   Manifest::Type type = Manifest::TYPE_EXTENSION;
 
   base::StringValue fooValue("fooValue");
   base::StringValue barValue("barValue");
 
   TestingValueStoreFactory* testing_factory = new TestingValueStoreFactory();
   storage_factory_->Reset(testing_factory);
 
   ValueStore* good = AddExtensionAndGetStorage("good", type);
   ValueStore* bad = AddExtensionAndGetStorage("bad", type);
 
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
   // bad will fail to send changes.
   good->Set(DEFAULTS, "foo", fooValue);
   sync_processor_->set_fail_all_requests(true);
   bad->Set(DEFAULTS, "foo", fooValue);
   sync_processor_->set_fail_all_requests(false);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   // No further changes should be sent from bad.
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "foo", barValue);
   bad->Set(DEFAULTS, "foo", barValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(1u, sync_processor_->changes().size());
 
   // Changes from sync will be sent to good, not to bad.
   {
     syncer::SyncChangeList change_list;
     change_list.push_back(settings_sync_util::CreateAdd(
           "good", "bar", barValue, model_type));
     change_list.push_back(settings_sync_util::CreateAdd(
           "bad", "bar", barValue, model_type));
     GetSyncableService(model_type)->ProcessSyncChanges(FROM_HERE, change_list);
@@ skipping 16 matching lines @@
   GetSyncableService(model_type)->StopSyncing(model_type);
   sync_processor_wrapper_.reset(
       new syncer::SyncChangeProcessorWrapperForTest(sync_processor_.get()));
   GetSyncableService(model_type)
       ->MergeDataAndStartSyncing(
           model_type,
           syncer::SyncDataList(),
           sync_processor_wrapper_.Pass(),
           make_scoped_ptr(new syncer::SyncErrorFactoryMock()));
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("good", "bar").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_ADD,
-      sync_processor_->GetOnlyChange("bad", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("good", "bar")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
+            sync_processor_->GetOnlyChange("bad", "foo")->change_type());
   EXPECT_EQ(3u, sync_processor_->changes().size());
 
   sync_processor_->ClearChanges();
   good->Set(DEFAULTS, "foo", fooValue);
   bad->Set(DEFAULTS, "foo", fooValue);
 
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
-  EXPECT_EQ(
-      syncer::SyncChange::ACTION_UPDATE,
-      sync_processor_->GetOnlyChange("good", "foo").change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
+  EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
+            sync_processor_->GetOnlyChange("good", "foo")->change_type());
   EXPECT_EQ(2u, sync_processor_->changes().size());
 }
 
 TEST_F(ExtensionSettingsSyncTest,
        LargeOutgoingChangeRejectedButIncomingAccepted) {
   syncer::ModelType model_type = syncer::APP_SETTINGS;
   Manifest::Type type = Manifest::TYPE_LEGACY_PACKAGED_APP;
 
   // This value should be larger than the limit in sync_storage_backend.cc.
   std::string string_10k;
@@ skipping 65 matching lines @@
         new base::StringValue("value"));
     EXPECT_TRUE(base::Value::Equals(&expected_data, &data->settings()));
   }
 
   // Test dots in keys going to sync.
   {
     scoped_ptr<base::Value> string_value(new base::StringValue("spot"));
     storage->Set(DEFAULTS, "key.with.spot", *string_value);
 
     ASSERT_EQ(1u, sync_processor_->changes().size());
-    SettingSyncData sync_data = sync_processor_->changes()[0];
-    EXPECT_EQ(syncer::SyncChange::ACTION_ADD, sync_data.change_type());
-    EXPECT_EQ("ext", sync_data.extension_id());
-    EXPECT_EQ("key.with.spot", sync_data.key());
-    EXPECT_TRUE(sync_data.value().Equals(string_value.get()));
+    SettingSyncData* sync_data = sync_processor_->changes()[0];
+    EXPECT_EQ(syncer::SyncChange::ACTION_ADD, sync_data->change_type());
+    EXPECT_EQ("ext", sync_data->extension_id());
+    EXPECT_EQ("key.with.spot", sync_data->key());
+    EXPECT_TRUE(sync_data->value().Equals(string_value.get()));
   }
 }
 
 // In other (frontend) tests, we assume that the result of GetStorage
 // is a pointer to the a Storage owned by a Frontend object, but for
 // the unlimitedStorage case, this might not be true. So, write the
 // tests in a "callback" style.  We should really rewrite all tests to
 // be asynchronous in this way.
 
 namespace {
@@ skipping 45 matching lines @@
                             settings_namespace::SYNC,
                             base::Bind(&UnlimitedSyncStorageTestCallback));
   frontend_->RunWithStorage(extension,
                             settings_namespace::LOCAL,
                             base::Bind(&UnlimitedLocalStorageTestCallback));
 
   base::MessageLoop::current()->RunUntilIdle();
 }
 
 }  // namespace extensions