drive: Support offline delete

drive: Support offline delete

Remove API related code from RemoveOperation, they are handled by RemovePerformer.
Add OperationObserver::OnEntryRemovedByOperation to kick SyncClient from RemoveOperation.
Add SyncClient::AddRemoveTask to kick RemovePerformer.
Modify SyncClient::StartProcessingBacklog to kick RemovePerformer on start up.

BUG=260542
TEST=unit_tests

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

[hashimoto, 2013-11-12 01:21:38]

This change depends on https://codereview.chromium.org/66293005/ ("Add
drive/trash")

[hashimoto, 2013-11-12 02:11:10]

Sorry, I forgot to update the change description.
Updated.

[hashimoto, 2013-11-12 05:53:19]

Changed the approach to move deleted entries to drive/other and
set_deleted(true), instead of moving them to drive/trash.

Changes introduced in Patch Set 3:
- Add ResourceMetadata::ReadDirectoryById to mark entries as deleted recursively
from RemoveOperation.
- ChangeListProcessor ignores changes if an entry is locallly deleted.
- SearchMetadata ignores deleted entries.

This change no longer depends on https://codereview.chromium.org/66293005/ ("Add
drive/trash")

[kinaba, 2013-11-12 06:21:28]

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/change_list_processor.cc (right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/change_list_processor.cc:325:
existing_entry.deleted()) {
Is it correct that a file inside a deleted directory (= .deleted() but parent !=
<other>) is refreshed? If it is refreshed, the 'deleted' flag is cleared and
hence it will appear in the SearchMetadata results.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/file_system/remove_operation.h (right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/file_system/remove_operation.h:33: class
RemoveOperation {
Please keep some brief class comment.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/file_system/remove_operation_unittest.cc
(right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/file_system/remove_operation_unittest.cc:128: }
Could you add a check for the recursive deleted marking is working as expected?

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/sync_client.cc (right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client.cc:63: to_remove->push_back(local_id);
Probably we don't want to, say, fetch a pinned and removed file. It might be
better to continue to the next entry at this point.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client.cc:391: AddRemoveTask(local_id);
AddTaskToQueue posts the task after a long_delay_, while AddRemoveTask retries
immediately. Is it fine?

[hashimoto, 2013-11-25 10:13:57]

New patch set.
Stopped deleting local entry after RemovePerformer finishes deletion, to avoid
conflicting with server side changes. (e.g. A child of locally deleted entry
moves to another directory)

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/change_list_processor.cc (right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/change_list_processor.cc:325:
existing_entry.deleted()) {
On 2013/11/12 06:21:29, kinaba wrote:
> Is it correct that a file inside a deleted directory (= .deleted() but parent
!=
> <other>) is refreshed? If it is refreshed, the 'deleted' flag is cleared and
> hence it will appear in the SearchMetadata results.

Recursively marking entries as deleted might be bug-prone.
Switched back to use drive/trash.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/file_system/remove_operation.h (right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/file_system/remove_operation.h:33: class
RemoveOperation {
On 2013/11/12 06:21:29, kinaba wrote:
> Please keep some brief class comment.

Done.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/file_system/remove_operation_unittest.cc
(right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/file_system/remove_operation_unittest.cc:128: }
On 2013/11/12 06:21:29, kinaba wrote:
> Could you add a check for the recursive deleted marking is working as
expected?

Stopped marking entries as deleted.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/sync_client.cc (right):

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client.cc:63: to_remove->push_back(local_id);
On 2013/11/12 06:21:29, kinaba wrote:
> Probably we don't want to, say, fetch a pinned and removed file. It might be
> better to continue to the next entry at this point.

Done.

https://codereview.chromium.org/68543002/diff/250001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client.cc:391: AddRemoveTask(local_id);
On 2013/11/12 06:21:29, kinaba wrote:
> AddTaskToQueue posts the task after a long_delay_, while AddRemoveTask retries
> immediately. Is it fine?

Done.

[kinaba, 2013-11-26 06:49:23]

lgtm. let's see how it goes.

[commit-bot: I haz the power, 2013-11-26 07:12:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/68543002/640001

[commit-bot: I haz the power, 2013-11-26 07:59:52]

Retried try job too often on mac_rel for step(s) telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-11-26 08:02:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/68543002/640001

[commit-bot: I haz the power, 2013-11-26 10:27:54]

Change committed as 237296