Refactor common invalidation framework types

sync/notifier/invalidator_registrar.cc

 // 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 "sync/notifier/invalidator_registrar.h"
 
 #include <cstddef>
 #include <utility>
 
 #include "base/logging.h"
+#include "sync/notifier/object_id_invalidation_map.h"
 
 namespace syncer {
 
 InvalidatorRegistrar::InvalidatorRegistrar()
     : state_(DEFAULT_INVALIDATION_ERROR) {}
 
 InvalidatorRegistrar::~InvalidatorRegistrar() {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(!handlers_.might_have_observers());
-  // |id_to_handler_map_| may be non-empty but that's okay.

[tim (not reviewing), 2013/09/20 21:53:46]

Hmm.. remind me what was happening here?  Why was this OK?

[rlarocque, 2013/09/23 18:38:19]

In this part of the code, "registration" is an overloaded term.

The invalidations client registers a set of object IDs with the server.  Those
registrations persist across restarts.

There are also handlers of invalidations that register with the invalidation
service because they want to be called back when certain invalidations are
received.  These registrations must be unregistered during shutdown; otherwise
we would risk calling into deleted handlers.

We expose this two our clients by exposing a two-step
registration/unregistration process.  Registration for callbacks is done with
RegisterHandler and UnregisterHandler.  Setting up to listen for specific
ObjectIDs (and requesting that the client register with the server as a listener
of these IDs) is done with UpdateRegisteredIds.  

During normal shutdown, we expect that handlers will call UnregisterHandler(),
but not call UpdateReisteredIds().  The id_to_handler_map_ is managed mainly by
UpdateRegisteredIds(), so it won't be empty at shutdown.

[tim (not reviewing), 2013/09/24 21:16:54]

I see.  So the two parts are separate in your new implementation and hence you
can assert one of them is empty.  Thanks.

+  CHECK(handler_to_id_map_.empty());
 }
 
 void InvalidatorRegistrar::RegisterHandler(InvalidationHandler* handler) {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(handler);
   CHECK(!handlers_.HasObserver(handler));
   handlers_.AddObserver(handler);
 }
 
 void InvalidatorRegistrar::UpdateRegisteredIds(
     InvalidationHandler* handler,
     const ObjectIdSet& ids) {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(handler);
   CHECK(handlers_.HasObserver(handler));
-  // Remove all existing entries for |handler|.
-  for (IdHandlerMap::iterator it = id_to_handler_map_.begin();
-       it != id_to_handler_map_.end(); ) {
-    if (it->second == handler) {
-      IdHandlerMap::iterator erase_it = it;
-      ++it;
-      id_to_handler_map_.erase(erase_it);
-    } else {
-      ++it;
+
+  for (HandlerIdMap::const_iterator it = handler_to_id_map_.begin();
+       it != handler_to_id_map_.end(); ++it) {
+    if (it->first == handler) {
+      continue;
     }
+
+    std::vector<invalidation::ObjectId> intersection;
+    std::set_intersection(
+        it->second.begin(), it->second.end(),
+        ids.begin(), ids.end(),
+        intersection.begin(), ObjectIdLessThan());
+    CHECK(intersection.empty())
+        << "Duplicate registration: trying to register "
+        << ObjectIdToString(*intersection.begin()) << " for "
+        << handler << " when it's already registered for "
+        << it->first;
   }
 
-  // Now add the entries for |handler|. We keep track of the last insertion
-  // point so we only traverse the map once to insert all the new entries.
-  IdHandlerMap::iterator insert_it = id_to_handler_map_.begin();
-  for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
-    insert_it =
-        id_to_handler_map_.insert(insert_it, std::make_pair(*it, handler));
-    CHECK_EQ(handler, insert_it->second)
-        << "Duplicate registration: trying to register "
-        << ObjectIdToString(insert_it->first) << " for "
-        << handler << " when it's already registered for "
-        << insert_it->second;
+  if (ids.empty()) {
+    handler_to_id_map_.erase(handler);
+  } else {
+    handler_to_id_map_[handler] = ids;
   }
 }
 
 void InvalidatorRegistrar::UnregisterHandler(InvalidationHandler* handler) {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(handler);
   CHECK(handlers_.HasObserver(handler));
   handlers_.RemoveObserver(handler);
+  handler_to_id_map_.erase(handler);
 }
 
 ObjectIdSet InvalidatorRegistrar::GetRegisteredIds(
     InvalidationHandler* handler) const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  ObjectIdSet registered_ids;
-  for (IdHandlerMap::const_iterator it = id_to_handler_map_.begin();
-       it != id_to_handler_map_.end(); ++it) {
-    if (it->second == handler) {
-      registered_ids.insert(it->first);
-    }
+  HandlerIdMap::const_iterator lookup = handler_to_id_map_.find(handler);
+  if (lookup != handler_to_id_map_.end()) {
+    return lookup->second;
+  } else {
+    return ObjectIdSet();
   }
-  return registered_ids;
 }
 
 ObjectIdSet InvalidatorRegistrar::GetAllRegisteredIds() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   ObjectIdSet registered_ids;
-  for (IdHandlerMap::const_iterator it = id_to_handler_map_.begin();
-       it != id_to_handler_map_.end(); ++it) {
-    registered_ids.insert(it->first);
+  for (HandlerIdMap::const_iterator it = handler_to_id_map_.begin();
+       it != handler_to_id_map_.end(); ++it) {
+    registered_ids.insert(it->second.begin(), it->second.end());
   }
   return registered_ids;
 }
 
 void InvalidatorRegistrar::DispatchInvalidationsToHandlers(
     const ObjectIdInvalidationMap& invalidation_map) {

[tim (not reviewing), 2013/09/20 21:53:46]

So, |invalidation_map| contains all the invalidations... and the task of the
filtering below is to collect the ones that pertain to each handler in our
handler map and then tell that handler about them?  

[rlarocque, 2013/09/23 18:38:19]

Correct.

   DCHECK(thread_checker_.CalledOnValidThread());
   // If we have no handlers, there's nothing to do.
   if (!handlers_.might_have_observers()) {
     return;
   }
 
-  typedef std::map<InvalidationHandler*, ObjectIdInvalidationMap> DispatchMap;
-  DispatchMap dispatch_map;
-  for (ObjectIdInvalidationMap::const_iterator it = invalidation_map.begin();
-       it != invalidation_map.end(); ++it) {
-    InvalidationHandler* const handler = ObjectIdToHandler(it->first);
-    // Filter out invalidations for IDs with no handler.
-    if (handler)
-      dispatch_map[handler].insert(*it);
-  }
-
-  // Emit invalidations only for handlers in |handlers_|.
-  ObserverListBase<InvalidationHandler>::Iterator it(handlers_);
-  InvalidationHandler* handler = NULL;
-  while ((handler = it.GetNext()) != NULL) {
-    DispatchMap::const_iterator dispatch_it = dispatch_map.find(handler);
-    if (dispatch_it != dispatch_map.end())
-      handler->OnIncomingInvalidation(dispatch_it->second);
+  for (HandlerIdMap::iterator it = handler_to_id_map_.begin();
+       it != handler_to_id_map_.end(); ++it) {
+    ObjectIdInvalidationMap to_emit = invalidation_map.WithObjects(it->second);
+    if (!to_emit.Empty()) {
+      it->first->OnIncomingInvalidation(to_emit);
+    }
   }
 }
 
 void InvalidatorRegistrar::UpdateInvalidatorState(InvalidatorState state) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "New invalidator state: " << InvalidatorStateToString(state_)
       << " -> " << InvalidatorStateToString(state);
   state_ = state;
   FOR_EACH_OBSERVER(InvalidationHandler, handlers_,
                     OnInvalidatorStateChange(state));
 }
 
 InvalidatorState InvalidatorRegistrar::GetInvalidatorState() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return state_;
 }
 
 bool InvalidatorRegistrar::IsHandlerRegisteredForTest(
     InvalidationHandler* handler) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return handlers_.HasObserver(handler);
 }
 
 void InvalidatorRegistrar::DetachFromThreadForTest() {
   DCHECK(thread_checker_.CalledOnValidThread());
   thread_checker_.DetachFromThread();
 }
 
-InvalidationHandler* InvalidatorRegistrar::ObjectIdToHandler(
-    const invalidation::ObjectId& id) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  IdHandlerMap::const_iterator it = id_to_handler_map_.find(id);
-  return (it == id_to_handler_map_.end()) ? NULL : it->second;
-}
-
 }  // namespace syncer