Bind: Merge FileUtilProxy and FileSystemFileUtilProxy: Delete/Touch/Truncate/Copy/Move

Merge FileUtilProxy and FileSystemFileUtilProxy using PostTaskAndReply: Delete/Touch/Truncate/Copy/Move

BUG=none
TEST=test_shell_tests:FileSystem*

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109010

[willchan no longer on Chromium, 2011-11-01 23:56:48]

base/ LGTM, but I'd like someone familiar with the webkit/ code to review that
portion. I've added adamk@. I pointed out my memory ownership concern that I'd
like someone to doublecheck.

http://codereview.chromium.org/8424007/diff/1/webkit/fileapi/file_system_oper...
File webkit/fileapi/file_system_operation.cc (right):

http://codereview.chromium.org/8424007/diff/1/webkit/fileapi/file_system_oper...
webkit/fileapi/file_system_operation.cc:174:
base::Unretained(&operation_context_),
Sorry, I'm not familiar with the code. It's not obvious to me that this is
correct. What prevents operation_context_ from going away? I think this
FileSystemOperation is supposed to be self-deleting, right? That said, I don't
know when it gets deleted, and it looks like
FileSystemOperation::DidFinishOperation() uses a weak pointer, so that makes me
a bit worried. Is base::Unretained() correct here? Is there someone else who can
look at this to doublecheck? Adam Klein perhaps?

[kinuko, 2011-11-02 08:04:28]

http://codereview.chromium.org/8424007/diff/1/webkit/fileapi/file_system_oper...
File webkit/fileapi/file_system_operation.cc (right):

http://codereview.chromium.org/8424007/diff/1/webkit/fileapi/file_system_oper...
webkit/fileapi/file_system_operation.cc:174:
base::Unretained(&operation_context_),
On 2011/11/01 23:56:48, willchan wrote:
> Sorry, I'm not familiar with the code. It's not obvious to me that this is
> correct. What prevents operation_context_ from going away? I think this
> FileSystemOperation is supposed to be self-deleting, right? That said, I don't
> know when it gets deleted, and it looks like
> FileSystemOperation::DidFinishOperation() uses a weak pointer, so that makes
me
> a bit worried. Is base::Unretained() correct here? Is there someone else who
can
> look at this to doublecheck? Adam Klein perhaps?

Right, it's self-deleting and WeakPtr makes things confusing.  I converted some
'delete this' with base::Owned(this) and removed WeakPtr usage.  Verified that
valgrind says ok in my local env.  Hoping that Eric, Taiju or Adam can
doublecheck the changes under fileapi.

[tzik, 2011-11-02 08:55:33]

On 2011/11/02 08:04:28, kinuko wrote:
>
http://codereview.chromium.org/8424007/diff/1/webkit/fileapi/file_system_oper...
> File webkit/fileapi/file_system_operation.cc (right):
> 
>
http://codereview.chromium.org/8424007/diff/1/webkit/fileapi/file_system_oper...
> webkit/fileapi/file_system_operation.cc:174:
> base::Unretained(&operation_context_),
> On 2011/11/01 23:56:48, willchan wrote:
> > Sorry, I'm not familiar with the code. It's not obvious to me that this is
> > correct. What prevents operation_context_ from going away? I think this
> > FileSystemOperation is supposed to be self-deleting, right? That said, I
don't
> > know when it gets deleted, and it looks like
> > FileSystemOperation::DidFinishOperation() uses a weak pointer, so that makes
> me
> > a bit worried. Is base::Unretained() correct here? Is there someone else who
> can
> > look at this to doublecheck? Adam Klein perhaps?
> 
> Right, it's self-deleting and WeakPtr makes things confusing.  I converted
some
> 'delete this' with base::Owned(this) and removed WeakPtr usage.  Verified that
> valgrind says ok in my local env.  Hoping that Eric, Taiju or Adam can
> doublecheck the changes under fileapi.

webkit/fileapi/ LGTM as doublecheck.