[abandoned] Remove the destination file after failed copy.

Remove the destination file after failed copy.

Previously, even file copy was failed, the destination file was kept
with empty or broken content.

BUG=729529

[tetsui, 2017-06-05 02:02:28]

The CQ bit was checked by tetsui@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-05 02:02:40]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-05 02:31:11]

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

[commit-bot: I haz the power, 2017-06-05 02:31:12]

Dry run: This issue passed the CQ dry run.

[tetsui, 2017-06-05 02:32:05]

tetsui@chromium.org changed reviewers:
+ fukino@chromium.org

[tetsui, 2017-06-05 02:32:05]

PTAL. Thanks!

[fukino, 2017-06-05 04:20:08]

https://codereview.chromium.org/2918273002/diff/1/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
(right):

https://codereview.chromium.org/2918273002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:183:
destination_url, false, base::Bind(&OnRemoveAfterFailedCopyCompleted));
Is it safe to call NotifyCopyCompletion before Remove operation is done?

https://codereview.chromium.org/2918273002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:209:
*operation_id = file_system_context->operation_runner()->Copy(
It might be better to remove incomplete file inside the copy operation in
(native?) file system.

[fukino, 2017-06-05 11:43:07]

Could you file a bug, since 499642 is already fixed?

The empty file will be created when opening a source file succeeds but reading
the content of the source file fails.
This sounds rare in actual use cases. Do I understand correctly?

Regarding the incomplete destination file, it can be easily created by
cancelling copy operation from progress center.
Maybe we should fix this case by looking into the cancel operation.

[tetsui, 2017-06-05 11:56:55]

On 2017/06/05 11:43:07, fukino wrote:
> Could you file a bug, since 499642 is already fixed?

I filed http://crbug.com/729529

> The empty file will be created when opening a source file succeeds but reading
> the content of the source file fails.
> This sounds rare in actual use cases. Do I understand correctly?

Yes, I think it is correct.

> Regarding the incomplete destination file, it can be easily created by
> cancelling copy operation from progress center.
> Maybe we should fix this case by looking into the cancel operation.

Which is the better place to do this: JavaScript side or fileManagerPrivate?

Thanks!

[tetsui, 2017-06-05 11:57:07]

Description was changed from

==========
Remove the destination file after failed copy.

Previously, even file copy was failed, the destination file was kept
with empty or broken content.

BUG=499642
==========

to

==========
Remove the destination file after failed copy.

Previously, even file copy was failed, the destination file was kept
with empty or broken content.

BUG=729529
==========

[fukino, 2017-06-06 01:55:17]

> Which is the better place to do this: JavaScript side or fileManagerPrivate?

Let's try cleaning the incomplete file on cancel operation in JS side.