Introduce error handling in HttpStreamParser on UploadDataStream::Read() failure.

net/http/http_stream_parser.cc

Index: net/http/http_stream_parser.cc
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index c718761ec7856dd132f14cfb211b977d58b7ed38..b37da9819c8665a0091cd468d8e9c79527a2c2de 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -273,7 +273,6 @@ int HttpStreamParser::SendRequest(const std::string& request_line,
   }
 
   io_state_ = STATE_SEND_HEADERS;
-
   // If we have a small request body, then we'll merge with the headers into a
   // single write.
   bool did_merge = false;
@@ -286,17 +285,17 @@ int HttpStreamParser::SendRequest(const std::string& request_line,
     // body.
     request_headers_ = new DrainableIOBuffer(
         merged_request_headers_and_body.get(), merged_size);
-
     memcpy(request_headers_->data(), request.data(), request_headers_length_);
     request_headers_->DidConsume(request_headers_length_);
 
     uint64_t todo = request_->upload_data_stream->size();
     while (todo) {
-      int consumed = request_->upload_data_stream->Read(
+      int result = request_->upload_data_stream->Read(
           request_headers_.get(), static_cast<int>(todo), CompletionCallback());
-      DCHECK_GT(consumed, 0);  // Read() won't fail if not chunked.
-      request_headers_->DidConsume(consumed);
-      todo -= consumed;
+      if (result == ERR_FILE_READ_FAIL)
+        return result;

[mmenke, 2016/05/24 17:13:47]

I think we can actually just keep the DCHECK here, instead - if it's in memory,
reads can't fail.

[maksims (do not use this acc), 2016/05/25 07:13:38]

Done.

+      request_headers_->DidConsume(result);
+      todo -= result;
     }
     DCHECK(request_->upload_data_stream->IsEOF());
     // Reset the offset, so the buffer can be read from the beginning.
@@ -318,11 +317,9 @@ int HttpStreamParser::SendRequest(const std::string& request_line,
     request_headers_ =
         new DrainableIOBuffer(headers_io_buf.get(), headers_io_buf->size());
   }
-
   result = DoLoop(OK);

[mmenke, 2016/05/24 17:13:47]

Should keep these line break, for readability.

[maksims (do not use this acc), 2016/05/25 07:13:38]

Done.

   if (result == ERR_IO_PENDING)
     callback_ = callback;
-
   return result > 0 ? OK : result;
 }
 
@@ -549,7 +546,8 @@ int HttpStreamParser::DoSendBodyComplete(int result) {
 int HttpStreamParser::DoSendRequestReadBodyComplete(int result) {
   // |result| is the result of read from the request body from the last call to
   // DoSendBody().
-  DCHECK_GE(result, 0);  // There won't be errors.
+  if (result == ERR_FILE_READ_FAIL)

[mmenke, 2016/05/24 17:13:47]

This should be "if (result < 0)" - if we see any read failure, we should fail,
it doesn't matter which one it is.  Also note that we can't get ERR_IO_PENDING
here.

[maksims (do not use this acc), 2016/05/25 07:13:38]

Done. Or should it be DCHECK_NE(result, ERR_IO_PENDING) ?

[mmenke, 2016/05/25 15:13:30]

We already have that DCHECK in HttpStreamParser::DoLoop, before calling this,
and other comparable methods in this file don't, so probably best just to keep
things as-is.  However, that wouldn't be unreasonable here.

+    return result;
 
   // Chunked data needs to be encoded.
   if (request_->upload_data_stream->is_chunked()) {