Do PPB_FileIO Query and Read in the plugin process.

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/files/file_util_proxy.h"
 #include "ipc/ipc_message.h"
 #include "ppapi/c/pp_errors.h"
+#include "ppapi/proxy/plugin_globals.h"
 #include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/shared_impl/array_writer.h"
+#include "ppapi/shared_impl/file_type_conversion.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
+#include "ppapi/shared_impl/proxy_lock.h"
 #include "ppapi/shared_impl/resource_tracker.h"
 #include "ppapi/thunk/enter.h"
 #include "ppapi/thunk/ppb_file_ref_api.h"
 
 using ppapi::thunk::EnterResourceNoLock;
 using ppapi::thunk::PPB_FileIO_API;
 using ppapi::thunk::PPB_FileRef_API;
 
 namespace {
 
 // An adapter to let Read() share the same implementation with ReadToArray().
 void* DummyGetDataBuffer(void* user_data, uint32_t count, uint32_t size) {
   return user_data;
 }
 
+// All potentially blocking file tasks should be done on the file thread. In
+// most cases, we can use base::FileUtilProxy to do the task and callback to the
+// original thread. This won't work for blocking PPAPI callbacks, since we
+// block the calling thread, which will cause FileUtilProxy to hang. Instead,

[dmichael (off chromium), 2013/08/02 15:44:29]

how about instead of "which will cause FileUtilProxy to hang"
...
"and so it can not receive the callback from FileUtilProxy."
or something (just to add more detail to why there's a hang)

[bbudge, 2013/08/02 20:56:44]

n/a now that we've eliminated FileUtilProxy.

+// do the same work in these tasks, which correctly wake the original thread.
+// However, we can't use these tasks for non-blocking callbacks because the
+// in-process proxy has no lock, which would lead to unsafe access to the
+// resource by both threads.
+
+void DoQuery(base::PlatformFile file,
+             PP_FileInfo* info,
+             const base::FileUtilProxy::GetFileInfoCallback& callback) {
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  base::PlatformFileInfo file_info;
+  if (!GetPlatformFileInfo(file, &file_info))
+    error = base::PLATFORM_FILE_ERROR_FAILED;
+
+  callback.Run(error, file_info);
+}
+
+void DoRead(base::PlatformFile file,
+            int64 offset,
+            int bytes_to_read,
+            const base::FileUtilProxy::ReadCallback& callback) {
+  scoped_ptr<char[]> buffer(new char[bytes_to_read]);

[dmichael (off chromium), 2013/08/02 15:44:29]

We shouldn't have to make a copy for the "Read" case. Maybe you can pass the
ArrayOutput here and do the read directly in to it?

In fact...  I hadn't realized that FileUtilProxy forces us to make a copy! We
probably shouldn't be using FileUtilProxy at all :-(. I think you can either:
1) Pass is_blocking along to DoRead, and if it's non-blocking, PostTask to the
main thread.
or...
2) Don't bother with is_blocking(). Check in here if ProxyLock is NULL. If so,
you know you need to PostTask to the main thread.

I think I like 2 better than 1. I think it's clearer, and gives you a good place
to stick a "TODO" to get rid of the special case. Maybe more importantly, if we
do (1), for any non-blocking callback that is *not* coming from the main thread,
we'd be bouncing to the main thread only to then bounce it to the real thread.

[bbudge, 2013/08/02 20:56:44]

Looks like we can't avoid the copy due to Abort semantics. Either we hold the
proxy lock for the entire file operation, or we copy.

I did eliminate FileUtilProxy, so will remove it from the IRT build in another
CL.

Note: we're no longer using is_blocking(), so I'll revert it too.

+  int bytes_read =
+      base::ReadPlatformFile(file, offset, buffer.get(), bytes_to_read);
+  base::PlatformFileError error = (bytes_read < 0) ?
+      base::PLATFORM_FILE_ERROR_FAILED : base::PLATFORM_FILE_OK;
+
+  callback.Run(error, buffer.get(), bytes_read);
+}
+
+void DoClose(base::PlatformFile file) {
+  base::ClosePlatformFile(file);
+}
+
 }  // namespace
 
 namespace ppapi {
 namespace proxy {
 
 FileIOResource::FileIOResource(Connection connection, PP_Instance instance)
-    : PluginResource(connection, instance) {
+    : PluginResource(connection, instance),
+      file_handle_(PP_kInvalidFileHandle),
+      file_system_type_(PP_FILESYSTEMTYPE_INVALID) {
   SendCreate(RENDERER, PpapiHostMsg_FileIO_Create());
 }
 
 FileIOResource::~FileIOResource() {
+  CloseFileHandle();
 }
 
 PPB_FileIO_API* FileIOResource::AsPPB_FileIO_API() {
   return this;
 }
 
 int32_t FileIOResource::Open(PP_Resource file_ref,
                              int32_t open_flags,
                              scoped_refptr<TrackedCallback> callback) {
   EnterResourceNoLock<PPB_FileRef_API> enter(file_ref, true);
   if (enter.failed())
     return PP_ERROR_BADRESOURCE;
 
+  PPB_FileRef_API* file_ref_api = enter.object();
+  PP_FileSystemType type = file_ref_api->GetFileSystemType();
+  if (type != PP_FILESYSTEMTYPE_LOCALPERSISTENT &&
+      type != PP_FILESYSTEMTYPE_LOCALTEMPORARY &&
+      type != PP_FILESYSTEMTYPE_EXTERNAL &&
+      type != PP_FILESYSTEMTYPE_ISOLATED) {
+    NOTREACHED();
+    return PP_ERROR_FAILED;
+  }
+  file_system_type_ = type;
+
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_EXCLUSIVE, false);
   if (rv != PP_OK)
     return rv;
 
   Call<PpapiPluginMsg_FileIO_OpenReply>(RENDERER,
       PpapiHostMsg_FileIO_Open(
           enter.resource()->host_resource().host_resource(),
           open_flags),
       base::Bind(&FileIOResource::OnPluginMsgOpenFileComplete, this,
                  callback));
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileIOResource::Query(PP_FileInfo* info,
                               scoped_refptr<TrackedCallback> callback) {
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_EXCLUSIVE, true);
   if (rv != PP_OK)
     return rv;
 
-  Call<PpapiPluginMsg_FileIO_QueryReply>(RENDERER,
-      PpapiHostMsg_FileIO_Query(),
-      base::Bind(&FileIOResource::OnPluginMsgQueryComplete, this,
-                 callback, info));
+  if (file_handle_ == base::kInvalidPlatformFileValue)
+    return PP_ERROR_FAILED;
+
+  base::TaskRunner* file_task_runner =
+      PpapiGlobals::Get()->GetFileTaskRunner(pp_instance());
+  base::FileUtilProxy::GetFileInfoCallback file_task = RunWhileLocked(
+      base::Bind(&FileIOResource::OnQueryComplete, this, callback, info));
+  if (callback->is_blocking()) {
+    file_task_runner->PostTask(
+        FROM_HERE,
+        Bind(&DoQuery, file_handle_, info, file_task));
+  } else {
+    if (!base::FileUtilProxy::GetFileInfoFromPlatformFile(file_task_runner,
+                                                          file_handle_,
+                                                          file_task)) {
+      return PP_ERROR_FAILED;
+    }
+  }
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileIOResource::Touch(PP_Time last_access_time,
                               PP_Time last_modified_time,
                               scoped_refptr<TrackedCallback> callback) {
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_EXCLUSIVE, true);
@@ skipping 14 matching lines @@
                              int32_t bytes_to_read,
                              scoped_refptr<TrackedCallback> callback) {
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_READ, true);
   if (rv != PP_OK)
     return rv;
 
   PP_ArrayOutput output_adapter;
   output_adapter.GetDataBuffer = &DummyGetDataBuffer;
   output_adapter.user_data = buffer;
-  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_READ);
   return ReadValidated(offset, bytes_to_read, output_adapter, callback);
 }
 
 int32_t FileIOResource::ReadToArray(int64_t offset,
                                     int32_t max_read_length,
                                     PP_ArrayOutput* array_output,
                                     scoped_refptr<TrackedCallback> callback) {
   DCHECK(array_output);
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_READ, true);
   if (rv != PP_OK)
     return rv;
 
-  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_READ);
   return ReadValidated(offset, max_read_length, *array_output, callback);
 }
 
 int32_t FileIOResource::Write(int64_t offset,
                               const char* buffer,
                               int32_t bytes_to_write,
                               scoped_refptr<TrackedCallback> callback) {
   int32_t rv = state_manager_.CheckOperationState(
       FileIOStateManager::OPERATION_WRITE, true);
   if (rv != PP_OK)
@@ skipping 36 matching lines @@
   Call<PpapiPluginMsg_FileIO_GeneralReply>(RENDERER,
       PpapiHostMsg_FileIO_Flush(),
       base::Bind(&FileIOResource::OnPluginMsgGeneralComplete, this,
                  callback));
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
 }
 
 void FileIOResource::Close() {
+  CloseFileHandle();
   Post(RENDERER, PpapiHostMsg_FileIO_Close());
 }
 
 int32_t FileIOResource::GetOSFileDescriptor() {
   int32_t file_descriptor;
   // Only available when running in process.
   SyncCall<PpapiPluginMsg_FileIO_GetOSFileDescriptorReply>(
       RENDERER, PpapiHostMsg_FileIO_GetOSFileDescriptor(), &file_descriptor);
   return file_descriptor;
 }
 
+int32_t FileIOResource::RequestOSFileHandle(
+    PP_FileHandle* handle,
+    scoped_refptr<TrackedCallback> callback) {
+  int32_t rv = state_manager_.CheckOperationState(
+      FileIOStateManager::OPERATION_EXCLUSIVE, true);
+  if (rv != PP_OK)
+    return rv;
+
+  Call<PpapiPluginMsg_FileIO_RequestOSFileHandleReply>(RENDERER,
+      PpapiHostMsg_FileIO_RequestOSFileHandle(),
+      base::Bind(&FileIOResource::OnPluginMsgRequestOSFileHandleComplete, this,
+                 callback, handle));
+
+  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
+  return PP_OK_COMPLETIONPENDING;
+}
+
 int32_t FileIOResource::WillWrite(int64_t offset,
                                   int32_t bytes_to_write,
                                   scoped_refptr<TrackedCallback> callback) {
   Call<PpapiPluginMsg_FileIO_GeneralReply>(RENDERER,
       PpapiHostMsg_FileIO_WillWrite(offset, bytes_to_write),
       base::Bind(&FileIOResource::OnPluginMsgGeneralComplete, this,
                  callback));
+
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileIOResource::WillSetLength(int64_t length,
                                       scoped_refptr<TrackedCallback> callback) {
   Call<PpapiPluginMsg_FileIO_GeneralReply>(RENDERER,
       PpapiHostMsg_FileIO_WillSetLength(length),
       base::Bind(&FileIOResource::OnPluginMsgGeneralComplete, this,
                  callback));
+
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileIOResource::ReadValidated(int64_t offset,
                                       int32_t bytes_to_read,
                                       const PP_ArrayOutput& array_output,
                                       scoped_refptr<TrackedCallback> callback) {
-  Call<PpapiPluginMsg_FileIO_ReadReply>(RENDERER,
-      PpapiHostMsg_FileIO_Read(offset, bytes_to_read),
-      base::Bind(&FileIOResource::OnPluginMsgReadComplete, this,
-                 callback, array_output));
+  if (file_handle_ == base::kInvalidPlatformFileValue)
+    return PP_ERROR_FAILED;
+  if (bytes_to_read < 0)
+    return PP_ERROR_FAILED;
+
+  base::TaskRunner* file_task_runner =
+      PpapiGlobals::Get()->GetFileTaskRunner(pp_instance());
+  base::FileUtilProxy::ReadCallback file_task = RunWhileLocked(
+      base::Bind(&FileIOResource::OnReadComplete, this, callback,
+                 array_output));
+  if (callback->is_blocking()) {
+    file_task_runner->PostTask(
+        FROM_HERE,
+        Bind(&DoRead, file_handle_, offset, bytes_to_read, file_task));
+  } else {
+    if (!base::FileUtilProxy::Read(file_task_runner,
+                                   file_handle_,
+                                   offset,
+                                   bytes_to_read,
+                                   file_task)) {
+      return PP_ERROR_FAILED;
+    }
+  }
+
+  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_READ);
   return PP_OK_COMPLETIONPENDING;
 }
 
-int32_t FileIOResource::RequestOSFileHandle(
-    PP_FileHandle* handle,
-    scoped_refptr<TrackedCallback> callback) {
-  int32_t rv = state_manager_.CheckOperationState(
-      FileIOStateManager::OPERATION_EXCLUSIVE, true);
-  if (rv != PP_OK)
-    return rv;
+void FileIOResource::CloseFileHandle() {
+  if (file_handle_ != base::kInvalidPlatformFileValue) {
+    base::TaskRunner* file_task_runner =
+        PpapiGlobals::Get()->GetFileTaskRunner(pp_instance());
+    file_task_runner->PostTask(FROM_HERE,
+                               base::Bind(&DoClose, file_handle_));
 
-  Call<PpapiPluginMsg_FileIO_RequestOSFileHandleReply>(RENDERER,
-      PpapiHostMsg_FileIO_RequestOSFileHandle(),
-      base::Bind(&FileIOResource::OnPluginMsgRequestOSFileHandleComplete, this,
-                 callback, handle));
+    file_handle_ = base::kInvalidPlatformFileValue;
+  }
+}
 
-  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
-  return PP_OK_COMPLETIONPENDING;
+void FileIOResource::OnQueryComplete(
+    scoped_refptr<TrackedCallback> callback,
+    PP_FileInfo* output_info,
+    base::PlatformFileError error_code,
+    const base::PlatformFileInfo& file_info) {
+  DCHECK(state_manager_.get_pending_operation() ==
+         FileIOStateManager::OPERATION_EXCLUSIVE);
+
+  if (!TrackedCallback::IsPending(callback)) {
+    state_manager_.SetOperationFinished();
+    return;
+  }
+
+  int32_t result = ::ppapi::PlatformFileErrorToPepperError(error_code);
+  if (result == PP_OK) {
+    ppapi::PlatformFileInfoToPepperFileInfo(file_info, file_system_type_,
+                                            output_info);
+  }
+
+  // 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);
+}
+
+void FileIOResource::OnReadComplete(
+    scoped_refptr<TrackedCallback> callback,
+    PP_ArrayOutput array_output,
+    base::PlatformFileError error_code,
+    const char* data, int bytes_read) {
+  DCHECK(state_manager_.get_pending_operation() ==
+         FileIOStateManager::OPERATION_READ);
+
+  if (!TrackedCallback::IsPending(callback)) {
+    state_manager_.SetOperationFinished();
+    return;
+  }
+
+  int32_t result = ::ppapi::PlatformFileErrorToPepperError(error_code);
+  if (result == PP_OK) {
+    result = std::max(0, bytes_read);
+    ArrayWriter output;
+    output.set_pp_array_output(array_output);
+    if (output.is_valid())
+      output.StoreArray(data, result);
+    else
+      result = PP_ERROR_FAILED;
+  }
+
+  // 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);
 }
 
 void FileIOResource::OnPluginMsgGeneralComplete(
     scoped_refptr<TrackedCallback> callback,
     const ResourceMessageReplyParams& params) {
   DCHECK(state_manager_.get_pending_operation() ==
          FileIOStateManager::OPERATION_EXCLUSIVE ||
          state_manager_.get_pending_operation() ==
          FileIOStateManager::OPERATION_WRITE);
-  // End the operation now. The callback may perform another file operation.
+  // 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(params.result());
 }
 
 void FileIOResource::OnPluginMsgOpenFileComplete(
     scoped_refptr<TrackedCallback> callback,
     const ResourceMessageReplyParams& params) {
   DCHECK(state_manager_.get_pending_operation() ==
          FileIOStateManager::OPERATION_EXCLUSIVE);
   if (params.result() == PP_OK)
     state_manager_.SetOpenSucceed();
-  // End the operation now. The callback may perform another file operation.
+
+  int32_t result = params.result();
+  IPC::PlatformFileForTransit transit_file;
+  if (result == PP_OK && !params.TakeFileHandleAtIndex(0, &transit_file))
+    result = PP_ERROR_FAILED;
+  file_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(params.result());
 }
 
-void FileIOResource::OnPluginMsgQueryComplete(
-    scoped_refptr<TrackedCallback> callback,
-    PP_FileInfo* output_info,
-    const ResourceMessageReplyParams& params,
-    const PP_FileInfo& info) {
-  DCHECK(state_manager_.get_pending_operation() ==
-         FileIOStateManager::OPERATION_EXCLUSIVE);
-  *output_info = info;
-  // End the operation now. The callback may perform another file operation.
-  state_manager_.SetOperationFinished();
-  callback->Run(params.result());
-}
-
-void FileIOResource::OnPluginMsgReadComplete(
-    scoped_refptr<TrackedCallback> callback,
-    PP_ArrayOutput array_output,
-    const ResourceMessageReplyParams& params,
-    const std::string& data) {
-  DCHECK(state_manager_.get_pending_operation() ==
-         FileIOStateManager::OPERATION_READ);
-
-  // The result code should contain the data size if it's positive.
-  int32_t result = params.result();
-  DCHECK((result < 0 && data.size() == 0) ||
-         result == static_cast<int32_t>(data.size()));
-
-  ArrayWriter output;
-  output.set_pp_array_output(array_output);
-  if (output.is_valid())
-    output.StoreArray(data.data(), std::max(0, result));
-  else
-    result = PP_ERROR_FAILED;
-
-  // End the operation now. The callback may perform another file operation.
-  state_manager_.SetOperationFinished();
-  callback->Run(result);
-}
-
 void FileIOResource::OnPluginMsgRequestOSFileHandleComplete(
     scoped_refptr<TrackedCallback> callback,
     PP_FileHandle* output_handle,
     const ResourceMessageReplyParams& params) {
   DCHECK(state_manager_.get_pending_operation() ==
          FileIOStateManager::OPERATION_EXCLUSIVE);
 
   if (!TrackedCallback::IsPending(callback)) {
     state_manager_.SetOperationFinished();
     return;
   }
 
   int32_t result = params.result();
   IPC::PlatformFileForTransit transit_file;
   if (!params.TakeFileHandleAtIndex(0, &transit_file))
     result = PP_ERROR_FAILED;
   *output_handle = IPC::PlatformFileForTransitToPlatformFile(transit_file);
 
-  // End the operation now. The callback may perform another file operation.
+  // 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