[fsp] Simplify aborting logic.

[fsp] Simplify aborting logic.

Convert the aborting callback to a simple closure, as we don't need to care
about the result code, nor about the completion. Note, that an error callback
with FILE_ERROR_ABORTED will be called for operations which are aborted.

TEST=unit_tests, browser_tests: *FileSystemProvider*
BUG=448298
Committed: https://crrev.com/ccc39f293a78dcd09cc05800823b90a74fe19c1b
Cr-Commit-Position: refs/heads/master@{#311449}

[mtomasz, 2015-01-13 11:15:10]

mtomasz@chromium.org changed reviewers:
+ hirono@chromium.org

[mtomasz, 2015-01-13 11:15:21]

@hirono: PTAL. Thanks!

[hirono, 2015-01-14 05:36:06]

Thanks!

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc
(right):

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc:258:
base::ThreadTaskRunnerHandle::Get()->PostTask(
Could you explain why it does not return net::ERR_UNEXPECTED directly?

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/queue.cc (right):

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/queue.cc:89: executed_.erase(it);
Aborted task should not be removed from the queue.

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/queue.cc:99: pending_.erase(it);
ditto

[mtomasz, 2015-01-14 08:55:10]

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc
(right):

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc:258:
base::ThreadTaskRunnerHandle::Get()->PostTask(
On 2015/01/14 05:36:06, hirono wrote:
> Could you explain why it does not return net::ERR_UNEXPECTED directly?

Not a strong reason, but for simplicity both FileStreamWriter and
FileStreamReader return all values asynchronously. Additionally,
fsp::BufferingFileStreamReader has an assumption that FileStreamReader never
completes synchronously. For BufferingFileStreamWriter there is no such
assumption.

I changed to return UNEXPECTED synchronously. Done.

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/queue.cc (right):

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/queue.cc:89: executed_.erase(it);
On 2015/01/14 05:36:06, hirono wrote:
> Aborted task should not be removed from the queue.

Good catch. Fixed + upgraded tests. Done.

https://codereview.chromium.org/845083005/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/queue.cc:99: pending_.erase(it);
On 2015/01/14 05:36:06, hirono wrote:
> ditto

Done.

[hirono, 2015-01-14 09:27:04]

lgtm with a nit. Thanks!

https://codereview.chromium.org/845083005/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/queue.cc (right):

https://codereview.chromium.org/845083005/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/queue.cc:68: // If not completed,
then it must be aborted.
nit: it must be aborted -> It sounds the task is aborted here. Maybe the task
must have been aborted, or the task must be in the aborted set?

[mtomasz, 2015-01-14 09:31:13]

https://codereview.chromium.org/845083005/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/queue.cc (right):

https://codereview.chromium.org/845083005/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/queue.cc:68: // If not completed,
then it must be aborted.
On 2015/01/14 09:27:04, hirono wrote:
> nit: it must be aborted -> It sounds the task is aborted here. Maybe the task
> must have been aborted, or the task must be in the aborted set?

Good point. Done.

[mtomasz, 2015-01-14 09:31:25]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2015-01-14 09:32:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/845083005/60001

[commit-bot: I haz the power, 2015-01-14 10:49:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-14 10:50:31]

Patchset 4 (id:??) landed as
https://crrev.com/ccc39f293a78dcd09cc05800823b90a74fe19c1b
Cr-Commit-Position: refs/heads/master@{#311449}