[fsp] Add support for aborting running operations.

[fsp] Add support for aborting running operations.

When an operation is aborted in C++ layer, it should be also terminated in the
providing extension.

This patch adds aborting feature for each operation. However, it is not used
everywhere. Especially, FileStreamReader does not utilize it yet. It will be
done in a separate CL.

TBR=nkostylev@chromium.org
TEST=unit_tests: *FileSystemProvider*Abort*, *FileSystemProvider*RequestManager*
BUG=400574
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290223

[mtomasz, 2014-08-06 03:58:20]

@hirono: PTAL. Thanks.

[hirono, 2014-08-06 08:33:50]

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

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc:84:
&FakeProvidedFileSystem::Abort, weak_ptr_factory_.GetWeakPtr(), task_id);
How about adding a helper function to call PostTask and returns bound Abort?

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h
(right):

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h:81:
virtual AbortCallback OpenFile(const base::FilePath& file_path,
nit: Make it the rule of breaking lines consistent with others.

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

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc:86:
file_system_->WriteFile(file_handle_, buffer, offset, length, callback);
Should we check file_handle_ != -1?

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js
(right):

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:23:
';
'; -> * ?

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:340:
if (!hadAbort) {
Is it OK that onwriteend is called even if the onWriteFileRequested handler does
not call any callbacks?

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:351:
hadAbort = true;
The onabort is acalled after onAbortRequested?
Can we put onTestSuccess() in the onabort?

[mtomasz, 2014-08-08 06:29:40]

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

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc:84:
&FakeProvidedFileSystem::Abort, weak_ptr_factory_.GetWeakPtr(), task_id);
On 2014/08/06 08:33:50, hirono wrote:
> How about adding a helper function to call PostTask and returns bound Abort?

Done.

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h
(right):

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h:81:
virtual AbortCallback OpenFile(const base::FilePath& file_path,
On 2014/08/06 08:33:50, hirono wrote:
> nit: Make it the rule of breaking lines consistent with others.

It is done by git cl format. AFAIR it is correct. First we should align to the
first asgument, but if doesn't fit then use 4-space ident.

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js
(right):

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:23:
';
On 2014/08/06 08:33:50, hirono wrote:
> '; -> * ?

Oops. Done.

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:340:
if (!hadAbort) {
On 2014/08/06 08:33:50, hirono wrote:
> Is it OK that onwriteend is called even if the onWriteFileRequested handler
does
> not call any callbacks?

That's the point. We call abort, so the operation is aborted, and file writer
doesn't wait for any callbacks. onwriteend is always called, even if there was
an error, or the operation got aborted.

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:351:
hadAbort = true;
On 2014/08/06 08:33:50, hirono wrote:
> The onabort is acalled after onAbortRequested?
> Can we put onTestSuccess() in the onabort?

It seems that FileWriter doesn't wait for result of onAbortRequested.
Surprisingly, onAbortRequested is executed *after* onabort, hence
onTestSuccess() is there.

[mtomasz, 2014-08-08 07:16:38]

I added a comment. PTAL, Thanks!

[hirono, 2014-08-08 07:23:38]

Thank you! LGTM!

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h
(right):

https://codereview.chromium.org/440653003/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h:81:
virtual AbortCallback OpenFile(const base::FilePath& file_path,
On 2014/08/08 06:29:40, mtomasz wrote:
> On 2014/08/06 08:33:50, hirono wrote:
> > nit: Make it the rule of breaking lines consistent with others.
> 
> It is done by git cl format. AFAIR it is correct. First we should align to the
> first asgument, but if doesn't fit then use 4-space ident.

Acknowledged.

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js
(right):

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:340:
if (!hadAbort) {
On 2014/08/08 06:29:40, mtomasz wrote:
> On 2014/08/06 08:33:50, hirono wrote:
> > Is it OK that onwriteend is called even if the onWriteFileRequested handler
> does
> > not call any callbacks?
> 
> That's the point. We call abort, so the operation is aborted, and file writer
> doesn't wait for any callbacks. onwriteend is always called, even if there was
> an error, or the operation got aborted.

Acknowledged.

https://codereview.chromium.org/440653003/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_system_provider/write_file/test.js:351:
hadAbort = true;
On 2014/08/08 06:29:40, mtomasz wrote:
> On 2014/08/06 08:33:50, hirono wrote:
> > The onabort is acalled after onAbortRequested?
> > Can we put onTestSuccess() in the onabort?
> 
> It seems that FileWriter doesn't wait for result of onAbortRequested.
> Surprisingly, onAbortRequested is executed *after* onabort, hence
> onTestSuccess() is there.

Acknowledged.

[mtomasz, 2014-08-08 07:30:08]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-08-08 07:31:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mtomasz@chromium.org/440653003/80001

[commit-bot: I haz the power, 2014-08-08 10:13:25]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-08 10:17:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 10:17:09]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mtomasz, 2014-08-11 02:14:09]

@benwells: PTAL at IDL. Thanks.

[benwells, 2014-08-11 06:48:47]

lgtm

[mtomasz, 2014-08-18 00:45:41]

On 2014/08/11 06:48:47, benwells wrote:
> lgtm

@hirono: I fixed thread DCHECKs. PTAL one more time at the last patchset.

[hirono, 2014-08-18 05:01:02]

still lgtm

[mtomasz, 2014-08-18 05:05:12]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-08-18 05:06:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mtomasz@chromium.org/440653003/100001

[commit-bot: I haz the power, 2014-08-18 06:11:19]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-18 06:14:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-18 06:14:47]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mtomasz, 2014-08-18 07:42:59]

@nkostylev: TBR for a refactoring change in webui.

[mtomasz, 2014-08-18 07:43:04]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-08-18 07:44:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mtomasz@chromium.org/440653003/100001

[commit-bot: I haz the power, 2014-08-18 07:48:39]

Committed patchset #6 (100001) as 290223