Convert InvalidationStateTracker to use invalidation::ObjectId instead of syncable::ModelType

BUG=124145,124149
TEST=unit tests

Review URL: https://chromiumcodereview.appspot.com/10545168

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

17 files changed, 453 insertions, 112 deletions

diff --git a/chrome/browser/sync/invalidations/invalidator_storage.cc b/chrome/browser/sync/invalidations/invalidator_storage.cc
index 98e1169..defe257 100644
--- a/chrome/browser/sync/invalidations/invalidator_storage.cc
+++ b/chrome/browser/sync/invalidations/invalidator_storage.cc
@@ -10,22 +10,74 @@
 #include "base/values.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
+#include "sync/internal_api/public/syncable/model_type.h"
 
 using csync::InvalidationVersionMap;
 
 namespace browser_sync {
 
+namespace {
+
+const char kSourceKey[] = "source";
+const char kNameKey[] = "name";
+const char kMaxVersionKey[] = "max-version";
+
+bool ValueToObjectIdAndVersion(const DictionaryValue& value,
+                               invalidation::ObjectId* id,
+                               int64* max_version) {
+  std::string source_str;
+  int source = 0;
+  std::string name;
+  std::string max_version_str;
+  if (!value.GetString(kSourceKey, &source_str)) {
+    DLOG(WARNING) << "Unable to deserialize source";
+    return false;
+  }
+  if (!value.GetString(kNameKey, &name)) {
+    DLOG(WARNING) << "Unable to deserialize name";
+    return false;
+  }
+  if (!value.GetString(kMaxVersionKey, &max_version_str)) {
+    DLOG(WARNING) << "Unable to deserialize max version";
+    return false;
+  }
+  if (!base::StringToInt(source_str, &source)) {
+    DLOG(WARNING) << "Invalid source: " << source_str;
+    return false;
+  }
+  if (!base::StringToInt64(max_version_str, max_version)) {
+    DLOG(WARNING) << "Invalid max invalidation version: " << max_version_str;
+    return false;
+  }
+  *id = invalidation::ObjectId(source, name);
+  return true;
+}
+
+// The caller owns the returned value.
+DictionaryValue* ObjectIdAndVersionToValue(const invalidation::ObjectId& id,
+                                           int64 max_version) {
+  DictionaryValue* value = new DictionaryValue;
+  value->SetString(kSourceKey, base::IntToString(id.source()));
+  value->SetString(kNameKey, id.name());
+  value->SetString(kMaxVersionKey, base::Int64ToString(max_version));
+  return value;
+}
+
+}  // namespace
+
 InvalidatorStorage::InvalidatorStorage(PrefService* pref_service)
     : pref_service_(pref_service) {
   // TODO(tim): Create a Mock instead of maintaining the if(!pref_service_) case
   // throughout this file.  This is a problem now due to lack of injection at
   // ProfileSyncService. Bug 130176.
   if (pref_service_) {
-    pref_service_->RegisterDictionaryPref(prefs::kSyncMaxInvalidationVersions,
-                                          PrefService::UNSYNCABLE_PREF);
+    pref_service_->RegisterListPref(prefs::kInvalidatorMaxInvalidationVersions,
+                                    PrefService::UNSYNCABLE_PREF);
     pref_service_->RegisterStringPref(prefs::kInvalidatorInvalidationState,
                                       std::string(),
                                       PrefService::UNSYNCABLE_PREF);
+
+    MigrateMaxInvalidationVersionsPref();
   }
 }
 
@@ -34,46 +86,92 @@ InvalidatorStorage::~InvalidatorStorage() {
 
 InvalidationVersionMap InvalidatorStorage::GetAllMaxVersions() const {
   DCHECK(non_thread_safe_.CalledOnValidThread());
+  InvalidationVersionMap max_versions;
   if (!pref_service_) {
-    return InvalidationVersionMap();
+    return max_versions;
   }
-
-  const base::DictionaryValue* max_versions_dict =
-      pref_service_->GetDictionary(prefs::kSyncMaxInvalidationVersions);
-  CHECK(max_versions_dict);
-  InvalidationVersionMap max_versions;
-  DeserializeMap(max_versions_dict, &max_versions);
+  const base::ListValue* max_versions_list =
+      pref_service_->GetList(prefs::kInvalidatorMaxInvalidationVersions);
+  CHECK(max_versions_list);
+  DeserializeFromList(*max_versions_list, &max_versions);
   return max_versions;
 }
 
-void InvalidatorStorage::SetMaxVersion(syncable::ModelType model_type,
+void InvalidatorStorage::SetMaxVersion(const invalidation::ObjectId& id,
                                        int64 max_version) {
   DCHECK(non_thread_safe_.CalledOnValidThread());
-  DCHECK(syncable::IsRealDataType(model_type));
   CHECK(pref_service_);
-  InvalidationVersionMap max_versions =
-      GetAllMaxVersions();
-  InvalidationVersionMap::iterator it =
-      max_versions.find(model_type);
+  InvalidationVersionMap max_versions = GetAllMaxVersions();
+  InvalidationVersionMap::iterator it = max_versions.find(id);
   if ((it != max_versions.end()) && (max_version <= it->second)) {
     NOTREACHED();
     return;
   }
-  max_versions[model_type] = max_version;
+  max_versions[id] = max_version;
+
+  base::ListValue max_versions_list;
+  SerializeToList(max_versions, &max_versions_list);
+  pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions,
+                     max_versions_list);
+}
+
+// static
+void InvalidatorStorage::DeserializeFromList(
+    const base::ListValue& max_versions_list,
+    InvalidationVersionMap* max_versions_map) {
+  max_versions_map->clear();
+  for (size_t i = 0; i < max_versions_list.GetSize(); ++i) {
+    DictionaryValue* value = NULL;
+    if (!max_versions_list.GetDictionary(i, &value)) {
+      DLOG(WARNING) << "Unable to deserialize entry " << i;
+      continue;
+    }
+    invalidation::ObjectId id;
+    int64 max_version = 0;
+    if (!ValueToObjectIdAndVersion(*value, &id, &max_version)) {
+      DLOG(WARNING) << "Error while deserializing entry " << i;
+      continue;
+    }
+    (*max_versions_map)[id] = max_version;
+  }
+}
 
-  base::DictionaryValue max_versions_dict;
-  SerializeMap(max_versions, &max_versions_dict);
-  pref_service_->Set(prefs::kSyncMaxInvalidationVersions, max_versions_dict);
+// static
+void InvalidatorStorage::SerializeToList(
+    const InvalidationVersionMap& max_versions_map,
+    base::ListValue* max_versions_list) {
+  for (InvalidationVersionMap::const_iterator it = max_versions_map.begin();
+       it != max_versions_map.end(); ++it) {
+    max_versions_list->Append(ObjectIdAndVersionToValue(it->first, it->second));
+  }
+}
+
+// Legacy migration code.
+void InvalidatorStorage::MigrateMaxInvalidationVersionsPref() {
+  pref_service_->RegisterDictionaryPref(prefs::kSyncMaxInvalidationVersions,
+                                        PrefService::UNSYNCABLE_PREF);
+  const base::DictionaryValue* max_versions_dict =
+      pref_service_->GetDictionary(prefs::kSyncMaxInvalidationVersions);
+  CHECK(max_versions_dict);
+  if (!max_versions_dict->empty()) {
+    InvalidationVersionMap max_versions;
+    DeserializeMap(max_versions_dict, &max_versions);
+    base::ListValue max_versions_list;
+    SerializeToList(max_versions, &max_versions_list);
+    pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions,
+                       max_versions_list);
+  }
+  pref_service_->ClearPref(prefs::kSyncMaxInvalidationVersions);
 }
 
+// Legacy migration code.
 // static
 void InvalidatorStorage::DeserializeMap(
     const base::DictionaryValue* max_versions_dict,
     InvalidationVersionMap* map) {
   map->clear();
   // Convert from a string -> string DictionaryValue to a
-  // ModelType -> int64 map
-  // .
+  // ModelType -> int64 map.
   for (base::DictionaryValue::key_iterator it =
            max_versions_dict->begin_keys();
        it != max_versions_dict->end_keys(); ++it) {
@@ -98,20 +196,12 @@ void InvalidatorStorage::DeserializeMap(
                    << max_version_str;
       continue;
     }
-    (*map)[model_type] = max_version;
-  }
-}
-
-// static
-void InvalidatorStorage::SerializeMap(
-    const InvalidationVersionMap& map, base::DictionaryValue* to_dict) {
-  // Convert from a ModelType -> int64 map to a string -> string
-  // DictionaryValue.
-  for (InvalidationVersionMap::const_iterator it = map.begin();
-       it != map.end(); ++it) {
-    to_dict->SetString(
-        base::IntToString(it->first),
-        base::Int64ToString(it->second));
+    invalidation::ObjectId id;
+    if (!csync::RealModelTypeToObjectId(model_type, &id)) {
+      DLOG(WARNING) << "Invalid model type: " << model_type;
+      continue;
+    }
+    (*map)[id] = max_version;
   }
 }
 
@@ -133,8 +223,8 @@ void InvalidatorStorage::SetInvalidationState(const std::string& state) {
 
 void InvalidatorStorage::Clear() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
+  pref_service_->ClearPref(prefs::kInvalidatorMaxInvalidationVersions);
   pref_service_->ClearPref(prefs::kInvalidatorInvalidationState);
-  pref_service_->ClearPref(prefs::kSyncMaxInvalidationVersions);
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/invalidations/invalidator_storage.h b/chrome/browser/sync/invalidations/invalidator_storage.h
index 2137761..af06ea2 100644
--- a/chrome/browser/sync/invalidations/invalidator_storage.h
+++ b/chrome/browser/sync/invalidations/invalidator_storage.h
@@ -13,13 +13,13 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
-#include "sync/internal_api/public/syncable/model_type.h"
 #include "sync/notifier/invalidation_state_tracker.h"
 
 class PrefService;
 
 namespace base {
-  class DictionaryValue;
+class DictionaryValue;
+class ListValue;
 }
 
 namespace browser_sync {
@@ -43,7 +43,7 @@ class InvalidatorStorage : public base::SupportsWeakPtr<InvalidatorStorage>,
   // InvalidationStateTracker implementation.
   virtual csync::InvalidationVersionMap GetAllMaxVersions() const
       OVERRIDE;
-  virtual void SetMaxVersion(syncable::ModelType model_type,
+  virtual void SetMaxVersion(const invalidation::ObjectId& id,
                              int64 max_version) OVERRIDE;
   // TODO(tim): These are not yet used. Bug 124140.
   virtual void SetInvalidationState(const std::string& state) OVERRIDE;
@@ -51,18 +51,37 @@ class InvalidatorStorage : public base::SupportsWeakPtr<InvalidatorStorage>,
 
  private:
   FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, SerializeEmptyMap);
-  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeOutOfRange);
-  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeInvalidFormat);
-  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeEmptyDictionary);
-  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeBasic);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest,
+                           DeserializeFromListOutOfRange);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest,
+                           DeserializeFromListInvalidFormat);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest,
+                           DeserializeFromListWithDuplicates);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest,
+                           DeserializeFromEmptyList);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeFromListBasic);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeMapOutOfRange);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeMapInvalidFormat);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest,
+                           DeserializeMapEmptyDictionary);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, DeserializeMapBasic);
+  FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, MigrateLegacyPreferences);
 
   base::NonThreadSafe non_thread_safe_;
 
-  // Helpers to convert between InvalidationVersionMap <--> DictionaryValue.
+  // Helpers to convert between InvalidationVersionMap <--> ListValue.
+  static void DeserializeFromList(
+      const base::ListValue& max_versions_list,
+      csync::InvalidationVersionMap* max_versions_map);
+  static void SerializeToList(
+      const csync::InvalidationVersionMap& max_versions_map,
+      base::ListValue* max_versions_list);
+
+  // Code for migrating from old MaxInvalidationVersions pref, which was a map
+  // from sync types to max invalidation versions.
+  void MigrateMaxInvalidationVersionsPref();
   static void DeserializeMap(const base::DictionaryValue* max_versions_dict,
                              csync::InvalidationVersionMap* map);
-  static void SerializeMap(const csync::InvalidationVersionMap& map,
-                           base::DictionaryValue* to_dict);
 
   // May be NULL.
   PrefService* const pref_service_;
diff --git a/chrome/browser/sync/invalidations/invalidator_storage_unittest.cc b/chrome/browser/sync/invalidations/invalidator_storage_unittest.cc
index d00b339..c40e887 100644
--- a/chrome/browser/sync/invalidations/invalidator_storage_unittest.cc
+++ b/chrome/browser/sync/invalidations/invalidator_storage_unittest.cc
@@ -8,8 +8,8 @@
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
-#include "sync/internal_api/public/syncable/model_type.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -17,37 +17,62 @@ using csync::InvalidationVersionMap;
 
 namespace browser_sync {
 
+namespace {
+
+const char kSourceKey[] = "source";
+const char kNameKey[] = "name";
+const char kMaxVersionKey[] = "max-version";
+
+const int kChromeSyncSourceId = 1004;
+
+}  // namespace
+
 class InvalidatorStorageTest : public testing::Test {
+ public:
+  InvalidatorStorageTest()
+      : kBookmarksId_(kChromeSyncSourceId, "BOOKMARK"),
+        kPreferencesId_(kChromeSyncSourceId, "PREFERENCE"),
+        kAppNotificationsId_(kChromeSyncSourceId, "APP_NOTIFICATION"),
+        kAutofillId_(kChromeSyncSourceId, "AUTOFILL") {}
+
  protected:
   TestingPrefService pref_service_;
 
+  const invalidation::ObjectId kBookmarksId_;
+  const invalidation::ObjectId kPreferencesId_;
+  const invalidation::ObjectId kAppNotificationsId_;
+  const invalidation::ObjectId kAutofillId_;
+
  private:
   MessageLoop loop_;
 };
 
+// Set max versions for various keys and verify that they are written and read
+// back correctly.
 TEST_F(InvalidatorStorageTest, MaxInvalidationVersions) {
   InvalidatorStorage storage(&pref_service_);
 
   InvalidationVersionMap expected_max_versions;
   EXPECT_EQ(expected_max_versions, storage.GetAllMaxVersions());
 
-  expected_max_versions[syncable::BOOKMARKS] = 2;
-  storage.SetMaxVersion(syncable::BOOKMARKS, 2);
+  expected_max_versions[kBookmarksId_] = 2;
+  storage.SetMaxVersion(kBookmarksId_, 2);
   EXPECT_EQ(expected_max_versions, storage.GetAllMaxVersions());
 
-  expected_max_versions[syncable::PREFERENCES] = 5;
-  storage.SetMaxVersion(syncable::PREFERENCES, 5);
+  expected_max_versions[kPreferencesId_] = 5;
+  storage.SetMaxVersion(kPreferencesId_, 5);
   EXPECT_EQ(expected_max_versions, storage.GetAllMaxVersions());
 
-  expected_max_versions[syncable::APP_NOTIFICATIONS] = 3;
-  storage.SetMaxVersion(syncable::APP_NOTIFICATIONS, 3);
+  expected_max_versions[kAppNotificationsId_] = 3;
+  storage.SetMaxVersion(kAppNotificationsId_, 3);
   EXPECT_EQ(expected_max_versions, storage.GetAllMaxVersions());
 
-  expected_max_versions[syncable::APP_NOTIFICATIONS] = 4;
-  storage.SetMaxVersion(syncable::APP_NOTIFICATIONS, 4);
+  expected_max_versions[kAppNotificationsId_] = 4;
+  storage.SetMaxVersion(kAppNotificationsId_, 4);
   EXPECT_EQ(expected_max_versions, storage.GetAllMaxVersions());
 }
 
+// Clearing the storage should result in an empty version map.
 TEST_F(InvalidatorStorageTest, Clear) {
   InvalidatorStorage storage(&pref_service_);
   EXPECT_TRUE(storage.GetAllMaxVersions().empty());
@@ -57,8 +82,8 @@ TEST_F(InvalidatorStorageTest, Clear) {
   EXPECT_EQ("test", storage.GetInvalidationState());
   {
     InvalidationVersionMap expected_max_versions;
-    expected_max_versions[syncable::APP_NOTIFICATIONS] = 3;
-    storage.SetMaxVersion(syncable::APP_NOTIFICATIONS, 3);
+    expected_max_versions[kAppNotificationsId_] = 3;
+    storage.SetMaxVersion(kAppNotificationsId_, 3);
     EXPECT_EQ(expected_max_versions, storage.GetAllMaxVersions());
   }
 
@@ -70,12 +95,144 @@ TEST_F(InvalidatorStorageTest, Clear) {
 
 TEST_F(InvalidatorStorageTest, SerializeEmptyMap) {
   InvalidationVersionMap empty_map;
-  base::DictionaryValue dict;
-  InvalidatorStorage::SerializeMap(empty_map, &dict);
-  EXPECT_TRUE(dict.empty());
+  base::ListValue list;
+  InvalidatorStorage::SerializeToList(empty_map, &list);
+  EXPECT_TRUE(list.empty());
+}
+
+// Make sure we don't choke on a variety of malformed input.
+TEST_F(InvalidatorStorageTest, DeserializeFromListInvalidFormat) {
+  InvalidationVersionMap map;
+  base::ListValue list_with_invalid_format;
+  DictionaryValue* value;
+
+  // The various cases below use distinct values to make it easier to track down
+  // failures.
+  value = new DictionaryValue();
+  list_with_invalid_format.Append(value);
+
+  value = new DictionaryValue();
+  value->SetString("completely", "invalid");
+  list_with_invalid_format.Append(value);
+
+  // Missing two required fields
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, "10");
+  list_with_invalid_format.Append(value);
+
+  value = new DictionaryValue();
+  value->SetString(kNameKey, "missing source and version");
+  list_with_invalid_format.Append(value);
+
+  value = new DictionaryValue();
+  value->SetString(kMaxVersionKey, "3");
+  list_with_invalid_format.Append(value);
+
+  // Missing one required field
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, "14");
+  value->SetString(kNameKey, "missing version");
+  list_with_invalid_format.Append(value);
+
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, "233");
+  value->SetString(kMaxVersionKey, "5");
+  list_with_invalid_format.Append(value);
+
+  value = new DictionaryValue();
+  value->SetString(kNameKey, "missing source");
+  value->SetString(kMaxVersionKey, "25");
+  list_with_invalid_format.Append(value);
+
+  // Invalid values in fields
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, "a");
+  value->SetString(kNameKey, "bad source");
+  value->SetString(kMaxVersionKey, "12");
+  list_with_invalid_format.Append(value);
+
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, "1");
+  value->SetString(kNameKey, "bad max version");
+  value->SetString(kMaxVersionKey, "a");
+  list_with_invalid_format.Append(value);
+
+  // And finally something that should work.
+  invalidation::ObjectId valid_id(42, "this should work");
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, "42");
+  value->SetString(kNameKey, valid_id.name());
+  value->SetString(kMaxVersionKey, "20");
+  list_with_invalid_format.Append(value);
+
+  InvalidatorStorage::DeserializeFromList(list_with_invalid_format, &map);
+
+  EXPECT_EQ(1U, map.size());
+  EXPECT_EQ(20, map[valid_id]);
+}
+
+// Tests behavior when there are duplicate entries for a single key. The value
+// of the last entry with that key should be used in the version map.
+TEST_F(InvalidatorStorageTest, DeserializeFromListWithDuplicates) {
+  InvalidationVersionMap map;
+  base::ListValue list;
+  DictionaryValue* value;
+
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, base::IntToString(kBookmarksId_.source()));
+  value->SetString(kNameKey, kBookmarksId_.name());
+  value->SetString(kMaxVersionKey, "20");
+  list.Append(value);
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, base::IntToString(kAutofillId_.source()));
+  value->SetString(kNameKey, kAutofillId_.name());
+  value->SetString(kMaxVersionKey, "10");
+  list.Append(value);
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, base::IntToString(kBookmarksId_.source()));
+  value->SetString(kNameKey, kBookmarksId_.name());
+  value->SetString(kMaxVersionKey, "15");
+  list.Append(value);
+
+  InvalidatorStorage::DeserializeFromList(list, &map);
+  EXPECT_EQ(2U, map.size());
+  EXPECT_EQ(10, map[kAutofillId_]);
+  EXPECT_EQ(15, map[kBookmarksId_]);
+}
+
+TEST_F(InvalidatorStorageTest, DeserializeFromEmptyList) {
+  InvalidationVersionMap map;
+  base::ListValue list;
+  InvalidatorStorage::DeserializeFromList(list, &map);
+  EXPECT_TRUE(map.empty());
 }
 
-TEST_F(InvalidatorStorageTest, DeserializeOutOfRange) {
+// Tests that deserializing a well-formed value results in the expected version
+// map.
+TEST_F(InvalidatorStorageTest, DeserializeFromListBasic) {
+  InvalidationVersionMap map;
+  base::ListValue list;
+  DictionaryValue* value;
+
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, base::IntToString(kAutofillId_.source()));
+  value->SetString(kNameKey, kAutofillId_.name());
+  value->SetString(kMaxVersionKey, "10");
+  list.Append(value);
+  value = new DictionaryValue();
+  value->SetString(kSourceKey, base::IntToString(kBookmarksId_.source()));
+  value->SetString(kNameKey, kBookmarksId_.name());
+  value->SetString(kMaxVersionKey, "15");
+  list.Append(value);
+
+  InvalidatorStorage::DeserializeFromList(list, &map);
+  EXPECT_EQ(2U, map.size());
+  EXPECT_EQ(10, map[kAutofillId_]);
+  EXPECT_EQ(15, map[kBookmarksId_]);
+}
+
+// Tests for legacy deserialization code.
+TEST_F(InvalidatorStorageTest, DeserializeMapOutOfRange) {
   InvalidationVersionMap map;
   base::DictionaryValue dict_with_out_of_range_type;
 
@@ -87,10 +244,10 @@ TEST_F(InvalidatorStorageTest, DeserializeOutOfRange) {
   InvalidatorStorage::DeserializeMap(&dict_with_out_of_range_type, &map);
 
   EXPECT_EQ(1U, map.size());
-  EXPECT_EQ(5, map[syncable::BOOKMARKS]);
+  EXPECT_EQ(5, map[kBookmarksId_]);
 }
 
-TEST_F(InvalidatorStorageTest, DeserializeInvalidFormat) {
+TEST_F(InvalidatorStorageTest, DeserializeMapInvalidFormat) {
   InvalidationVersionMap map;
   base::DictionaryValue dict_with_invalid_format;
 
@@ -104,17 +261,17 @@ TEST_F(InvalidatorStorageTest, DeserializeInvalidFormat) {
   InvalidatorStorage::DeserializeMap(&dict_with_invalid_format, &map);
 
   EXPECT_EQ(1U, map.size());
-  EXPECT_EQ(10, map[syncable::AUTOFILL]);
+  EXPECT_EQ(10, map[kAutofillId_]);
 }
 
-TEST_F(InvalidatorStorageTest, DeserializeEmptyDictionary) {
+TEST_F(InvalidatorStorageTest, DeserializeMapEmptyDictionary) {
   InvalidationVersionMap map;
   base::DictionaryValue dict;
   InvalidatorStorage::DeserializeMap(&dict, &map);
   EXPECT_TRUE(map.empty());
 }
 
-TEST_F(InvalidatorStorageTest, DeserializeBasic) {
+TEST_F(InvalidatorStorageTest, DeserializeMapBasic) {
   InvalidationVersionMap map;
   base::DictionaryValue dict;
 
@@ -123,8 +280,35 @@ TEST_F(InvalidatorStorageTest, DeserializeBasic) {
 
   InvalidatorStorage::DeserializeMap(&dict, &map);
   EXPECT_EQ(2U, map.size());
-  EXPECT_EQ(10, map[syncable::AUTOFILL]);
-  EXPECT_EQ(15, map[syncable::BOOKMARKS]);
+  EXPECT_EQ(10, map[kAutofillId_]);
+  EXPECT_EQ(15, map[kBookmarksId_]);
+}
+
+// Test that the migration code for the legacy preference works as expected.
+// Migration should happen on construction of InvalidatorStorage.
+TEST_F(InvalidatorStorageTest, MigrateLegacyPreferences) {
+  base::DictionaryValue* legacy_dict = new DictionaryValue;
+  legacy_dict->SetString(base::IntToString(syncable::AUTOFILL), "10");
+  legacy_dict->SetString(base::IntToString(syncable::BOOKMARKS), "32");
+  legacy_dict->SetString(base::IntToString(syncable::PREFERENCES), "54");
+  pref_service_.SetUserPref(prefs::kSyncMaxInvalidationVersions, legacy_dict);
+  InvalidatorStorage storage(&pref_service_);
+
+  // Legacy pref should be cleared.
+  const base::DictionaryValue* dict =
+      pref_service_.GetDictionary(prefs::kSyncMaxInvalidationVersions);
+  EXPECT_TRUE(dict->empty());
+
+  // Validate the new pref is set correctly.
+  InvalidationVersionMap map;
+  const base::ListValue* list =
+      pref_service_.GetList(prefs::kInvalidatorMaxInvalidationVersions);
+  InvalidatorStorage::DeserializeFromList(*list, &map);
+
+  EXPECT_EQ(3U, map.size());
+  EXPECT_EQ(10, map[kAutofillId_]);
+  EXPECT_EQ(32, map[kBookmarksId_]);
+  EXPECT_EQ(54, map[kPreferencesId_]);
 }
 
 TEST_F(InvalidatorStorageTest, SetGetInvalidationState) {
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 90f81a2..7a310b6aa 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -44,6 +44,7 @@
         # TODO(akalin): Depend only on syncapi_service from sync.
         '../sync/sync.gyp:syncapi_core',
         '../sync/sync.gyp:syncapi_service',
+        '../sync/sync.gyp:sync_notifier',
         '../third_party/adobe/flash/flash_player.gyp:flapper_version_h',
         '../third_party/bzip2/bzip2.gyp:bzip2',
         '../third_party/cld/cld.gyp:cld',
@@ -88,6 +89,9 @@
           '<@(nacl_defines)',
         ],
       },
+      'export_dependent_settings': [
+        '../sync/sync.gyp:sync_notifier',
+      ],
       'sources': [
         # All .cc, .h, .m, and .mm files under browser except for:
         # * tests and mocks.
diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
index 7fa79f3..1675a5c 100644
--- a/chrome/chrome_browser_extensions.gypi
+++ b/chrome/chrome_browser_extensions.gypi
@@ -8,6 +8,11 @@
       'target_name': 'browser_extensions',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
+      # Since browser and browser_extensions actually depend on each other,
+      # we must omit the dependency from browser_extensions to browser.
+      # However, this means browser_extensions and browser should more or less
+      # have the same dependencies. Once browser_extensions is untangled from
+      # browser, then we can clean up these dependencies.
       'dependencies': [
         'app/policy/cloud_policy_codegen.gyp:policy',
         '../sync/protocol/sync_proto.gyp:sync_proto',
@@ -27,6 +32,7 @@
         '../crypto/crypto.gyp:crypto',
         '../net/net.gyp:net',
         '../skia/skia.gyp:skia',
+        '../sync/sync.gyp:sync_notifier',
         '../third_party/bzip2/bzip2.gyp:bzip2',
         '../third_party/icu/icu.gyp:icui18n',
         '../third_party/icu/icu.gyp:icuuc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index b285de7..ccde326 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -166,8 +166,6 @@
         'browser/sessions/session_service_test_helper.h',
         'browser/ssl/ssl_client_auth_requestor_mock.cc',
         'browser/ssl/ssl_client_auth_requestor_mock.h',
-        'browser/sync/profile_sync_service_mock.cc',
-        'browser/sync/profile_sync_service_mock.h',
         'browser/ui/browser.h',
         'browser/ui/cocoa/run_loop_testing.h',
         'browser/ui/cocoa/run_loop_testing.mm',
@@ -364,14 +362,21 @@
       'dependencies': [
         'chrome_resources.gyp:chrome_resources',
         'chrome_resources.gyp:chrome_strings',
+        'browser',
+        'common',
         'test_support_common',
+        '../base/base.gyp:base',
         '../skia/skia.gyp:skia',
+        '../sync/sync.gyp:sync',
+        '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
       ],
       'include_dirs': [
         '..',
       ],
       'sources': [
+        'browser/sync/profile_sync_service_mock.cc',
+        'browser/sync/profile_sync_service_mock.h',
         'test/base/run_all_unittests.cc',
       ],
       'conditions': [
@@ -4041,6 +4046,7 @@
       'type': 'executable',
       'dependencies': [
         '../sync/protocol/sync_proto.gyp:sync_proto',
+        'browser',
         'chrome',
         'test_support_common',
         '../skia/skia.gyp:skia',
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index a2a1d75..016af5d 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -1600,6 +1600,12 @@ const char kSyncSessionsGUID[] = "sync.session_sync_guid";
 // The value is base 64 encoded.
 const char kInvalidatorInvalidationState[] = "invalidator.invalidation_state";
 
+// List of {source, name, max invalidation version} tuples. source is an int,
+// while max invalidation version is an int64; both are stored as string
+// representations though.
+const char kInvalidatorMaxInvalidationVersions[] =
+    "invalidator.max_invalidation_versions";
+
 // A string that can be used to restore sync encryption infrastructure on
 // startup so that the user doesn't need to provide credentials on each start.
 const char kSyncEncryptionBootstrapToken[] =
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 50b94b7..8373574 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -580,10 +580,12 @@ extern const char kGoogleServicesUsernamePattern[];
 extern const char kSyncUsingSecondaryPassphrase[];
 extern const char kSyncEncryptionBootstrapToken[];
 extern const char kSyncAcknowledgedSyncTypes[];
+// Deprecated in favor of kInvalidatorMaxInvalidationVersions.
 extern const char kSyncMaxInvalidationVersions[];
 extern const char kSyncSessionsGUID[];
 
 extern const char kInvalidatorInvalidationState[];
+extern const char kInvalidatorMaxInvalidationVersions[];
 
 extern const char kSyncPromoStartupCount[];
 extern const char kSyncPromoViewCount[];
diff --git a/sync/notifier/chrome_invalidation_client.cc b/sync/notifier/chrome_invalidation_client.cc
index 0c6c1e0..3557494 100644
--- a/sync/notifier/chrome_invalidation_client.cc
+++ b/sync/notifier/chrome_invalidation_client.cc
@@ -74,7 +74,7 @@ void ChromeInvalidationClient::Start(
              max_invalidation_versions_.begin();
          it != max_invalidation_versions_.end(); ++it) {
       DVLOG(2) << "Initial max invalidation version for "
-               << syncable::ModelTypeToString(it->first) << " is "
+               << ObjectIdToString(it->first) << " is "
                << it->second;
     }
   }
@@ -124,38 +124,41 @@ void ChromeInvalidationClient::Invalidate(
     const invalidation::AckHandle& ack_handle) {
   DCHECK(CalledOnValidThread());
   DVLOG(1) << "Invalidate: " << InvalidationToString(invalidation);
-  syncable::ModelType model_type;
-  if (!ObjectIdToRealModelType(invalidation.object_id(), &model_type)) {
-    LOG(WARNING) << "Could not get invalidation model type; "
-                 << "invalidating everything";
-    EmitInvalidation(registered_types_, std::string());
-    client->Acknowledge(ack_handle);
-    return;
-  }
+
+  const invalidation::ObjectId& id = invalidation.object_id();
+
   // The invalidation API spec allows for the possibility of redundant
   // invalidations, so keep track of the max versions and drop
   // invalidations with old versions.
   //
-  // TODO(akalin): Now that we keep track of registered types, we
-  // should drop invalidations for unregistered types.  We may also
+  // TODO(akalin): Now that we keep track of registered ids, we
+  // should drop invalidations for unregistered ids.  We may also
   // have to filter it at a higher level, as invalidations for
-  // newly-unregistered types may already be in flight.
+  // newly-unregistered ids may already be in flight.
   InvalidationVersionMap::const_iterator it =
-      max_invalidation_versions_.find(model_type);
+      max_invalidation_versions_.find(id);
   if ((it != max_invalidation_versions_.end()) &&
       (invalidation.version() <= it->second)) {
     // Drop redundant invalidations.
     client->Acknowledge(ack_handle);
     return;
   }
-  DVLOG(2) << "Setting max invalidation version for "
-           << syncable::ModelTypeToString(model_type) << " to "
-           << invalidation.version();
-  max_invalidation_versions_[model_type] = invalidation.version();
+  DVLOG(2) << "Setting max invalidation version for " << ObjectIdToString(id)
+           << " to " << invalidation.version();
+  max_invalidation_versions_[id] = invalidation.version();
   invalidation_state_tracker_.Call(
       FROM_HERE,
       &InvalidationStateTracker::SetMaxVersion,
-      model_type, invalidation.version());
+      id, invalidation.version());
+
+  syncable::ModelType model_type;
+  if (!ObjectIdToRealModelType(id, &model_type)) {
+    LOG(WARNING) << "Could not get invalidation model type; "
+                 << "invalidating everything";
+    EmitInvalidation(registered_types_, std::string());
+    client->Acknowledge(ack_handle);
+    return;
+  }
 
   std::string payload;
   // payload() CHECK()'s has_payload(), so we must check it ourselves first.
diff --git a/sync/notifier/chrome_invalidation_client_unittest.cc b/sync/notifier/chrome_invalidation_client_unittest.cc
index 40ddbdd..8cfe490 100644
--- a/sync/notifier/chrome_invalidation_client_unittest.cc
+++ b/sync/notifier/chrome_invalidation_client_unittest.cc
@@ -31,6 +31,8 @@ const char kClientInfo[] = "client_info";
 const char kState[] = "state";
 const char kNewState[] = "new_state";
 
+const int kChromeSyncSourceId = 1004;
+
 class MockInvalidationClient : public invalidation::InvalidationClient {
  public:
   MOCK_METHOD0(Start, void());
@@ -55,7 +57,11 @@ class ChromeInvalidationClientTest : public testing::Test {
  protected:
   ChromeInvalidationClientTest()
       : fake_push_client_(new notifier::FakePushClient()),
-        client_(scoped_ptr<notifier::PushClient>(fake_push_client_)) {}
+        client_(scoped_ptr<notifier::PushClient>(fake_push_client_)),
+        kBookmarksId_(kChromeSyncSourceId, "BOOKMARK"),
+        kPreferencesId_(kChromeSyncSourceId, "PREFERENCE"),
+        kExtensionsId_(kChromeSyncSourceId, "EXTENSION"),
+        kAppsId_(kChromeSyncSourceId, "APP") {}
 
   virtual void SetUp() {
     client_.Start(kClientId, kClientInfo, kState,
@@ -119,6 +125,11 @@ class ChromeInvalidationClientTest : public testing::Test {
   StrictMock<MockInvalidationClient> mock_invalidation_client_;
   notifier::FakePushClient* const fake_push_client_;
   ChromeInvalidationClient client_;
+
+  const invalidation::ObjectId kBookmarksId_;
+  const invalidation::ObjectId kPreferencesId_;
+  const invalidation::ObjectId kExtensionsId_;
+  const invalidation::ObjectId kAppsId_;
 };
 
 namespace {
@@ -141,15 +152,17 @@ TEST_F(ChromeInvalidationClientTest, InvalidateBadObjectId) {
   syncable::ModelTypeSet types(syncable::BOOKMARKS, syncable::APPS);
   client_.RegisterTypes(types);
   EXPECT_CALL(mock_listener_, OnInvalidate(MakeMapFromSet(types, "")));
+  EXPECT_CALL(mock_invalidation_state_tracker_,
+              SetMaxVersion(invalidation::ObjectId(kChromeSyncSourceId, "bad"),
+                            1));
   FireInvalidate("bad", 1, NULL);
-  message_loop_.RunAllPending();
 }
 
 TEST_F(ChromeInvalidationClientTest, InvalidateNoPayload) {
   EXPECT_CALL(mock_listener_,
               OnInvalidate(MakeMap(syncable::BOOKMARKS, "")));
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::BOOKMARKS, 1));
+              SetMaxVersion(kBookmarksId_, 1));
   FireInvalidate("BOOKMARK", 1, NULL);
 }
 
@@ -157,7 +170,7 @@ TEST_F(ChromeInvalidationClientTest, InvalidateWithPayload) {
   EXPECT_CALL(mock_listener_,
               OnInvalidate(MakeMap(syncable::PREFERENCES, "payload")));
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::PREFERENCES, 1));
+              SetMaxVersion(kPreferencesId_, 1));
   FireInvalidate("PREFERENCE", 1, "payload");
 }
 
@@ -167,7 +180,7 @@ TEST_F(ChromeInvalidationClientTest, InvalidateVersion) {
   EXPECT_CALL(mock_listener_,
               OnInvalidate(MakeMap(syncable::APPS, "")));
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::APPS, 1));
+              SetMaxVersion(kAppsId_, 1));
 
   // Should trigger.
   FireInvalidate("APP", 1, NULL);
@@ -200,9 +213,9 @@ TEST_F(ChromeInvalidationClientTest, InvalidateVersionMultipleTypes) {
               OnInvalidate(MakeMap(syncable::EXTENSIONS, "")));
 
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::APPS, 3));
+              SetMaxVersion(kAppsId_, 3));
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::EXTENSIONS, 2));
+              SetMaxVersion(kExtensionsId_, 2));
 
   // Should trigger both.
   FireInvalidate("APP", 3, NULL);
@@ -233,11 +246,11 @@ TEST_F(ChromeInvalidationClientTest, InvalidateVersionMultipleTypes) {
               OnInvalidate(MakeMap(syncable::APPS, "")));
 
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::PREFERENCES, 5));
+              SetMaxVersion(kPreferencesId_, 5));
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::EXTENSIONS, 3));
+              SetMaxVersion(kExtensionsId_, 3));
   EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(syncable::APPS, 4));
+              SetMaxVersion(kAppsId_, 4));
 
   // Should trigger all three.
   FireInvalidate("PREFERENCE", 5, NULL);
diff --git a/sync/notifier/chrome_system_resources.cc b/sync/notifier/chrome_system_resources.cc
index 62a44ac..702078b 100644
--- a/sync/notifier/chrome_system_resources.cc
+++ b/sync/notifier/chrome_system_resources.cc
@@ -14,6 +14,7 @@
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
+#include "google/cacheinvalidation/deps/callback.h"
 #include "google/cacheinvalidation/include/types.h"
 #include "jingle/notifier/listener/push_client.h"
 #include "sync/notifier/invalidation_util.h"
@@ -123,8 +124,9 @@ void ChromeScheduler::SetSystemResources(
 
 void ChromeScheduler::RunPostedTask(invalidation::Closure* task) {
   CHECK_EQ(created_on_loop_, MessageLoop::current());
-  RunAndDeleteClosure(task);
+  task->Run();
   posted_tasks_.erase(task);
+  delete task;
 }
 
 ChromeStorage::ChromeStorage(StateWriter* state_writer,
diff --git a/sync/notifier/invalidation_state_tracker.h b/sync/notifier/invalidation_state_tracker.h
index b09f72e..9dbc352 100644
--- a/sync/notifier/invalidation_state_tracker.h
+++ b/sync/notifier/invalidation_state_tracker.h
@@ -11,11 +11,13 @@
 #include <map>
 
 #include "base/basictypes.h"
-#include "sync/internal_api/public/syncable/model_type.h"
+#include "google/cacheinvalidation/include/types.h"
+#include "sync/notifier/invalidation_util.h"
 
 namespace csync {
 
-typedef std::map<syncable::ModelType, int64> InvalidationVersionMap;
+typedef std::map<invalidation::ObjectId, int64, ObjectIdLessThan>
+    InvalidationVersionMap;
 
 class InvalidationStateTracker {
  public:
@@ -25,7 +27,7 @@ class InvalidationStateTracker {
 
   // |max_version| should be strictly greater than any existing max
   // version for |model_type|.
-  virtual void SetMaxVersion(syncable::ModelType model_type,
+  virtual void SetMaxVersion(const invalidation::ObjectId& id,
                              int64 max_version) = 0;
 
   // Used by InvalidationClient for persistence. |state| is opaque data we can
diff --git a/sync/notifier/invalidation_util.cc b/sync/notifier/invalidation_util.cc
index 27bb851..3e680ec 100644
--- a/sync/notifier/invalidation_util.cc
+++ b/sync/notifier/invalidation_util.cc
@@ -11,9 +11,10 @@
 
 namespace csync {
 
-void RunAndDeleteClosure(invalidation::Closure* task) {
-  task->Run();
-  delete task;
+bool ObjectIdLessThan::operator()(const invalidation::ObjectId& lhs,
+                                  const invalidation::ObjectId& rhs) const {
+  return (lhs.source() < rhs.source()) ||
+         (lhs.source() == rhs.source() && lhs.name() < rhs.name());
 }
 
 bool RealModelTypeToObjectId(syncable::ModelType model_type,
diff --git a/sync/notifier/invalidation_util.h b/sync/notifier/invalidation_util.h
index 8d7de66..628ffed 100644
--- a/sync/notifier/invalidation_util.h
+++ b/sync/notifier/invalidation_util.h
@@ -10,7 +10,6 @@
 
 #include <string>
 
-#include "google/cacheinvalidation/deps/callback.h"
 #include "sync/internal_api/public/syncable/model_type.h"
 
 namespace invalidation {
@@ -22,7 +21,10 @@ class ObjectId;
 
 namespace csync {
 
-void RunAndDeleteClosure(invalidation::Closure* task);
+struct ObjectIdLessThan {
+  bool operator()(const invalidation::ObjectId& lhs,
+                  const invalidation::ObjectId& rhs) const;
+};
 
 bool RealModelTypeToObjectId(syncable::ModelType model_type,
                              invalidation::ObjectId* object_id);
diff --git a/sync/notifier/mock_invalidation_state_tracker.h b/sync/notifier/mock_invalidation_state_tracker.h
index f5a84da..ac884f5 100644
--- a/sync/notifier/mock_invalidation_state_tracker.h
+++ b/sync/notifier/mock_invalidation_state_tracker.h
@@ -19,7 +19,7 @@ class MockInvalidationStateTracker
   virtual ~MockInvalidationStateTracker();
 
   MOCK_CONST_METHOD0(GetAllMaxVersions, InvalidationVersionMap());
-  MOCK_METHOD2(SetMaxVersion, void(syncable::ModelType, int64));
+  MOCK_METHOD2(SetMaxVersion, void(const invalidation::ObjectId&, int64));
   MOCK_CONST_METHOD0(GetInvalidationState, std::string());
   MOCK_METHOD1(SetInvalidationState, void(const std::string&));
 };
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 8202ea2..7fca6b9 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -386,8 +386,6 @@
         'internal_api/public/syncable/model_type_test_util.h',
         'js/js_test_util.cc',
         'js/js_test_util.h',
-        'notifier/mock_invalidation_state_tracker.cc',
-        'notifier/mock_invalidation_state_tracker.h',
         'sessions/test_util.cc',
         'sessions/test_util.h',
         'syncable/syncable_mock.cc',
@@ -436,6 +434,8 @@
         'sync_notifier',
       ],
       'sources': [
+        'notifier/mock_invalidation_state_tracker.cc',
+        'notifier/mock_invalidation_state_tracker.h',
         'notifier/mock_sync_notifier_observer.cc',
         'notifier/mock_sync_notifier_observer.h',
       ],
diff --git a/sync/tools/sync_listen_notifications.cc b/sync/tools/sync_listen_notifications.cc
index 58e116f..e13f3e3 100644
--- a/sync/tools/sync_listen_notifications.cc
+++ b/sync/tools/sync_listen_notifications.cc
@@ -23,6 +23,7 @@
 #include "sync/internal_api/public/syncable/model_type.h"
 #include "sync/internal_api/public/syncable/model_type_payload_map.h"
 #include "sync/notifier/invalidation_state_tracker.h"
+#include "sync/notifier/invalidation_util.h"
 #include "sync/notifier/sync_notifier.h"
 #include "sync/notifier/sync_notifier_factory.h"
 #include "sync/notifier/sync_notifier_observer.h"
@@ -78,10 +79,10 @@ class NullInvalidationStateTracker
   }
 
   virtual void SetMaxVersion(
-      syncable::ModelType model_type,
+      const invalidation::ObjectId& id,
       int64 max_invalidation_version) OVERRIDE {
     LOG(INFO) << "Setting max invalidation version for "
-              << syncable::ModelTypeToString(model_type) << " to "
+              << csync::ObjectIdToString(id) << " to "
               << max_invalidation_version;
   }