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

net/url_request/url_request.h

Index: net/url_request/url_request.h
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 73e02d99c8710171d0c83c780eafabdcd1097645..86e5a0361ab53b0c0694bd71d99bd09edb8a42c0 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -196,20 +196,22 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
                                        bool fatal);
 
     // After calling Start(), the delegate will receive an OnResponseStarted
-    // callback when the request has completed.  If an error occurred, the
-    // request->status() will be set.  On success, all redirects have been
+    // callback when the request has completed. |net_error| will be set to OK
+    // or an actual net error.  On success, all redirects have been
     // followed and the final response is beginning to arrive.  At this point,
     // meta data about the response is available, including for example HTTP
     // response headers if this is a request for a HTTP resource.
-    virtual void OnResponseStarted(URLRequest* request) = 0;
+    virtual void OnResponseStarted(URLRequest* request, int net_error);
+    // Deprecated.
+    // TODO(maksims): Remove this;
+    virtual void OnResponseStarted(URLRequest* request);
 
     // Called when the a Read of the response body is completed after an
     // IO_PENDING status from a Read() call.
     // The data read is filled into the buffer which the caller passed
     // to Read() previously.
     //
-    // If an error occurred, request->status() will contain the error,
-    // and bytes read will be -1.
+    // If an error occurred, |bytes_read| will be set to the error.
     virtual void OnReadCompleted(URLRequest* request, int bytes_read) = 0;
 
    protected:
@@ -524,9 +526,6 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   // URL but has not yet initiated the new request.
   bool is_redirecting() const { return is_redirecting_; }
 
-  // Returns the error status of the request.
-  const URLRequestStatus& status() const { return status_; }
-
   // Returns a globally unique identifier for this request.
   uint64_t identifier() const { return identifier_; }
 
@@ -540,12 +539,14 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   // no effect once the response has completed.  It is guaranteed that no
   // methods of the delegate will be called after the request has been
   // cancelled, except that this may call the delegate's OnReadCompleted()
-  // during the call to Cancel itself.
-  void Cancel();
+  // during the call to Cancel itself. Returns |ERR_ABORTED| or other net error
+  // if there was one.
+  int Cancel();
 
-  // Cancels the request and sets the error to |error| (see net_error_list.h
-  // for values).
-  void CancelWithError(int error);
+  // Cancels the request and sets the error to |error|, unless the request
+  // already failed with another error code (see net_error_list.h). Returns
+  // final network error code.
+  int CancelWithError(int error);
 
   // Cancels the request and sets the error to |error| (see net_error_list.h
   // for values) and attaches |ssl_info| as the SSLInfo for that request.  This
@@ -553,28 +554,23 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   // request.
   void CancelWithSSLError(int error, const SSLInfo& ssl_info);
 
-  // Read initiates an asynchronous read from the response, and must only
-  // be called after the OnResponseStarted callback is received with a
-  // successful status.
-  // If data is available, Read will return true, and the data and length will
-  // be returned immediately.  If data is not available, Read returns false,
-  // and an asynchronous Read is initiated.  The Read is finished when
-  // the caller receives the OnReadComplete callback.  Unless the request was
-  // cancelled, OnReadComplete will always be called, even if the read failed.
+  //  Read initiates an asynchronous read from the response, and must only be
+  // called after the OnResponseStarted callback is received with a net::OK. If
+  // data is available, length and the data will be returned immediately. If the
+  // request has failed, an error code will be returned. If data is not yet
+  // available, Read returns net::ERR_IO_PENDING, and the Delegate's
+  // OnReadComplete method will be called asynchronously with the result of the
+  // read, unless the URLRequest is canceled.
   //
-  // The buf parameter is a buffer to receive the data.  If the operation
+  // The |buf| parameter is a buffer to receive the data. If the operation
   // completes asynchronously, the implementation will reference the buffer
-  // until OnReadComplete is called.  The buffer must be at least max_bytes in
+  // until OnReadComplete is called. The buffer must be at least |max_bytes| in
   // length.
   //
-  // The max_bytes parameter is the maximum number of bytes to read.
-  //
-  // The bytes_read parameter is an output parameter containing the
-  // the number of bytes read.  A value of 0 indicates that there is no
-  // more data available to read from the stream.
-  //
-  // If a read error occurs, Read returns false and the request->status
-  // will be set to an error.
+  // The |max_bytes| parameter is the maximum number of bytes to read.
+  int Read(IOBuffer* buf, int max_bytes);
+  // Deprecated.
+  // TODO(maksims): Remove this.
   bool Read(IOBuffer* buf, int max_bytes, int* bytes_read);
 
   // If this request is being cached by the HTTP cache, stop subsequent caching.
@@ -648,6 +644,10 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   // or after the response headers are received.
   void GetConnectionAttempts(ConnectionAttempts* out) const;
 
+  // Returns the error status of the request.
+  // Do not use! Going to be protected!
+  const URLRequestStatus& status() const { return status_; }
+
  protected:
   // Allow the URLRequestJob class to control the is_pending() flag.
   void set_is_pending(bool value) { is_pending_ = value; }
@@ -671,6 +671,10 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   friend class URLRequestJob;
   friend class URLRequestContext;
 
+  // For testing purposes.
+  // TODO(maksims): Remove this.
+  friend class TestNetworkDelegate;
+
   // URLRequests are always created by calling URLRequestContext::CreateRequest.
   //
   // If no network delegate is passed in, will use the ones from the
@@ -702,8 +706,8 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   void OrphanJob();
 
   // Cancels the request and set the error and ssl info for this request to the
-  // passed values.
-  void DoCancel(int error, const SSLInfo& ssl_info);
+  // passed values. Returns the error that was set.
+  int DoCancel(int error, const SSLInfo& ssl_info);
 
   // Called by the URLRequestJob when the headers are received, before any other
   // method, to allow caching of load timing information.