Keep sync tasks alive as long as it's not finished

Keep sync tasks alive as long as it's not finished

After this change, sync tasks are erased only when it completes or SyncClient gives up.
This change is preparation for implementing a mechanism to allow CopyOperation to wait for a specific sync task to complete.

In EntryUpdatePerformer:
Add a new error code FILE_ERROR_PARENT_NEEDS_TO_BE_SYNCED.
Return the new error code when the sync task cannot be completed because of the lack of the parent resource ID.

In SyncClient:
Add a new task state SUSPENDED to allow tasks to be in an inactive state.
Add a new member |context| to SyncTask to make it possible to change the context without erasing the task.
OnTaskComplete() takes almost all responsibilities to handle errors. OnFetchFileComplete() and OnUpdateCompleter() are now almost empty or removed.
Handle the new error code FILE_ERROR_PARENT_NEEDS_TO_BE_SYNCED in OnTaskComplete().

BUG=384213
TEST=unit_tests

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

[hashimoto, 2014-07-16 11:05:16]

https://codereview.chromium.org/391343002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/sync_client_unittest.cc (right):

https://codereview.chromium.org/391343002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client_unittest.cc:480:
sync_client_->AddUpdateTask(ClientContext(USER_INITIATED), local_id2);
In the new code, RunUntilIdle() is not called here because AddDependentTask()
requires the parent's task to be present.

In the production environment, AddUpdateTask should be called in parent-to-child
order.
We are reversing the order here just for convenience.

[kinaba, 2014-07-17 06:07:48]

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

https://codereview.chromium.org/391343002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client.cc:453: if (it_parent == tasks_.end())
{
Can't there be a danger that the parent task has completed during the blocking
GetResourceEntryById operation? Is it still ok to fail in such a case?

[hashimoto, 2014-07-18 06:12:17]

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

https://codereview.chromium.org/391343002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/sync_client.cc:453: if (it_parent == tasks_.end())
{
On 2014/07/17 06:07:46, kinaba wrote:
> Can't there be a danger that the parent task has completed during the blocking
> GetResourceEntryById operation? Is it still ok to fail in such a case?

Extremely good catch.
Changed EntryUpdatePerformer to return the parent's local ID when necessary.

The new patch set fixes the addressed race problem while keeping the following
two contracts:
1. No AddTask() call should not result in a chain of an unlimited number of
tasks.
2. No task should stay in the SUSPENDED state forever.

[hashimoto, 2014-07-18 08:16:59]

A new patch set which performs the parent resource ID check in SyncClient.
PTAL

[kinaba, 2014-07-18 08:25:20]

lgtm. thanks!

[hashimoto, 2014-07-18 08:34:00]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2014-07-18 08:34:53]

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

[commit-bot: I haz the power, 2014-07-18 10:43:18]

Change committed as 284051