[Invalidation] Drop TopicInvalidationMap

This is a follow-up to https://crrev.com/c/4941730 As we now send only one invalidation at a time, this CLS drops the TopicInvalidationMap and related classes. Bug: b/305177486 b/305203290 Tests: Adjusted unit tests Change-Id: I6f46423018213bbeb9915c4ba08ce9cf0336e770 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4946411 Reviewed-by: Artem Sumaneev <asumaneev@google.com> Commit-Queue: Roland Bock <rbock@google.com> Cr-Commit-Position: refs/heads/main@{#1212684}
chromium · Oct 20, 2023 · e135ddc · e135ddc
1 parent e9cf776
commit e135ddc
Show file tree

Hide file tree

Showing 22 changed files with 48 additions and 552 deletions.
diff --git a/components/invalidation/impl/BUILD.gn b/components/invalidation/impl/BUILD.gn
@@ -99,14 +99,6 @@ source_set("unit_tests") {
     "//testing/gmock",
     "//testing/gtest",
   ]
-
-  if (!is_android) {
-    # Non-Android tests.
-    sources += [
-      "single_topic_invalidation_set_unittest.cc",
-      "topic_invalidation_map_unittest.cc",
-    ]
-  }
 }
 
 static_library("test_support") {

diff --git a/components/invalidation/impl/fake_invalidation_service.cc b/components/invalidation/impl/fake_invalidation_service.cc
@@ -4,11 +4,9 @@
 
 #include "components/invalidation/impl/fake_invalidation_service.h"
 
-#include "base/functional/callback.h"
-#include "base/values.h"
 #include "components/invalidation/impl/invalidation_service_util.h"
+#include "components/invalidation/public/invalidation.h"
 #include "components/invalidation/public/invalidation_util.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 
 namespace invalidation {
 
@@ -76,9 +74,7 @@ void FakeInvalidationService::EmitInvalidationForTest(
   // Otherwise, register the invalidation with the fake_ack_handler_ and deliver
   // it to the appropriate consumer.
   fake_ack_handler_.RegisterInvalidation(&invalidation_copy);
-  TopicInvalidationMap invalidation_map;
-  invalidation_map.Insert(invalidation_copy);
-  invalidator_registrar_->DispatchInvalidationsToHandlers(invalidation_map);
+  invalidator_registrar_->DispatchInvalidationToHandlers(invalidation_copy);
 }
 
 FakeAckHandler* FakeInvalidationService::GetFakeAckHandler() {

diff --git a/components/invalidation/impl/fcm_invalidation_listener.cc b/components/invalidation/impl/fcm_invalidation_listener.cc
@@ -9,7 +9,6 @@
 #include "base/task/single_thread_task_runner.h"
 #include "components/invalidation/public/invalidation.h"
 #include "components/invalidation/public/invalidation_util.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 
 namespace invalidation {
 
@@ -114,15 +113,13 @@ void FCMInvalidationListener::DispatchInvalidation(
 
   // Emit invalidation to registered handlers (if any).
   if (interested_topics_.contains(invalidation.topic())) {
-    TopicInvalidationMap topic_invalidation_map;
-    topic_invalidation_map.Insert(invalidation);
-    EmitSavedInvalidations(topic_invalidation_map);
+    EmitSavedInvalidation(invalidation);
   }
 }
 
-void FCMInvalidationListener::EmitSavedInvalidations(
-    const TopicInvalidationMap& to_emit) {
-  delegate_->OnInvalidate(to_emit);
+void FCMInvalidationListener::EmitSavedInvalidation(
+    const Invalidation& invalidation) {
+  delegate_->OnInvalidate(invalidation);
 }
 
 void FCMInvalidationListener::TokenReceived(
@@ -162,20 +159,18 @@ void FCMInvalidationListener::DoSubscriptionUpdate() {
   // Go over all stored unacked invalidations and dispatch them if their topics
   // have become interesting.
   // Note: We might dispatch invalidations for a second time here, if they were
-  // already dispatched but not acked yet.
+  // already dispatched but not acknowledged yet.
   // TODO(melandory): remove unacked invalidations for unregistered topics.
-  TopicInvalidationMap topic_invalidation_map;
   for (const auto& [topic, invalidation] : unacked_invalidations_map_) {
-    if (interested_topics_.find(topic) == interested_topics_.end()) {
+    if (!interested_topics_.contains(topic)) {
       continue;
     }
-    topic_invalidation_map.Insert(invalidation);
-  }
 
-  // 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(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.
+    EmitSavedInvalidation(invalidation);
+  }
 }
 
 void FCMInvalidationListener::StartForTest(Delegate* delegate) {
@@ -186,9 +181,9 @@ void FCMInvalidationListener::EmitStateChangeForTest(InvalidatorState state) {
   delegate_->OnInvalidatorStateChange(state);
 }
 
-void FCMInvalidationListener::EmitSavedInvalidationsForTest(
-    const TopicInvalidationMap& to_emit) {
-  EmitSavedInvalidations(to_emit);
+void FCMInvalidationListener::EmitSavedInvalidationForTest(
+    const Invalidation& invalidation) {
+  EmitSavedInvalidation(invalidation);
 }
 
 void FCMInvalidationListener::Stop() {

diff --git a/components/invalidation/impl/fcm_invalidation_listener.h b/components/invalidation/impl/fcm_invalidation_listener.h
@@ -19,7 +19,7 @@
 
 namespace invalidation {
 
-class TopicInvalidationMap;
+class Invalidation;
 
 // Receives InstanceID tokens and actual invalidations from FCM via
 // FCMSyncNetworkChannel, and dispatches them to its delegate (in practice, the
@@ -38,7 +38,7 @@ class FCMInvalidationListener
    public:
     virtual ~Delegate() = default;
 
-    virtual void OnInvalidate(const TopicInvalidationMap& invalidations) = 0;
+    virtual void OnInvalidate(const Invalidation& invalidation) = 0;
 
     virtual void OnInvalidatorStateChange(InvalidatorState state) = 0;
   };
@@ -78,7 +78,7 @@ class FCMInvalidationListener
 
   void StartForTest(Delegate* delegate);
   void EmitStateChangeForTest(InvalidatorState state);
-  void EmitSavedInvalidationsForTest(const TopicInvalidationMap& to_emit);
+  void EmitSavedInvalidationForTest(const Invalidation& invalidation);
 
  private:
   // Callbacks for the |network_channel_|.
@@ -103,8 +103,8 @@ class FCMInvalidationListener
   // Cache `invalidation` and emit it to registered handlers (if any).
   void DispatchInvalidation(const Invalidation& invalidation);
 
-  // Emits previously saved invalidations to their registered observers.
-  void EmitSavedInvalidations(const TopicInvalidationMap& to_emit);
+  // Emits previously saved invalidation to their registered observers.
+  void EmitSavedInvalidation(const Invalidation& invalidation);
 
   std::unique_ptr<FCMSyncNetworkChannel> network_channel_;
   std::map<Topic, Invalidation> unacked_invalidations_map_;

diff --git a/components/invalidation/impl/fcm_invalidation_listener_unittest.cc b/components/invalidation/impl/fcm_invalidation_listener_unittest.cc
@@ -13,9 +13,9 @@
 #include "base/test/task_environment.h"
 #include "components/invalidation/impl/fcm_invalidation_listener.h"
 #include "components/invalidation/impl/per_user_topic_subscription_manager.h"
+#include "components/invalidation/public/invalidation.h"
 #include "components/invalidation/public/invalidation_util.h"
 #include "components/invalidation/public/invalidator_state.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -80,14 +80,8 @@ class FakeDelegate : public FCMInvalidationListener::Delegate {
   InvalidatorState GetInvalidatorState() const { return state_; }
 
   // FCMInvalidationListener::Delegate implementation.
-  void OnInvalidate(const TopicInvalidationMap& invalidation_map) override {
-    TopicSet topics = invalidation_map.GetTopics();
-    for (const auto& topic : topics) {
-      const SingleTopicInvalidationSet& incoming =
-          invalidation_map.ForTopic(topic);
-      List& list = invalidations_[topic];
-      list.insert(list.end(), incoming.begin(), incoming.end());
-    }
+  void OnInvalidate(const Invalidation& invalidation) override {
+    invalidations_[invalidation.topic()].push_back(invalidation);
   }
 
   void OnInvalidatorStateChange(InvalidatorState state) override {

diff --git a/components/invalidation/impl/fcm_invalidation_service_base.cc b/components/invalidation/impl/fcm_invalidation_service_base.cc
@@ -14,7 +14,6 @@
 #include "components/invalidation/impl/invalidation_prefs.h"
 #include "components/invalidation/public/invalidator_state.h"
 #include "components/invalidation/public/topic_data.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 #include "components/prefs/scoped_user_pref_update.h"
 
 namespace invalidation {
@@ -122,8 +121,8 @@ std::string FCMInvalidationServiceBase::GetInvalidatorClientId() const {
 }
 
 void FCMInvalidationServiceBase::OnInvalidate(
-    const TopicInvalidationMap& invalidation_map) {
-  invalidator_registrar_.DispatchInvalidationsToHandlers(invalidation_map);
+    const Invalidation& invalidation) {
+  invalidator_registrar_.DispatchInvalidationToHandlers(invalidation);
 }
 
 void FCMInvalidationServiceBase::OnInvalidatorStateChange(

diff --git a/components/invalidation/impl/fcm_invalidation_service_base.h b/components/invalidation/impl/fcm_invalidation_service_base.h
@@ -72,7 +72,7 @@ class FCMInvalidationServiceBase : public InvalidationService,
   std::string GetInvalidatorClientId() const override;
 
   // FCMInvalidationListener::Delegate implementation.
-  void OnInvalidate(const TopicInvalidationMap& invalidation_map) override;
+  void OnInvalidate(const Invalidation& invalidation) override;
   void OnInvalidatorStateChange(InvalidatorState state) override;
 
  protected:

diff --git a/components/invalidation/impl/fcm_invalidation_service_unittest.cc b/components/invalidation/impl/fcm_invalidation_service_unittest.cc
@@ -7,12 +7,9 @@
 #include <memory>
 #include <utility>
 
-#include "base/files/file_path.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
-#include "base/memory/ptr_util.h"
 #include "base/memory/raw_ptr.h"
-#include "base/memory/weak_ptr.h"
 #include "base/strings/strcat.h"
 #include "base/test/task_environment.h"
 #include "base/values.h"
@@ -27,7 +24,7 @@
 #include "components/invalidation/impl/invalidation_prefs.h"
 #include "components/invalidation/impl/invalidation_service_test_template.h"
 #include "components/invalidation/impl/profile_identity_provider.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
+#include "components/invalidation/public/invalidation.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/prefs/testing_pref_service.h"
@@ -165,9 +162,8 @@ class FCMInvalidationServiceTestDelegate {
     fake_listener_->EmitStateChangeForTest(state);
   }
 
-  void TriggerOnIncomingInvalidation(
-      const TopicInvalidationMap& invalidation_map) {
-    fake_listener_->EmitSavedInvalidationsForTest(invalidation_map);
+  void TriggerOnIncomingInvalidation(const Invalidation& invalidation) {
+    fake_listener_->EmitSavedInvalidationForTest(invalidation);
   }
 
   base::test::TaskEnvironment task_environment_;

diff --git a/components/invalidation/impl/invalidation_service_test_template.h b/components/invalidation/impl/invalidation_service_test_template.h
@@ -69,20 +69,16 @@
 #include "base/compiler_specific.h"
 #include "base/memory/raw_ref.h"
 #include "components/invalidation/impl/fake_invalidation_handler.h"
-#include "components/invalidation/public/ack_handle.h"
 #include "components/invalidation/public/invalidation.h"
 #include "components/invalidation/public/invalidation_service.h"
 #include "components/invalidation/public/invalidation_util.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace invalidation {
 
 template <class Delegate, class... Inv>
 void TriggerOnIncomingInvalidation(Delegate& delegate, Inv... inv) {
-  TopicInvalidationMap invalidation_map;
-  (invalidation_map.Insert(inv), ...);
-  delegate.TriggerOnIncomingInvalidation(invalidation_map);
+  (delegate.TriggerOnIncomingInvalidation(inv), ...);
 }
 
 template <class... Inv>

diff --git a/components/invalidation/impl/invalidator_registrar_with_memory.cc b/components/invalidation/impl/invalidator_registrar_with_memory.cc
@@ -5,7 +5,6 @@
 #include "components/invalidation/impl/invalidator_registrar_with_memory.h"
 
 #include <cstddef>
-#include <iterator>
 #include <string>
 #include <utility>
 #include <vector>
@@ -15,8 +14,7 @@
 #include "base/observer_list.h"
 #include "base/stl_util.h"
 #include "base/values.h"
-#include "components/invalidation/public/single_topic_invalidation_set.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
+#include "components/invalidation/public/invalidation.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -203,41 +201,26 @@ Topics InvalidatorRegistrarWithMemory::GetAllSubscribedTopics() const {
   return ConvertTopicSetToLegacyTopicMap(subscribed_topics);
 }
 
-void InvalidatorRegistrarWithMemory::DispatchInvalidationsToHandlers(
-    const TopicInvalidationMap& invalidation_map) {
+void InvalidatorRegistrarWithMemory::DispatchInvalidationToHandlers(
+    const Invalidation& invalidation) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // If we have no handlers, there's nothing to do.
   if (handlers_.empty()) {
     return;
   }
 
-  const auto invalidation_topics = invalidation_map.GetTopics();
-
   // Each handler has a set of registered topics. In order to send the incoming
-  // invalidations to the correct handlers we are going through each handler and
+  // invalidation to the correct handlers we are going through each handler and
   // each of their sets of topics.
   for (const auto& [handler, registered_topics] :
        registered_handler_to_topics_map_) {
     for (const auto& registered_topic : registered_topics) {
-      // If a registered topic is present in the incoming invalidations, then we
-      // extract all invalidations for that topic from the map and send the one
-      // with the highest version to the respective handler.
-      if (!invalidation_topics.contains(registered_topic.name)) {
-        continue;
-      }
-      SingleTopicInvalidationSet invalidations =
-          invalidation_map.ForTopic(registered_topic.name);
-      if (invalidations.IsEmpty()) {
+      // If the topic of the invalidation matches a registered topic, we send
+      // the invalidation to the respective handler.
+      if (invalidation.topic() != registered_topic.name) {
         continue;
       }
-      handler->OnIncomingInvalidation(invalidations.back());
-
-      // Acknowledge all except the invalidation with the highest version.
-      auto it = invalidations.rbegin();
-      ++it;
-      for (; it != invalidations.rend(); ++it) {
-        it->Acknowledge();
-      }
+      handler->OnIncomingInvalidation(invalidation);
     }
   }
 }

diff --git a/components/invalidation/impl/invalidator_registrar_with_memory.h b/components/invalidation/impl/invalidator_registrar_with_memory.h
@@ -13,11 +13,9 @@
 #include "base/memory/raw_ptr.h"
 #include "base/observer_list.h"
 #include "base/sequence_checker.h"
-#include "base/values.h"
 #include "components/invalidation/public/invalidation_export.h"
 #include "components/invalidation/public/invalidation_handler.h"
 #include "components/invalidation/public/topic_data.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 
@@ -26,6 +24,8 @@ class PrefService;
 
 namespace invalidation {
 
+class Invalidation;
+
 BASE_DECLARE_FEATURE(kRestoreInterestingTopicsFeature);
 
 // A helper class for FCMInvalidationService.  It helps keep track of registered
@@ -88,12 +88,10 @@ class INVALIDATION_EXPORT InvalidatorRegistrarWithMemory {
   // itself again yet).
   Topics GetAllSubscribedTopics() const;
 
-  // Sorts incoming invalidations into a bucket for each handler and then
-  // dispatches the batched invalidations to the corresponding handler.
-  // Invalidations for topics with no corresponding handler are dropped, as are
-  // invalidations for handlers that are not added.
-  void DispatchInvalidationsToHandlers(
-      const TopicInvalidationMap& invalidation_map);
+  // Dispatches incoming invalidation to the corresponding handler based on its
+  // topic.
+  // Invalidations for topics with no corresponding handler are dropped.
+  void DispatchInvalidationToHandlers(const Invalidation& invalidation);
 
   // Updates the invalidator state to the given one and then notifies
   // all handlers.  Note that the order is important; handlers that

diff --git a/components/invalidation/impl/invalidator_registrar_with_memory_unittest.cc b/components/invalidation/impl/invalidator_registrar_with_memory_unittest.cc
@@ -14,7 +14,6 @@
 #include "components/invalidation/public/invalidation.h"
 #include "components/invalidation/public/invalidation_util.h"
 #include "components/invalidation/public/invalidator_state.h"
-#include "components/invalidation/public/topic_invalidation_map.h"
 #include "components/prefs/testing_pref_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -29,9 +28,7 @@ namespace {
 
 template <class... Inv>
 void Dispatch(InvalidatorRegistrarWithMemory& registrar, Inv... inv) {
-  TopicInvalidationMap invalidation_map;
-  (invalidation_map.Insert(inv), ...);
-  registrar.DispatchInvalidationsToHandlers(invalidation_map);
+  (registrar.DispatchInvalidationToHandlers(inv), ...);
 }
 
 template <class... Inv>