[UnackedInvalidationSet] Update API to use Topic instead of ObjectId

This CL is a part of ObjectId -> Topic API migration. It's a pure
refactoring and there should be no behavioral changes.

Bug: 1029698
Change-Id: Ieab9c5a76c009a3ec93dd2a8737b9f9b6119a101
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2059376Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#743064}

components/invalidation/impl/fcm_invalidation_listener.cc

@@ -103,15 +103,14 @@ void FCMInvalidationListener::DispatchInvalidations(
 
 void FCMInvalidationListener::SaveInvalidations(
     const TopicInvalidationMap& to_save) {
-  ObjectIdSet objects_to_save = ConvertTopicsToIds(to_save.GetTopics());
-  for (const invalidation::ObjectId& id : objects_to_save) {
-    auto lookup = unacked_invalidations_map_.find(id);
+  for (const Topic& topic : to_save.GetTopics()) {
+    auto lookup = unacked_invalidations_map_.find(topic);
     if (lookup == unacked_invalidations_map_.end()) {
-      lookup =
-          unacked_invalidations_map_.emplace(id, UnackedInvalidationSet(id))
-              .first;
+      lookup = unacked_invalidations_map_
+                   .emplace(topic, UnackedInvalidationSet(topic))
+                   .first;
     }
-    lookup->second.AddSet(to_save.ForTopic(id.name()));
+    lookup->second.AddSet(to_save.ForTopic(topic));
   }
 }
 
@@ -134,7 +133,7 @@ void FCMInvalidationListener::TokenReceived(
 
 void FCMInvalidationListener::Acknowledge(const invalidation::ObjectId& id,
                                           const syncer::AckHandle& handle) {
-  auto lookup = unacked_invalidations_map_.find(id);
+  auto lookup = unacked_invalidations_map_.find(id.name());
   if (lookup == unacked_invalidations_map_.end()) {
     DLOG(WARNING) << "Received acknowledgement for untracked object ID";
     return;
@@ -144,7 +143,7 @@ void FCMInvalidationListener::Acknowledge(const invalidation::ObjectId& id,
 
 void FCMInvalidationListener::Drop(const invalidation::ObjectId& id,
                                    const syncer::AckHandle& handle) {
-  auto lookup = unacked_invalidations_map_.find(id);
+  auto lookup = unacked_invalidations_map_.find(id.name());
   if (lookup == unacked_invalidations_map_.end()) {
     DLOG(WARNING) << "Received drop for untracked object ID";
     return;
@@ -164,24 +163,22 @@ void FCMInvalidationListener::DoSubscriptionUpdate() {
   // have become interesting.
   // Note: We might dispatch invalidations for a second time here, if they were
   // already dispatched but not acked yet.
-  // TODO(melandory): remove unacked invalidations for unregistered objects.
-  ObjectIdInvalidationMap object_id_invalidation_map;
+  // TODO(melandory): remove unacked invalidations for unregistered topics.
+  TopicInvalidationMap topic_invalidation_map;
   for (const auto& unacked : unacked_invalidations_map_) {
-    if (interested_topics_.find(unacked.first.name()) ==
-        interested_topics_.end()) {
+    if (interested_topics_.find(unacked.first) == interested_topics_.end()) {
       continue;
     }
 
     unacked.second.ExportInvalidations(weak_factory_.GetWeakPtr(),
                                        base::ThreadTaskRunnerHandle::Get(),
-                                       &object_id_invalidation_map);
+                                       &topic_invalidation_map);
   }
 
   // There's no need to run these through DispatchInvalidations(); they've
   // already been saved to storage (that's where we found them) so all we need
   // to do now is emit them.
-  EmitSavedInvalidations(ConvertObjectIdInvalidationMapToTopicInvalidationMap(
-      object_id_invalidation_map));
+  EmitSavedInvalidations(topic_invalidation_map);
 }
 
 void FCMInvalidationListener::RequestDetailedStatus(

components/invalidation/impl/unacked_invalidation_set.cc

@@ -5,29 +5,22 @@
 #include "components/invalidation/impl/unacked_invalidation_set.h"
 
 #include "components/invalidation/public/ack_handle.h"
-#include "components/invalidation/public/object_id_invalidation_map.h"
+#include "components/invalidation/public/topic_invalidation_map.h"
 
 namespace syncer {
 
 const size_t UnackedInvalidationSet::kMaxBufferedInvalidations = 5;
 
-// static
-UnackedInvalidationSet::UnackedInvalidationSet(
-    invalidation::ObjectId id)
-    : registered_(false),
-      object_id_(id) {}
+UnackedInvalidationSet::UnackedInvalidationSet(const Topic& topic)
+    : registered_(false), topic_(topic) {}
 
 UnackedInvalidationSet::UnackedInvalidationSet(
-    const UnackedInvalidationSet& other)
-    : registered_(other.registered_),
-      object_id_(other.object_id_),
-      invalidations_(other.invalidations_) {
-}
+    const UnackedInvalidationSet& other) = default;
 
-UnackedInvalidationSet::~UnackedInvalidationSet() {}
+UnackedInvalidationSet::~UnackedInvalidationSet() = default;
 
-const invalidation::ObjectId& UnackedInvalidationSet::object_id() const {
-  return object_id_;
+const Topic& UnackedInvalidationSet::topic() const {
+  return topic_;
 }
 
 void UnackedInvalidationSet::Add(
@@ -49,12 +42,12 @@ void UnackedInvalidationSet::AddSet(
 void UnackedInvalidationSet::ExportInvalidations(
     base::WeakPtr<AckHandler> ack_handler,
     scoped_refptr<base::SingleThreadTaskRunner> ack_handler_task_runner,
-    ObjectIdInvalidationMap* out) const {
-  for (auto it = invalidations_.begin(); it != invalidations_.end(); ++it) {
+    TopicInvalidationMap* out) const {
+  for (const Invalidation& invalidation : invalidations_) {
     // Copy the invalidation and set the copy's ack_handler.
-    Invalidation inv(*it);
-    inv.SetAckHandler(ack_handler, ack_handler_task_runner);
-    out->Insert(inv);
+    Invalidation invalidation_copy = invalidation;
+    invalidation_copy.SetAckHandler(ack_handler, ack_handler_task_runner);
+    out->Insert(invalidation_copy);
   }
 }
 
@@ -81,8 +74,7 @@ void UnackedInvalidationSet::Acknowledge(const AckHandle& handle) {
       break;
     }
   }
-  DLOG_IF(WARNING, !handle_found)
-      << "Unrecognized to ack for object " << ObjectIdToString(object_id_);
+  DLOG_IF(WARNING, !handle_found) << "Unrecognized to ack for topic " << topic_;
   (void)handle_found;  // Silence unused variable warning in release builds.
 }
 
@@ -98,8 +90,7 @@ void UnackedInvalidationSet::Drop(const AckHandle& handle) {
     }
   }
   if (it == invalidations_.end()) {
-    DLOG(WARNING) << "Unrecognized drop request for object "
-                  << ObjectIdToString(object_id_);
+    DLOG(WARNING) << "Unrecognized drop request for topic " << topic_;
     return;
   }
 

components/invalidation/impl/unacked_invalidation_set.h

@@ -19,26 +19,25 @@
 namespace syncer {
 
 class SingleObjectInvalidationSet;
-class ObjectIdInvalidationMap;
+class TopicInvalidationMap;
 class AckHandle;
 class UnackedInvalidationSet;
 
-using UnackedInvalidationsMap =
-    std::map<invalidation::ObjectId, UnackedInvalidationSet, ObjectIdLessThan>;
+using UnackedInvalidationsMap = std::map<Topic, UnackedInvalidationSet>;
 
 // Manages the set of invalidations that are awaiting local acknowledgement for
-// a particular ObjectId.  This set of invalidations will be persisted across
+// a particular Topic.  This set of invalidations will be persisted across
 // restarts, though this class is not directly responsible for that.
 class INVALIDATION_EXPORT UnackedInvalidationSet {
  public:
   static const size_t kMaxBufferedInvalidations;
 
-  explicit UnackedInvalidationSet(invalidation::ObjectId id);
+  explicit UnackedInvalidationSet(const Topic& topic);
   UnackedInvalidationSet(const UnackedInvalidationSet& other);
   ~UnackedInvalidationSet();
 
-  // Returns the ObjectID of the invalidations this class is tracking.
-  const invalidation::ObjectId& object_id() const;
+  // Returns the Topic of the invalidations this class is tracking.
+  const Topic& topic() const;
 
   // Adds a new invalidation to the set awaiting acknowledgement.
   void Add(const Invalidation& invalidation);
@@ -47,7 +46,7 @@ class INVALIDATION_EXPORT UnackedInvalidationSet {
   void AddSet(const SingleObjectInvalidationSet& invalidations);
 
   // Exports the set of invalidations awaiting acknowledgement as an
-  // ObjectIdInvalidationMap.  Each of these invalidations will be associated
+  // TopicInvalidationMap. Each of these invalidations will be associated
   // with the given |ack_handler|.
   //
   // The contents of the UnackedInvalidationSet are not directly modified by
@@ -56,7 +55,7 @@ class INVALIDATION_EXPORT UnackedInvalidationSet {
   void ExportInvalidations(
       base::WeakPtr<AckHandler> ack_handler,
       scoped_refptr<base::SingleThreadTaskRunner> ack_handler_task_runner,
-      ObjectIdInvalidationMap* out) const;
+      TopicInvalidationMap* out) const;
 
   // Removes all stored invalidations from this object.
   void Clear();
@@ -100,7 +99,7 @@ class INVALIDATION_EXPORT UnackedInvalidationSet {
   void Truncate(size_t max_size);
 
   bool registered_;
-  invalidation::ObjectId object_id_;
+  const Topic topic_;
   InvalidationsSet invalidations_;
 };
 

components/invalidation/impl/unacked_invalidation_set_unittest.cc

@@ -11,33 +11,31 @@
 #include "base/json/json_string_value_serializer.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
-#include "components/invalidation/public/object_id_invalidation_map.h"
 #include "components/invalidation/public/single_object_invalidation_set.h"
+#include "components/invalidation/public/topic_invalidation_map.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
 
 class UnackedInvalidationSetTest : public testing::Test {
  public:
-  UnackedInvalidationSetTest()
-      : kObjectId_(10, "ASDF"),
-        unacked_invalidations_(kObjectId_) {}
+  UnackedInvalidationSetTest() : unacked_invalidations_(kTopic) {}
 
   SingleObjectInvalidationSet GetStoredInvalidations() {
-    ObjectIdInvalidationMap map;
+    TopicInvalidationMap map;
     unacked_invalidations_.ExportInvalidations(
         base::WeakPtr<AckHandler>(),
         scoped_refptr<base::SingleThreadTaskRunner>(),
         &map);
-    ObjectIdSet ids = map.GetObjectIds();
-    if (ids.find(kObjectId_) != ids.end()) {
-      return map.ForObject(kObjectId_);
-    } else {
+    if (map.Empty()) {
       return SingleObjectInvalidationSet();
+    } else {
+      return map.ForTopic(kTopic);
     }
   }
 
-  const invalidation::ObjectId kObjectId_;
+  const Topic kTopic = "ASDF";
+  const invalidation::ObjectId kObjectId = ConvertTopicToId(kTopic);
   UnackedInvalidationSet unacked_invalidations_;
 };
 
@@ -50,7 +48,7 @@ TEST_F(UnackedInvalidationSetTest, Empty) {
 
 // Test storage and retrieval of a single invalidation.
 TEST_F(UnackedInvalidationSetTest, OneInvalidation) {
-  Invalidation inv1 = Invalidation::Init(kObjectId_, 10, "payload");
+  Invalidation inv1 = Invalidation::Init(kObjectId, 10, "payload");
   unacked_invalidations_.Add(inv1);
 
   SingleObjectInvalidationSet set = GetStoredInvalidations();
@@ -60,7 +58,7 @@ TEST_F(UnackedInvalidationSetTest, OneInvalidation) {
 
 // Test that calling Clear() returns us to the empty state.
 TEST_F(UnackedInvalidationSetTest, Clear) {
-  Invalidation inv1 = Invalidation::Init(kObjectId_, 10, "payload");
+  Invalidation inv1 = Invalidation::Init(kObjectId, 10, "payload");
   unacked_invalidations_.Add(inv1);
   unacked_invalidations_.Clear();
 
@@ -69,9 +67,9 @@ TEST_F(UnackedInvalidationSetTest, Clear) {
 
 // Test that repeated unknown version invalidations are squashed together.
 TEST_F(UnackedInvalidationSetTest, UnknownVersions) {
-  Invalidation inv1 = Invalidation::Init(kObjectId_, 10, "payload");
-  Invalidation inv2 = Invalidation::InitUnknownVersion(kObjectId_);
-  Invalidation inv3 = Invalidation::InitUnknownVersion(kObjectId_);
+  Invalidation inv1 = Invalidation::Init(kObjectId, 10, "payload");
+  Invalidation inv2 = Invalidation::InitUnknownVersion(kObjectId);
+  Invalidation inv3 = Invalidation::InitUnknownVersion(kObjectId);
   unacked_invalidations_.Add(inv1);
   unacked_invalidations_.Add(inv2);
   unacked_invalidations_.Add(inv3);
@@ -86,7 +84,7 @@ TEST_F(UnackedInvalidationSetTest, NoTruncation) {
   size_t kMax = UnackedInvalidationSet::kMaxBufferedInvalidations;
 
   for (size_t i = 0; i < kMax; ++i) {
-    Invalidation inv = Invalidation::Init(kObjectId_, i, "payload");
+    Invalidation inv = Invalidation::Init(kObjectId, i, "payload");
     unacked_invalidations_.Add(inv);
   }
 
@@ -102,7 +100,7 @@ TEST_F(UnackedInvalidationSetTest, Truncation) {
   size_t kMax = UnackedInvalidationSet::kMaxBufferedInvalidations;
 
   for (size_t i = 0; i < kMax + 1; ++i) {
-    Invalidation inv = Invalidation::Init(kObjectId_, i, "payload");
+    Invalidation inv = Invalidation::Init(kObjectId, i, "payload");
     unacked_invalidations_.Add(inv);
   }
 
@@ -120,7 +118,7 @@ TEST_F(UnackedInvalidationSetTest, RegistrationAndTruncation) {
   size_t kMax = UnackedInvalidationSet::kMaxBufferedInvalidations;
 
   for (size_t i = 0; i < kMax + 1; ++i) {
-    Invalidation inv = Invalidation::Init(kObjectId_, i, "payload");
+    Invalidation inv = Invalidation::Init(kObjectId, i, "payload");
     unacked_invalidations_.Add(inv);
   }
 
@@ -148,8 +146,8 @@ TEST_F(UnackedInvalidationSetTest, Acknowledge) {
   // Not that it makes much of a difference in behavior.
   unacked_invalidations_.SetHandlerIsRegistered();
 
-  Invalidation inv1 = Invalidation::Init(kObjectId_, 10, "payload");
-  Invalidation inv2 = Invalidation::InitUnknownVersion(kObjectId_);
+  Invalidation inv1 = Invalidation::Init(kObjectId, 10, "payload");
+  Invalidation inv2 = Invalidation::InitUnknownVersion(kObjectId);
   AckHandle inv1_handle = inv1.ack_handle();
 
   unacked_invalidations_.Add(inv1);
@@ -171,8 +169,8 @@ TEST_F(UnackedInvalidationSetTest, Drop) {
   // Not that it makes much of a difference in behavior.
   unacked_invalidations_.SetHandlerIsRegistered();
 
-  Invalidation inv1 = Invalidation::Init(kObjectId_, 10, "payload");
-  Invalidation inv2 = Invalidation::Init(kObjectId_, 15, "payload");
+  Invalidation inv1 = Invalidation::Init(kObjectId, 10, "payload");
+  Invalidation inv2 = Invalidation::Init(kObjectId, 15, "payload");
   AckHandle inv1_handle = inv1.ack_handle();
 
   unacked_invalidations_.Add(inv1);