Refactor FileIO to the new design

content/renderer/pepper/pepper_file_io_host.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 "content/renderer/pepper/pepper_file_io_host.h"
+
+#include <string>

[raymes, 2012/11/27 06:54:28]

This should probably be in the header instead

[victorhsieh, 2012/11/27 09:44:42]

Done.

[raymes, 2012/11/27 16:41:30]

I think you forgot to upload a patch set because I don't see this (or the
comments below) fixed.

[victorhsieh, 2012/11/28 04:11:55]

Oops, done.

+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/file_util_proxy.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/host/dispatch_host_message.h"
+#include "ppapi/host/ppapi_host.h"
+#include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/shared_impl/file_type_conversion.h"
+#include "ppapi/shared_impl/file_type_conversion.h"

[raymes, 2012/11/27 06:54:28]

you've included this twice

[victorhsieh, 2012/11/27 09:44:42]

Done.

+#include "ppapi/shared_impl/time_conversion.h"
+#include "ppapi/shared_impl/time_conversion.h"

[raymes, 2012/11/27 06:54:28]

you've included this twice

[victorhsieh, 2012/11/27 09:44:42]

Done.

+#include "webkit/fileapi/file_system_callback_dispatcher.h"
+#include "webkit/plugins/ppapi/file_callbacks.h"
+#include "webkit/plugins/ppapi/ppb_file_ref_impl.h"
+#include "webkit/plugins/ppapi/quota_file_io.h"
+
+namespace content {
+
+using ppapi::PPTimeToTime;
+using ppapi::TimeToPPTime;
+using ppapi::thunk::PPB_FileRef_API;
+using webkit::ppapi::PPB_FileRef_Impl;
+using webkit::ppapi::PluginDelegate;

[raymes, 2012/11/27 06:54:28]

I think this list is slightly out of order (at least the last line is)

[victorhsieh, 2012/11/27 09:44:42]

How should I sort them?  Capital letter shouldn't come first?

[raymes, 2012/11/27 16:41:30]

Oh sorry you're right the order is good.

+
+namespace {
+
+// The maximum size we'll support reading in one chunk. The renderer process
+// must allocate a buffer sized according to the request of the plugin. To
+// keep things from getting out of control, we cap the read size to this value.
+// This should generally be OK since the API specifies that it may perform a
+// partial read.
+static const int32_t kMaxReadSize = 32 * 1024 * 1024;  // 32MB
+
+typedef base::Callback<void (base::PlatformFileError)> PlatformGeneralCallback;
+
+class PlatformGeneralCallbackTranslator
+    : public fileapi::FileSystemCallbackDispatcher {
+ public:
+  PlatformGeneralCallbackTranslator(
+      const PlatformGeneralCallback& callback)

[raymes, 2012/11/27 06:54:28]

nit: push this onto the previous line

[victorhsieh, 2012/11/27 09:44:42]

Done.

+    : callback_(callback) {}
+
+  virtual ~PlatformGeneralCallbackTranslator() {}
+
+  virtual void DidSucceed() OVERRIDE {
+    callback_.Run(base::PLATFORM_FILE_OK);
+  }
+
+  virtual void DidReadMetadata(
+      const base::PlatformFileInfo& file_info,

[raymes, 2012/11/27 06:54:28]

nit: Push this onto the previous line

[victorhsieh, 2012/11/27 09:44:42]

Done.

+      const FilePath& platform_path) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidReadDirectory(
+      const std::vector<base::FileUtilProxy::Entry>& entries,
+      bool has_more) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidOpenFileSystem(const std::string& name,
+                                 const GURL& root) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidFail(base::PlatformFileError error_code) OVERRIDE {
+    callback_.Run(error_code);
+  }
+
+  virtual void DidWrite(int64 bytes, bool complete) OVERRIDE {
+    NOTREACHED();
+  }
+
+  virtual void DidOpenFile(base::PlatformFile file) OVERRIDE {
+    NOTREACHED();
+  }
+
+ private:
+  PlatformGeneralCallback callback_;
+};
+
+}  // namespace
+
+PepperFileIOHost::PepperFileIOHost(RendererPpapiHost* host,
+                                   PP_Instance instance,
+                                   PP_Resource resource)
+    : ResourceHost(host->GetPpapiHost(), instance, resource),
+      ppapi::PPB_FileIO_Shared(),
+      plugin_delegate_(host->GetDelegateForInstance(instance)),
+      file_(base::kInvalidPlatformFileValue),
+      file_system_type_(PP_FILESYSTEMTYPE_INVALID),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+}
+
+PepperFileIOHost::~PepperFileIOHost() {

[raymes, 2012/11/27 06:54:28]

Should this call into the same code as OnHostMsgClose()?

[victorhsieh, 2012/11/27 09:44:42]

Yes, the old implementation does, so I made it the same now.

+}
+
+int32_t PepperFileIOHost::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    ppapi::host::HostMessageContext* context) {
+  IPC_BEGIN_MESSAGE_MAP(PepperFileIOHost, msg)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Open,
+                                      OnHostMsgOpen)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_FileIO_Query,
+                                        OnHostMsgQuery)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Touch,
+                                      OnHostMsgTouch)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Read,
+                                      OnHostMsgRead)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_Write,
+                                      OnHostMsgWrite)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_SetLength,
+                                      OnHostMsgSetLength)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_FileIO_Flush,
+                                        OnHostMsgFlush)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_FileIO_Close,
+                                        OnHostMsgClose)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_WillWrite,
+                                      OnHostMsgWillWrite)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_FileIO_WillSetLength,
+                                      OnHostMsgWillSetLength)
+  IPC_END_MESSAGE_MAP()
+  return PP_ERROR_FAILED;
+}
+
+int32_t PepperFileIOHost::OnHostMsgOpen(
+    ppapi::host::HostMessageContext* context,
+    PP_Resource file_ref_resource,
+    int32_t open_flags) {
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoOpen(file_ref_resource, open_flags);
+}
+
+int32_t PepperFileIOHost::OnHostMsgQuery(
+    ppapi::host::HostMessageContext* context) {
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoQuery();
+}
+
+int32_t PepperFileIOHost::OnHostMsgTouch(
+    ppapi::host::HostMessageContext* context,
+    PP_Time last_access_time,
+    PP_Time last_modified_time) {
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoTouch(last_access_time, last_modified_time);
+}
+
+int32_t PepperFileIOHost::OnHostMsgRead(
+    ppapi::host::HostMessageContext* context,
+    int64_t offset,
+    int32_t bytes_to_read) {
+

[raymes, 2012/11/27 06:54:28]

uneeded newline

[victorhsieh, 2012/11/27 09:44:42]

Done.

+  // Validate bytes_to_read before allocating below. This value is coming from
+  // the untrusted plugin.
+  if (bytes_to_read < 0) {
+    ppapi::host::ReplyMessageContext reply_context =
+        context->MakeReplyMessageContext();
+    reply_context.params.set_result(PP_ERROR_FAILED);
+    host()->SendReply(reply_context,
+                      PpapiPluginMsg_FileIO_ReadComplete(std::string()));
+    return PP_OK_COMPLETIONPENDING;
+  }
+
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoRead(offset, std::min(bytes_to_read, kMaxReadSize));
+}
+
+int32_t PepperFileIOHost::OnHostMsgWrite(
+    ppapi::host::HostMessageContext* context,
+    int64_t offset,
+    const std::string& buffer) {
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoWrite(offset, buffer.c_str(), buffer.size());
+}
+
+int32_t PepperFileIOHost::OnHostMsgSetLength(
+    ppapi::host::HostMessageContext* context,
+    int64_t length) {
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoSetLength(length);
+}
+
+int32_t PepperFileIOHost::OnHostMsgFlush(
+    ppapi::host::HostMessageContext* context) {
+  temp_reply_context_ = context->MakeReplyMessageContext();
+  return DoFlush();
+}
+
+int32_t PepperFileIOHost::OnHostMsgClose(
+    ppapi::host::HostMessageContext* context) {
+  if (file_ != base::kInvalidPlatformFileValue && plugin_delegate_) {
+    base::FileUtilProxy::Close(
+        plugin_delegate_->GetFileThreadMessageLoopProxy(),

[raymes, 2012/11/27 06:54:28]

Please look at the code for this function in the delegate. You should be able to
replace this call to the delegate with that code directly. Similarly for other
uses of the delegate. The aim is to remove all of these functions from the
delegate.

That said, it's probably better to do this in a separate CL. So please just add
a TODO (probably next to the definition of the plugin_delegate_ instance
variable)

[victorhsieh, 2012/11/27 09:44:42]

Done.  Leave a TODO.

+        file_,
+        base::ResetAndReturn(&notify_close_file_callback_));
+    file_ = base::kInvalidPlatformFileValue;
+    quota_file_io_.reset();
+  }
+  return PP_OK;
+}
+
+int32_t PepperFileIOHost::OnHostMsgWillWrite(
+    ppapi::host::HostMessageContext* context,
+    int64_t offset,
+    int32_t bytes_to_write) {
+  int32_t rv = CommonPreCondition(true, OPERATION_EXCLUSIVE);
+  if (rv != PP_OK)
+    return rv;
+
+  if (!quota_file_io_.get())
+    return PP_OK;
+
+  if (!quota_file_io_->WillWrite(
+          offset, bytes_to_write,
+          base::Bind(&PepperFileIOHost::ExecutePlatformWillWriteCallback,
+                     weak_factory_.GetWeakPtr(),
+                     context->MakeReplyMessageContext())))
+    return PP_ERROR_FAILED;
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::OnHostMsgWillSetLength(
+    ppapi::host::HostMessageContext* context,
+    int64_t length) {
+  int32_t rv = CommonPreCondition(true, OPERATION_EXCLUSIVE);
+  if (rv != PP_OK)
+    return rv;
+
+  if (!quota_file_io_.get())
+    return PP_OK;
+
+  if (!quota_file_io_->WillSetLength(
+          length,
+          base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
+                     weak_factory_.GetWeakPtr(),
+                     context->MakeReplyMessageContext())))
+    return PP_ERROR_FAILED;
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::CommonPreCondition(bool should_be_open,
+                                             OperationType new_op) {
+  if (!plugin_delegate_ || !CheckOpenState(should_be_open))
+    return PP_ERROR_FAILED;
+  return PP_OK;
+}
+
+void PepperFileIOHost::CommonPostCondition(OperationType new_op) {
+}
+
+int32_t PepperFileIOHost::OpenValidated(
+    PP_Resource file_ref_resource,
+    PPB_FileRef_API* file_ref_api,
+    int32_t open_flags) {
+  int flags = 0;
+  if (!::ppapi::PepperFileOpenFlagsToPlatformFileFlags(open_flags, &flags))
+    return PP_ERROR_BADARGUMENT;
+
+  PP_FileSystemType type = file_ref_api->GetFileSystemType();
+  if (type != PP_FILESYSTEMTYPE_LOCALPERSISTENT &&
+      type != PP_FILESYSTEMTYPE_LOCALTEMPORARY &&
+      type != PP_FILESYSTEMTYPE_EXTERNAL)
+    return PP_ERROR_FAILED;
+  file_system_type_ = type;
+
+  PPB_FileRef_Impl* file_ref = static_cast<PPB_FileRef_Impl*>(file_ref_api);
+  if (file_ref->HasValidFileSystem()) {
+    file_system_url_ = file_ref->GetFileSystemURL();
+    if (!plugin_delegate_->AsyncOpenFileSystemURL(
+            file_system_url_, flags,
+            base::Bind(
+                &PepperFileIOHost::ExecutePlatformOpenFileSystemURLCallback,
+                weak_factory_.GetWeakPtr(),
+                temp_reply_context_)))
+      return PP_ERROR_FAILED;
+  } else {
+    if (file_system_type_ != PP_FILESYSTEMTYPE_EXTERNAL)
+      return PP_ERROR_FAILED;
+    if (!plugin_delegate_->AsyncOpenFile(
+            file_ref->GetSystemPath(), flags,
+            base::Bind(&PepperFileIOHost::ExecutePlatformOpenFileCallback,
+                       weak_factory_.GetWeakPtr(),
+                       temp_reply_context_)))
+      return PP_ERROR_FAILED;
+  }
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::QueryValidated() {
+  if (!base::FileUtilProxy::GetFileInfoFromPlatformFile(
+          plugin_delegate_->GetFileThreadMessageLoopProxy(), file_,
+          base::Bind(&PepperFileIOHost::ExecutePlatformQueryCallback,
+                     weak_factory_.GetWeakPtr(),
+                     temp_reply_context_)))
+    return PP_ERROR_FAILED;
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::TouchValidated(
+    PP_Time last_access_time,
+    PP_Time last_modified_time) {
+  if (file_system_type_ != PP_FILESYSTEMTYPE_EXTERNAL) {
+    if (!plugin_delegate_->Touch(
+            file_system_url_,
+            PPTimeToTime(last_access_time),
+            PPTimeToTime(last_modified_time),
+            new PlatformGeneralCallbackTranslator(
+                base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
+                           weak_factory_.GetWeakPtr(),
+                           temp_reply_context_))))
+      return PP_ERROR_FAILED;
+    return PP_OK_COMPLETIONPENDING;
+  }
+
+  // TODO(nhiroki): fix a failure of FileIO.Touch for an external filesystem on
+  // Mac and Linux due to sandbox restrictions (http://crbug.com/101128).
+  if (!base::FileUtilProxy::Touch(
+          plugin_delegate_->GetFileThreadMessageLoopProxy(),
+          file_, PPTimeToTime(last_access_time),
+          PPTimeToTime(last_modified_time),
+          base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
+                     weak_factory_.GetWeakPtr(),
+                     temp_reply_context_)))
+    return PP_ERROR_FAILED;
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::ReadValidated(
+    int64_t offset,
+    int32_t max_read_length) {
+  if (!base::FileUtilProxy::Read(
+          plugin_delegate_->GetFileThreadMessageLoopProxy(), file_, offset,
+          max_read_length,
+          base::Bind(&PepperFileIOHost::ExecutePlatformReadCallback,
+                     weak_factory_.GetWeakPtr(),
+                     temp_reply_context_)))
+    return PP_ERROR_FAILED;
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::WriteValidated(
+    int64_t offset,
+    const char* buffer,
+    int32_t bytes_to_write) {
+  if (quota_file_io_.get()) {
+    if (!quota_file_io_->Write(
+            offset, buffer, bytes_to_write,
+            base::Bind(&PepperFileIOHost::ExecutePlatformWriteCallback,
+                       weak_factory_.GetWeakPtr(),
+                       temp_reply_context_)))
+      return PP_ERROR_FAILED;
+  } else {
+    if (!base::FileUtilProxy::Write(
+            plugin_delegate_->GetFileThreadMessageLoopProxy(), file_, offset,
+            buffer, bytes_to_write,
+            base::Bind(&PepperFileIOHost::ExecutePlatformWriteCallback,
+                       weak_factory_.GetWeakPtr(),
+                       temp_reply_context_)))
+      return PP_ERROR_FAILED;
+  }
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::SetLengthValidated(
+    int64_t length) {
+  if (quota_file_io_.get()) {
+    if (!quota_file_io_->SetLength(

[raymes, 2012/11/27 06:54:28]

Why does the existing implementation call plugin_delegate.SetLength here as
opposed to quota_file_io_.SetLength? Also this happens under a different
condition

[victorhsieh, 2012/11/27 09:44:42]

Thanks for catching!  A change (http://crrev.com/164030) happened after my
initial change so I didn't realize it.

+            length,
+            base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
+                       weak_factory_.GetWeakPtr(),
+                       temp_reply_context_)))
+      return PP_ERROR_FAILED;
+  } else {
+    if (!base::FileUtilProxy::Truncate(

[raymes, 2012/11/27 06:54:28]

You've erased a TODO here. Is http://crbug.com/156077 no longer applicable here?

[victorhsieh, 2012/11/27 09:44:42]

Oops!  Have put it back.

+            plugin_delegate_->GetFileThreadMessageLoopProxy(), file_, length,
+            base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
+                       weak_factory_.GetWeakPtr(),
+                       temp_reply_context_)))
+      return PP_ERROR_FAILED;
+  }
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperFileIOHost::FlushValidated() {
+  if (!base::FileUtilProxy::Flush(
+          plugin_delegate_->GetFileThreadMessageLoopProxy(), file_,
+          base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
+                     weak_factory_.GetWeakPtr(),
+                     temp_reply_context_)))
+    return PP_ERROR_FAILED;
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PepperFileIOHost::ExecutePlatformGeneralCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code) {
+  reply_context.params.set_result(
+      ::ppapi::PlatformFileErrorToPepperError(error_code));
+  host()->SendReply(reply_context, PpapiPluginMsg_FileIO_GeneralComplete());
+}
+
+void PepperFileIOHost::ExecutePlatformOpenFileCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code,
+    base::PassPlatformFile file) {
+  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);
+  if (pp_error == PP_OK)
+    SetOpenSucceed();
+
+  DCHECK(file_ == base::kInvalidPlatformFileValue);
+  file_ = file.ReleaseValue();
+
+  DCHECK(!quota_file_io_.get());
+  if (file_ != base::kInvalidPlatformFileValue &&
+      (file_system_type_ == PP_FILESYSTEMTYPE_LOCALTEMPORARY ||
+       file_system_type_ == PP_FILESYSTEMTYPE_LOCALPERSISTENT)) {
+    quota_file_io_.reset(new webkit::ppapi::QuotaFileIO(
+        pp_instance(), file_, file_system_url_, file_system_type_));
+  }
+
+  reply_context.params.set_result(pp_error);
+  host()->SendReply(reply_context, PpapiPluginMsg_FileIO_OpenFileComplete());
+}
+
+void PepperFileIOHost::ExecutePlatformOpenFileSystemURLCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code,
+    base::PassPlatformFile file,
+    const PluginDelegate::NotifyCloseFileCallback& callback) {
+  if (error_code == base::PLATFORM_FILE_OK)
+    notify_close_file_callback_ = callback;
+  ExecutePlatformOpenFileCallback(reply_context, error_code, file);
+}
+
+void PepperFileIOHost::ExecutePlatformQueryCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code,
+    const base::PlatformFileInfo& file_info) {
+  PP_FileInfo pp_info;
+  pp_info.size = file_info.size;
+  pp_info.creation_time = TimeToPPTime(file_info.creation_time);
+  pp_info.last_access_time = TimeToPPTime(file_info.last_accessed);
+  pp_info.last_modified_time = TimeToPPTime(file_info.last_modified);
+  pp_info.system_type = file_system_type_;
+  if (file_info.is_directory)
+    pp_info.type = PP_FILETYPE_DIRECTORY;
+  else
+    pp_info.type = PP_FILETYPE_REGULAR;
+
+  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);
+  reply_context.params.set_result(pp_error);
+  host()->SendReply(reply_context,
+                    PpapiPluginMsg_FileIO_QueryComplete(pp_info));
+}
+
+void PepperFileIOHost::ExecutePlatformReadCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code,
+    const char* data, int bytes_read) {
+  // Map the error code, OK getting mapped to the # of bytes read.
+  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);
+  pp_error = pp_error == PP_OK ? bytes_read : pp_error;

[raymes, 2012/11/27 06:54:28]

I think it would be better to avoid doing funny stuff with pp_error in this
function. It makes things very confusing and I guess there is no longer a good
reason to do this.

[victorhsieh, 2012/11/27 09:44:42]

The callback still expect a parameter that is bytes_read if non-negative, or
pp_error if negative.  We can make the code readable here, but still need to
translate again in the resource.

I leave a comment here for now, but am happy to change for a benefit to move to
resource.

+
+  // Debug checks

[raymes, 2012/11/27 06:54:28]

I would get rid of these checks as well.

[victorhsieh, 2012/11/27 09:44:42]

Done.

+  if (pp_error >= 0)
+    DCHECK(pp_error <= bytes_read);
+  else
+    DCHECK(bytes_read == 0);
+
+  // Only send the amount of data in the string that was actually read.
+  std::string buffer(data, pp_error > 0 ? pp_error : 0);

[raymes, 2012/11/27 06:54:28]

Just change this to:
std::string buffer;
if (pp_error == PP_OK)
  buffer.append(data, bytes_read);
reply_context.params.set_result(pp_error);

(or equivalent)

[victorhsieh, 2012/11/27 09:44:42]

Done.

+  reply_context.params.set_result(pp_error);
+  host()->SendReply(reply_context,
+                    PpapiPluginMsg_FileIO_ReadComplete(buffer));
+}
+
+void PepperFileIOHost::ExecutePlatformWriteCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code,
+    int bytes_written) {
+  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);
+  reply_context.params.set_result(pp_error == PP_OK ? bytes_written : pp_error);

[raymes, 2012/11/27 06:54:28]

It would be better to set the result to pp_error and return the # bytes in the
reply message.

[victorhsieh, 2012/11/27 09:44:42]

Same as read.  Leave a comment for now.  I'm happy to change if there is any
reason to move it to resource.

+  host()->SendReply(reply_context, PpapiPluginMsg_FileIO_GeneralComplete());
+}
+
+void PepperFileIOHost::ExecutePlatformWillWriteCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::PlatformFileError error_code,
+    int bytes_written) {
+  if (error_code != base::PLATFORM_FILE_OK)
+    reply_context.params.set_result(
+        ::ppapi::PlatformFileErrorToPepperError(error_code));
+  else
+    reply_context.params.set_result(bytes_written);

[raymes, 2012/11/27 06:54:28]

Same here

[victorhsieh, 2012/11/27 09:44:42]

Done.

+  host()->SendReply(reply_context,
+                    PpapiPluginMsg_FileIO_GeneralComplete());
+}
+
+}  // namespace content