Wire up Syncable operation runner to the local file sync context

Wire up Syncable operation runner to the local file sync context

* Adding LocalFileSyncContext::PrepareForSync method which disables writing on the given URL (the design is roughly based on the skeleton http://codereview.chromium.org/11234025/)

BUG=148897, 154234
TEST=LocalFileSyncContextTest.*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=164033

[kinuko, 2012-10-22 12:43:07]

More wiring for queueing & synchronization (this is getting complicated now)
PTL Thanks!

[kinuko, 2012-10-23 05:20:54]

Separated out the runner change from this one:
http://codereview.chromium.org/11238054/

[kinuko, 2012-10-24 03:17:12]

Rebased.  PTL.

[tzik, 2012-10-24 03:45:12]

http://codereview.chromium.org/11235027/diff/13011/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.cc (right):

http://codereview.chromium.org/11235027/diff/13011/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:80:
DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
I think this is a bit too redundant.

http://codereview.chromium.org/11235027/diff/13011/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:144:
DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
could you add TODO comment?

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.cc (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:110: url_syncable_callback_ =
on_syncable_callback;
Could you support multiple URL to wait?
Remote side might change its URL for next sync if new change arrived for the
URL. In that case, it calls RegisterURLForWaitingSync with another URL.

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.h (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.h:101: ~LocalFileSyncContext();
could you readd virtual?

[kinuko, 2012-10-24 09:39:56]

http://codereview.chromium.org/11235027/diff/13011/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.cc (right):

http://codereview.chromium.org/11235027/diff/13011/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:80:
DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
On 2012/10/24 03:45:13, tzik wrote:
> I think this is a bit too redundant.

I may prefer keeping this as we chatted offline.

http://codereview.chromium.org/11235027/diff/13011/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:144:
DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
On 2012/10/24 03:45:13, tzik wrote:
> could you add TODO comment?

I have no plan to add anything for now.. :) will add some comments.

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.cc (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:110: url_syncable_callback_ =
on_syncable_callback;
On 2012/10/24 03:45:13, tzik wrote:
> Could you support multiple URL to wait?
> Remote side might change its URL for next sync if new change arrived for the
> URL. In that case, it calls RegisterURLForWaitingSync with another URL.

Does it work if we simply replace the URL to wait for or will we really want to
wait on multiple URLs?  (I don't want to allow having too many URLs to wait on,
so if we make it std::set or so probably I'd want to have some upper limit...
and making it work like LRU?)

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.h (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.h:101: ~LocalFileSyncContext();
On 2012/10/24 03:45:13, tzik wrote:
> could you readd virtual?

Done.

[kinuko, 2012-10-24 14:07:05]

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.cc (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:110: url_syncable_callback_ =
on_syncable_callback;
For now I changed the code to simply replace the URL.  Let me hear your
comments.

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.h (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.h:101: ~LocalFileSyncContext();
(This is done by the other change)

[tzik, 2012-10-25 08:14:42]

lgtm

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
File webkit/fileapi/syncable/local_file_sync_context.cc (right):

http://codereview.chromium.org/11235027/diff/15001/webkit/fileapi/syncable/lo...
webkit/fileapi/syncable/local_file_sync_context.cc:110: url_syncable_callback_ =
on_syncable_callback;
On 2012/10/24 14:07:05, kinuko wrote:
> For now I changed the code to simply replace the URL.  Let me hear your
> comments.

Sorry for delayed comment, the replacement seems to works.

[commit-bot: I haz the power, 2012-10-25 08:22:32]

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

[commit-bot: I haz the power, 2012-10-25 09:13:00]

Retried try job too often for step(s) unit_tests