Add RequestOSFileHandle as a private PPAPI

ppapi/proxy/file_io_resource.cc

Index: ppapi/proxy/file_io_resource.cc
diff --git a/ppapi/proxy/file_io_resource.cc b/ppapi/proxy/file_io_resource.cc
index 77105a17555e41679191b8d3349ff565025a3644..61e32f758ac726d8252bb51a99b11e813a49059e 100644
--- a/ppapi/proxy/file_io_resource.cc
+++ b/ppapi/proxy/file_io_resource.cc
@@ -185,11 +185,25 @@ void FileIOResource::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;
+  IPC::Message reply;
+  ResourceMessageReplyParams reply_params;
+  int32_t error = GenericSyncCall(RENDERER,
+      PpapiHostMsg_FileIO_GetOSFileDescriptor(), &reply, &reply_params);
+  if (error != PP_OK)
+    return error;
+
+  IPC::PlatformFileForTransit transit_file;
+  if (!reply_params.TakeFileHandleAtIndex(0, &transit_file))
+    return PP_ERROR_FAILED;
+  base::PlatformFile file =
+    IPC::PlatformFileForTransitToPlatformFile(transit_file);
+#if defined(OS_POSIX)
+    return file;
+#elif defined(OS_WIN)
+    return reinterpret_cast<uintptr_t>(file);
+#else
+    return -1;  // Platform not supported.
+#endif
 }
 
 int32_t FileIOResource::WillWrite(int64_t offset,
@@ -224,6 +238,23 @@ int32_t FileIOResource::ReadValidated(int64_t offset,
   return PP_OK_COMPLETIONPENDING;
 }
 
+int32_t FileIOResource::GetOSFileHandle(

[dmichael (off chromium), 2013/03/27 19:40:53]

Maybe RequestOSFileHandle? I think synchronous when I read Get.

[hamaji, 2013/03/28 00:17:29]

Done.

+    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_GetOSFileDescriptorReply>(RENDERER,
+      PpapiHostMsg_FileIO_GetOSFileDescriptor(),
+      base::Bind(&FileIOResource::OnPluginMsgGetOSFileHandleComplete, this,
+                 callback, handle));
+
+  state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
+  return PP_OK_COMPLETIONPENDING;
+}
+
 void FileIOResource::OnPluginMsgGeneralComplete(
     scoped_refptr<TrackedCallback> callback,
     const ResourceMessageReplyParams& params) {
@@ -286,6 +317,23 @@ void FileIOResource::OnPluginMsgReadComplete(
   callback->Run(result);
 }
 
+void FileIOResource::OnPluginMsgGetOSFileHandleComplete(
+    scoped_refptr<TrackedCallback> callback,
+    PP_FileHandle* output_handle,
+    const ResourceMessageReplyParams& params) {
+  DCHECK(state_manager_.get_pending_operation() ==
+         FileIOStateManager::OPERATION_EXCLUSIVE);

[yzshen1, 2013/03/27 20:47:58]

Please check whether |callback| is pending. It is possible that |callback| is
aborted. (For example, the last plugin ref has been released.) If yes, nothing
needs to be done except changing state_manager_. Especially, |output_handle|
should not be touched. The plugin has probably deleted the memory.

[hamaji, 2013/03/28 00:17:29]

Done. I'm not 100% sure if I understood your comment. Hope this change is what
you said.

[yzshen1, 2013/03/28 20:59:49]

Yes.

What I was talking about is that the plugin may have already got its callback
run with a PP_ERRROR_ABORTED return value. In that case, it won't get another
notification even if you do callback->Run(). Moreover, because the callback has
been run, the plugin may have decided to delete the memory allocated for
|output_handle| because from its point of view the call has completed.

On 2013/03/28 00:17:29, hamaji wrote:

[hamaji, 2013/03/28 21:37:15]

Thanks for your explanation!

On 2013/03/28 20:59:49, yzshen1 wrote:

+
+  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.
+  state_manager_.SetOperationFinished();
+  callback->Run(result);
+}
+
 }  // namespace proxy
 }  // namespace ppapi
-