Add truncate and cancel for FileWriter; write and tests will come in later CL...

Add truncate and cancel for FileWriter; write and more tests will come in later CLs.
This CL also contains a fair amount of plumbing for the write call itself.

BUG=none
TEST=none

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

[kinuko, 2010-10-02 04:43:16]

http://codereview.chromium.org/3599011/diff/11001/12001
File base/file_util_proxy.cc (right):

http://codereview.chromium.org/3599011/diff/11001/12001#newcode612
base/file_util_proxy.cc:612: base::PLATFORM_FILE_WRITE |
base::PLATFORM_FILE_TRUNCATE,
Doesn't this (PLATFORM_FILE_TRUNCATE) truncate the file to length 0?

http://codereview.chromium.org/3599011/diff/11001/12008
File webkit/fileapi/file_system_operation.cc (right):

http://codereview.chromium.org/3599011/diff/11001/12008#newcode207
webkit/fileapi/file_system_operation.cc:207:
cancel_op->dispatcher_->DidSucceed();
Hmm the calling frame remains?  Well it seems like.

[kinuko, 2010-10-04 00:41:00]

http://codereview.chromium.org/3599011/diff/11001/12001
File base/file_util_proxy.cc (right):

http://codereview.chromium.org/3599011/diff/11001/12001#newcode585
base/file_util_proxy.cc:585: class RelayTruncate : public
RelayWithStatusCallback {
It might be better to separate classes for FilePath and PlatformFile versions
(like RelayGetFileInfo and RelayGetFileInfoFromPlatformFile) given that they
don't share much of the code.

[ericu, 2010-10-04 18:04:47]

Also cleaned up some lint errors.

http://codereview.chromium.org/3599011/diff/11001/12001
File base/file_util_proxy.cc (right):

http://codereview.chromium.org/3599011/diff/11001/12001#newcode585
base/file_util_proxy.cc:585: class RelayTruncate : public
RelayWithStatusCallback {
On 2010/10/04 00:41:00, Kinuko Yasuda wrote:
> It might be better to separate classes for FilePath and PlatformFile versions
> (like RelayGetFileInfo and RelayGetFileInfoFromPlatformFile) given that they
> don't share much of the code.

Done.

http://codereview.chromium.org/3599011/diff/11001/12001#newcode612
base/file_util_proxy.cc:612: base::PLATFORM_FILE_WRITE |
base::PLATFORM_FILE_TRUNCATE,
On 2010/10/02 04:43:16, Kinuko Yasuda wrote:
> Doesn't this (PLATFORM_FILE_TRUNCATE) truncate the file to length 0?

Oops!  It sure does.  That's what I get for testing with zero-length initial
input files.  Fixed.

http://codereview.chromium.org/3599011/diff/11001/12008
File webkit/fileapi/file_system_operation.cc (right):

http://codereview.chromium.org/3599011/diff/11001/12008#newcode207
webkit/fileapi/file_system_operation.cc:207:
cancel_op->dispatcher_->DidSucceed();
On 2010/10/02 04:43:16, Kinuko Yasuda wrote:
> Hmm the calling frame remains?  Well it seems like.

Right--you can delete the class out from under the thread, but you're not doing
anything to the thread or its frame.

[kinuko, 2010-10-04 23:04:55]

LGTM

Would be really really great if we could add a unittest for Truncate in
file_system_operation_unittest.cc.

On 2010/10/04 18:04:47, ericu wrote:
> Also cleaned up some lint errors.
> 
> http://codereview.chromium.org/3599011/diff/11001/12001
> File base/file_util_proxy.cc (right):
> 
> http://codereview.chromium.org/3599011/diff/11001/12001#newcode585
> base/file_util_proxy.cc:585: class RelayTruncate : public
> RelayWithStatusCallback {
> On 2010/10/04 00:41:00, Kinuko Yasuda wrote:
> > It might be better to separate classes for FilePath and PlatformFile
versions
> > (like RelayGetFileInfo and RelayGetFileInfoFromPlatformFile) given that they
> > don't share much of the code.
> 
> Done.
> 
> http://codereview.chromium.org/3599011/diff/11001/12001#newcode612
> base/file_util_proxy.cc:612: base::PLATFORM_FILE_WRITE |
> base::PLATFORM_FILE_TRUNCATE,
> On 2010/10/02 04:43:16, Kinuko Yasuda wrote:
> > Doesn't this (PLATFORM_FILE_TRUNCATE) truncate the file to length 0?
> 
> Oops!  It sure does.  That's what I get for testing with zero-length initial
> input files.  Fixed.
> 
> http://codereview.chromium.org/3599011/diff/11001/12008
> File webkit/fileapi/file_system_operation.cc (right):
> 
> http://codereview.chromium.org/3599011/diff/11001/12008#newcode207
> webkit/fileapi/file_system_operation.cc:207:
> cancel_op->dispatcher_->DidSucceed();
> On 2010/10/02 04:43:16, Kinuko Yasuda wrote:
> > Hmm the calling frame remains?  Well it seems like.
> 
> Right--you can delete the class out from under the thread, but you're not
doing
> anything to the thread or its frame.

[kinuko, 2010-10-04 23:05:21]

http://codereview.chromium.org/3599011/diff/31001/32009
File webkit/fileapi/file_system_operation.h (right):

http://codereview.chromium.org/3599011/diff/31001/32009#newcode24
webkit/fileapi/file_system_operation.h:24: class URLRequestContext;
nit: are they for coming code?

[ericu, 2010-10-04 23:13:29]

I'll add some unit tests and post again shortly.

http://codereview.chromium.org/3599011/diff/31001/32009
File webkit/fileapi/file_system_operation.h (right):

http://codereview.chromium.org/3599011/diff/31001/32009#newcode24
webkit/fileapi/file_system_operation.h:24: class URLRequestContext;
On 2010/10/04 23:05:21, Kinuko Yasuda wrote:
> nit: are they for coming code?

Oops!  Yeah, I'll take them out before committing.

[ericu, 2010-10-05 00:26:27]

Added a unit test for truncate.

I'll add cancel tests when I've got write in.

[kinuko, 2010-10-05 00:37:24]

LGTM

http://codereview.chromium.org/3599011/diff/52001/6012
File webkit/fileapi/file_system_operation_unittest.cc (right):

http://codereview.chromium.org/3599011/diff/52001/6012#newcode565
webkit/fileapi/file_system_operation_unittest.cc:565: 
nit: no need for this empty line?

http://codereview.chromium.org/3599011/diff/52001/6012#newcode592
webkit/fileapi/file_system_operation_unittest.cc:592: int buffer_size = 100;
nit: this is not used.

http://codereview.chromium.org/3599011/diff/52001/6012#newcode595
webkit/fileapi/file_system_operation_unittest.cc:595: for (int i=0; i < length;
++i) {
nit: s/i=0/i = 0/

http://codereview.chromium.org/3599011/diff/52001/6012#newcode613
webkit/fileapi/file_system_operation_unittest.cc:613: for (int i=0; i < length;
++i)
nit: s/i=0/i = 0/

[ericu, 2010-10-05 00:40:32]

http://codereview.chromium.org/3599011/diff/52001/6012
File webkit/fileapi/file_system_operation_unittest.cc (right):

http://codereview.chromium.org/3599011/diff/52001/6012#newcode565
webkit/fileapi/file_system_operation_unittest.cc:565: 
On 2010/10/05 00:37:26, Kinuko Yasuda wrote:
> nit: no need for this empty line?

Done.

http://codereview.chromium.org/3599011/diff/52001/6012#newcode592
webkit/fileapi/file_system_operation_unittest.cc:592: int buffer_size = 100;
On 2010/10/05 00:37:26, Kinuko Yasuda wrote:
> nit: this is not used.

Removed.

http://codereview.chromium.org/3599011/diff/52001/6012#newcode595
webkit/fileapi/file_system_operation_unittest.cc:595: for (int i=0; i < length;
++i) {
On 2010/10/05 00:37:26, Kinuko Yasuda wrote:
> nit: s/i=0/i = 0/

Done.

http://codereview.chromium.org/3599011/diff/52001/6012#newcode613
webkit/fileapi/file_system_operation_unittest.cc:613: for (int i=0; i < length;
++i)
On 2010/10/05 00:37:26, Kinuko Yasuda wrote:
> nit: s/i=0/i = 0/

Done.