Make URLRequest::Read to return net errors or bytes read instead of a bool

net/url_request/url_request_job.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/url_request/url_request_job.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 142 matching lines @@
   // Kill().
   // TODO(mmenke):  The URLRequest is currently deleted before this method
   // invokes its async callback whenever this is called by the URLRequest.
   // Try to simplify how cancellation works.
   NotifyCanceled();
 }
 
 // This function calls ReadRawData to get stream data. If a filter exists, it
 // passes the data to the attached filter. It then returns the output from
 // filter back to the caller.
-bool URLRequestJob::Read(IOBuffer* buf, int buf_size, int *bytes_read) {
+int URLRequestJob::Read(IOBuffer* buf, int buf_size) {
   DCHECK_LT(buf_size, 1000000);  // Sanity check.
   DCHECK(buf);
-  DCHECK(bytes_read);
   DCHECK(!filtered_read_buffer_);
   DCHECK_EQ(0, filtered_read_buffer_len_);
 
   Error error = OK;
-  *bytes_read = 0;
+  int bytes_read = 0;
 
   // Skip Filter if not present.
   if (!filter_) {
-    error = ReadRawDataHelper(buf, buf_size, bytes_read);
+    error = ReadRawDataHelper(buf, buf_size, &bytes_read);
   } else {
     // Save the caller's buffers while we do IO
     // in the filter's buffers.
     filtered_read_buffer_ = buf;
     filtered_read_buffer_len_ = buf_size;
 
-    error = ReadFilteredData(bytes_read);
+    error = ReadFilteredData(&bytes_read);
 
     // Synchronous EOF from the filter.
-    if (error == OK && *bytes_read == 0)
+    if (error == OK && bytes_read == 0)
       DoneReading();
   }
 
   if (error == OK) {
     // If URLRequestJob read zero bytes, the job is at EOF.
-    if (*bytes_read == 0)
+    if (bytes_read == 0)
       NotifyDone(URLRequestStatus());
   } else if (error == ERR_IO_PENDING) {
-    *bytes_read = ERR_IO_PENDING;
+    bytes_read = ERR_IO_PENDING;
   } else {
     NotifyDone(URLRequestStatus::FromError(error));
-    *bytes_read = error;
+    bytes_read = error;
   }
-  return error == OK;
+  return bytes_read;

[mmenke, 2016/08/30 22:09:12]

Suggest re-ordering these and using early returns.  Think separate cases are
easier to follow, and now don't need separate logic to set bytes_read and handle
the return value.

Something like:

if (error == ERR_IO_PENDING)
  return ERR_IO_PENDING;

if (error < 0) {
  ...
  return error;
}

if (bytes_read == 0)
  // Early return not needed here.

[maksims (do not use this acc), 2016/08/31 11:16:59]

Done.

 }
 
 void URLRequestJob::StopCaching() {
   // Nothing to do here.
 }
 
 bool URLRequestJob::GetFullRequestHeaders(HttpRequestHeaders* headers) const {
   // Most job types don't send request headers.
   return false;
 }
@@ skipping 793 matching lines @@
   int64_t total_sent_bytes = GetTotalSentBytes();
   DCHECK_GE(total_sent_bytes, last_notified_total_sent_bytes_);
   if (total_sent_bytes > last_notified_total_sent_bytes_) {
     network_delegate_->NotifyNetworkBytesSent(
         request_, total_sent_bytes - last_notified_total_sent_bytes_);
   }
   last_notified_total_sent_bytes_ = total_sent_bytes;
 }
 
 }  // namespace net