PNaCl cleanup: Remove TempFile class and use base::File instead

Following earlier cleanups, TempFile is now just a wrapper around
base::File anyway.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

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

Cr-Commit-Position: refs/heads/master@{#370752}

components/nacl/renderer/plugin/BUILD.gn

@@ -12,7 +12,6 @@ source_set("nacl_trusted_plugin") {
     "pnacl_translate_thread.cc",
     "ppapi_entrypoints.cc",
     "service_runtime.cc",
-    "temporary_file.cc",
     "utility.cc",
   ]
 

components/nacl/renderer/plugin/plugin.gypi

@@ -14,7 +14,6 @@
       'pnacl_translate_thread.cc',
       'ppapi_entrypoints.cc',
       'service_runtime.cc',
-      'temporary_file.cc',
       'utility.cc',
     ],
   },

components/nacl/renderer/plugin/pnacl_coordinator.cc

@@ -13,7 +13,6 @@
 #include "components/nacl/renderer/plugin/plugin_error.h"
 #include "components/nacl/renderer/plugin/pnacl_translate_thread.h"
 #include "components/nacl/renderer/plugin/service_runtime.h"
-#include "components/nacl/renderer/plugin/temporary_file.h"
 #include "ppapi/c/pp_bool.h"
 #include "ppapi/c/pp_errors.h"
 
@@ -128,13 +127,11 @@ PnaclCoordinator::~PnaclCoordinator() {
   // since the thread may be accessing those fields.
   // It will also be accessing obj_files_.
   translate_thread_.reset(NULL);
-  for (size_t i = 0; i < obj_files_.size(); i++)
-    delete obj_files_[i];
 }
 
 PP_FileHandle PnaclCoordinator::TakeTranslatedFileHandle() {
-  DCHECK(temp_nexe_file_ != NULL);
-  return temp_nexe_file_->TakeFileHandle();
+  DCHECK(temp_nexe_file_.IsValid());
+  return temp_nexe_file_.TakePlatformFile();
 }
 
 void PnaclCoordinator::ReportNonPpapiError(PP_NaClError err_code,
@@ -187,10 +184,10 @@ void PnaclCoordinator::TranslateFinished(int32_t pp_error) {
                                       pexe_bytes_compiled_,
                                       expected_pexe_size_);
   }
-  int64_t nexe_size = temp_nexe_file_->GetLength();
-  // The nexe is written to the temp_nexe_file_.  We must Reset() the file
+  int64_t nexe_size = temp_nexe_file_.GetLength();
+  // The nexe is written to the temp_nexe_file_.  We must reset the file
   // pointer to be able to read it again from the beginning.
-  temp_nexe_file_->Reset();
+  temp_nexe_file_.Seek(base::File::FROM_BEGIN, 0);
 
   // Report to the browser that translation finished. The browser will take
   // care of storing the nexe in the cache.
@@ -204,7 +201,7 @@ void PnaclCoordinator::TranslateFinished(int32_t pp_error) {
 }
 
 void PnaclCoordinator::NexeReadDidOpen() {
-  if (!temp_nexe_file_->IsValid()) {
+  if (!temp_nexe_file_.IsValid()) {
     ReportNonPpapiError(PP_NACL_ERROR_PNACL_CACHE_FETCH_OTHER,
                         "Failed to open translated nexe.");
     return;
@@ -240,8 +237,7 @@ void PnaclCoordinator::BitcodeStreamCacheHit(PP_FileHandle handle) {
     BitcodeStreamDidFinish(PP_ERROR_FAILED);
     return;
   }
-  temp_nexe_file_.reset(new TempFile(plugin_, handle));
-  // Open it for reading as the cached nexe file.
+  temp_nexe_file_ = base::File(handle);
   NexeReadDidOpen();
 }
 
@@ -277,22 +273,20 @@ void PnaclCoordinator::BitcodeStreamCacheMiss(int64_t expected_pexe_size,
   expected_pexe_size_ = expected_pexe_size;
 
   for (int i = 0; i < split_module_count_; i++) {
-    PP_FileHandle obj_handle =
-        plugin_->nacl_interface()->CreateTemporaryFile(plugin_->pp_instance());
-    scoped_ptr<TempFile> temp_file(new TempFile(plugin_, obj_handle));
-    if (!temp_file->IsValid()) {
+    base::File temp_file(
+        plugin_->nacl_interface()->CreateTemporaryFile(plugin_->pp_instance()));
+    if (!temp_file.IsValid()) {
       ReportNonPpapiError(PP_NACL_ERROR_PNACL_CREATE_TEMP,
                           "Failed to open scratch object file.");
       return;
-    } else {
-      obj_files_.push_back(temp_file.release());
     }
+    obj_files_.push_back(std::move(temp_file));
   }
 
-  temp_nexe_file_.reset(new TempFile(plugin_, nexe_handle));
+  temp_nexe_file_ = base::File(nexe_handle);
   // Open the nexe file for connecting ld and sel_ldr.
   // Start translation when done with this last step of setup!
-  if (!temp_nexe_file_->IsValid()) {
+  if (!temp_nexe_file_.IsValid()) {
     ReportNonPpapiError(
         PP_NACL_ERROR_PNACL_CREATE_TEMP,
         std::string(
@@ -419,7 +413,7 @@ void PnaclCoordinator::RunCompile(int32_t pp_error,
   CHECK(translate_thread_ != NULL);
   translate_thread_->SetupState(
       report_translate_finished, &compiler_subprocess_, &ld_subprocess_,
-      &obj_files_, num_threads_, temp_nexe_file_.get(),
+      &obj_files_, num_threads_, &temp_nexe_file_,
       &error_info_, &pnacl_options_, architecture_attributes_, this);
   translate_thread_->RunCompile(compile_finished);
 }

components/nacl/renderer/plugin/pnacl_coordinator.h

@@ -10,6 +10,7 @@
 
 #include <vector>
 
+#include "base/files/file.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/nacl/renderer/plugin/nacl_subprocess.h"
@@ -25,7 +26,6 @@ namespace plugin {
 class Plugin;
 class PnaclCoordinator;
 class PnaclTranslateThread;
-class TempFile;
 
 // A class invoked by Plugin to handle PNaCl client-side translation.
 // Usage:
@@ -154,14 +154,14 @@ class PnaclCoordinator {
   std::string architecture_attributes_;
 
   // Object file, produced by the translator and consumed by the linker.
-  std::vector<TempFile*> obj_files_;
+  std::vector<base::File> obj_files_;
   // Number of split modules for llc.
   int split_module_count_;
   // Number of threads for llc / subzero.
   int num_threads_;
 
   // Translated nexe file, produced by the linker.
-  scoped_ptr<TempFile> temp_nexe_file_;
+  base::File temp_nexe_file_;
 
   // Used to report information when errors (PPAPI or otherwise) are reported.
   ErrorInfo error_info_;

components/nacl/renderer/plugin/pnacl_translate_thread.cc

@@ -12,7 +12,6 @@
 #include "base/logging.h"
 #include "components/nacl/renderer/plugin/plugin.h"
 #include "components/nacl/renderer/plugin/plugin_error.h"
-#include "components/nacl/renderer/plugin/temporary_file.h"
 #include "components/nacl/renderer/plugin/utility.h"
 #include "content/public/common/sandbox_init.h"
 #include "native_client/src/shared/platform/nacl_sync_raii.h"
@@ -83,9 +82,9 @@ void PnaclTranslateThread::SetupState(
     const pp::CompletionCallback& finish_callback,
     NaClSubprocess* compiler_subprocess,
     NaClSubprocess* ld_subprocess,
-    const std::vector<TempFile*>* obj_files,
+    std::vector<base::File>* obj_files,
     int num_threads,
-    TempFile* nexe_file,
+    base::File* nexe_file,
     ErrorInfo* error_info,
     PP_PNaClOptions* pnacl_options,
     const std::string& architecture_attributes,
@@ -196,12 +195,13 @@ void PnaclTranslateThread::EndStream() {
 }
 
 ppapi::proxy::SerializedHandle PnaclTranslateThread::GetHandleForSubprocess(
-    TempFile* file, int32_t open_flags, base::ProcessId peer_pid) {
+    base::File* file, int32_t open_flags, base::ProcessId peer_pid) {
   IPC::PlatformFileForTransit file_for_transit;
 
+  DCHECK(file->IsValid());
 #if defined(OS_WIN)
   if (!content::BrokerDuplicateHandle(
-          file->GetFileHandle(),
+          file->GetPlatformFile(),
           peer_pid,
           &file_for_transit,
           0,  // desired_access is 0 since we're using DUPLICATE_SAME_ACCESS.
@@ -209,7 +209,7 @@ ppapi::proxy::SerializedHandle PnaclTranslateThread::GetHandleForSubprocess(
     return ppapi::proxy::SerializedHandle();
   }
 #else
-  file_for_transit = base::FileDescriptor(dup(file->GetFileHandle()), true);
+  file_for_transit = base::FileDescriptor(dup(file->GetPlatformFile()), true);
 #endif
 
   // Using 0 disables any use of quota enforcement for this file handle.
@@ -236,9 +236,9 @@ void PnaclTranslateThread::DoCompile() {
   }
 
   std::vector<ppapi::proxy::SerializedHandle> compiler_output_files;
-  for (TempFile* obj_file : *obj_files_) {
+  for (base::File& obj_file : *obj_files_) {
     compiler_output_files.push_back(
-        GetHandleForSubprocess(obj_file, PP_FILEOPENFLAG_WRITE,
+        GetHandleForSubprocess(&obj_file, PP_FILEOPENFLAG_WRITE,
                                compiler_channel_peer_pid_));
   }
 
@@ -365,10 +365,10 @@ void PnaclTranslateThread::DoLink() {
   }
 
   // Reset object files for reading first.  We do this before duplicating
-  // handles/FDs to prevent any handle/FD leaks in case any of the Reset()
+  // handles/FDs to prevent any handle/FD leaks in case any of the Seek()
   // calls fail.
-  for (TempFile* obj_file : *obj_files_) {
-    if (!obj_file->Reset()) {
+  for (base::File& obj_file : *obj_files_) {
+    if (obj_file.Seek(base::File::FROM_BEGIN, 0) != 0) {
       TranslateFailed(PP_NACL_ERROR_PNACL_LD_SETUP,
                       "Link process could not reset object file");
       return;
@@ -379,9 +379,9 @@ void PnaclTranslateThread::DoLink() {
       GetHandleForSubprocess(nexe_file_, PP_FILEOPENFLAG_WRITE,
                              ld_channel_peer_pid_);
   std::vector<ppapi::proxy::SerializedHandle> ld_input_files;
-  for (TempFile* obj_file : *obj_files_) {
+  for (base::File& obj_file : *obj_files_) {
     ld_input_files.push_back(
-        GetHandleForSubprocess(obj_file, PP_FILEOPENFLAG_READ,
+        GetHandleForSubprocess(&obj_file, PP_FILEOPENFLAG_READ,
                                ld_channel_peer_pid_));
   }
 

components/nacl/renderer/plugin/pnacl_translate_thread.h

@@ -10,6 +10,7 @@
 #include <deque>
 #include <vector>
 
+#include "base/files/file.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/nacl/renderer/plugin/plugin_error.h"
@@ -24,7 +25,6 @@ namespace plugin {
 
 class NaClSubprocess;
 class PnaclCoordinator;
-class TempFile;
 
 class PnaclTranslateThread {
  public:
@@ -37,9 +37,9 @@ class PnaclTranslateThread {
   void SetupState(const pp::CompletionCallback& finish_callback,
                   NaClSubprocess* compiler_subprocess,
                   NaClSubprocess* ld_subprocess,
-                  const std::vector<TempFile*>* obj_files,
+                  std::vector<base::File>* obj_files,
                   int num_threads,
-                  TempFile* nexe_file,
+                  base::File* nexe_file,
                   ErrorInfo* error_info,
                   PP_PNaClOptions* pnacl_options,
                   const std::string& architecture_attributes,
@@ -79,7 +79,7 @@ class PnaclTranslateThread {
 
  private:
   ppapi::proxy::SerializedHandle GetHandleForSubprocess(
-      TempFile* file, int32_t open_flags, base::ProcessId peer_pid);
+      base::File* file, int32_t open_flags, base::ProcessId peer_pid);
 
   // Helper thread entry point for compilation. Takes a pointer to
   // PnaclTranslateThread and calls DoCompile().
@@ -138,9 +138,9 @@ class PnaclTranslateThread {
   int64_t compile_time_;
 
   // Data about the translation files, owned by the coordinator
-  const std::vector<TempFile*>* obj_files_;
+  std::vector<base::File>* obj_files_;
   int num_threads_;
-  TempFile* nexe_file_;
+  base::File* nexe_file_;
   ErrorInfo* coordinator_error_info_;
   PP_PNaClOptions* pnacl_options_;
   std::string architecture_attributes_;

components/nacl/renderer/plugin/temporary_file.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 "components/nacl/renderer/plugin/temporary_file.h"
-
-#include "base/logging.h"
-#include "build/build_config.h"
-#include "components/nacl/renderer/plugin/plugin.h"
-#include "components/nacl/renderer/plugin/utility.h"
-#include "ppapi/c/private/pp_file_handle.h"
-#include "ppapi/cpp/core.h"
-#include "ppapi/cpp/instance.h"
-#include "ppapi/cpp/module.h"
-
-namespace plugin {
-
-TempFile::TempFile(Plugin* plugin, PP_FileHandle handle)
-    : plugin_(plugin),
-      file_handle_(handle) { }
-
-TempFile::~TempFile() { }
-
-bool TempFile::Reset() {
-  // file_handle_, read_wrapper_ and write_wrapper_ are all backed by the
-  // same file handle/descriptor, so resetting the seek position of one
-  // will reset them all.
-  int64_t newpos = file_handle_.Seek(base::File::FROM_BEGIN, 0);
-  return newpos == 0;
-}
-
-int64_t TempFile::GetLength() {
-  return file_handle_.GetLength();
-}
-
-PP_FileHandle TempFile::TakeFileHandle() {
-  DCHECK(file_handle_.IsValid());
-  return file_handle_.TakePlatformFile();
-}
-
-PP_FileHandle TempFile::GetFileHandle() {
-  DCHECK(file_handle_.IsValid());
-  return file_handle_.GetPlatformFile();
-}
-
-}  // namespace plugin

components/nacl/renderer/plugin/temporary_file.h

-// 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.
-
-#ifndef COMPONENTS_NACL_RENDERER_PLUGIN_TEMPORARY_FILE_H_
-#define COMPONENTS_NACL_RENDERER_PLUGIN_TEMPORARY_FILE_H_
-
-#include <stdint.h>
-
-#include "base/files/file.h"
-#include "base/macros.h"
-#include "base/memory/scoped_ptr.h"
-
-#include "ppapi/c/private/pp_file_handle.h"
-
-namespace plugin {
-
-class Plugin;
-
-// Translation creates two temporary files.  The first temporary file holds
-// the object file created by llc.  The second holds the nexe produced by
-// the linker.  Both of these temporary files are used to both write and
-// read according to the following matrix:
-//
-// PnaclCoordinator::obj_file_:
-//     written by: llc     (passed in explicitly through SRPC)
-//     read by:    ld      (returned via lookup service from SRPC)
-// PnaclCoordinator::nexe_file_:
-//     written by: ld      (passed in explicitly through SRPC)
-//     read by:    sel_ldr (passed in explicitly to command channel)
-//
-
-// TempFile represents a file used as a temporary between stages in
-// translation.  It is automatically deleted when all handles are closed
-// (or earlier -- immediately unlinked on POSIX systems).  The file is only
-// opened, once, but because both reading and writing are necessary (and in
-// different processes), the user should reset / seek back to the beginning
-// of the file between sessions.
-class TempFile {
- public:
-  // Create a TempFile.
-  TempFile(Plugin* plugin, PP_FileHandle handle);
-  ~TempFile();
-
-  bool IsValid() { return file_handle_.IsValid(); }
-  // Resets file position of the handle, for reuse.
-  bool Reset();
-
-  // Returns the current size of this file.
-  int64_t GetLength();
-
-  // Returns a handle to the file, transferring ownership of it.
-  PP_FileHandle TakeFileHandle();
-
-  // Returns a handle to the file, without transferring ownership of it.
-  // This handle remains valid until the TempFile object is destroyed or
-  // TakeFileHandle() is called.
-  PP_FileHandle GetFileHandle();
-
- private:
-  Plugin* plugin_;
-  base::File file_handle_;
-
-  DISALLOW_COPY_AND_ASSIGN(TempFile);
-};
-
-}  // namespace plugin
-
-#endif  // COMPONENTS_NACL_RENDERER_PLUGIN_TEMPORARY_FILE_H_