Avoid memory allocation for PPB_FileIO Read when callback is blocking.

Performs file read directly into caller's buffer.

BUG=194304

Review URL: https://codereview.chromium.org/27730003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230282 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 68 insertions, 25 deletions

diff --git a/ppapi/api/pp_array_output.idl b/ppapi/api/pp_array_output.idl
index be1aca7..8e04a4c 100644
--- a/ppapi/api/pp_array_output.idl
+++ b/ppapi/api/pp_array_output.idl
@@ -9,7 +9,7 @@
  *
  * This function will be called reentrantly. This means that if you call a
  * function PPB_Foo.GetData(&array_output), GetData will call your
- * GetDataBuffer function before it returns. 
+ * GetDataBuffer function before it returns.
  *
  * This function will be called even when returning 0-length arrays, so be sure
  * your implementation can support that. You can return NULL for 0 length
@@ -34,7 +34,10 @@
  * @param element_size The size of each element in bytes.
  *
  * @return Returns a pointer to the allocated memory. On failure, returns null.
- * You can also return null if the element_count is 0.
+ * You can also return null if the element_count is 0. When a non-null value is
+ * returned, the buffer must remain valid until after the callback runs. If used
+ * with a blocking callback, the buffer must remain valid until after the
+ * function returns. The plugin can then free any memory that it allocated.
  */
 typedef mem_t PP_ArrayOutput_GetDataBuffer([inout] mem_t user_data,
                                            [in] uint32_t element_count,
@@ -78,7 +81,7 @@ typedef mem_t PP_ArrayOutput_GetDataBuffer([inout] mem_t user_data,
 [passByValue]
 struct PP_ArrayOutput {
   /**
-   * A pointer to the allocation function that the browser implements.
+   * A pointer to the allocation function that the browser will call.
    */
   PP_ArrayOutput_GetDataBuffer GetDataBuffer;
 
diff --git a/ppapi/api/ppb_file_io.idl b/ppapi/api/ppb_file_io.idl
index 2e43ec5..9b8c9e2 100644
--- a/ppapi/api/ppb_file_io.idl
+++ b/ppapi/api/ppb_file_io.idl
@@ -121,7 +121,9 @@ interface PPB_FileIO {
    * @param[out] info The <code>PP_FileInfo</code> structure representing all
    * information about the file.
    * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
-   * completion of Query().
+   * completion of Query(). <code>info</code> must remain valid until after the
+   * callback runs. If you pass a blocking callback, <code>info</code> must
+   * remain valid until after Query() returns.
    *
    * @return An int32_t containing an error code from <code>pp_errors.h</code>.
    * PP_ERROR_FAILED will be returned if the file isn't opened, and
@@ -158,6 +160,7 @@ interface PPB_FileIO {
    * large enough to hold the specified number of bytes to read.  This function
    * might perform a partial read, meaning all the requested bytes
    * might not be returned, even if the end of the file has not been reached.
+   * The FileIO object must have been opened with read access.
    *
    * ReadToArray() is preferred to Read() when doing asynchronous operations.
    *
@@ -168,7 +171,9 @@ interface PPB_FileIO {
    * @param[in] bytes_to_read The number of bytes to read from
    * <code>offset</code>.
    * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
-   * completion of Read().
+   * completion of Read(). <code>buffer</code> must remain valid until after
+   * the callback runs. If you pass a blocking callback, <code>buffer</code>
+   * must remain valid until after Read() returns.
    *
    * @return The number of bytes read or an error code from
    * <code>pp_errors.h</code>. If the return value is 0, then end-of-file was
@@ -260,7 +265,8 @@ interface PPB_FileIO {
   /**
    * ReadToArray() reads from an offset in the file.  A PP_ArrayOutput must be
    * provided so that output will be stored in its allocated buffer.  This
-   * function might perform a partial read.
+   * function might perform a partial read. The FileIO object must have been
+   * opened with read access.
    *
    * @param[in] file_io A <code>PP_Resource</code> corresponding to a file
    * FileIO.
diff --git a/ppapi/c/pp_array_output.h b/ppapi/c/pp_array_output.h
index add873c..2272a8f 100644
--- a/ppapi/c/pp_array_output.h
+++ b/ppapi/c/pp_array_output.h
@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From pp_array_output.idl modified Thu Mar 28 11:07:53 2013. */
+/* From pp_array_output.idl modified Tue Oct 22 15:09:25 2013. */
 
 #ifndef PPAPI_C_PP_ARRAY_OUTPUT_H_
 #define PPAPI_C_PP_ARRAY_OUTPUT_H_
@@ -43,7 +43,10 @@
  * @param element_size The size of each element in bytes.
  *
  * @return Returns a pointer to the allocated memory. On failure, returns null.
- * You can also return null if the element_count is 0.
+ * You can also return null if the element_count is 0. When a non-null value is
+ * returned, the buffer must remain valid until after the callback runs. If used
+ * with a blocking callback, the buffer must remain valid until after the
+ * function returns. The plugin can then free any memory that it allocated.
  */
 
 
@@ -99,7 +102,7 @@ typedef void* (*PP_ArrayOutput_GetDataBuffer)(void* user_data,
  */
 struct PP_ArrayOutput {
   /**
-   * A pointer to the allocation function that the browser implements.
+   * A pointer to the allocation function that the browser will call.
    */
   PP_ArrayOutput_GetDataBuffer GetDataBuffer;
   /**
diff --git a/ppapi/c/ppb_file_io.h b/ppapi/c/ppb_file_io.h
index fe0e1bd..8f272bf 100644
--- a/ppapi/c/ppb_file_io.h
+++ b/ppapi/c/ppb_file_io.h
@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From ppb_file_io.idl modified Tue Jun 11 15:21:38 2013. */
+/* From ppb_file_io.idl modified Tue Oct 22 15:09:47 2013. */
 
 #ifndef PPAPI_C_PPB_FILE_IO_H_
 #define PPAPI_C_PPB_FILE_IO_H_
@@ -135,7 +135,9 @@ struct PPB_FileIO_1_1 {
    * @param[out] info The <code>PP_FileInfo</code> structure representing all
    * information about the file.
    * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
-   * completion of Query().
+   * completion of Query(). <code>info</code> must remain valid until after the
+   * callback runs. If you pass a blocking callback, <code>info</code> must
+   * remain valid until after Query() returns.
    *
    * @return An int32_t containing an error code from <code>pp_errors.h</code>.
    * PP_ERROR_FAILED will be returned if the file isn't opened, and
@@ -170,6 +172,7 @@ struct PPB_FileIO_1_1 {
    * large enough to hold the specified number of bytes to read.  This function
    * might perform a partial read, meaning all the requested bytes
    * might not be returned, even if the end of the file has not been reached.
+   * The FileIO object must have been opened with read access.
    *
    * ReadToArray() is preferred to Read() when doing asynchronous operations.
    *
@@ -180,7 +183,9 @@ struct PPB_FileIO_1_1 {
    * @param[in] bytes_to_read The number of bytes to read from
    * <code>offset</code>.
    * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
-   * completion of Read().
+   * completion of Read(). <code>buffer</code> must remain valid until after
+   * the callback runs. If you pass a blocking callback, <code>buffer</code>
+   * must remain valid until after Read() returns.
    *
    * @return The number of bytes read or an error code from
    * <code>pp_errors.h</code>. If the return value is 0, then end-of-file was
@@ -267,7 +272,8 @@ struct PPB_FileIO_1_1 {
   /**
    * ReadToArray() reads from an offset in the file.  A PP_ArrayOutput must be
    * provided so that output will be stored in its allocated buffer.  This
-   * function might perform a partial read.
+   * function might perform a partial read. The FileIO object must have been
+   * opened with read access.
    *
    * @param[in] file_io A <code>PP_Resource</code> corresponding to a file
    * FileIO.
diff --git a/ppapi/cpp/file_io.h b/ppapi/cpp/file_io.h
index 99d8ce7..bbd6b57 100644
--- a/ppapi/cpp/file_io.h
+++ b/ppapi/cpp/file_io.h
@@ -71,7 +71,9 @@ class FileIO : public Resource {
   /// @param[in] result_buf The <code>PP_FileInfo</code> structure representing
   /// all information about the file.
   /// @param[in] cc A <code>CompletionCallback</code> to be called upon
-  /// completion of Query().
+  /// completion of Query(). <code>result_buf</code> must remain valid until
+  /// after the callback runs. If you pass a blocking callback,
+  /// <code>result_buf</code> must remain valid until after Query() returns.
   ///
   /// @return An int32_t containing an error code from
   /// <code>pp_errors.h</code>.
@@ -138,7 +140,9 @@ class FileIO : public Resource {
   /// @param[in] bytes_to_read The number of bytes to read from
   /// <code>offset</code>.
   /// @param[in] cc A <code>CompletionCallback</code> to be called upon
-  /// completion of Read().
+  /// completion of Read(). <code>buffer</code> must remain valid until after
+  /// the callback runs. If you pass a blocking callback, <code>buffer</code>
+  /// must remain valid until after Read() returns.
   ///
   /// @return An The number of bytes read an error code from
   /// <code>pp_errors.h</code>. If the return value is 0, then end-of-file was
diff --git a/ppapi/proxy/file_io_resource.cc b/ppapi/proxy/file_io_resource.cc
index b1eb450..347d4ef 100644
--- a/ppapi/proxy/file_io_resource.cc
+++ b/ppapi/proxy/file_io_resource.cc
@@ -138,21 +138,33 @@ int32_t FileIOResource::Query(PP_FileInfo* info,
     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;
+    }
+    if (result == PP_OK) {
+      // This writes the file info into the plugin's PP_FileInfo struct.
+      ppapi::PlatformFileInfoToPepperFileInfo(file_info,
+                                              file_system_type_,
+                                              info);
     }
-    return OnQueryComplete(query_op, info, result);
+    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,
@@ -316,19 +328,28 @@ int32_t FileIOResource::ReadValidated(int64_t offset,
   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));
+    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,