Initial commit of sync engine code to browser/sync....

Initial commit of sync engine code to browser/sync.

The code is not built on any platform yet.  That will arrive
as a subsequent checkin.

This is an implementation of the interface exposed earlier
through syncapi.h.  It is the client side of a sync
protocol that lets users sync their browser data
(currently, just bookmarks) with their Google Account.

Table of contents:

browser/sync/
  protocol - The protocol definition, and
             other definitions necessary to connect to
             the service.
  syncable/ - defines a data model for syncable objects,
              and provides a sqlite-based backing store
              for this model.
  engine/ - includes the core sync logic, including commiting
            changes to the server, downloading changes from
            the server, resolving conflicts, other parts of
            the sync algorithm.
  engine/net - parts of the sync engine focused on the
               business of talking to the server.  Some of
               this is binds a generic "server connection"
               interface to a concrete implementation
               provided by Chromium.
  notifier - the part of the syncer focused on the business
             of sending and receiving xmpp notifications.
             Notifications are used instead of polling to
             achieve very low latency change propagation.
  util - not necessarily sync specific utility code.  Much
         of this is scaffolding which should either be
         replaced by, or merged with, the utility code
         in base/.

BUG=none
TEST=this code includes its own suite of unit tests.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25850

[ncarter (slow), 2009-09-10 01:52:30]

Idan,

Please review.

Nick

[ncarter (slow), 2009-09-10 02:27:35]

New patch set uploaded.  

I changed the gtest.h and gtest_prod.h include paths to appease check_deps.py. 
Also, I added files to test/

Please diff patch set 3 vs. patch set 1 -- these are the differences versus our
internal development branch.

[idana, 2009-09-10 05:44:36]

LGTM++

Looks great.

I have numerous nit comment. None of my comment are particularly concerning so
you can go ahead and land this while I fix these nit issues tomorrow if you
want.

http://codereview.chromium.org/194065/diff/5154/2134
File chrome/browser/sync/engine/apply_updates_command_unittest.cc (right):

http://codereview.chromium.org/194065/diff/5154/2134#newcode164
Line 164: 
nit: please remove extra blank line.

http://codereview.chromium.org/194065/diff/5154/3121
File chrome/browser/sync/engine/authenticator.h (right):

http://codereview.chromium.org/194065/diff/5154/3121#newcode90
Line 90: const char * display_name() const { return display_name_.c_str(); }
nit: "const char *" -> "const char*" in above two lines.

http://codereview.chromium.org/194065/diff/5154/9102
File chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
(right):

http://codereview.chromium.org/194065/diff/5154/9102#newcode29
Line 29: SyncerSession *session) {
nit: "SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/9102#newcode34
Line 34: SyncerSession *session) {
nit: "SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/9102#newcode180
Line 180: // appear in order.
Can you please change this comment paragraph so that it resides above
rollback_ids?

http://codereview.chromium.org/194065/diff/5154/9102#newcode183
Line 183: // to |rollback_ids|.
Same as above.

http://codereview.chromium.org/194065/diff/5154/5176
File chrome/browser/sync/engine/build_commit_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/5176#newcode33
Line 33: void BuildCommitCommand::ExecuteImpl(SyncerSession *session) {
nit: "SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/5176#newcode67
Line 67: // deleted items with negative parent ids can be a problem so we set
the
"deleted" -> "Deleted"

http://codereview.chromium.org/194065/diff/5154/5176#newcode68
Line 68: // parent to 0. (TODO(sync): Still true in protocol?
Missing ")." at the end of this sentence.

http://codereview.chromium.org/194065/diff/5154/5168
File chrome/browser/sync/engine/build_commit_command.h (right):

http://codereview.chromium.org/194065/diff/5154/5168#newcode19
Line 19: virtual void ExecuteImpl(SyncerSession *session);
nit: "SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/9105
File chrome/browser/sync/engine/client_command_channel.h (right):

http://codereview.chromium.org/194065/diff/5154/9105#newcode21
Line 21: static inline bool IsChannelShutdownEvent(const EventType &event) {
nit: "const EventType &event" -> "const EventType& event"

http://codereview.chromium.org/194065/diff/5154/8102
File chrome/browser/sync/engine/conflict_resolver.cc (right):

http://codereview.chromium.org/194065/diff/5154/8102#newcode134
Line 134: MutableEntry * entry) {
nit: "MutableEntry * entry" -> "MutableEntry* entry"

http://codereview.chromium.org/194065/diff/5154/8102#newcode620
Line 620: 
Please remove extra blank line.

http://codereview.chromium.org/194065/diff/5154/8102#newcode655
Line 655: SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/8102#newcode696
Line 696: SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/1113
File chrome/browser/sync/engine/conflict_resolver.h (right):

http://codereview.chromium.org/194065/diff/5154/1113#newcode42
Line 42: SyncerSession *session);
nit: "SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/1120
File chrome/browser/sync/engine/download_updates_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/1120#newcode25
Line 25: void DownloadUpdatesCommand::ExecuteImpl(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/2132
File chrome/browser/sync/engine/download_updates_command.h (right):

http://codereview.chromium.org/194065/diff/5154/2132#newcode19
Line 19: virtual void ExecuteImpl(SyncerSession *session);
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/1129
File chrome/browser/sync/engine/get_commit_ids_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/1129#newcode12
Line 12: #include "chrome/browser/sync/engine/syncer_session.h"
syncer_session.h should come before syncer_util.h

http://codereview.chromium.org/194065/diff/5154/1129#newcode26
Line 26: void GetCommitIdsCommand::ExecuteImpl(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/1129#newcode31
Line 31: &all_unsynced_handles);
Indentation

http://codereview.chromium.org/194065/diff/5154/1129#newcode143
Line 143: void GetCommitIdsCommand::AddDeletes(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/1129#newcode225
Line 225: void GetCommitIdsCommand::BuildCommitIds(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/7093
File chrome/browser/sync/engine/get_commit_ids_command.h (right):

http://codereview.chromium.org/194065/diff/5154/7093#newcode13
Line 13: #include "chrome/browser/sync/engine/syncer_session.h"
syncer_session.h should come before syncer_util.h

http://codereview.chromium.org/194065/diff/5154/7093#newcode28
Line 28: virtual void ExecuteImpl(SyncerSession *session);
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/2151
File chrome/browser/sync/engine/model_changing_syncer_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/2151#newcode13
Line 13: void ModelChangingSyncerCommand::ExecuteImpl(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/8111
File chrome/browser/sync/engine/net/server_connection_manager.cc (right):

http://codereview.chromium.org/194065/diff/5154/8111#newcode92
Line 92: string ServerConnectionManager::Post::MakeConnectionURL(
Indentation

http://codereview.chromium.org/194065/diff/5154/8104
File chrome/browser/sync/engine/post_commit_message_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/8104#newcode22
Line 22: void PostCommitMessageCommand::ExecuteImpl(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/1115
File chrome/browser/sync/engine/process_commit_response_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/1115#newcode11
Line 11: #include "chrome/browser/sync/engine/syncer_util.h"
syncer_util.h should be moved after syncer_status.h

http://codereview.chromium.org/194065/diff/5154/3122
File chrome/browser/sync/engine/process_updates_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/3122#newcode11
Line 11: #include "chrome/browser/sync/engine/syncer_util.h"
Move sync_util.h after syncer_session.h

http://codereview.chromium.org/194065/diff/5154/7087
File chrome/browser/sync/engine/resolve_conflicts_command.h (right):

http://codereview.chromium.org/194065/diff/5154/7087#newcode7
Line 7: 
Extra blank line.

http://codereview.chromium.org/194065/diff/5154/4187
File chrome/browser/sync/engine/sync_cycle_state.h (right):

http://codereview.chromium.org/194065/diff/5154/4187#newcode216
Line 216: 
Extra blank line.

http://codereview.chromium.org/194065/diff/5154/2142
File chrome/browser/sync/engine/sync_process_state.h (right):

http://codereview.chromium.org/194065/diff/5154/2142#newcode55
Line 55: const {
'const' should be moved to the previous line.

http://codereview.chromium.org/194065/diff/5154/2142#newcode319
Line 319: ServerConnectionManager *connection_manager_;
"ServerConnectionManager *connection_manager_;" -> "ServerConnectionManager*
connection_manager_;"

http://codereview.chromium.org/194065/diff/5154/2148
File chrome/browser/sync/engine/syncer.cc (right):

http://codereview.chromium.org/194065/diff/5154/2148#newcode52
Line 52: const PathString &account_name,
"const PathString &account_name," -> "const PathString& account_name,"

http://codereview.chromium.org/194065/diff/5154/2148#newcode91
Line 91: bool Syncer::SyncShare(SyncProcessState *process_state) {
"SyncProcessState *process_state" -> "SyncProcessState* process_state"

http://codereview.chromium.org/194065/diff/5154/2148#newcode110
Line 110: void Syncer::SyncShare(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSession* session" (appears in other places in
this file)

http://codereview.chromium.org/194065/diff/5154/6111
File chrome/browser/sync/engine/syncer_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/6111#newcode19
Line 19: void SyncerCommand::Execute(SyncerSession *session) {
"SyncerSession *session" -> "SyncerSessio* session" (appears more than once)

http://codereview.chromium.org/194065/diff/5154/1118
File chrome/browser/sync/engine/syncer_command.h (right):

http://codereview.chromium.org/194065/diff/5154/1118#newcode33
Line 33: void Execute(SyncerSession *session);
"SyncerSession *session" -> "SyncerSession* session""

http://codereview.chromium.org/194065/diff/5154/4196
File chrome/browser/sync/engine/syncer_end_command.h (right):

http://codereview.chromium.org/194065/diff/5154/4196#newcode29
Line 29: };
Add extra blank line after "};"

http://codereview.chromium.org/194065/diff/5154/4191
File chrome/browser/sync/engine/syncer_proto_util.cc (right):

http://codereview.chromium.org/194065/diff/5154/4191#newcode99
Line 99: SyncerSession *session) {
Same old "SyncerSession *session"...

http://codereview.chromium.org/194065/diff/5154/4191#newcode274
Line 274: 
Remove extra blank line.

http://codereview.chromium.org/194065/diff/5154/3127
File chrome/browser/sync/engine/syncer_proto_util.h (right):

http://codereview.chromium.org/194065/diff/5154/3127#newcode38
Line 38: sync_pb::ClientToServerResponse* response, SyncerSession *session);
"SyncerSession *session" -> "SyncerSession* session"

http://codereview.chromium.org/194065/diff/5154/3127#newcode70
Line 70: };
Add extra blank line.

http://codereview.chromium.org/194065/diff/5154/6113
File chrome/browser/sync/engine/syncer_status.cc (right):

http://codereview.chromium.org/194065/diff/5154/6113#newcode8
Line 8: namespace browser_sync {
Add blank line.

http://codereview.chromium.org/194065/diff/5154/9109
File chrome/browser/sync/engine/syncer_status.h (right):

http://codereview.chromium.org/194065/diff/5154/9109#newcode248
Line 248: private:
Blank lines should be added/removed in the 6 lines below.

http://codereview.chromium.org/194065/diff/5154/76
File chrome/browser/sync/engine/syncer_unittest.cc (right):

http://codereview.chromium.org/194065/diff/5154/76#newcode24
Line 24: #include "chrome/browser/sync/engine/syncer_util.h"
syncer_util.h should be moved after syncer_session.h

http://codereview.chromium.org/194065/diff/5154/76#newcode92
Line 92: 
Extra blank line.

http://codereview.chromium.org/194065/diff/5154/76#newcode4587
Line 4587: SyncerTest::CommitOrderingTest::LAST_COMMIT_ITEM = {-1,
TestIdFactory::root()};
Add a blank line.

http://codereview.chromium.org/194065/diff/5154/1114
File chrome/browser/sync/engine/syncer_util.h (right):

http://codereview.chromium.org/194065/diff/5154/1114#newcode26
Line 26: 
Extra blank line.

http://codereview.chromium.org/194065/diff/5154/7100
File chrome/browser/sync/engine/verify_updates_command.cc (right):

http://codereview.chromium.org/194065/diff/5154/7100#newcode9
Line 9: #include "chrome/browser/sync/engine/syncer_util.h"
syncer_util.h is misplaced

http://codereview.chromium.org/194065/diff/5154/2154
File chrome/browser/sync/engine/verify_updates_command.h (right):

http://codereview.chromium.org/194065/diff/5154/2154#newcode33
Line 33: };
Add blank line.

http://codereview.chromium.org/194065/diff/5154/1105
File chrome/browser/sync/notifier/base/async_dns_lookup.cc (right):

http://codereview.chromium.org/194065/diff/5154/1105#newcode132
Line 132: }
Add blank line.

http://codereview.chromium.org/194065/diff/5154/2124
File chrome/browser/sync/notifier/base/async_dns_lookup.h (right):

http://codereview.chromium.org/194065/diff/5154/2124#newcode47
Line 47: };
Add two blank missing blank lines below.

http://codereview.chromium.org/194065/diff/5154/6102
File chrome/browser/sync/notifier/base/async_network_alive.h (right):

http://codereview.chromium.org/194065/diff/5154/6102#newcode50
Line 50: };
Missing blank lines below.

http://codereview.chromium.org/194065/diff/5154/8091
File chrome/browser/sync/notifier/base/network_status_detector_task.h (right):

http://codereview.chromium.org/194065/diff/5154/8091#newcode53
Line 53: };
Blank lines.

http://codereview.chromium.org/194065/diff/5154/6098
File chrome/browser/sync/notifier/base/task_pump.cc (right):

http://codereview.chromium.org/194065/diff/5154/6098#newcode41
Line 41: }
Missing blank line.

http://codereview.chromium.org/194065/diff/5154/8077
File chrome/browser/sync/notifier/communicator/auto_reconnect.h (right):

http://codereview.chromium.org/194065/diff/5154/8077#newcode70
Line 70: }  // namespace notifier
Add extra blank space.

[ncarter (slow), 2009-09-10 06:12:39]

I committed the change without addressing the nits, since it seems safer to fix
these issues up once we have a way to build this code.