Make asynchronous writes with PPB_FileIO copy the plugin's buffer.

This fixes a problem that was introduced by moving the implementation
of Write to the plugin side. The plugin must maintain its buffer until
completion.

BUG=none

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

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

3 files changed, 57 insertions, 28 deletions

diff --git a/ppapi/proxy/file_io_resource.cc b/ppapi/proxy/file_io_resource.cc
index 7157574..0564fdf 100644
--- a/ppapi/proxy/file_io_resource.cc
+++ b/ppapi/proxy/file_io_resource.cc
@@ -85,28 +85,28 @@ int32_t FileIOResource::ReadOp::DoWork() {
 
 FileIOResource::WriteOp::WriteOp(scoped_refptr<FileHandleHolder> file_handle,
                                  int64_t offset,
-                                 const char* buffer,
+                                 scoped_ptr<char[]> buffer,
                                  int32_t bytes_to_write,
                                  bool append)
-  : file_handle_(file_handle),
-    offset_(offset),
-    buffer_(buffer),
-    bytes_to_write_(bytes_to_write),
-    append_(append) {
+    : file_handle_(file_handle),
+      offset_(offset),
+      buffer_(buffer.Pass()),
+      bytes_to_write_(bytes_to_write),
+      append_(append) {
 }
 
 FileIOResource::WriteOp::~WriteOp() {
 }
 
 int32_t FileIOResource::WriteOp::DoWork() {
-  // We can't just call WritePlatformFile in append mode, since NaCl doesn't
+  // In append mode, we can't call WritePlatformFile, since NaCl doesn't
   // implement fcntl, causing the function to call pwrite, which is incorrect.
   if (append_) {
     return base::WritePlatformFileAtCurrentPos(
-        file_handle_->raw_handle(), buffer_, bytes_to_write_);
+        file_handle_->raw_handle(), buffer_.get(), bytes_to_write_);
   } else {
     return base::WritePlatformFile(
-        file_handle_->raw_handle(), offset_, buffer_, bytes_to_write_);
+        file_handle_->raw_handle(), offset_, buffer_.get(), bytes_to_write_);
   }
 }
 
@@ -306,12 +306,19 @@ int32_t FileIOResource::Write(int64_t offset,
     }
 
     if (increase > 0) {
+      // Request a quota reservation. This makes the Write asynchronous, so we
+      // must copy the plugin's buffer.
+      scoped_ptr<char[]> copy(new char[bytes_to_write]);
+      memcpy(copy.get(), buffer, bytes_to_write);
       int64_t result =
           file_system_resource_->AsPPB_FileSystem_API()->RequestQuota(
               increase,
               base::Bind(&FileIOResource::OnRequestWriteQuotaComplete,
                          this,
-                         offset, buffer, bytes_to_write, callback));
+                         offset,
+                         base::Passed(&copy),
+                         bytes_to_write,
+                         callback));
       if (result == PP_OK_COMPLETIONPENDING)
         return PP_OK_COMPLETIONPENDING;
       DCHECK(result == increase);
@@ -515,16 +522,18 @@ int32_t FileIOResource::WriteValidated(
     return result;
   }
 
-  // For the non-blocking case, post a task to the file thread.
+  // For the non-blocking case, post a task to the file thread. We must copy the
+  // plugin's buffer at this point.
+  scoped_ptr<char[]> copy(new char[bytes_to_write]);
+  memcpy(copy.get(), buffer, bytes_to_write);
   scoped_refptr<WriteOp> write_op(
-      new WriteOp(file_handle_, offset, buffer, bytes_to_write, append));
+      new WriteOp(file_handle_, offset, copy.Pass(), bytes_to_write, append));
   base::PostTaskAndReplyWithResult(
       PpapiGlobals::Get()->GetFileTaskRunner(),
       FROM_HERE,
       Bind(&FileIOResource::WriteOp::DoWork, write_op),
       RunWhileLocked(Bind(&TrackedCallback::Run, callback)));
-  callback->set_completion_task(
-      Bind(&FileIOResource::OnWriteComplete, this, write_op));
+  callback->set_completion_task(Bind(&FileIOResource::OnWriteComplete, this));
 
   return PP_OK_COMPLETIONPENDING;
 }
@@ -582,7 +591,7 @@ int32_t FileIOResource::OnReadComplete(scoped_refptr<ReadOp> read_op,
 
 void FileIOResource::OnRequestWriteQuotaComplete(
     int64_t offset,
-    const char* buffer,
+    scoped_ptr<char[]> buffer,
     int32_t bytes_to_write,
     scoped_refptr<TrackedCallback> callback,
     int64_t granted) {
@@ -602,9 +611,22 @@ void FileIOResource::OnRequestWriteQuotaComplete(
       max_written_offset_ = max_offset;
   }
 
-  int32_t result = WriteValidated(offset, buffer, bytes_to_write, callback);
-  if (result != PP_OK_COMPLETIONPENDING)
+  if (callback->is_blocking()) {
+    int32_t result =
+        WriteValidated(offset, buffer.get(), bytes_to_write, callback);
+    DCHECK(result != PP_OK_COMPLETIONPENDING);
     callback->Run(result);
+  } else {
+    bool append = (open_flags_ & PP_FILEOPENFLAG_APPEND) != 0;
+    scoped_refptr<WriteOp> write_op(new WriteOp(
+        file_handle_, offset, buffer.Pass(), bytes_to_write, append));
+    base::PostTaskAndReplyWithResult(
+        PpapiGlobals::Get()->GetFileTaskRunner(),
+        FROM_HERE,
+        Bind(&FileIOResource::WriteOp::DoWork, write_op),
+        RunWhileLocked(Bind(&TrackedCallback::Run, callback)));
+    callback->set_completion_task(Bind(&FileIOResource::OnWriteComplete, this));
+  }
 }
 
 void FileIOResource::OnRequestSetLengthQuotaComplete(
@@ -623,8 +645,7 @@ void FileIOResource::OnRequestSetLengthQuotaComplete(
   SetLengthValidated(length, callback);
 }
 
-int32_t FileIOResource::OnWriteComplete(scoped_refptr<WriteOp> write_op,
-                                        int32_t result) {
+int32_t FileIOResource::OnWriteComplete(int32_t result) {
   DCHECK(state_manager_.get_pending_operation() ==
          FileIOStateManager::OPERATION_WRITE);
   // |result| is the return value of WritePlatformFile; -1 indicates failure.
diff --git a/ppapi/proxy/file_io_resource.h b/ppapi/proxy/file_io_resource.h
index 5d58035..557847f 100644
--- a/ppapi/proxy/file_io_resource.h
+++ b/ppapi/proxy/file_io_resource.h
@@ -153,7 +153,7 @@ class PPAPI_PROXY_EXPORT FileIOResource
    public:
     WriteOp(scoped_refptr<FileHandleHolder> file_handle,
             int64_t offset,
-            const char* buffer,
+            scoped_ptr<char[]> buffer,
             int32_t bytes_to_write,
             bool append);
 
@@ -167,13 +167,13 @@ class PPAPI_PROXY_EXPORT FileIOResource
 
     scoped_refptr<FileHandleHolder> file_handle_;
     int64_t offset_;
-    const char* buffer_;
+    scoped_ptr<char[]> buffer_;
     int32_t bytes_to_write_;
     bool append_;
   };
 
   void OnRequestWriteQuotaComplete(int64_t offset,
-                                   const char* buffer,
+                                   scoped_ptr<char[]> buffer,
                                    int32_t bytes_to_write,
                                    scoped_refptr<TrackedCallback> callback,
                                    int64_t granted);
@@ -199,8 +199,7 @@ class PPAPI_PROXY_EXPORT FileIOResource
   int32_t OnReadComplete(scoped_refptr<ReadOp> read_op,
                          PP_ArrayOutput array_output,
                          int32_t result);
-  int32_t OnWriteComplete(scoped_refptr<WriteOp> write_op,
-                          int32_t result);
+  int32_t OnWriteComplete(int32_t result);
 
   // Reply message handlers for operations that are done in the host.
   void OnPluginMsgGeneralComplete(scoped_refptr<TrackedCallback> callback,
diff --git a/ppapi/tests/test_file_io.cc b/ppapi/tests/test_file_io.cc
index 9adb6e6..5b26804 100644
--- a/ppapi/tests/test_file_io.cc
+++ b/ppapi/tests/test_file_io.cc
@@ -11,6 +11,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <algorithm>
 #include <vector>
 
 #include "ppapi/c/pp_errors.h"
@@ -875,12 +876,20 @@ std::string TestFileIO::TestParallelWrites() {
   int32_t rv_2 = PP_OK;
   while (size_1 >= 0 && size_2 >= 0 && size_1 + size_2 > 0) {
     if (size_1 > 0) {
-      rv_1 = file_io.Write(write_offset_1, buf_1, size_1,
-                           callback_1.GetCallback());
+      // Copy the buffer so we can erase it below.
+      std::string str_1(buf_1);
+      rv_1 = file_io.Write(
+          write_offset_1, &str_1[0], str_1.size(), callback_1.GetCallback());
+      // Erase the buffer to test that async writes copy it.
+      std::fill(str_1.begin(), str_1.end(), 0);
     }
     if (size_2 > 0) {
-      rv_2 = file_io.Write(write_offset_2, buf_2, size_2,
-                           callback_2.GetCallback());
+      // Copy the buffer so we can erase it below.
+      std::string str_2(buf_2);
+      rv_2 = file_io.Write(
+          write_offset_2, &str_2[0], str_2.size(), callback_2.GetCallback());
+      // Erase the buffer to test that async writes copy it.
+      std::fill(str_2.begin(), str_2.end(), 0);
     }
 
     if (size_1 > 0) {