sync: Remove ModelTypeInvalidationMap

sync/internal_api/sync_manager_impl.cc

Index: sync/internal_api/sync_manager_impl.cc
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index a541032691f0f7aa11aaa420185cc585d1a5b213..c289035638069b9f80a86c2ad69e1953715b455a 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -20,7 +20,6 @@
 #include "sync/engine/syncer_types.h"
 #include "sync/internal_api/change_reorder_buffer.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/internal_api/public/base/model_type_invalidation_map.h"
 #include "sync/internal_api/public/base_node.h"
 #include "sync/internal_api/public/configure_reason.h"
 #include "sync/internal_api/public/engine/polling_constants.h"
@@ -1152,12 +1151,15 @@ JsArgList SyncManagerImpl::GetChildNodeIds(const JsArgList& args) {
 }
 
 void SyncManagerImpl::UpdateNotificationInfo(
-    const ModelTypeInvalidationMap& invalidation_map) {
-  for (ModelTypeInvalidationMap::const_iterator it = invalidation_map.begin();
+    const ObjectIdInvalidationMap& invalidation_map) {
+  for (ObjectIdInvalidationMap::const_iterator it = invalidation_map.begin();
        it != invalidation_map.end(); ++it) {
-    NotificationInfo* info = &notification_info_map_[it->first];
-    info->total_count++;
-    info->payload = it->second.payload;
+    ModelType type;

[tim (not reviewing), 2013/08/28 00:13:00]

Initialize to UNSPECIFIED.

[rlarocque, 2013/08/28 01:00:35]

Done.

+    if (ObjectIdToRealModelType(it->first, &type)) {
+      NotificationInfo* info = &notification_info_map_[type];
+      info->total_count++;
+      info->payload = it->second.payload;
+    }
   }
 }
 
@@ -1186,29 +1188,27 @@ void SyncManagerImpl::OnIncomingInvalidation(
     const ObjectIdInvalidationMap& invalidation_map) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  const ModelTypeInvalidationMap& type_invalidation_map =
-      ObjectIdInvalidationMapToModelTypeInvalidationMap(invalidation_map);
-  if (type_invalidation_map.empty()) {
+  if (invalidation_map.empty()) {
     LOG(WARNING) << "Sync received invalidation without any type information.";

[tim (not reviewing), 2013/08/28 00:13:00]

Do we expect to be invoked here with non-sync Object IDs?

[rlarocque, 2013/08/28 01:00:35]

No, that should never happen.

[tim (not reviewing), 2013/08/28 01:11:07]

I ask just because before we were checking for that condition, whereas now it's
"did we get anything at all".  Maybe add a DCHECK(!nudged_types.empty()) below?

[rlarocque, 2013/08/28 17:50:22]

Done.

While we're on the subject, I also can't think of any scenario where
OnInvalidationReceived would be sent an empty invalidation map.  I'll try to
convert more of these conditions to DCHECKs in a future CL.  For now, though,
I'd prefer to be conservative.  This code needs to be compatible with the
Android version of the invalidator, which I don't understand very well.

(In case you're wondering, I think adding the DCHECK(!nudged_types.empty()) is
safe because I'm pretty sure the Android invalidator is incapable of sending
invalidations with object IDs that are not convertible to model types.)

   } else {
     allstatus_.IncrementNudgeCounter(NUDGE_SOURCE_NOTIFICATION);
     scheduler_->ScheduleInvalidationNudge(
         TimeDelta::FromMilliseconds(kSyncSchedulerDelayMsec),
-        type_invalidation_map, FROM_HERE);
+        invalidation_map, FROM_HERE);
     allstatus_.IncrementNotificationsReceived();
-    UpdateNotificationInfo(type_invalidation_map);
-    debug_info_event_listener_.OnIncomingNotification(type_invalidation_map);
+    UpdateNotificationInfo(invalidation_map);
+    debug_info_event_listener_.OnIncomingNotification(invalidation_map);
   }
 
   if (js_event_handler_.IsInitialized()) {
     base::DictionaryValue details;
     base::ListValue* changed_types = new base::ListValue();
     details.Set("changedTypes", changed_types);
-    for (ModelTypeInvalidationMap::const_iterator it =
-             type_invalidation_map.begin(); it != type_invalidation_map.end();
-         ++it) {
-      const std::string& model_type_str =
-          ModelTypeToString(it->first);
+    ObjectIdSet id_set = ObjectIdInvalidationMapToSet(invalidation_map);
+    ModelTypeSet nudged_types = ObjectIdSetToModelTypeSet(id_set);
+    for (ModelTypeSet::Iterator it = nudged_types.First();
+         it.Good(); it.Inc()) {
+      const std::string& model_type_str = ModelTypeToString(it.Get());

[tim (not reviewing), 2013/08/28 00:13:00]

nit - I tend to prefer explicitly forcing the copy (i.e const std::string vs
const std::string&) in cases like this vs relying on the
assign-result-to-const-ref-surprisingly-works-and-might-not-require-copy,
because it looks less weird.  But not a big deal.

[rlarocque, 2013/08/28 01:00:35]

Fixed.  Either way is OK with me.

       changed_types->Append(new base::StringValue(model_type_str));
     }
     details.SetString("source", "REMOTE_INVALIDATION");