Add skeleton code to wire Local- and Remote- file sync services

Add skeleton code to wire Local- and Remote- file sync services

BUG=156599, 156041
TEST=to be added when we add implementation
TBR=ben@chromium.org


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

[kinuko, 2012-10-22 08:57:29]

A skeleton code for the proposal I sent in a comment for
http://codereview.chromium.org/11187021/. This change doesn't include any code
for RemoteFileSyncService (or RemoteFileProvider).  Could you take a look if
this works for you in your side?

Some notes which you may want to check for:
- This uses fileapi::FileChange to express a file change for inter-module
communication.
- Local service will implement RemoteChangeProcessor while Remote service will
implement LocalChangeProcessor; which might be a bit confusing
- In this model most of the Remote Sync logic is to be implemented in
RemoteFileSyncService (and vice versa for Local Sync)

[kinuko, 2012-10-22 09:21:06]

http://codereview.chromium.org/11234025/diff/2001/chrome/browser/sync_file_sy...
File chrome/browser/sync_file_system/remote_change_processor.h (right):

http://codereview.chromium.org/11234025/diff/2001/chrome/browser/sync_file_sy...
chrome/browser/sync_file_system/remote_change_processor.h:42: const
PrepareChangeCallback& callback) = 0;
The remote sync service will call this method when it is asked by
SyncFileSystemService to perform one remote sync.

* If the current remote change is DELETE and the url is in writing or we have
local changes for the url we can ignore/skip the remote change
* If the current remote change is ADD/DELETE and the url is in writing we will
likely have a conflict; may want to suspend there and return the process back to
the SyncFileSystemService (or we can possibly just record a conflict here though
if the ongoing local change was delete we'll need to remove the conflict mark
later)

Conflict status must be recorded in the remote service as well as to be
propagated to the SyncFileSystemService (via callback).
In the current design we don't need to record the file metadata at this stage.

http://codereview.chromium.org/11234025/diff/2001/chrome/browser/sync_file_sy...
chrome/browser/sync_file_system/remote_change_processor.h:55: const
StatusCallback& callback) = 0;
The remote sync service will call this method to actually apply the change to
the local side.  If the change was ADD/DELETE it needs to finish downloading the
remote file before calling this.

[tzik, 2012-10-22 12:28:01]

On 2012/10/22 08:57:29, kinuko wrote:
> A skeleton code for the proposal I sent in a comment for
> http://codereview.chromium.org/11187021/. This change doesn't include any code
> for RemoteFileSyncService (or RemoteFileProvider).  Could you take a look if
> this works for you in your side?
> 
> Some notes which you may want to check for:
> - This uses fileapi::FileChange to express a file change for inter-module
> communication.
> - Local service will implement RemoteChangeProcessor while Remote service will
> implement LocalChangeProcessor; which might be a bit confusing
> - In this model most of the Remote Sync logic is to be implemented in
> RemoteFileSyncService (and vice versa for Local Sync)

Looks great to me. I like this much better.

[tzik, 2012-10-22 12:28:19]

http://codereview.chromium.org/11234025/diff/12/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/remote_change_processor.h (right):

http://codereview.chromium.org/11234025/diff/12/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/remote_change_processor.h:39: // which is
supposed to be done by LocalChangeProcessor)
Should we invoke the callback soon, when we fail to acquire the lock? or store
the callback and invoke it after we acquire the lock?

I prefer returning fail immediately.

[kinuko, 2012-10-22 12:38:03]

Thanks!

http://codereview.chromium.org/11234025/diff/12/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/remote_change_processor.h (right):

http://codereview.chromium.org/11234025/diff/12/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/remote_change_processor.h:39: // which is
supposed to be done by LocalChangeProcessor)
On 2012/10/22 12:28:19, tzik wrote:
> Should we invoke the callback soon, when we fail to acquire the lock? or store
> the callback and invoke it after we acquire the lock?
> 
> I prefer returning fail immediately.

I'm thinking about let it fail with an error like SYNC_STATUS_ERROR_SUSPENDED
immediately with the URL (so that the caller==SyncFileSystemService can register
the URL if necessary).

[tzik, 2012-10-24 03:46:10]

lgtm

[commit-bot: I haz the power, 2012-10-24 04:47:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/11234025/12

[commit-bot: I haz the power, 2012-10-24 04:48:07]

Presubmit check for 11234025-12 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 1.1s to calculate.

[kinuko, 2012-10-24 04:56:04]

TBR-ing ben@ for simple gypi changes in chrome/. Thanks!

[commit-bot: I haz the power, 2012-10-24 04:56:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/11234025/18001

[commit-bot: I haz the power, 2012-10-24 08:18:40]

Change committed as 163787

[Ben Goodger (Google), 2012-10-24 17:34:04]

lgtm