Use MetadataChangeList and EntityChangeList in SharedModelTypeProcessor

sync/api/metadata_change_list.h

 // Copyright 2015 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.
 
 #ifndef SYNC_API_METADATA_CHANGE_LIST_H_
 #define SYNC_API_METADATA_CHANGE_LIST_H_
 
+#include <string>
+
 #include "sync/base/sync_export.h"
 
+namespace sync_pb {
+class EntityMetadata;
+}  // namespace sync_pb
+
 namespace syncer_v2 {
+struct DataTypeState;
 
 // Interface used by the processor and service to communicate about metadata.
 class SYNC_EXPORT MetadataChangeList {
  public:
   MetadataChangeList() {}
   virtual ~MetadataChangeList() {}
+
+  // Requests DataTypeState to be updated in the storage.

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

Should we be able to clear the DTS as well?

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

I guess clearing DTS is equivalent to replacing it with a default one.

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

I guess my problem here is that this interface is inconsistent. It seems wrong
to me that to clear the DTS you set it with default values. And to clear the
entity-metadata you call ClearMetadata.

I lean towards adding an explicit ClearDataTypeState. I would hate to be a data
type owner and check the passed in DTS during UpdateDataTypeState to see if all
values are defaulted and then if so delete some resource.

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

OK, I've added ClearDataTypeState.

+  virtual void UpdateDataTypeState(const DataTypeState& data_type_state);
+
+  // Requests metadata entry to be updated in the storage.
+  // Please note that the update might contain a deleted entry if
+  // metadata.is_deleted() is true (as opposed to clearing the entry from the
+  // storage completely by calling the Clear method).
+  // Please note that update requests for the same entry are coalesced.

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

What does 'entry' mean? What does 'coalesced' mean?

Also remember that these comments are being written with the intention that a
service owner will write an implementation for them. They should be phrased as
such.

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

What I tried to say that multiple updates with the same key would overwrite each
other. But I guess that ended up being confusing. I'll go ahead and just remove
this sentence.

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

I lean towards being explicit about how the implementer should behave with
repeated keys. It's fine if we don't want to require them to handle this case in
a specific way, but lets say that.

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

Done.

+  virtual void UpdateMetadata(const std::string& client_tag,
+                              const sync_pb::EntityMetadata& metadata) = 0;
+
+  // Requests metadata entry to be cleared from the storage.
+  virtual void ClearMetadata(const std::string& client_tag) = 0;
 };
 
 }  // namespace syncer_v2
 
 #endif  // SYNC_API_METADATA_CHANGE_LIST_H_