Migrate startup URLs pref.

chrome/browser/prefs/pref_model_associator.cc

Index: chrome/browser/prefs/pref_model_associator.cc
diff --git a/chrome/browser/prefs/pref_model_associator.cc b/chrome/browser/prefs/pref_model_associator.cc
index 28759a10b6919b681ac7fcea46e5fc8227e7fc00..9c932256063c39508a48b49a52fa71ad6b73f4a5 100644
--- a/chrome/browser/prefs/pref_model_associator.cc
+++ b/chrome/browser/prefs/pref_model_associator.cc
@@ -48,6 +48,41 @@ sync_pb::PreferenceSpecifics* GetMutableSpecifics(
   }
 }
 
+// List of migrated preference name pairs. If a preference is migrated adding
+// the old and new preference names here will do the right thing by the sync

[grt (UTC plus 2), 2013/10/07 20:15:16]

what is "the right thing"? maybe spell out that it causes values under the old
name that come down from sync to warp to the new name (assuming that's what it
means).

[robertshield, 2013/10/08 01:52:13]

Done.

+// engine. Preference migration itself doesn't happen here. It may happen in
+// session_startup_pref.cc.
+struct MigratedPreferences {

[grt (UTC plus 2), 2013/10/07 20:15:16]

const struct
mega nit: this type doesn't need a name, but maybe it's happier with one.

[robertshield, 2013/10/08 01:52:13]

Done. 
I prefer useful symbol names and dislike anonymous types.

+  const char* old_name;

[grt (UTC plus 2), 2013/10/07 20:15:16]

const char* const

[robertshield, 2013/10/08 01:52:13]

Done.

+  const char* new_name;
+} kMigratedPreferences[] = {
+  { prefs::kURLsToRestoreOnStartupOld, prefs::kURLsToRestoreOnStartup },
+};
+
+std::string GetOldMigratedPreferenceName(const char* preference_name) {
+  for (int i = 0; i < arraysize(kMigratedPreferences); ++i) {

[grt (UTC plus 2), 2013/10/07 20:15:16]

int -> size_t since you're comparing with arraysize, which returns size_t (here
and line 71).

[robertshield, 2013/10/08 01:52:13]

Done.

+    if (!strcmp(kMigratedPreferences[i].new_name, preference_name))
+      return kMigratedPreferences[i].old_name;
+  }
+  return "";

[grt (UTC plus 2), 2013/10/07 20:15:16]

"" -> std::string() here and line 75

[robertshield, 2013/10/08 01:52:13]

Done.

+}
+
+std::string GetNewMigratedPreferenceName(const char* old_preference_name) {
+  for (int i = 0; i < arraysize(kMigratedPreferences); ++i) {
+    if (!strcmp(kMigratedPreferences[i].old_name, old_preference_name))
+      return kMigratedPreferences[i].new_name;
+  }
+  return "";
+}
+
+bool IsMigratedPreference(const char* preference_name) {
+  return !GetOldMigratedPreferenceName(preference_name).empty();
+}
+
+bool IsOldMigratedPreference(const char* old_preference_name) {
+  return !GetNewMigratedPreferenceName(old_preference_name).empty();
+}
+
 }  // namespace
 
 PrefModelAssociator::PrefModelAssociator(syncer::ModelType type)
@@ -78,7 +113,10 @@ void PrefModelAssociator::InitPrefAndAssociate(
 
   if (sync_pref.IsValid()) {
     const sync_pb::PreferenceSpecifics& preference = GetSpecifics(sync_pref);
-    DCHECK_EQ(pref_name, preference.name());
+    DCHECK(pref_name == preference.name() ||
+           (IsMigratedPreference(pref_name.c_str()) &&
+            preference.name() ==
+                GetOldMigratedPreferenceName(pref_name.c_str())));
 
     base::JSONReader reader;
     scoped_ptr<Value> sync_value(reader.ReadToValue(preference.value()));
@@ -119,6 +157,21 @@ void PrefModelAssociator::InitPrefAndAssociate(
             syncer::SyncChange(FROM_HERE,
                                syncer::SyncChange::ACTION_UPDATE,
                                sync_data));
+        // This preference has been migrated from an old version that must be
+        // kept in sync on older versions of Chrome.
+        if (IsMigratedPreference(pref_name.c_str())) {
+          if (!CreatePrefSyncData(
+                  GetOldMigratedPreferenceName(pref_name.c_str()),
+                  *new_value,
+                  &sync_data)) {
+            LOG(ERROR) << "Failed to update preference.";
+            return;
+          }
+          sync_changes->push_back(
+              syncer::SyncChange(FROM_HERE,
+                                 syncer::SyncChange::ACTION_UPDATE,
+                                 sync_data));
+        }
       }
     } else if (!sync_value->IsType(Value::TYPE_NULL)) {
       // Only a server value exists. Just set the local user value.
@@ -179,18 +232,26 @@ syncer::SyncMergeResult PrefModelAssociator::MergeDataAndStartSyncing(
     DCHECK_EQ(type_, sync_iter->GetDataType());
 
     const sync_pb::PreferenceSpecifics& preference = GetSpecifics(*sync_iter);
-    const std::string& sync_pref_name = preference.name();
+    std::string sync_pref_name = preference.name();
 
     if (remaining_preferences.count(sync_pref_name) == 0) {
-      // We're not syncing this preference locally, ignore the sync data.
-      // TODO(zea): Eventually we want to be able to have the syncable service
-      // reconstruct all sync data for it's datatype (therefore having
-      // GetAllSyncData be a complete representation). We should store this data
-      // somewhere, even if we don't use it.
-      continue;
+      if (IsOldMigratedPreference(sync_pref_name.c_str())) {
+        // This old pref name is not syncable locally anymore but we accept
+        // changes from other Chrome installs of previous versions and migrate
+        // them to the new name. Note that we will be merging any differences
+        // between the new and old values and sync'ing them back.
+        sync_pref_name = GetNewMigratedPreferenceName(sync_pref_name.c_str());
+      } else {
+        // We're not syncing this preference locally, ignore the sync data.
+        // TODO(zea): Eventually we want to be able to have the syncable service
+        // reconstruct all sync data for its datatype (therefore having
+        // GetAllSyncData be a complete representation). We should store this
+        // data somewhere, even if we don't use it.
+        continue;
+      }
+    } else {
+      remaining_preferences.erase(sync_pref_name);
     }
-
-    remaining_preferences.erase(sync_pref_name);
     InitPrefAndAssociate(*sync_iter, sync_pref_name, &new_changes);
   }
 
@@ -225,7 +286,10 @@ scoped_ptr<Value> PrefModelAssociator::MergePreference(
     const std::string& name,
     const Value& local_value,
     const Value& server_value) {
-  if (name == prefs::kURLsToRestoreOnStartup) {
+  // This function special cases preferences individually, so don't attempt
+  // to merge for all migrated values.
+  if (name == prefs::kURLsToRestoreOnStartup ||
+      name == prefs::kURLsToRestoreOnStartupOld) {
     return scoped_ptr<Value>(MergeListValues(local_value, server_value)).Pass();
   }
 
@@ -388,8 +452,14 @@ syncer::SyncError PrefModelAssociator::ProcessSyncChanges(
     // Windows client, the Windows client does not support
     // kConfirmToQuitEnabled. Ignore updates from these preferences.
     const char* pref_name = name.c_str();
-    if (!IsPrefRegistered(pref_name))
+    std::string new_name;
+    // We migrated this preference name, so do as if the name had not changed.
+    if (IsOldMigratedPreference(pref_name)) {
+      new_name = GetNewMigratedPreferenceName(pref_name);
+      pref_name = new_name.c_str();
+    } else if (!IsPrefRegistered(pref_name)) {

[grt (UTC plus 2), 2013/10/07 20:15:16]

in the block above, it's safe to assume that new_name.c_str() is always
registered?

[robertshield, 2013/10/08 01:52:13]

That's interesting. MAD's patch was doing so, but looking at line 464 I am not
so sure.

I see no reason to bypass the IsPrefRegistered check here just because of the
migration, so I'll drop the 'else'.

@Nicolas, please holler if you object.

       continue;
+    }
 
     const PrefService::Preference* pref =
         pref_service_->FindPreference(pref_name);
@@ -508,6 +578,21 @@ void PrefModelAssociator::ProcessPrefChange(const std::string& name) {
         syncer::SyncChange(FROM_HERE,
                            syncer::SyncChange::ACTION_UPDATE,
                            sync_data));
+    // This preference has been migrated from an old version that must be kept
+    // in sync on older versions of Chrome.
+    if (IsMigratedPreference(name.c_str())) {
+      if (!CreatePrefSyncData(GetOldMigratedPreferenceName(name.c_str()),
+                              *preference->GetValue(),
+                              &sync_data)) {
+        LOG(ERROR) << "Failed to update preference.";
+        return;
+      }
+
+      changes.push_back(
+          syncer::SyncChange(FROM_HERE,
+                             syncer::SyncChange::ACTION_UPDATE,
+                             sync_data));
+    }
   }
 
   syncer::SyncError error =