Reland: URLRequestJob: change ReadRawData contract

content/browser/net/view_http_cache_job_factory.cc

Index: content/browser/net/view_http_cache_job_factory.cc
diff --git a/content/browser/net/view_http_cache_job_factory.cc b/content/browser/net/view_http_cache_job_factory.cc
index 823ced9cc1066d1639571d9ec4d39caf40367723..38c1655f276602f33ac5e32b2a41a394965033ce 100644
--- a/content/browser/net/view_http_cache_job_factory.cc
+++ b/content/browser/net/view_http_cache_job_factory.cc
@@ -45,15 +45,8 @@ class ViewHttpCacheJob : public net::URLRequestJob {
   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:
@@ -73,7 +66,7 @@ class ViewHttpCacheJob : public net::URLRequestJob {
 
     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>;
@@ -172,18 +165,13 @@ bool ViewHttpCacheJob::Core::GetCharset(std::string* charset) {
   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) {