Avoid memory allocation for PPB_FileIO Read when callback is blocking.

ppapi/proxy/file_io_resource.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 "ppapi/proxy/file_io_resource.h"
 
 #include "base/bind.h"
 #include "base/task_runner_util.h"
 #include "ipc/ipc_message.h"
 #include "ppapi/c/pp_errors.h"
@@ skipping 120 matching lines @@
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_EXCLUSIVE, true);
   if (rv != PP_OK)
     return rv;
   if (!info)
     return PP_ERROR_BADARGUMENT;
   if (!FileHandleHolder::IsValid(file_handle_))
     return PP_ERROR_FAILED;
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
-  scoped_refptr<QueryOp> query_op(new QueryOp(file_handle_));
 
   // If the callback is blocking, perform the task on the calling thread.
   if (callback->is_blocking()) {
-    int32_t result;
+    int32_t result = PP_ERROR_FAILED;
+    base::PlatformFileInfo file_info;
+    // The plugin could release its reference to this instance when we release
+    // the proxy lock below.
+    scoped_refptr<FileIOResource> protect(this);
     {
       // Release the proxy lock while making a potentially slow file call.
       ProxyAutoUnlock unlock;
-      result = query_op->DoWork();
+      if (base::GetPlatformFileInfo(file_handle_->raw_handle(), &file_info))
+        result = PP_OK;
     }
-    return OnQueryComplete(query_op, info, result);
+    if (result == PP_OK) {
+      // This writes the file info into the plugin's PP_FileInfo struct.
+      ppapi::PlatformFileInfoToPepperFileInfo(file_info,
+                                              file_system_type_,
+                                              info);
+    }
+    state_manager_.SetOperationFinished();
+    return result;
   }
 
   // For the non-blocking case, post a task to the file thread and add a
   // completion task to write the result.
+  scoped_refptr<QueryOp> query_op(new QueryOp(file_handle_));
   base::PostTaskAndReplyWithResult(
       PpapiGlobals::Get()->GetFileTaskRunner(),
       FROM_HERE,
       Bind(&FileIOResource::QueryOp::DoWork, query_op),
       RunWhileLocked(Bind(&TrackedCallback::Run, callback)));
   callback->set_completion_task(
       Bind(&FileIOResource::OnQueryComplete, this, query_op, info));
 
   return PP_OK_COMPLETIONPENDING;
 }
@@ skipping 143 matching lines @@
                                       const PP_ArrayOutput& array_output,
                                       scoped_refptr<TrackedCallback> callback) {
   if (bytes_to_read < 0)
     return PP_ERROR_FAILED;
   if (!FileHandleHolder::IsValid(file_handle_))
     return PP_ERROR_FAILED;
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_READ);
 
   bytes_to_read = std::min(bytes_to_read, kMaxReadSize);
-  scoped_refptr<ReadOp> read_op(
-      new ReadOp(file_handle_, offset, bytes_to_read));
   if (callback->is_blocking()) {
-    int32_t result;
-    {
+    char* buffer = static_cast<char*>(
+        array_output.GetDataBuffer(array_output.user_data, bytes_to_read, 1));

[dmichael (off chromium), 2013/10/22 18:02:58]

I had to read a bunch of code to convince myself we don't leak this in error
conditions for the ReadToArray case. It looks like our C++ wrapper code handles
it properly.

It is slightly confusing. Maybe we should note in the C interface somewhere
(perhaps in PP_ArrayOutput) that the plugin is responsible for freeing the
memory returned by GetDataBuffer when it's finished with that buffer, even in
the case of error.

[bbudge, 2013/10/22 20:51:04]

Done.

+    int32_t result = PP_ERROR_FAILED;
+    // The plugin could release its reference to this instance when we release
+    // the proxy lock below.
+    scoped_refptr<FileIOResource> protect(this);
+    if (buffer) {
       // Release the proxy lock while making a potentially slow file call.
       ProxyAutoUnlock unlock;
-      result = read_op->DoWork();
+      result = base::ReadPlatformFile(
+          file_handle_->raw_handle(), offset, buffer, bytes_to_read);
+      if (result < 0)
+        result = PP_ERROR_FAILED;
     }
-    return OnReadComplete(read_op, array_output, result);
+    state_manager_.SetOperationFinished();
+    return result;
   }
 
   // For the non-blocking case, post a task to the file thread.
+  scoped_refptr<ReadOp> read_op(
+      new ReadOp(file_handle_, offset, bytes_to_read));
   base::PostTaskAndReplyWithResult(
       PpapiGlobals::Get()->GetFileTaskRunner(),
       FROM_HERE,
       Bind(&FileIOResource::ReadOp::DoWork, read_op),
       RunWhileLocked(Bind(&TrackedCallback::Run, callback)));
   callback->set_completion_task(
       Bind(&FileIOResource::OnReadComplete, this, read_op, array_output));
 
   return PP_OK_COMPLETIONPENDING;
 }
@@ skipping 89 matching lines @@
   *output_handle = IPC::PlatformFileForTransitToPlatformFile(transit_file);
 
   // End this operation now, so the user's callback can execute another FileIO
   // operation, assuming there are no other pending operations.
   state_manager_.SetOperationFinished();
   callback->Run(result);
 }
 
 }  // namespace proxy
 }  // namespace ppapi