ExtensionServiceSyncTest cleanup

chrome/browser/extensions/extension_service_sync_unittest.cc

Index: chrome/browser/extensions/extension_service_sync_unittest.cc
diff --git a/chrome/browser/extensions/extension_service_sync_unittest.cc b/chrome/browser/extensions/extension_service_sync_unittest.cc
index a43f8bd03b2a94b258f53b6284243ffb51173bd8..a3f4421b694e1d95f8ef7d08408bd858e8500006 100644
--- a/chrome/browser/extensions/extension_service_sync_unittest.cc
+++ b/chrome/browser/extensions/extension_service_sync_unittest.cc
@@ -55,6 +55,10 @@ using extensions::ExtensionSyncData;
 using extensions::ExtensionSystem;
 using extensions::Manifest;
 using extensions::PermissionSet;
+using syncer::SyncChange;
+using syncer::SyncChangeList;
+
+namespace {
 
 const char good0[] = "behllobkkfkfnphdnhnkndlbkcpglgmj";
 const char good2[] = "bjafgdebaacbbbecmhlhpofkepfkgcpa";
@@ -62,6 +66,17 @@ const char good_crx[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char page_action[] = "obcimlgaoabeegjmmpldobjndiealpln";
 const char theme2_crx[] = "pjpgmfcmabopnnfonnhmdjglfpjjfkbf";
 
+SyncChangeList MakeSyncChangeList(const std::string& id,

[Devlin, 2015/12/02 17:24:50]

Doesn't bother me much, but isn't the id in the specifics already?

[Marc Treib, 2015/12/03 09:29:41]

Hm. Yes, but it's in the ExtensionSpecifics, so we'd have to get it either via
.extension() or via .app().extension() depending on whether it's an App or an
Extension. That seems more trouble than it's worth.

+                                  const sync_pb::EntitySpecifics& specifics,
+                                  SyncChange::SyncChangeType change_type) {
+  syncer::SyncData sync_data =
+      syncer::SyncData::CreateLocalData(id, "Name", specifics);
+  SyncChange sync_change(FROM_HERE, change_type, sync_data);
+  return SyncChangeList(1, sync_change);

[Devlin, 2015/12/02 17:24:50]

nit: could even inline sync_change here.

[Marc Treib, 2015/12/03 09:29:41]

Done.

+}
+
+}  // namespace
+
 class ExtensionServiceSyncTest
     : public extensions::ExtensionServiceTestWithInstall {
  public:
@@ -161,11 +176,9 @@ TEST_F(ExtensionServiceSyncTest, DeferredSyncStartupOnInstall) {
 
   // Once sync starts, flare should no longer be invoked.
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),

[Devlin, 2015/12/02 17:24:50]

nitty nit: even though it doesn't matter, prefer explicit construction with ().

[Marc Treib, 2015/12/03 09:29:41]

Done. (Is there anything in the style guide about this? I see both styles with
roughly the same frequency.)

[Devlin, 2015/12/03 16:55:27]

Probably not, except as it pertains to POD structs, where it actually matters. 
But not 100% sure.  So I'm just using my awesome OWNERS powers of preferences :)

+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
   path = data_dir().AppendASCII("page_action.crx");
   InstallCRX(path, INSTALL_NEW);
   EXPECT_FALSE(flare_was_called);
@@ -198,19 +211,16 @@ TEST_F(ExtensionServiceSyncTest, DisableExtensionFromSync) {
 
   // Sync starts up.
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
+      syncer::EXTENSIONS, syncer::SyncDataList(),
       make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   // Then sync data arrives telling us to disable |good0|.
   ExtensionSyncData disable_good_crx(*extension, false,
                                      Extension::DISABLE_USER_ACTION, false,
                                      false, ExtensionSyncData::BOOLEAN_UNSET);
-  syncer::SyncChange sync_change(FROM_HERE,
-                                 syncer::SyncChange::ACTION_UPDATE,
-                                 disable_good_crx.GetSyncData());
-  syncer::SyncChangeList list(1, sync_change);
+  SyncChangeList list(
+      1, disable_good_crx.GetSyncChange(SyncChange::ACTION_UPDATE));
   extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
   ASSERT_FALSE(service()->IsExtensionEnabled(good0));
@@ -315,8 +325,7 @@ TEST_F(ExtensionServiceSyncTest, DontSelfNotify) {
     // Disable the extension.
     ExtensionSyncData data(*extension, false, Extension::DISABLE_USER_ACTION,
                            false, false, ExtensionSyncData::BOOLEAN_UNSET);
-    syncer::SyncChangeList list(
-        1, data.GetSyncChange(syncer::SyncChange::ACTION_UPDATE));
+    SyncChangeList list(1, data.GetSyncChange(SyncChange::ACTION_UPDATE));
 
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
@@ -330,8 +339,7 @@ TEST_F(ExtensionServiceSyncTest, DontSelfNotify) {
     // Set incognito enabled to true.
     ExtensionSyncData data(*extension, false, Extension::DISABLE_NONE, true,
                            false, ExtensionSyncData::BOOLEAN_UNSET);
-    syncer::SyncChangeList list(
-        1, data.GetSyncChange(syncer::SyncChange::ACTION_UPDATE));
+    SyncChangeList list(1, data.GetSyncChange(SyncChange::ACTION_UPDATE));
 
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
@@ -347,8 +355,7 @@ TEST_F(ExtensionServiceSyncTest, DontSelfNotify) {
                            Extension::DISABLE_USER_ACTION |
                                Extension::DISABLE_PERMISSIONS_INCREASE,
                            false, false, ExtensionSyncData::BOOLEAN_UNSET);
-    syncer::SyncChangeList list(
-        1, data.GetSyncChange(syncer::SyncChange::ACTION_UPDATE));
+    SyncChangeList list(1, data.GetSyncChange(SyncChange::ACTION_UPDATE));
 
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
@@ -364,8 +371,7 @@ TEST_F(ExtensionServiceSyncTest, DontSelfNotify) {
                            Extension::DISABLE_USER_ACTION |
                                Extension::DISABLE_PERMISSIONS_INCREASE,
                            false, false, ExtensionSyncData::BOOLEAN_UNSET);
-    syncer::SyncChangeList list(
-        1, data.GetSyncChange(syncer::SyncChange::ACTION_DELETE));
+    SyncChangeList list(1, data.GetSyncChange(SyncChange::ACTION_DELETE));
 
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
@@ -380,11 +386,9 @@ TEST_F(ExtensionServiceSyncTest, GetSyncData) {
   ASSERT_TRUE(extension);
 
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   syncer::SyncDataList list =
       extension_sync_service()->GetAllSyncData(syncer::EXTENSIONS);
@@ -410,13 +414,10 @@ TEST_F(ExtensionServiceSyncTest, GetSyncDataDisableReasons) {
       InstallCRX(data_dir().AppendASCII("good.crx"), INSTALL_NEW);
   ASSERT_TRUE(extension);
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   {
     syncer::SyncDataList list =
@@ -485,13 +486,10 @@ TEST_F(ExtensionServiceSyncTest, GetSyncDataTerminated) {
   const Extension* extension = service()->GetInstalledExtension(good_crx);
   ASSERT_TRUE(extension);
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   syncer::SyncDataList list =
       extension_sync_service()->GetAllSyncData(syncer::EXTENSIONS);
@@ -517,13 +515,10 @@ TEST_F(ExtensionServiceSyncTest, GetSyncDataFilter) {
   const Extension* extension = service()->GetInstalledExtension(good_crx);
   ASSERT_TRUE(extension);
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::APPS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::APPS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   syncer::SyncDataList list =
       extension_sync_service()->GetAllSyncData(syncer::EXTENSIONS);
@@ -536,13 +531,10 @@ TEST_F(ExtensionServiceSyncTest, GetSyncExtensionDataUserSettings) {
   const Extension* extension = service()->GetInstalledExtension(good_crx);
   ASSERT_TRUE(extension);
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   {
     syncer::SyncDataList list =
@@ -607,13 +599,10 @@ TEST_F(ExtensionServiceSyncTest, SyncForUninstalledExternalExtension) {
   const Extension* extension = service()->GetInstalledExtension(good_crx);
   ASSERT_TRUE(extension);
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   UninstallExtension(good_crx, false);
   EXPECT_TRUE(
@@ -627,13 +616,8 @@ TEST_F(ExtensionServiceSyncTest, SyncForUninstalledExternalExtension) {
   extension_specifics->set_version("1.0");
   extension_specifics->set_enabled(true);
 
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE,
-                                 syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList list(1);
-  list[0] = sync_change;
+  SyncChangeList list =
+      MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
   extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
   EXPECT_TRUE(
@@ -647,13 +631,10 @@ TEST_F(ExtensionServiceSyncTest, GetSyncAppDataUserSettings) {
   ASSERT_TRUE(app);
   ASSERT_TRUE(app->is_app());
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::APPS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::APPS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   syncer::StringOrdinal initial_ordinal =
       syncer::StringOrdinal::CreateInitialOrdinal();
@@ -712,13 +693,10 @@ TEST_F(ExtensionServiceSyncTest, GetSyncAppDataUserSettingsOnExtensionMoved) {
     ASSERT_TRUE(apps[i]->is_app());
   }
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::APPS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::APPS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   ExtensionSystem::Get(service()->GetBrowserContext())
       ->app_sorting()
@@ -757,19 +735,14 @@ TEST_F(ExtensionServiceSyncTest, GetSyncDataList) {
   InstallCRX(data_dir().AppendASCII("theme.crx"), INSTALL_NEW);
   InstallCRX(data_dir().AppendASCII("theme2.crx"), INSTALL_NEW);
 
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::APPS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::APPS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   service()->DisableExtension(page_action, Extension::DISABLE_USER_ACTION);
   TerminateExtension(theme2_crx);
@@ -781,25 +754,18 @@ TEST_F(ExtensionServiceSyncTest, GetSyncDataList) {
 
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataUninstall) {
   InitializeEmptyExtensionService();
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   sync_pb::EntitySpecifics specifics;
   sync_pb::ExtensionSpecifics* ext_specifics = specifics.mutable_extension();
   ext_specifics->set_id(good_crx);
   ext_specifics->set_version("1.0");
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE,
-                                 syncer::SyncChange::ACTION_DELETE,
-                                 sync_data);
-  syncer::SyncChangeList list(1);
-  list[0] = sync_change;
+
+  SyncChangeList list =
+      MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_DELETE);
 
   // Should do nothing.
   extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
@@ -837,13 +803,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataWrongType) {
 
   {
     extension_specifics->set_enabled(true);
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_DELETE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_DELETE);
 
     // Should do nothing
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
@@ -852,13 +814,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataWrongType) {
 
   {
     extension_specifics->set_enabled(false);
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
     // Should again do nothing.
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
@@ -868,13 +826,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataWrongType) {
 
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
   InitializeEmptyExtensionService();
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   InstallCRX(data_dir().AppendASCII("good.crx"), INSTALL_NEW);
   EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
@@ -894,13 +849,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
   ext_specifics->set_enabled(false);
 
   {
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_FALSE(service()->IsExtensionEnabled(good_crx));
     EXPECT_FALSE(extensions::util::IsIncognitoEnabled(good_crx, profile()));
@@ -913,13 +864,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
   {
     ext_specifics->set_enabled(true);
     ext_specifics->set_incognito_enabled(true);
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
     EXPECT_TRUE(extensions::util::IsIncognitoEnabled(good_crx, profile()));
@@ -928,13 +876,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
   {
     ext_specifics->set_enabled(false);
     ext_specifics->set_incognito_enabled(true);
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_FALSE(service()->IsExtensionEnabled(good_crx));
     EXPECT_TRUE(extensions::util::IsIncognitoEnabled(good_crx, profile()));
@@ -943,13 +888,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
   {
     ext_specifics->set_enabled(true);
     ext_specifics->set_all_urls_enabled(!kDefaultAllowedScripting);
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
     EXPECT_TRUE(extensions::util::HasSetAllowedScriptingOnAllUrls(
@@ -960,13 +902,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
 
   {
     ext_specifics->set_all_urls_enabled(kDefaultAllowedScripting);
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1);
-    list[0] = sync_change;
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
     EXPECT_TRUE(extensions::util::HasSetAllowedScriptingOnAllUrls(
@@ -980,13 +919,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataSettings) {
 
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataNewExtension) {
   InitializeEmptyExtensionService();
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   const base::FilePath path = data_dir().AppendASCII("good.crx");
   const ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
@@ -1033,12 +969,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataNewExtension) {
     if (test_case.sync_disable_reasons != -1)
       ext_specifics->set_disable_reasons(test_case.sync_disable_reasons);
 
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1, sync_change);
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
     ASSERT_TRUE(service()->pending_extension_manager()->IsIdPending(good_crx));
@@ -1060,13 +993,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataNewExtension) {
 
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataTerminatedExtension) {
   InitializeExtensionServiceWithUpdater();
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   InstallCRX(data_dir().AppendASCII("good.crx"), INSTALL_NEW);
   TerminateExtension(good_crx);
@@ -1080,13 +1010,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataTerminatedExtension) {
       service()->GetInstalledExtension(good_crx)->version()->GetString());
   ext_specifics->set_enabled(false);
   ext_specifics->set_incognito_enabled(true);
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE,
-                                 syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList list(1);
-  list[0] = sync_change;
+
+  SyncChangeList list =
+      MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
   extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
   EXPECT_FALSE(service()->IsExtensionEnabled(good_crx));
@@ -1097,13 +1023,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataTerminatedExtension) {
 
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataVersionCheck) {
   InitializeExtensionServiceWithUpdater();
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   InstallCRX(data_dir().AppendASCII("good.crx"), INSTALL_NEW);
   EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
@@ -1119,12 +1042,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataVersionCheck) {
 
   {
     ext_specifics->set_version(installed_version.GetString());
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1, sync_change);
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
     // Should do nothing if extension version == sync version.
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
@@ -1142,12 +1062,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataVersionCheck) {
   // Should do nothing if extension version > sync version.
   {
     ext_specifics->set_version("0.0.0.0");
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1, sync_change);
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_FALSE(service()->updater()->WillCheckSoon());
@@ -1165,12 +1082,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataVersionCheck) {
   {
     const base::Version new_version("9.9.9.9");
     ext_specifics->set_version(new_version.GetString());
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1, sync_change);
+
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
     EXPECT_TRUE(service()->updater()->WillCheckSoon());
@@ -1191,13 +1105,10 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataVersionCheck) {
 
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataNotInstalled) {
   InitializeExtensionServiceWithUpdater();
-  syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   sync_pb::EntitySpecifics specifics;
   sync_pb::ExtensionSpecifics* ext_specifics = specifics.mutable_extension();
@@ -1206,13 +1117,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataNotInstalled) {
   ext_specifics->set_incognito_enabled(true);
   ext_specifics->set_update_url("http://www.google.com/");
   ext_specifics->set_version("1.2.3.4");
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE,
-                                 syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList list(1);
-  list[0] = sync_change;
+
+  SyncChangeList list =
+      MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
 
   EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
   EXPECT_FALSE(extensions::util::IsIncognitoEnabled(good_crx, profile()));
@@ -1233,11 +1140,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataNotInstalled) {
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataEnableDisable) {
   InitializeEmptyExtensionService();
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   const ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
 
@@ -1318,12 +1223,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataEnableDisable) {
     if (test_case.sync_disable_reasons != -1)
       ext_specifics->set_disable_reasons(test_case.sync_disable_reasons);
 
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1, sync_change);
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
     // Check expectations.
@@ -1340,11 +1242,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataEnableDisable) {
 TEST_F(ExtensionServiceSyncTest, ProcessSyncDataDeferredEnable) {
   InitializeEmptyExtensionService();
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   base::FilePath base_path = data_dir().AppendASCII("permissions_increase");
   base::FilePath pem_path = base_path.AppendASCII("permissions.pem");
@@ -1370,12 +1270,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataDeferredEnable) {
   ext_specifics->set_enabled(true);
   ext_specifics->set_disable_reasons(Extension::DISABLE_NONE);
 
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE,
-                                 syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList list(1, sync_change);
+  SyncChangeList list =
+      MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
   extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
   // Since the version didn't match, the extension should still be disabled.
@@ -1395,11 +1292,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataPermissionApproval) {
 
   InitializeEmptyExtensionService();
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   const base::FilePath base_path =
       data_dir().AppendASCII("permissions_increase");
@@ -1487,12 +1382,9 @@ TEST_F(ExtensionServiceSyncTest, ProcessSyncDataPermissionApproval) {
     if (test_case.sync_disable_reasons != -1)
       ext_specifics->set_disable_reasons(test_case.sync_disable_reasons);
 
-    syncer::SyncData sync_data =
-        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
-    syncer::SyncChange sync_change(FROM_HERE,
-                                   syncer::SyncChange::ACTION_UPDATE,
-                                   sync_data);
-    syncer::SyncChangeList list(1, sync_change);
+    SyncChangeList list =
+        MakeSyncChangeList(good_crx, specifics, SyncChange::ACTION_UPDATE);
+
     extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
     // Check expectations.
@@ -1740,12 +1632,10 @@ TEST_F(ExtensionServiceTestSupervised,
   EXPECT_CALL(*creator, CreateExtensionUpdateRequest(
                             UpdateRequestId(id, version2), testing::_));
 
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(id, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE, syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList change_list(1, sync_change);
-  extension_sync_service()->ProcessSyncChanges(FROM_HERE, change_list);
+  SyncChangeList list =
+      MakeSyncChangeList(id, specifics, SyncChange::ACTION_UPDATE);
+
+  extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
   // The re-enable should be ignored, since the version doesn't match.
   EXPECT_FALSE(registry()->enabled_extensions().Contains(id));
   EXPECT_FALSE(extension_sync_service()->HasPendingReenable(
@@ -1785,12 +1675,10 @@ TEST_F(ExtensionServiceTestSupervised,
   ext_specifics->set_installed_by_custodian(true);
   ext_specifics->set_version(version2);
 
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(id, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE, syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList change_list(1, sync_change);
-  extension_sync_service()->ProcessSyncChanges(FROM_HERE, change_list);
+  SyncChangeList list =
+      MakeSyncChangeList(id, specifics, SyncChange::ACTION_UPDATE);
+
+  extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
   // The extension should have gotten re-enabled.
   EXPECT_TRUE(registry()->enabled_extensions().Contains(id));
 }
@@ -1832,12 +1720,10 @@ TEST_F(ExtensionServiceTestSupervised,
                             UpdateRequestId(id, version3), testing::_))
       .Times(0);
 
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(id, "Name", specifics);
-  syncer::SyncChange sync_change(FROM_HERE, syncer::SyncChange::ACTION_UPDATE,
-                                 sync_data);
-  syncer::SyncChangeList change_list(1, sync_change);
-  extension_sync_service()->ProcessSyncChanges(FROM_HERE, change_list);
+  SyncChangeList list =
+      MakeSyncChangeList(id, specifics, SyncChange::ACTION_UPDATE);
+
+  extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
   // The re-enable should be delayed until the extension is updated to the
   // matching version.
   EXPECT_FALSE(registry()->enabled_extensions().Contains(id));
@@ -1851,11 +1737,9 @@ TEST_F(ExtensionServiceTestSupervised,
 TEST_F(ExtensionServiceSyncTest, SyncUninstallByCustodianSkipsPolicy) {
   InitializeEmptyExtensionService();
   extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      syncer::SyncDataList(),
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      syncer::EXTENSIONS, syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
   // Install two extensions.
   base::FilePath path1 = data_dir().AppendASCII("good.crx");
@@ -1872,7 +1756,7 @@ TEST_F(ExtensionServiceSyncTest, SyncUninstallByCustodianSkipsPolicy) {
       browser_context())->management_policy()->RegisterProvider(&provider);
 
   // Create a sync deletion for each extension.
-  syncer::SyncChangeList change_list;
+  SyncChangeList list;
   for (size_t i = 0; i < arraysize(extensions); i++) {
     const std::string& id = extensions[i]->id();
     sync_pb::EntitySpecifics specifics;
@@ -1883,9 +1767,7 @@ TEST_F(ExtensionServiceSyncTest, SyncUninstallByCustodianSkipsPolicy) {
         extensions[i]->was_installed_by_custodian());
     syncer::SyncData sync_data =
         syncer::SyncData::CreateLocalData(id, "Name", specifics);
-    change_list.push_back(syncer::SyncChange(FROM_HERE,
-                                             syncer::SyncChange::ACTION_DELETE,
-                                             sync_data));
+    list.push_back(SyncChange(FROM_HERE, SyncChange::ACTION_DELETE, sync_data));
   }
 
   // Save the extension ids, as uninstalling destroys the Extension instance.
@@ -1895,7 +1777,7 @@ TEST_F(ExtensionServiceSyncTest, SyncUninstallByCustodianSkipsPolicy) {
   };
 
   // Now apply the uninstallations.
-  extension_sync_service()->ProcessSyncChanges(FROM_HERE, change_list);
+  extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
   // Uninstalling the extension without installed_by_custodian should have been
   // blocked by policy, so it should still be there.
@@ -1931,8 +1813,7 @@ TEST_F(ExtensionServiceSyncTest, SyncExtensionHasAllhostsWithheld) {
   EXPECT_TRUE(registry()->enabled_extensions().GetByID(id));
 
   // Simulate a sync node coming in where the extension had all-hosts withheld.
-  // This means that it should have all-hosts withheld on this machine, too.
-  syncer::SyncChangeList change_list;
+  // This means that it should have all-hosts withheld on this machine,

[Devlin, 2015/12/02 17:24:50]

?

[Marc Treib, 2015/12/03 09:29:41]

Err... no idea how that happened?! Reverted, thanks!

   sync_pb::EntitySpecifics specifics;
   sync_pb::ExtensionSpecifics* ext_specifics = specifics.mutable_extension();
   ext_specifics->set_id(id);
@@ -1940,13 +1821,11 @@ TEST_F(ExtensionServiceSyncTest, SyncExtensionHasAllhostsWithheld) {
   ext_specifics->set_version("1.0");
   ext_specifics->set_all_urls_enabled(false);
   ext_specifics->set_enabled(true);
-  syncer::SyncData sync_data =
-      syncer::SyncData::CreateLocalData(id, "Name", specifics);
-  change_list.push_back(syncer::SyncChange(FROM_HERE,
-                                           syncer::SyncChange::ACTION_UPDATE,
-                                           sync_data));
 
-  extension_sync_service()->ProcessSyncChanges(FROM_HERE, change_list);
+  SyncChangeList list =
+      MakeSyncChangeList(id, specifics, SyncChange::ACTION_UPDATE);
+
+  extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
 
   EXPECT_TRUE(registry()->enabled_extensions().GetByID(id));
   EXPECT_FALSE(extensions::util::AllowedScriptingOnAllUrls(id, profile()));