Use MetadataChangeList and EntityChangeList in SharedModelTypeProcessor

sync/api/entity_change.h

Index: sync/api/entity_change.h
diff --git a/sync/api/entity_change.h b/sync/api/entity_change.h
index 88e3812353ed2f487a9ddf67e28988330c98f199..866d38ca2059c798d76cd2abb31b69ca52e97ad1 100644
--- a/sync/api/entity_change.h
+++ b/sync/api/entity_change.h
@@ -34,9 +34,9 @@ class SYNC_EXPORT EntityChange {
  private:
   EntityChange(std::string client_key, ChangeType type, EntityDataPtr data);
 
-  const std::string client_key_;
-  const ChangeType type_;
-  const EntityDataPtr data_;
+  std::string client_key_;

[skym, 2016/01/07 00:40:48]

I don't quite follow why these need to be mutable based off the other changes in
this CL.

[stanisc, 2016/01/11 19:56:47]

The compiler complains that copy constructor is deleted when these fields are
consts. The copy constructor is used when inserting EntityChange into
EntityChangeList.

[skym, 2016/01/11 20:45:40]

That makes me so sad. It should be completely possible for a copy constructor to
set all these fields correctly. Maybe we should define it? The assignment
operator on the other hand, won't work here regardless, right? And I think
std::vector::push_back requires the type the be assignable.

If we made this type movable, I think it might actually solve this problem. I
think Chromium style has recently been updated to allow explicit move
constructors http://chromium-cpp.appspot.com/ . Also if this works, you could
throw in DISALLOW_COPY_AND_ASSIGN as well!

There's also the emplace_back option (although not currently allowed, presumably
this is going to change in the near future). However, since we're using a static
method and private constructor, we'd have to write a custom allocator, yuck.

[stanisc, 2016/01/12 19:32:11]

I think these fields have to be assignable because when an element is inserted
into a vector the vector is first expanded to a larger size, which involves
construction of new elements at the end, then the new element is copied over. So
essentially existing elements are assigned new values. I am not sure if we need
to further complicate that now.

[skym, 2016/01/12 19:38:54]

You're right, in order for EntityChange to be assignable, these fields must be
assignable. But one of the points I was trying to make was I think EntityChange
only needs to be either assignable or movable to work with std::vector. You make
a good point though, no need to deal with this now.

+  ChangeType type_;
+  EntityDataPtr data_;
 };
 
 typedef std::vector<EntityChange> EntityChangeList;