Check response code before writing blob to file.

Check response code before writing blob to file.

In current impl, FileWriterDelegate does not check response code of blob scheme request.
This causes falsely success write-job when writing broken blob to file.

This patch makes it explicitly fail.

BUG=None
TEST="FileWriterDelegateTest.*, FileSystemOperationWriteTest.*"

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

[tzik, 2012-01-05 03:34:30]

Could you review this patch?

[ericu, 2012-01-05 05:47:20]

http://codereview.chromium.org/9007055/diff/8001/webkit/fileapi/file_writer_d...
File webkit/fileapi/file_writer_delegate.cc (right):

http://codereview.chromium.org/9007055/diff/8001/webkit/fileapi/file_writer_d...
webkit/fileapi/file_writer_delegate.cc:172: if (!request->status().is_success()
|| request->GetResponseCode() != 200) {
Why is is_success true here, if the blob wasn't valid?
[Sorry--I'm away from my workstation and can't easily look that up.]

http://codereview.chromium.org/9007055/diff/8001/webkit/fileapi/file_writer_d...
webkit/fileapi/file_writer_delegate.cc:187: if (!request->status().is_success())
{
Is it possible to get is_success == true, but request->GetResponseCode() != 200
here?  What if it's a write of a large File that goes away halfway through the
operation?

[tzik, 2012-01-05 06:59:45]

http://codereview.chromium.org/9007055/diff/8001/webkit/fileapi/file_writer_d...
File webkit/fileapi/file_writer_delegate.cc (right):

http://codereview.chromium.org/9007055/diff/8001/webkit/fileapi/file_writer_d...
webkit/fileapi/file_writer_delegate.cc:172: if (!request->status().is_success()
|| request->GetResponseCode() != 200) {
On 2012/01/05 05:47:20, ericu wrote:
> Why is is_success true here, if the blob wasn't valid?
> [Sorry--I'm away from my workstation and can't easily look that up.]

Looking at other use of URLRequestStatus, !is_success() seems to mean error on
lower layer. e.g. failed to connect to server.
Errors on upper layer (like 404) should be caught in response code.

If blob is invalid, status code is set to 404 and content of the response is
empty in current impl.
(In case of HTTP, if a resource is not found, response code is set to 404 and
content of response can be non empty error page.)

http://codereview.chromium.org/9007055/diff/8001/webkit/fileapi/file_writer_d...
webkit/fileapi/file_writer_delegate.cc:187: if (!request->status().is_success())
{
On 2012/01/05 05:47:20, ericu wrote:
> Is it possible to get is_success == true, but request->GetResponseCode() !=
200
> here?  What if it's a write of a large File that goes away halfway through the
> operation?

The response code is invariant after header of the blob protocol reached, so
it's enough to check response code in start time.
If we encounter an error on reading blob, we'll get !is_success().

[ericu, 2012-01-05 16:22:33]

lgtm