Introduce NaClFileInfoAutoCloser as an RAII wrapper.

ppapi/native_client/src/trusted/plugin/file_downloader.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 NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_FILE_DOWNLOADER_H_
 #define NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_FILE_DOWNLOADER_H_
 
 #include <deque>
 
 #include "native_client/src/include/nacl_macros.h"
@@ skipping 23 matching lines @@
 typedef enum {
   SCHEME_CHROME_EXTENSION,
   SCHEME_DATA,
   SCHEME_OTHER
 } UrlSchemeType;
 
 typedef std::vector<char>* FileStreamData;
 typedef CallbackSource<FileStreamData> StreamCallbackSource;
 typedef pp::CompletionCallbackWithOutput<FileStreamData> StreamCallback;
 
+// RAII-style wrapper class
+class NaClFileInfoAutoCloser {
+ public:
+  NaClFileInfoAutoCloser();
+
+  explicit NaClFileInfoAutoCloser(NaClFileInfo pass_ownership);

[bbudge, 2014/01/29 21:43:31]

It would be nice if pass_ownership's desc field was set to -1 by this ctor, but
I suppose you'd have to pass it as a pointer here.

Same for TakeOwnership below.

[bsy, 2014/01/30 00:29:40]

I'll set the whole thing to NoFileInfo(), since the file token fields are single
use and should be part of the ownership passing.

It makes usage a bit more awkward, but I agree that this change makes it a more
defensive API.

+
+  ~NaClFileInfoAutoCloser() {
+    FreeResources();
+  }
+
+  // Frees owned resources
+  void FreeResources();
+
+  void TakeOwnership(NaClFileInfo pass_ownership);
+
+  // Return NaClFileInfo for temporary use, retaining ownership.
+  NaClFileInfo get() { return info_; }

[bbudge, 2014/01/29 21:43:31]

Could this return a const & instead?

[bsy, 2014/01/30 00:29:40]

Done.

+
+  // Returns POSIX descriptor for temporary use, retaining ownership.
+  int get_desc() { return info_.desc; }
+
+  // Returns ownership to caller
+  NaClFileInfo Release();
+
+ private:
+  NACL_DISALLOW_COPY_AND_ASSIGN(NaClFileInfoAutoCloser);
+
+  NaClFileInfo info_;
+};
+
 // A class that wraps PPAPI URLLoader and FileIO functionality for downloading
 // the url into a file and providing an open file descriptor.
 class FileDownloader {
  public:
   // Ctor initializes |instance_| to NULL, be sure to call Initialize() before
   // calling Open(), or Open() will fail.
   FileDownloader()
       : instance_(NULL),
         file_open_notify_callback_(pp::BlockUntilComplete()),
         stream_finish_callback_(pp::BlockUntilComplete()),
-        file_handle_(PP_kInvalidFileHandle),
         file_io_private_interface_(NULL),
         url_loader_trusted_interface_(NULL),
         open_time_(-1),
         mode_(DOWNLOAD_NONE),
         open_and_stream_(true),
         url_scheme_(SCHEME_OTHER),
         data_stream_callback_source_(NULL) {}
   ~FileDownloader() {}
 
   // Initialize() can only be called once during the lifetime of this instance.
@@ skipping 113 matching lines @@
   void GotFileHandleNotify(int32_t pp_error, PP_FileHandle handle);
 
   Plugin* instance_;
   nacl::string url_to_open_;
   nacl::string url_;
   nacl::string extra_request_headers_;
   pp::URLResponseInfo url_response_;
   pp::CompletionCallback file_open_notify_callback_;
   pp::CompletionCallback stream_finish_callback_;
   pp::FileIO file_reader_;
-  PP_FileHandle file_handle_;
-  struct NaClFileToken file_token_;
   const PPB_FileIO_Private* file_io_private_interface_;
   const PPB_URLLoaderTrusted* url_loader_trusted_interface_;
   pp::URLLoader url_loader_;
   pp::CompletionCallbackFactory<FileDownloader> callback_factory_;
   int64_t open_time_;
   int32_t status_code_;
   DownloadMode mode_;
   bool open_and_stream_;
   static const uint32_t kTempBufferSize = 2048;
   std::vector<char> temp_buffer_;
   std::deque<char> buffer_;
   UrlSchemeType url_scheme_;
   StreamCallbackSource* data_stream_callback_source_;
-  NaClFileInfo cached_file_info_;
+  NaClFileInfoAutoCloser file_info_;
 };
 }  // namespace plugin;
 #endif  // NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_FILE_DOWNLOADER_H_