Reland: URLRequestJob: change ReadRawData contract

content/browser/net/view_http_cache_job_factory.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 "content/browser/net/view_http_cache_job_factory.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ skipping 27 matching lines @@
 
   // net::URLRequestJob implementation.
   void Start() override;
   void Kill() override;
   bool GetMimeType(std::string* mime_type) const override {
     return core_->GetMimeType(mime_type);
   }
   bool GetCharset(std::string* charset) override {
     return core_->GetCharset(charset);
   }
-  bool ReadRawData(net::IOBuffer* buf,
-                   int buf_size,
-                   int* out_bytes_read) override {
-    size_t bytes_read;
-    if (!core_->ReadRawData(buf, base::checked_cast<size_t>(buf_size),
-                            &bytes_read))
-      return false;
-    *out_bytes_read = base::checked_cast<int>(bytes_read);
-    return true;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override {
+    return core_->ReadRawData(buf, buf_size);
   }
 
  private:
   class Core : public base::RefCounted<Core> {
    public:
     Core()
         : data_offset_(0),
           callback_(base::Bind(&Core::OnIOComplete, this)) {
     }
 
     int Start(const net::URLRequest& request, const base::Closure& callback);
 
     // Prevents it from invoking its callback. It will self-delete.
     void Orphan() {
       user_callback_.Reset();
     }
 
     bool GetMimeType(std::string* mime_type) const;
     bool GetCharset(std::string* charset);
-    bool ReadRawData(net::IOBuffer* buf, size_t buf_size, size_t* bytes_read);
+    int ReadRawData(net::IOBuffer* buf, int buf_size);
 
    private:
     friend class base::RefCounted<Core>;
 
     ~Core() {}
 
     // Called when ViewCacheHelper completes the operation.
     void OnIOComplete(int result);
 
     std::string data_;
@@ skipping 78 matching lines @@
 bool ViewHttpCacheJob::Core::GetMimeType(std::string* mime_type) const {
   mime_type->assign("text/html");
   return true;
 }
 
 bool ViewHttpCacheJob::Core::GetCharset(std::string* charset) {
   charset->assign("UTF-8");
   return true;
 }
 
-bool ViewHttpCacheJob::Core::ReadRawData(net::IOBuffer* buf,
-                                         size_t buf_size,
-                                         size_t* bytes_read) {
-  DCHECK(bytes_read);
-  DCHECK_LE(data_offset_, data_.size());
-  size_t remaining = data_.size() - data_offset_;
+int ViewHttpCacheJob::Core::ReadRawData(net::IOBuffer* buf, int buf_size) {
+  int remaining = base::checked_cast<int>(data_.size()) - data_offset_;

[davidben, 2015/11/17 19:54:06]

Oh oops. I switched data_offset_ to size_t but intended to do so only after you
landed this one. I missed that yours had since been reverted. Since data_offset_
is also a size_t now let's do

  DCHECK_LE(data_offset_, data_.size());
  int remaining = base::checked_cast<int>(data_.size() - data_offset_);
  [...]
  data_offset_ += base::checked_cast<size_t>(buf_size);

We'll sort out the rest of the size_t / int confusion later since this interface
is now fairly int-specific in the return value.

[davidben, 2015/11/17 19:55:02]

(Er, I guess negative buf_size would have already exploded, so we can leave the
data_offset_ line alone.)

[xunjieli, 2015/11/17 20:28:35]

Acknowledged. Thanks, David.

[xunjieli, 2015/11/17 20:37:55]

Done.

   if (buf_size > remaining)
     buf_size = remaining;
   memcpy(buf->data(), data_.data() + data_offset_, buf_size);
   data_offset_ += buf_size;
-  *bytes_read = buf_size;
-  return true;
+  return buf_size;
 }
 
 void ViewHttpCacheJob::Core::OnIOComplete(int result) {
   DCHECK_EQ(net::OK, result);
 
   if (!user_callback_.is_null())
     user_callback_.Run();
 
   // We may be holding the last reference to this job. Do not access |this|
   // after Release().
   Release();  // Acquired on Start().
 }
 
 }  // namespace.
 
 // Static.
 bool ViewHttpCacheJobFactory::IsSupportedURL(const GURL& url) {
   return url.SchemeIs(kChromeUIScheme) &&
          url.host() == kChromeUINetworkViewCacheHost;
 }
 
 // Static.
 net::URLRequestJob* ViewHttpCacheJobFactory::CreateJobForRequest(
     net::URLRequest* request, net::NetworkDelegate* network_delegate) {
   return new ViewHttpCacheJob(request, network_delegate);
 }
 
 }  // namespace content