Replace FileUtilProxy with FileProxy in renderer_host/pepper

BUG=322664

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

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

Author: rvargas@chromium.org

base/files/file_proxy.cc

@@ -265,10 +265,19 @@ bool FileProxy::IsValid() const {
   return file_.IsValid();
 }
 
+void FileProxy::SetFile(File file) {
+  DCHECK(!file_.IsValid());
+  file_ = file.Pass();
+}
+
 File FileProxy::TakeFile() {
   return file_.Pass();
 }
 
+PlatformFile FileProxy::GetPlatformFile() const {
+  return file_.GetPlatformFile();
+}
+
 bool FileProxy::Close(const StatusCallback& callback) {
   DCHECK(file_.IsValid());
   GenericFileHelper* helper = new GenericFileHelper(this, file_.Pass());
@@ -347,9 +356,4 @@ bool FileProxy::Flush(const StatusCallback& callback) {
       Bind(&GenericFileHelper::Reply, Owned(helper), callback));
 }
 
-void FileProxy::SetFile(File file) {
-  DCHECK(!file_.IsValid());
-  file_ = file.Pass();
-}
-
 }  // namespace base

base/files/file_proxy.h

@@ -33,12 +33,9 @@ class Time;
 // In other words, having a sequence like
 //
 //   proxy.Write(...);
-//   delete proxy;
+//   proxy.Write(...);
 //
-// will keep the file valid during the Write operation but will cause the file
-// to be closed in the current thread, when the operation finishes. If Close is
-// called right away after Write, the second call will fail because there is an
-// operation in progress.
+// means the second Write will always fail.
 class BASE_EXPORT FileProxy : public SupportsWeakPtr<FileProxy> {
  public:
   // This callback is used by methods that report only an error code. It is
@@ -88,8 +85,14 @@ class BASE_EXPORT FileProxy : public SupportsWeakPtr<FileProxy> {
   // length to simulate a new file), and false otherwise.
   bool created() const { return file_.created(); }
 
+  // Claims ownership of |file|. It is an error to call this method when
+  // IsValid() returns true.
+  void SetFile(File file);
+
   File TakeFile();
 
+  PlatformFile GetPlatformFile() const;
+
   // Proxies File::Close. The callback can be null.
   // This returns false if task posting to |task_runner| has failed.
   bool Close(const StatusCallback& callback);
@@ -127,7 +130,6 @@ class BASE_EXPORT FileProxy : public SupportsWeakPtr<FileProxy> {
 
  private:
   friend class FileHelper;
-  void SetFile(File file);
   TaskRunner* task_runner() { return task_runner_.get(); }
 
   scoped_refptr<TaskRunner> task_runner_;

base/files/file_proxy_unittest.cc

@@ -207,6 +207,20 @@ TEST_F(FileProxyTest, CreateTemporary) {
   EXPECT_TRUE(base::DeleteFile(path_, false));
 }
 
+TEST_F(FileProxyTest, SetAndTake) {
+  File file(test_path(), File::FLAG_CREATE | File::FLAG_READ);
+  ASSERT_TRUE(file.IsValid());
+  FileProxy proxy(file_task_runner());
+  EXPECT_FALSE(proxy.IsValid());
+  proxy.SetFile(file.Pass());
+  EXPECT_TRUE(proxy.IsValid());
+  EXPECT_FALSE(file.IsValid());
+
+  file = proxy.TakeFile();
+  EXPECT_FALSE(proxy.IsValid());
+  EXPECT_TRUE(file.IsValid());
+}
+
 TEST_F(FileProxyTest, GetInfo) {
   // Setup.
   ASSERT_EQ(4, base::WriteFile(test_path(), "test", 4));

content/browser/renderer_host/pepper/pepper_file_io_host.cc

@@ -86,7 +86,7 @@ PepperFileIOHost::PepperFileIOHost(BrowserPpapiHostImpl* host,
     : ResourceHost(host->GetPpapiHost(), instance, resource),
       browser_ppapi_host_(host),
       render_process_host_(NULL),
-      file_(base::kInvalidPlatformFileValue),
+      file_(BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)),
       open_flags_(0),
       file_system_type_(PP_FILESYSTEMTYPE_INVALID),
       max_written_offset_(0),
@@ -97,8 +97,6 @@ PepperFileIOHost::PepperFileIOHost(BrowserPpapiHostImpl* host,
           instance, &render_process_id_, &unused)) {
     render_process_id_ = -1;
   }
-  file_message_loop_ =
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE);
 }
 
 PepperFileIOHost::~PepperFileIOHost() {}
@@ -265,15 +263,14 @@ void PepperFileIOHost::DidOpenInternalFile(
 void PepperFileIOHost::GotResolvedRenderProcessId(
     ppapi::host::ReplyMessageContext reply_context,
     base::FilePath path,
-    int platform_file_flags,
+    int file_flags,
     base::ProcessId resolved_render_process_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   resolved_render_process_id_ = resolved_render_process_id;
-  base::FileUtilProxy::CreateOrOpen(
-      file_message_loop_,
+  file_.CreateOrOpen(
       path,
-      platform_file_flags,
-      base::Bind(&PepperFileIOHost::ExecutePlatformOpenFileCallback,
+      file_flags,
+      base::Bind(&PepperFileIOHost::OnOpenProxyCallback,
                  weak_factory_.GetWeakPtr(),
                  reply_context));
 }
@@ -287,15 +284,14 @@ int32_t PepperFileIOHost::OnHostMsgTouch(
   if (rv != PP_OK)
     return rv;
 
-  if (!base::FileUtilProxy::Touch(
-          file_message_loop_,
-          file_,
+  if (!file_.SetTimes(
           PPTimeToTime(last_access_time),
           PPTimeToTime(last_modified_time),
           base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
                      weak_factory_.GetWeakPtr(),
-                     context->MakeReplyMessageContext())))
+                     context->MakeReplyMessageContext()))) {
     return PP_ERROR_FAILED;
+  }
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
@@ -314,14 +310,13 @@ int32_t PepperFileIOHost::OnHostMsgSetLength(
   // Quota checks are performed on the plugin side, in order to use the same
   // quota reservation and request system as Write.
 
-  if (!base::FileUtilProxy::Truncate(
-          file_message_loop_,
-          file_,
+  if (!file_.SetLength(
           length,
           base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
                      weak_factory_.GetWeakPtr(),
-                     context->MakeReplyMessageContext())))
+                     context->MakeReplyMessageContext()))) {
     return PP_ERROR_FAILED;
+  }
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
@@ -334,13 +329,12 @@ int32_t PepperFileIOHost::OnHostMsgFlush(
   if (rv != PP_OK)
     return rv;
 
-  if (!base::FileUtilProxy::Flush(
-          file_message_loop_,
-          file_,
+  if (!file_.Flush(
           base::Bind(&PepperFileIOHost::ExecutePlatformGeneralCallback,
                      weak_factory_.GetWeakPtr(),
-                     context->MakeReplyMessageContext())))
+                     context->MakeReplyMessageContext()))) {
     return PP_ERROR_FAILED;
+  }
 
   state_manager_.SetPendingOperation(FileIOStateManager::OPERATION_EXCLUSIVE);
   return PP_OK_COMPLETIONPENDING;
@@ -354,12 +348,9 @@ int32_t PepperFileIOHost::OnHostMsgClose(
     check_quota_ = false;
   }
 
-  if (file_ != base::kInvalidPlatformFileValue) {
-    base::FileUtilProxy::Close(file_message_loop_,
-                               file_,
-                               base::Bind(&PepperFileIOHost::DidCloseFile,
-                                          weak_factory_.GetWeakPtr()));
-    file_ = base::kInvalidPlatformFileValue;
+  if (file_.IsValid()) {
+    file_.Close(base::Bind(&PepperFileIOHost::DidCloseFile,
+                           weak_factory_.GetWeakPtr()));
   }
   return PP_OK;
 }
@@ -425,17 +416,23 @@ void PepperFileIOHost::ExecutePlatformGeneralCallback(
   state_manager_.SetOperationFinished();
 }
 
+// TODO(rvargas): this method should go away when FileApi moves to use File.
 void PepperFileIOHost::ExecutePlatformOpenFileCallback(
     ppapi::host::ReplyMessageContext reply_context,
     base::File::Error error_code,
     base::PassPlatformFile file,
     bool unused_created) {
-  int32_t pp_error = ppapi::FileErrorToPepperError(error_code);
-  DCHECK(file_ == base::kInvalidPlatformFileValue);
-  file_ = file.ReleaseValue();
+  DCHECK(!file_.IsValid());
+  file_.SetFile(base::File(file.ReleaseValue()));
 
-  if (file_ != base::kInvalidPlatformFileValue &&
-      !AddFileToReplyContext(open_flags_, &reply_context))
+  OnOpenProxyCallback(reply_context, error_code);
+}
+
+void PepperFileIOHost::OnOpenProxyCallback(
+    ppapi::host::ReplyMessageContext reply_context,
+    base::File::Error error_code) {
+  int32_t pp_error = ppapi::FileErrorToPepperError(error_code);
+  if (file_.IsValid() && !AddFileToReplyContext(open_flags_, &reply_context))
     pp_error = PP_ERROR_FAILED;
 
   PP_Resource quota_file_system = 0;
@@ -467,7 +464,8 @@ bool PepperFileIOHost::AddFileToReplyContext(
     plugin_process_id = resolved_render_process_id_;
 
   IPC::PlatformFileForTransit transit_file =
-      BrokerGetFileHandleForProcess(file_, plugin_process_id, false);
+      BrokerGetFileHandleForProcess(file_.GetPlatformFile(), plugin_process_id,
+                                    false);
   if (transit_file == IPC::InvalidPlatformFileForTransit())
     return false;
 

content/browser/renderer_host/pepper/pepper_file_io_host.h

@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
 #include "base/files/file.h"
+#include "base/files/file_proxy.h"
 #include "base/memory/weak_ptr.h"
 #include "base/platform_file.h"
 #include "content/browser/renderer_host/pepper/browser_ppapi_host_impl.h"
@@ -84,6 +85,9 @@ class PepperFileIOHost : public ppapi::host::ResourceHost,
       base::PassPlatformFile file,
       bool unused_created);
 
+  void OnOpenProxyCallback(ppapi::host::ReplyMessageContext reply_context,
+                           base::File::Error error_code);
+
   void GotUIThreadStuffForInternalFileSystems(
       ppapi::host::ReplyMessageContext reply_context,
       int platform_file_flags,
@@ -95,7 +99,7 @@ class PepperFileIOHost : public ppapi::host::ResourceHost,
   void GotResolvedRenderProcessId(
       ppapi::host::ReplyMessageContext reply_context,
       base::FilePath path,
-      int platform_file_flags,
+      int file_flags,
       base::ProcessId resolved_render_process_id);
 
   void DidOpenQuotaFile(ppapi::host::ReplyMessageContext reply_context,
@@ -119,7 +123,7 @@ class PepperFileIOHost : public ppapi::host::ResourceHost,
   int render_process_id_;
   base::ProcessId resolved_render_process_id_;
 
-  base::PlatformFile file_;
+  base::FileProxy file_;
   int32_t open_flags_;
 
   // The file system type specified in the Open() call. This will be
@@ -137,8 +141,6 @@ class PepperFileIOHost : public ppapi::host::ResourceHost,
 
   ppapi::FileIOStateManager state_manager_;
 
-  scoped_refptr<base::MessageLoopProxy> file_message_loop_;
-
   base::WeakPtrFactory<PepperFileIOHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PepperFileIOHost);