Remove PlatformFile from components/nacl

components/nacl/browser/pnacl_host.cc

 // Copyright 2013 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/browser/pnacl_host.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "components/nacl/browser/nacl_browser.h"
 #include "components/nacl/browser/pnacl_translation_cache.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 
 using content::BrowserThread;
 
 namespace {
 
 static const base::FilePath::CharType kTranslationCacheDirectoryName[] =
     FILE_PATH_LITERAL("PnaclTranslationCache");
 // Delay to wait for initialization of the cache backend
 static const int kTranslationCacheInitializationDelayMs = 20;
 
-void CloseBaseFile(base::File file) {
-  // Not really needed because the file will go out of scope here.
-  file.Close();
+void CloseBaseFile(base::File auto_file_closer) {
 }
 
+void CloseScopedFile(scoped_ptr<base::File> auto_file_closer) {
 }
 
+}  // namespace
+
 namespace pnacl {
 
+class FileProxy {
+ public:
+  FileProxy(scoped_ptr<base::File> file, base::WeakPtr<pnacl::PnaclHost> host);
+  int Write(scoped_refptr<net::DrainableIOBuffer> buffer);
+  void WriteDone(const PnaclHost::TranslationID& id, int result);
+
+ private:
+  scoped_ptr<base::File> file_;
+  base::WeakPtr<pnacl::PnaclHost> host_;
+};
+
+FileProxy::FileProxy(scoped_ptr<base::File> file,
+                     base::WeakPtr<pnacl::PnaclHost> host)
+    : file_(file.Pass()),
+      host_(host) {
+}
+
+int FileProxy::Write(scoped_refptr<net::DrainableIOBuffer> buffer) {
+  int rv = file_->Write(0, buffer->data(), buffer->size());
+  if (rv == -1)
+    PLOG(ERROR) << "FileProxy::Write error";
+  return rv;
+}
+
+void FileProxy::WriteDone(const PnaclHost::TranslationID& id, int result) {
+  if (host_) {
+    host_->OnBufferCopiedToTempFile(id, file_.Pass(), result);
+  } else {
+    BrowserThread::PostBlockingPoolTask(
+        FROM_HERE,
+        base::Bind(CloseScopedFile, Passed(&file_)));
+  }
+}
+
 PnaclHost::PnaclHost()
     : pending_backend_operations_(0),
       cache_state_(CacheUninitialized),
       weak_factory_(this) {}
 
 PnaclHost::~PnaclHost() {
   // When PnaclHost is destroyed, it's too late to post anything to the cache
   // thread (it will hang shutdown). So just leak the cache backend.
   pnacl::PnaclTranslationCache* cache = disk_cache_.release();
   (void)cache;
 }
 
-PnaclHost* PnaclHost::GetInstance() { return Singleton<PnaclHost>::get(); }
+PnaclHost* PnaclHost::GetInstance() {
+  return Singleton<PnaclHost>::get();
+}
 
 PnaclHost::PendingTranslation::PendingTranslation()
     : process_handle(base::kNullProcessHandle),
       render_view_id(0),
-      nexe_fd(base::kInvalidPlatformFileValue),
+      nexe_fd(NULL),
       got_nexe_fd(false),
       got_cache_reply(false),
       got_cache_hit(false),
       is_incognito(false),
       callback(NexeFdCallback()),
-      cache_info(nacl::PnaclCacheInfo()) {}
-PnaclHost::PendingTranslation::~PendingTranslation() {}
+      cache_info(nacl::PnaclCacheInfo()) {
+}
+
+PnaclHost::PendingTranslation::~PendingTranslation() {
+  if (nexe_fd)
+    delete nexe_fd;
+}
 
 bool PnaclHost::TranslationMayBeCached(
     const PendingTranslationMap::iterator& entry) {
   return !entry->second.is_incognito &&
          !entry->second.cache_info.has_no_store_header;
 }
 
 /////////////////////////////////////// Initialization
 
 static base::FilePath GetCachePath() {
@@ skipping 226 matching lines @@
     entry->second.callback.Run(base::kInvalidPlatformFileValue, false);
     bool may_be_cached = TranslationMayBeCached(entry);
     pending_translations_.erase(entry);
     // No translations will be waiting for entries that will not be stored.
     if (may_be_cached)
       RequeryMatchingTranslations(key);
     return;
   }
   PendingTranslation* pt = &entry->second;
   pt->got_nexe_fd = true;
-  pt->nexe_fd = file.TakePlatformFile();
+  pt->nexe_fd = new base::File(file.Pass());
   CheckCacheQueryReady(entry);
 }
 
 // Check whether both the cache query and the temp file have returned, and check
 // whether we actually got a hit or not.
 void PnaclHost::CheckCacheQueryReady(
     const PendingTranslationMap::iterator& entry) {
   PendingTranslation* pt = &entry->second;
   if (!(pt->got_cache_reply && pt->got_nexe_fd))
     return;
@@ skipping 12 matching lines @@
           // and it's already gotten past this check and returned the miss.
           it->second.got_cache_reply &&
           it->second.got_nexe_fd) {
         return;
       }
     }
     ReturnMiss(entry);
     return;
   }
 
+  scoped_ptr<base::File> file(pt->nexe_fd);
+  pt->nexe_fd = NULL;
+  pt->got_nexe_fd = false;
+  FileProxy* proxy(new FileProxy(file.Pass(), weak_factory_.GetWeakPtr()));
+
   if (!base::PostTaskAndReplyWithResult(
            BrowserThread::GetBlockingPool(),
            FROM_HERE,
-           base::Bind(
-               &PnaclHost::CopyBufferToFile, pt->nexe_fd, pt->nexe_read_buffer),
-           base::Bind(&PnaclHost::OnBufferCopiedToTempFile,
-                      weak_factory_.GetWeakPtr(),
+           base::Bind(&FileProxy::Write, base::Unretained(proxy),
+                      pt->nexe_read_buffer),
+           base::Bind(&FileProxy::WriteDone, base::Owned(proxy),
                       entry->first))) {
-    pt->callback.Run(base::kInvalidPlatformFileValue, false);
+    base::File invalid;
+    pt->callback.Run(invalid.GetPlatformFile(), false);
   }
 }
 
 //////////////////// GetNexeFd miss path
 // Return the temp fd to the renderer, reporting a miss.
 void PnaclHost::ReturnMiss(const PendingTranslationMap::iterator& entry) {
   // Return the fd
   PendingTranslation* pt = &entry->second;
   NexeFdCallback cb(pt->callback);
-  if (pt->nexe_fd == base::kInvalidPlatformFileValue) {
-    // Bad FD is unrecoverable, so clear out the entry
+  if (!pt->nexe_fd->IsValid()) {
+    // Bad FD is unrecoverable, so clear out the entry.
     pending_translations_.erase(entry);
+    // Does this has to be executed after the previous line?

[rvargas (doing something else), 2014/06/03 21:29:43]

This was also a comment for the review... but I decided that the order is not
important, and I also updated the callback signature.

[rvargas (doing something else), 2014/06/03 21:31:34]

(let me know if the order is indeed important)

[jvoung (off chromium), 2014/06/03 23:00:09]

I think it will be okay (dschuff can confirm, but is OoO), since this code
shouldn't be handling another request until this thread is done running the
erase().

+    base::File invalid;
+    cb.Run(invalid.GetPlatformFile(), false);
+  } else {
+    cb.Run(pt->nexe_fd->GetPlatformFile(), false);
   }
-  cb.Run(pt->nexe_fd, false);
 }
 
 // On error, just return a null refptr.
 // static
 scoped_refptr<net::DrainableIOBuffer> PnaclHost::CopyFileToBuffer(
-    base::PlatformFile fd) {
-  base::PlatformFileInfo info;
+    scoped_ptr<base::File> file) {
+  base::File::Info info;
   scoped_refptr<net::DrainableIOBuffer> buffer;
   bool error = false;
-  if (!base::GetPlatformFileInfo(fd, &info) ||
+  if (!file->GetInfo(&info) ||
       info.size >= std::numeric_limits<int>::max()) {
-    PLOG(ERROR) << "GetPlatformFileInfo failed";
+    PLOG(ERROR) << "File::GetInfo failed";
     error = true;
   } else {
     buffer = new net::DrainableIOBuffer(
         new net::IOBuffer(static_cast<int>(info.size)), info.size);
-    if (base::ReadPlatformFile(fd, 0, buffer->data(), buffer->size()) !=
-        info.size) {
+    if (file->Read(0, buffer->data(), buffer->size()) != info.size) {
       PLOG(ERROR) << "CopyFileToBuffer file read failed";
       error = true;
     }
   }
   if (error) {
     buffer = NULL;
   }
-  base::ClosePlatformFile(fd);
   return buffer;
 }
 
 // Called by the renderer in the miss path to report a finished translation
 void PnaclHost::TranslationFinished(int render_process_id,
                                     int pp_instance,
                                     bool success) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (cache_state_ != CacheReady)
     return;
   TranslationID id(render_process_id, pp_instance);
   PendingTranslationMap::iterator entry(pending_translations_.find(id));
   if (entry == pending_translations_.end()) {
     LOG(ERROR) << "TranslationFinished: TranslationID " << render_process_id
                << "," << pp_instance << " not found.";
     return;
   }
   bool store_nexe = true;
   // If this is a premature response (i.e. we haven't returned a temp file
   // yet) or if it's an unsuccessful translation, or if we are incognito,
   // don't store in the cache.
   // TODO(dschuff): use a separate in-memory cache for incognito
   // translations.
   if (!entry->second.got_nexe_fd || !entry->second.got_cache_reply ||
       !success || !TranslationMayBeCached(entry)) {
     store_nexe = false;
-  } else if (!base::PostTaskAndReplyWithResult(
-                  BrowserThread::GetBlockingPool(),
-                  FROM_HERE,
-                  base::Bind(&PnaclHost::CopyFileToBuffer,
-                             entry->second.nexe_fd),
-                  base::Bind(&PnaclHost::StoreTranslatedNexe,
-                             weak_factory_.GetWeakPtr(),
-                             id))) {
-    store_nexe = false;
+  } else {
+    scoped_ptr<base::File> file(entry->second.nexe_fd);
+    entry->second.nexe_fd = NULL;
+    entry->second.got_nexe_fd = false;
+
+    if (!base::PostTaskAndReplyWithResult(
+             BrowserThread::GetBlockingPool(),
+             FROM_HERE,
+             base::Bind(&PnaclHost::CopyFileToBuffer, Passed(&file)),
+             base::Bind(&PnaclHost::StoreTranslatedNexe,
+                        weak_factory_.GetWeakPtr(),
+                        id))) {
+      store_nexe = false;
+    }
   }
 
   if (!store_nexe) {
     // If store_nexe is true, the fd will be closed by CopyFileToBuffer.
     if (entry->second.got_nexe_fd) {
+      scoped_ptr<base::File> file(entry->second.nexe_fd);
+      entry->second.nexe_fd = NULL;
       BrowserThread::PostBlockingPoolTask(
           FROM_HERE,
-          base::Bind(base::IgnoreResult(base::ClosePlatformFile),
-                     entry->second.nexe_fd));
+          base::Bind(CloseScopedFile, Passed(&file)));
     }
     pending_translations_.erase(entry);
   }
 }
 
 // Store the translated nexe in the translation cache. Called back with the
 // TranslationID from the host and the result of CopyFileToBuffer.
 // (Bound callbacks must re-lookup the TranslationID because the translation
 // could be cancelled before they get called).
 void PnaclHost::StoreTranslatedNexe(
@@ skipping 55 matching lines @@
       disk_cache_->GetNexe(key,
                            base::Bind(&PnaclHost::OnCacheQueryReturn,
                                       weak_factory_.GetWeakPtr(),
                                       it->first));
     }
   }
 }
 
 //////////////////// GetNexeFd hit path
 
-// static
-int PnaclHost::CopyBufferToFile(base::PlatformFile fd,
-                                scoped_refptr<net::DrainableIOBuffer> buffer) {
-  int rv = base::WritePlatformFile(fd, 0, buffer->data(), buffer->size());
-  if (rv == -1)
-    PLOG(ERROR) << "CopyBufferToFile write error";
-  return rv;
-}
-
 void PnaclHost::OnBufferCopiedToTempFile(const TranslationID& id,
+                                         scoped_ptr<base::File> file,
                                          int file_error) {
   DCHECK(thread_checker_.CalledOnValidThread());
   PendingTranslationMap::iterator entry(pending_translations_.find(id));
   if (entry == pending_translations_.end()) {
+    BrowserThread::PostBlockingPoolTask(
+        FROM_HERE,
+        base::Bind(CloseScopedFile, Passed(&file)));
     return;
   }
   if (file_error == -1) {
     // Write error on the temp file. Request a new file and start over.
     BrowserThread::PostBlockingPoolTask(
         FROM_HERE,
-        base::Bind(base::IgnoreResult(base::ClosePlatformFile),
-                   entry->second.nexe_fd));
-    entry->second.got_nexe_fd = false;
+        base::Bind(CloseScopedFile, Passed(&file)));
     CreateTemporaryFile(base::Bind(&PnaclHost::OnTempFileReturn,
                                    weak_factory_.GetWeakPtr(),
                                    entry->first));
     return;
   }
-  base::PlatformFile fd = entry->second.nexe_fd;
-  entry->second.callback.Run(fd, true);
+  entry->second.callback.Run(file->GetPlatformFile(), true);
   BrowserThread::PostBlockingPoolTask(
-      FROM_HERE, base::Bind(base::IgnoreResult(base::ClosePlatformFile), fd));
+      FROM_HERE,
+      base::Bind(CloseScopedFile, Passed(&file)));
   pending_translations_.erase(entry);
 }
 
 ///////////////////
 
 void PnaclHost::RendererClosing(int render_process_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (cache_state_ != CacheReady)
     return;
   for (PendingTranslationMap::iterator it = pending_translations_.begin();
        it != pending_translations_.end();) {
     PendingTranslationMap::iterator to_erase(it++);
     if (to_erase->first.first == render_process_id) {
       // Clean up the open files.
+      scoped_ptr<base::File> file(to_erase->second.nexe_fd);
+      to_erase->second.nexe_fd = NULL;
       BrowserThread::PostBlockingPoolTask(
           FROM_HERE,
-          base::Bind(base::IgnoreResult(base::ClosePlatformFile),
-                     to_erase->second.nexe_fd));
+          base::Bind(CloseScopedFile, Passed(&file)));
       std::string key(to_erase->second.cache_key);
       bool may_be_cached = TranslationMayBeCached(to_erase);
       pending_translations_.erase(to_erase);
       // No translations will be waiting for entries that will not be stored.
       if (may_be_cached)
         RequeryMatchingTranslations(key);
     }
   }
   BrowserThread::PostTask(
       BrowserThread::IO,
@@ skipping 66 matching lines @@
   DCHECK(pending_backend_operations_ >= 0);
   if (pending_translations_.empty() &&
       pending_backend_operations_ <= 0 &&
       cache_state_ == CacheReady) {
     cache_state_ = CacheUninitialized;
     disk_cache_.reset();
   }
 }
 
 }  // namespace pnacl