(For-try) Divide recursive Copy/Move into multiple async tasks

(For-try) Divide recursive Copy/Move into multiple async tasks
+ FileAPI: Split recursive remove into multiple tasks.
+ More cleanup in recursive copy/move

Try for:
https://codereview.chromium.org/12036022/ splict copy/move
https://codereview.chromium.org/12018017/ split remove
https://codereview.chromium.org/11960003/ cleanup

[ericu, 2013-01-23 19:27:23]

A couple of drive-by comments, only.

https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/cross_opera...
File webkit/fileapi/cross_operation_delegate.cc (right):

https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/cross_opera...
webkit/fileapi/cross_operation_delegate.cc:145: if (same_file_system_) {
I'm getting a bit lost in all the redirection.  If we're doing a move of a local
directory, to the same filesystem, possibly overwriting an empty directory, will
we still do a fast local move, or will we recursively copy and then remove the
original?

https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/cross_opera...
webkit/fileapi/cross_operation_delegate.cc:199: // Remove the source for
finalizing move oepration.
typo: operation

https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/obfuscated_...
File webkit/fileapi/obfuscated_file_util.cc (right):

https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/obfuscated_...
webkit/fileapi/obfuscated_file_util.cc:880: if (error == base::PLATFORM_FILE_OK
&& file_info->is_directory)
Here you're returning the file_info, even if it's not valid.  If someone forgets
to check the return value, bad things might happen.  I don't want to slow things
down by requiring an extra copy in the common case, but perhaps in the case of
this error we should clear the fields of the file_info?  What do you think?

[kinuko, 2013-01-24 01:34:51]

On 2013/01/23 19:27:23, ericu wrote:
> A couple of drive-by comments, only.
>
>
https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/cross_opera...
> File webkit/fileapi/cross_operation_delegate.cc (right):
> 
>
https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/cross_opera...
> webkit/fileapi/cross_operation_delegate.cc:145: if (same_file_system_) {
> I'm getting a bit lost in all the redirection.  If we're doing a move of a
local
> directory, to the same filesystem, possibly overwriting an empty directory,
will
> we still do a fast local move, or will we recursively copy and then remove the
> original?

For directories we always create a new one and remove the original, and actually
that was what we were doing in the old code.  Probably we could do better, but
first I want to exactly replicate the original behavior.

>
https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/cross_opera...
> webkit/fileapi/cross_operation_delegate.cc:199: // Remove the source for
> finalizing move oepration.
> typo: operation

Thanks, will address (in the version I uploaded for reveiw:
https://codereview.chromium.org/12036022/)

>
https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/obfuscated_...
> File webkit/fileapi/obfuscated_file_util.cc (right):
> 
>
https://codereview.chromium.org/12051010/diff/8003/webkit/fileapi/obfuscated_...
> webkit/fileapi/obfuscated_file_util.cc:880: if (error ==
base::PLATFORM_FILE_OK
> && file_info->is_directory)
> Here you're returning the file_info, even if it's not valid.  If someone
forgets
> to check the return value, bad things might happen.  I don't want to slow
things
> down by requiring an extra copy in the common case, but perhaps in the case of
> this error we should clear the fields of the file_info?  What do you think?

How about having an explicit comment to note that the field values of file_info
will be 'undefined' (i.e. implementation dependent) on error case in the header
(i.e. file_system_file_util.h)?  I'll clear the fields in this file too.

[kinuko, 2013-01-24 03:36:38]

Addressed comments in the patch set 3 of
https://codereview.chromium.org/12036022/