MTP Streaming: Optimize block reading

chrome/browser/media_galleries/fileapi/mtp_file_stream_reader.cc

Index: chrome/browser/media_galleries/fileapi/mtp_file_stream_reader.cc
diff --git a/chrome/browser/media_galleries/fileapi/mtp_file_stream_reader.cc b/chrome/browser/media_galleries/fileapi/mtp_file_stream_reader.cc
index e8082ea0ea2b99648e210a51886a528dfab79644..d6578904fd9aa462bf53345928c0d88069c81078 100644
--- a/chrome/browser/media_galleries/fileapi/mtp_file_stream_reader.cc
+++ b/chrome/browser/media_galleries/fileapi/mtp_file_stream_reader.cc
@@ -41,27 +41,24 @@ void CallInt64CompletionCallbackWithPlatformFileError(
   callback.Run(net::FileErrorToNetError(file_error));
 }
 
-void ReadBytes(const fileapi::FileSystemURL& url, net::IOBuffer* buf,
-               int64 offset, int buf_len, const base::File::Info& file_info,
-               const net::CompletionCallback& callback) {
+void ReadBytes(

[vandebo (ex-Chrome), 2014/02/27 17:20:50]

You might be able to eliminate this function now.

[tommycli, 2014/02/27 18:57:33]

Will probably do this in the next CL (which refactors this class).

+    const fileapi::FileSystemURL& url, net::IOBuffer* buf, int64 offset,
+    int buf_len,
+    const MTPDeviceAsyncDelegate::ReadBytesSuccessCallback& success_callback,
+    const net::CompletionCallback& error_callback) {
   MTPDeviceAsyncDelegate* delegate = GetMTPDeviceDelegate(url);
   if (!delegate) {
-    callback.Run(net::ERR_FAILED);
+    error_callback.Run(net::ERR_FAILED);
     return;
   }
 
-  DCHECK_GE(file_info.size, offset);
-  int bytes_to_read = std::min(
-      buf_len,
-      base::saturated_cast<int>(file_info.size - offset));
-
   delegate->ReadBytes(
       url.path(),
       make_scoped_refptr(buf),
       offset,
-      bytes_to_read,
-      callback,
-      base::Bind(&CallCompletionCallbackWithPlatformFileError, callback));
+      buf_len,
+      success_callback,
+      base::Bind(&CallCompletionCallbackWithPlatformFileError, error_callback));
 }
 
 }  // namespace
@@ -90,12 +87,21 @@ int MTPFileStreamReader::Read(net::IOBuffer* buf, int buf_len,
   if (!delegate)
     return net::ERR_FAILED;
 
-  delegate->GetFileInfo(
-      url_.path(),
-      base::Bind(&MTPFileStreamReader::OnFileInfoForRead,
-                 weak_factory_.GetWeakPtr(),
-                 make_scoped_refptr(buf), buf_len, callback),
-      base::Bind(&CallCompletionCallbackWithPlatformFileError, callback));
+  if (!media_header_validated_) {
+    scoped_refptr<net::IOBuffer> header_buf(
+        new net::IOBuffer(net::kMaxBytesToSniff));
+    ReadBytes(url_, header_buf, 0, net::kMaxBytesToSniff,
+              base::Bind(&MTPFileStreamReader::FinishValidateMediaHeader,
+                         weak_factory_.GetWeakPtr(), header_buf,
+                         make_scoped_refptr(buf), buf_len, callback),
+              callback);
+    return net::ERR_IO_PENDING;
+  }
+
+  ReadBytes(url_, buf, current_offset_, buf_len,
+            base::Bind(&MTPFileStreamReader::FinishRead,
+                       weak_factory_.GetWeakPtr(), callback),
+            callback);
 
   return net::ERR_IO_PENDING;
 }
@@ -118,37 +124,11 @@ int64 MTPFileStreamReader::GetLength(
   return net::ERR_IO_PENDING;
 }
 
-void MTPFileStreamReader::OnFileInfoForRead(
-    net::IOBuffer* buf, int buf_len, const net::CompletionCallback& callback,
-    const base::File::Info& file_info) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
-
-  if (!VerifySnapshotTime(expected_modification_time_, file_info)) {
-    callback.Run(net::ERR_UPLOAD_FILE_CHANGED);
-    return;
-  }
-
-  if (!media_header_validated_) {
-    int header_bytes = std::min(net::kMaxBytesToSniff,
-                                base::saturated_cast<int>(file_info.size));
-    scoped_refptr<net::IOBuffer> header_buf(new net::IOBuffer(header_bytes));
-    ReadBytes(url_, header_buf, 0, header_bytes, file_info,
-              base::Bind(&MTPFileStreamReader::FinishValidateMediaHeader,
-                         weak_factory_.GetWeakPtr(), header_buf, file_info,
-                         make_scoped_refptr(buf), buf_len, callback));
-    return;
-  }
-
-  ReadBytes(url_, buf, current_offset_, buf_len, file_info,
-            base::Bind(&MTPFileStreamReader::FinishRead,
-                       weak_factory_.GetWeakPtr(), callback));
-}
-
 void MTPFileStreamReader::FinishValidateMediaHeader(
     net::IOBuffer* header_buf,
-    const base::File::Info& file_info,
     net::IOBuffer* buf, int buf_len,
     const net::CompletionCallback& callback,
+    const base::File::Info& file_info,
     int header_bytes_read) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   DCHECK_GE(header_bytes_read, 0);
@@ -162,14 +142,22 @@ void MTPFileStreamReader::FinishValidateMediaHeader(
   media_header_validated_ = true;
 
   // Complete originally requested read.
-  ReadBytes(url_, buf, current_offset_, buf_len, file_info,
+  ReadBytes(url_, buf, current_offset_, buf_len,
             base::Bind(&MTPFileStreamReader::FinishRead,
-                       weak_factory_.GetWeakPtr(), callback));
+                       weak_factory_.GetWeakPtr(), callback),
+            callback);
 }
 
 void MTPFileStreamReader::FinishRead(const net::CompletionCallback& callback,
+                                     const base::File::Info& file_info,
                                      int bytes_read) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+  if (!VerifySnapshotTime(expected_modification_time_, file_info)) {
+    callback.Run(net::ERR_UPLOAD_FILE_CHANGED);
+    return;
+  }
+
   DCHECK_GE(bytes_read, 0);
   current_offset_ += bytes_read;
   callback.Run(bytes_read);