UploadDataStream now returns errors on failures

net/base/elements_upload_data_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/elements_upload_data_stream.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "net/base/completion_callback.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/base/upload_element_reader.h"
 
 namespace net {
 
 ElementsUploadDataStream::ElementsUploadDataStream(
     std::vector<std::unique_ptr<UploadElementReader>> element_readers,
     int64_t identifier)
     : UploadDataStream(false, identifier),
       element_readers_(std::move(element_readers)),
       element_index_(0),
-      read_failed_(false),
+      read_failed_(OK),
       weak_ptr_factory_(this) {}
 
 ElementsUploadDataStream::~ElementsUploadDataStream() {
 }
 
 std::unique_ptr<UploadDataStream> ElementsUploadDataStream::CreateWithReader(
     std::unique_ptr<UploadElementReader> reader,
     int64_t identifier) {
   std::vector<std::unique_ptr<UploadElementReader>> readers;
   readers.push_back(std::move(reader));
@@ skipping 20 matching lines @@
   return true;
 }
 
 const std::vector<std::unique_ptr<UploadElementReader>>*
 ElementsUploadDataStream::GetElementReaders() const {
   return &element_readers_;
 }
 
 void ElementsUploadDataStream::ResetInternal() {
   weak_ptr_factory_.InvalidateWeakPtrs();
-  read_failed_ = false;
+  read_failed_ = OK;
   element_index_ = 0;
 }
 
 int ElementsUploadDataStream::InitElements(size_t start_index) {
   // Call Init() for all elements.
   for (size_t i = start_index; i < element_readers_.size(); ++i) {
     UploadElementReader* reader = element_readers_[i].get();
     // When new_result is ERR_IO_PENDING, InitInternal() will be called
     // with start_index == i + 1 when reader->Init() finishes.
     int result = reader->Init(
@@ skipping 21 matching lines @@
   // Check the last result.
   if (result == OK)
     result = InitElements(index + 1);
 
   if (result != ERR_IO_PENDING)
     OnInitCompleted(result);
 }
 
 int ElementsUploadDataStream::ReadElements(
     const scoped_refptr<DrainableIOBuffer>& buf) {
   while (!read_failed_ && element_index_ < element_readers_.size()) {

[mmenke, 2016/06/21 21:13:13]

!read_failed_ should be read_failed_ == OK, for clarity.

[maksims (do not use this acc), 2016/06/22 15:10:32]

Done.

     UploadElementReader* reader = element_readers_[element_index_].get();
 
     if (reader->BytesRemaining() == 0) {
       ++element_index_;
       continue;
     }
 
     if (buf->BytesRemaining() == 0)
       break;
 
     int result = reader->Read(
         buf.get(),
         buf->BytesRemaining(),
         base::Bind(&ElementsUploadDataStream::OnReadElementCompleted,
                    weak_ptr_factory_.GetWeakPtr(),
                    buf));
     if (result == ERR_IO_PENDING)
       return ERR_IO_PENDING;
     ProcessReadResult(buf, result);
   }
 
-  if (read_failed_) {
-    // If an error occured during read operation, then pad with zero.
-    // Otherwise the server will hang waiting for the rest of the data.
-    int num_bytes_to_fill =
-        static_cast<int>(std::min(static_cast<uint64_t>(buf->BytesRemaining()),
-                                  size() - position() - buf->BytesConsumed()));
-    DCHECK_GE(num_bytes_to_fill, 0);
-    memset(buf->data(), 0, num_bytes_to_fill);
-    buf->DidConsume(num_bytes_to_fill);
-  }
+  if (read_failed_ != OK)

[mmenke, 2016/06/21 21:13:13]

I'd suggest this instead:

if (buf->BytesConsumed() > 0)
  return buf->BytesConsumed();

return read_failed_;

That way, if we read bytes and have an error, we return the read bytes first. 
Probably doesn't matter, but seems safer.

[maksims (do not use this acc), 2016/06/22 15:10:32]

Done. It sitll has to check if there is now error, because it might fail on
another read round.

+    return read_failed_;
 
   return buf->BytesConsumed();
 }
 
 void ElementsUploadDataStream::OnReadElementCompleted(
     const scoped_refptr<DrainableIOBuffer>& buf,
     int result) {
   ProcessReadResult(buf, result);
 
   result = ReadElements(buf);
   if (result != ERR_IO_PENDING)
     OnReadCompleted(result);
 }
 
 void ElementsUploadDataStream::ProcessReadResult(
     const scoped_refptr<DrainableIOBuffer>& buf,
     int result) {
   DCHECK_NE(ERR_IO_PENDING, result);
   DCHECK(!read_failed_);
 
   if (result >= 0) {
     buf->DidConsume(result);
   } else {
-    read_failed_ = true;
+    read_failed_ = result;
   }
 }
 
 }  // namespace net