sync: Remove ModelTypeInvalidationMap

sync/internal_api/sync_manager_impl.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/internal_api/sync_manager_impl.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/json/json_writer.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram.h"
 #include "base/observer_list.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "sync/engine/sync_scheduler.h"
 #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"
 #include "sync/internal_api/public/http_post_provider_factory.h"
 #include "sync/internal_api/public/internal_components_factory.h"
 #include "sync/internal_api/public/read_node.h"
 #include "sync/internal_api/public/read_transaction.h"
 #include "sync/internal_api/public/user_share.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/write_node.h"
@@ skipping 1111 matching lines @@
                                                   id, &child_handles);
     for (syncable::Directory::Metahandles::const_iterator it =
              child_handles.begin(); it != child_handles.end(); ++it) {
       child_ids->Append(new base::StringValue(base::Int64ToString(*it)));
     }
   }
   return JsArgList(&return_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;
+    }
   }
 }
 
 void SyncManagerImpl::OnInvalidatorStateChange(InvalidatorState state) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   const std::string& state_str = InvalidatorStateToString(state);
   invalidator_state_ = state;
   DVLOG(1) << "Invalidator state changed to: " << state_str;
   const bool notifications_enabled =
       (invalidator_state_ == INVALIDATIONS_ENABLED);
   allstatus_.SetNotificationsEnabled(notifications_enabled);
   scheduler_->SetNotificationsEnabled(notifications_enabled);
 
   if (js_event_handler_.IsInitialized()) {
     base::DictionaryValue details;
     details.SetString("state", state_str);
     js_event_handler_.Call(FROM_HERE,
                            &JsEventHandler::HandleJsEvent,
                            "onNotificationStateChange",
                            JsEventDetails(&details));
   }
 }
 
 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");
     js_event_handler_.Call(FROM_HERE,
                            &JsEventHandler::HandleJsEvent,
                            "onIncomingNotification",
                            JsEventDetails(&details));
   }
 }
 
@@ skipping 99 matching lines @@
 int SyncManagerImpl::GetDefaultNudgeDelay() {
   return kDefaultNudgeDelayMilliseconds;
 }
 
 // static.
 int SyncManagerImpl::GetPreferencesNudgeDelay() {
   return kPreferencesNudgeDelayMilliseconds;
 }
 
 }  // namespace syncer