Serialize and deserialize InvalidationObjectId.

This CL implements routines for serializing and deserializing object
ids, which simplifies the logic for storing topics which were already
registered for.

TBR: pavely@chromium.org
Bug: 867402, 801985
Change-Id: Ie859cc87b8e9b3cff54ae2af0eeed68467af09af
Reviewed-on: https://chromium-review.googlesource.com/1154921Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583991}

components/BUILD.gn

@@ -220,6 +220,7 @@ test("components_unittests") {
       "//components/gcm_driver/instance_id:unit_tests",
       "//components/history/content/browser:unit_tests",
       "//components/invalidation/impl:unit_tests",
+      "//components/invalidation/public:unit_tests",
       "//components/keyed_service/content:unit_tests",
       "//components/language/content/browser:unit_tests",
       "//components/link_header_util:unit_tests",

components/invalidation/impl/per_user_topic_registration_manager.cc

@@ -81,9 +81,13 @@ PerUserTopicRegistrationManager::RegistrationEntry::RegistrationEntry(
       pref(pref) {
   const base::DictionaryValue* registered_for_invalidation =
       pref->GetDictionary(kTypeRegisteredForInvalidation);
-  if (registered_for_invalidation &&
-      registered_for_invalidation->FindKey(id.name())) {
-    state = RegistrationEntry::REGISTERED;
+  if (registered_for_invalidation) {
+    auto* value =
+        registered_for_invalidation->FindKey(SerializeInvalidationObjectId(id));
+    if (value) {
+      value->GetAsString(&private_topic_name);
+      state = RegistrationEntry::REGISTERED;
+    }
   }
 }
 
@@ -95,9 +99,9 @@ void PerUserTopicRegistrationManager::RegistrationEntry::RegistrationFinished(
   if (code.IsSuccess()) {
     private_topic_name = topic_name;
     state = RegistrationEntry::REGISTERED;
-    ScopedUserPrefUpdate<base::DictionaryValue, base::Value::Type::DICTIONARY>
-        topics_update(pref, kTypeRegisteredForInvalidation);
-    topics_update->SetKey(id.name(), base::Value(private_topic_name));
+    DictionaryPrefUpdate update(pref, kTypeRegisteredForInvalidation);
+    base::DictionaryValue* pref_data = update.Get();
+    pref_data->SetString(SerializeInvalidationObjectId(id), private_topic_name);
   }
 }
 

components/invalidation/impl/per_user_topic_registration_manager_unittest.cc

@@ -152,11 +152,10 @@ TEST_F(PerUserTopicRegistrationManagerTest, ShouldUpdateRegisteredIds) {
   EXPECT_EQ(ids, per_user_topic_registration_manager->GetRegisteredIds());
 
   for (const auto& id : ids) {
-    std::string topic_name = id.name();
     const base::DictionaryValue* topics =
         pref_service()->GetDictionary(kTypeRegisteredForInvalidation);
-    const base::Value* private_topic_value =
-        topics->FindKeyOfType(topic_name, base::Value::Type::STRING);
+    const base::Value* private_topic_value = topics->FindKeyOfType(
+        SerializeInvalidationObjectId(id), base::Value::Type::STRING);
     ASSERT_NE(private_topic_value, nullptr);
   }
 }

components/invalidation/public/BUILD.gn

@@ -35,3 +35,16 @@ static_library("public") {
     "//base",
   ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [
+    "invalidation_util_unittest.cc",
+  ]
+  deps = [
+    ":public",
+    "//base",
+    "//base/test:test_support",
+    "//testing/gtest",
+  ]
+}

components/invalidation/public/invalidation_util.cc

@@ -8,6 +8,7 @@
 #include <ostream>
 #include <sstream>
 
+#include "base/json/json_string_value_serializer.h"
 #include "base/json/json_writer.h"
 #include "base/values.h"
 #include "components/invalidation/public/invalidation.h"
@@ -15,6 +16,13 @@
 #include "google/cacheinvalidation/include/types.h"
 #include "google/cacheinvalidation/types.pb.h"
 
+namespace {
+
+const char kSourceKey[] = "source";
+const char kNameKey[] = "name";
+
+}  // namespace
+
 namespace syncer {
 
 bool ObjectIdLessThan::operator()(const invalidation::ObjectId& lhs,
@@ -77,11 +85,50 @@ bool InvalidationObjectIdLessThan::operator()(
 std::unique_ptr<base::DictionaryValue> InvalidationObjectIdToValue(
     const invalidation::InvalidationObjectId& object_id) {
   std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue());
-  value->SetInteger("source", object_id.source());
-  value->SetString("name", object_id.name());
+  value->SetString(kNameKey, object_id.name());
+  value->SetInteger(kSourceKey, object_id.source());
   return value;
 }
 
+std::string SerializeInvalidationObjectId(
+    const invalidation::InvalidationObjectId& object_id) {
+  std::unique_ptr<base::DictionaryValue> value =
+      InvalidationObjectIdToValue(object_id);
+  std::string serialized_id;
+  JSONStringValueSerializer serializer(&serialized_id);
+  serializer.Serialize(*value);
+  return serialized_id;
+}
+
+bool DeserializeInvalidationObjectId(const std::string& serialized_id,
+                                     invalidation::InvalidationObjectId* id) {
+  JSONStringValueDeserializer deserializer(serialized_id);
+  int error_code;
+  std::string error_msg;
+  std::unique_ptr<base::Value> value =
+      deserializer.Deserialize(&error_code, &error_msg);
+  base::DictionaryValue* dict = nullptr;
+
+  if (!value->GetAsDictionary(&dict)) {
+    DLOG(WARNING) << "Unable to get dictionary";
+    return false;
+  }
+
+  int source = 0;
+  if (!dict->GetInteger(kSourceKey, &source)) {
+    DLOG(WARNING) << "Unable to deserialize source";
+    return false;
+  }
+
+  std::string name;
+  if (!dict->GetString(kNameKey, &name)) {
+    DLOG(WARNING) << "Unable to deserialize name";
+    return false;
+  }
+  id->Init(source, name);
+  return true;
+}
+
 std::string InvalidationObjectIdToString(
     const invalidation::InvalidationObjectId& object_id) {
   std::string str;

components/invalidation/public/invalidation_util.h

@@ -79,6 +79,12 @@ typedef std::
 std::unique_ptr<base::DictionaryValue> InvalidationObjectIdToValue(
     const invalidation::InvalidationObjectId& object_id);
 
+// TODO(melandory): figure out the security implications for such serialization.
+std::string SerializeInvalidationObjectId(
+    const invalidation::InvalidationObjectId& object_id);
+bool DeserializeInvalidationObjectId(const std::string& serialized,
+                                     invalidation::InvalidationObjectId* id);
+
 INVALIDATION_EXPORT std::string InvalidationObjectIdToString(
     const invalidation::InvalidationObjectId& object_id);
 

components/invalidation/public/invalidation_util_unittest.cc

+// Copyright 2018 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 "components/invalidation/public/invalidation_util.h"
+
+#include "components/invalidation/public/invalidation_object_id.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+class InvalidationObjectIdSerializationTest : public testing::Test {
+ public:
+  InvalidationObjectIdSerializationTest() : kObjectId_(10, "ASDF") {}
+
+  const invalidation::InvalidationObjectId kObjectId_;
+};
+
+TEST_F(InvalidationObjectIdSerializationTest,
+       ShouldCorrectlyDeserializeSerialized) {
+  std::string serialized = SerializeInvalidationObjectId(kObjectId_);
+  invalidation::InvalidationObjectId id;
+  DeserializeInvalidationObjectId(serialized, &id);
+  EXPECT_EQ(kObjectId_, id);
+}
+
+}  // namespace syncer