sync: move invalidation version prefs out of SyncPrefs into InvalidatorStorage.

chrome/browser/sync/invalidations/invalidator_storage.cc

Index: chrome/browser/sync/invalidations/invalidator_storage.cc
diff --git a/chrome/browser/sync/invalidations/invalidator_storage.cc b/chrome/browser/sync/invalidations/invalidator_storage.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7137cb414b9a16638328e81ad65eee2bdc6ed86c
--- /dev/null
+++ b/chrome/browser/sync/invalidations/invalidator_storage.cc
@@ -0,0 +1,118 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sync/invalidations/invalidator_storage.h"
+
+#include "base/logging.h"
+#include "base/string_number_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/common/pref_names.h"
+
+namespace browser_sync {
+
+InvalidatorStorage::InvalidatorStorage(PrefService* pref_service)
+    : pref_service_(pref_service) {
+
+  if (pref_service_) {

[rlarocque, 2012/05/29 18:28:24]

It looks like there are if branches in several member functions to deal with the
case where this object is constructed with a NULL pref_service.  Consider
creating a separate class for this (ie. MockInvalidatorStorage) so you can keep
both cases separate.  

[tim (not reviewing), 2012/05/29 23:17:31]

As we discussed offline, there are a few gotchas to this. Added todos and filed
bug 130176.

+    pref_service_->RegisterDictionaryPref(prefs::kSyncMaxInvalidationVersions,
+                                          PrefService::UNSYNCABLE_PREF);
+    pref_service_->RegisterStringPref(prefs::kInvalidatorInvalidationState,
+                                      std::string(),
+                                      PrefService::UNSYNCABLE_PREF);
+  }
+}
+
+InvalidatorStorage::~InvalidatorStorage() {
+}
+
+sync_notifier::InvalidationVersionMap InvalidatorStorage::GetAllMaxVersions()
+    const {

[rlarocque, 2012/05/29 18:28:24]

This indent looks odd, though I think it's technically correct.  

Would it be possible to add 'using sync_notifier::InavlidationVersionMap' to the
top of this file?  That would make this declaration more readable.  

[tim (not reviewing), 2012/05/29 23:17:31]

Done.

+  DCHECK(non_thread_safe_.CalledOnValidThread());
+  if (!pref_service_) {
+    return sync_notifier::InvalidationVersionMap();
+  }
+  // Complicated gross code to convert from a string -> string

[rlarocque, 2012/05/29 18:28:24]

Consider moving this into a 'deserialize()' helper function?  

[tim (not reviewing), 2012/05/29 23:17:31]

Done.

+  // DictionaryValue to a ModelType -> int64 map.
+  const base::DictionaryValue* max_versions_dict =
+      pref_service_->GetDictionary(prefs::kSyncMaxInvalidationVersions);
+  CHECK(max_versions_dict);
+  sync_notifier::InvalidationVersionMap max_versions;
+  for (base::DictionaryValue::key_iterator it =
+           max_versions_dict->begin_keys();
+       it != max_versions_dict->end_keys(); ++it) {
+    int model_type_int = 0;
+    if (!base::StringToInt(*it, &model_type_int)) {
+      LOG(WARNING) << "Invalid model type key: " << *it;
+      continue;
+    }
+    if ((model_type_int < syncable::FIRST_REAL_MODEL_TYPE) ||
+        (model_type_int >= syncable::MODEL_TYPE_COUNT)) {
+      LOG(WARNING) << "Out-of-range model type key: " << model_type_int;
+      continue;
+    }
+    const syncable::ModelType model_type =
+        syncable::ModelTypeFromInt(model_type_int);
+    std::string max_version_str;
+    CHECK(max_versions_dict->GetString(*it, &max_version_str));
+    int64 max_version = 0;
+    if (!base::StringToInt64(max_version_str, &max_version)) {
+      LOG(WARNING) << "Invalid max invalidation version for "
+                   << syncable::ModelTypeToString(model_type) << ": "
+                   << max_version_str;
+      continue;
+    }
+    max_versions[model_type] = max_version;
+  }
+  return max_versions;
+}
+
+void InvalidatorStorage::SetMaxVersion(syncable::ModelType model_type,
+                                       int64 max_version) {
+  DCHECK(non_thread_safe_.CalledOnValidThread());
+  DCHECK(syncable::IsRealDataType(model_type));
+  CHECK(pref_service_);
+  sync_notifier::InvalidationVersionMap max_versions =
+      GetAllMaxVersions();
+  sync_notifier::InvalidationVersionMap::iterator it =
+      max_versions.find(model_type);
+  if ((it != max_versions.end()) && (max_version <= it->second)) {
+    NOTREACHED();
+    return;
+  }
+  max_versions[model_type] = max_version;
+
+  // Gross code to convert from a ModelType -> int64 map to a string

[rlarocque, 2012/05/29 18:28:24]

Consider moving this to a serialize() helper function?

[tim (not reviewing), 2012/05/29 23:17:31]

Done.

+  // -> string DictionaryValue.
+  base::DictionaryValue max_versions_dict;
+  for (sync_notifier::InvalidationVersionMap::const_iterator it =
+           max_versions.begin();
+       it != max_versions.end(); ++it) {
+    max_versions_dict.SetString(
+        base::IntToString(it->first),
+        base::Int64ToString(it->second));
+  }
+  pref_service_->Set(prefs::kSyncMaxInvalidationVersions, max_versions_dict);
+}
+
+std::string InvalidatorStorage::GetInvalidationState() const {
+  DLOG(WARNING) << "TODO(tim): Wire this up. Bug 124140.";
+  return pref_service_ ?
+      pref_service_->GetString(prefs::kInvalidatorInvalidationState) : "";
+}
+
+void InvalidatorStorage::SetInvalidationState(const std::string& state) {
+  DLOG(WARNING) << "TODO(tim): Wire this up. Bug 124140.";
+  DCHECK(non_thread_safe_.CalledOnValidThread());
+  pref_service_->SetString(prefs::kInvalidatorInvalidationState,
+                           state);
+}
+
+void InvalidatorStorage::Clear() {
+  DCHECK(non_thread_safe_.CalledOnValidThread());
+  pref_service_->ClearPref(prefs::kInvalidatorInvalidationState);
+  pref_service_->ClearPref(prefs::kSyncMaxInvalidationVersions);
+}
+
+}  // namespace browser_sync