Implement sync in the NonBlockingTypeProcessor

sync/engine/non_blocking_sync_common.h

+// Copyright 2014 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_ENGINE_NON_BLOCKING_SYNC_COMMON_H_
+#define SYNC_ENGINE_NON_BLOCKING_SYNC_COMMON_H_
+
+#include <string>
+#include <vector>
+
+#include "base/time/time.h"
+#include "sync/base/sync_export.h"
+#include "sync/protocol/sync.pb.h"
+
+namespace syncer {
+
+struct SYNC_EXPORT_PRIVATE DataTypeState {
+  DataTypeState();
+  ~DataTypeState();
+
+  sync_pb::DataTypeProgressMarker progress_marker;
+  sync_pb::DataTypeContext type_context;
+  std::string parent_id;

[Nicolas Zea, 2014/05/19 23:54:15]

why do we have parent_id and next_id here? Aren't those per item, rather than
per type?

[rlarocque, 2014/05/20 01:39:47]

parent_id: Other than bookmarks, sync doesn't really support hierarchy.  I have
no intention of attempting to support hierarchy for non-blocking sync types.

If you look at the structure of the Directory today, you'll see that all items
of the same model type (except bookmarks) have the same parent.  That's the
behavior I'm looking to mimic with this code.

According to the server devs, I could leave the parent_id unset for most
"unique_client_tag" types with no ill effects.  I'm not quite sure I believe
them yet.  I intend to start by mimicing the old behavior; we can experiment
with empty parent IDs later.

next_id: This is a completely different thing.  The name is misleading, though
it does match the name it had in directory sync.  This is the counter used as a
nonce to ensure that no two items have the same client-assigned ID.

According to the server devs, this is also useless.  Once again, I'm including
it just so I can mimic the old behavior.

See
https://code.google.com/p/chromium/codesearch#chromium/src/sync/syncable/dire...

[Nicolas Zea, 2014/05/20 21:51:08]

I think it would be clearer to rename these two fields then (in addition to
adding comments). parent_id could be type_root_id, and next_id could be
next_client_id

[rlarocque, 2014/05/20 22:29:21]

Renamed and added lots of comments for this struct.

+  int64 next_id;
+};
+
+struct SYNC_EXPORT_PRIVATE CommitRequestData {
+  CommitRequestData();
+  ~CommitRequestData();
+
+  std::string id;
+  std::string client_tag;
+  int64 sequence_number;

[Nicolas Zea, 2014/05/19 23:54:15]

what is sequence number?

[rlarocque, 2014/05/20 01:39:47]

A new concept for non-blocking sync.  Feel free to suggest a better name.

With non-blocking sync, it's possible that there are many variations of a
particular item with outstanding commit requests at any given time.  These can
not be identified by the base version or the server version alone.

For example, consider an item that is modified 10 times while we're offline. 
The model thread will send 10 commit requests to the sync thread.  When it
receives a commit response, how does it know if that response refers to the 1st
or 10th request?

My solution is to use a sequence number.  Think of it as a different
implementation of the "is_unsynced" and "syncing" bits.

The sequence number is never forwarded to the sync server (yet).  It's up to the
sync thread to handle some of the mapping.  There may be some benefits to
sending it all the way to the server, particularly if crashes are frequent, but
it's probably not worth exploring those possibilities just yet.

[Nicolas Zea, 2014/05/20 21:51:08]

The name is fine, I mainly want comments

[rlarocque, 2014/05/20 22:29:21]

Added a comment.  It points readers to look for the places where the value is
used, since that's going to be the most up-to-date source of information.  I
don't trust maintainers (including myself) to update the comments here if the
behavior in model_thread_sync_entity.cc changes.

+  int64 base_version;
+  base::Time ctime;
+  base::Time mtime;
+  bool deleted;
+  sync_pb::EntitySpecifics specifics;

[Nicolas Zea, 2014/05/19 23:54:15]

what about non unique name?

[rlarocque, 2014/05/20 01:39:47]

That's a bug.

I must have had the ProcessorCore extract it from the client_tag.  Now that the
client_tag is no longer available, I've added an explicit non_unique_name field.
 Which is the way it should have been implemented in the first place.

Fixed.

+};
+
+struct SYNC_EXPORT_PRIVATE CommitResponseData {
+  CommitResponseData();
+  ~CommitResponseData();
+
+  std::string id;
+  std::string client_tag_hash;
+  int64 sequence_number;
+  int64 response_version;
+};
+
+struct SYNC_EXPORT_PRIVATE UpdateResponseData {
+  UpdateResponseData();
+  ~UpdateResponseData();
+
+  std::string id;
+  std::string client_tag_hash;
+  int64 response_version;

[Nicolas Zea, 2014/05/19 23:54:15]

server_version?

[rlarocque, 2014/05/20 01:39:47]

I'm a bit reluctant to use the term 'server_version', since it could be out of
date by the time this field is read.

+  base::Time ctime;
+  base::Time mtime;
+  std::string non_unique_name;
+  bool deleted;
+  sync_pb::EntitySpecifics specifics;
+};
+
+typedef std::vector<CommitRequestData> CommitRequestDataList;
+typedef std::vector<CommitResponseData> CommitResponseDataList;
+typedef std::vector<UpdateResponseData> UpdateResponseDataList;
+
+}  // namespace syncer
+
+#endif  // SYNC_ENGINE_NON_BLOCKING_SYNC_COMMON_H_