Add CreateTemporaryFile to PPB_Flash_File_ModuleLocal.

content/browser/renderer_host/pepper_file_message_filter.cc

Index: content/browser/renderer_host/pepper_file_message_filter.cc
diff --git a/content/browser/renderer_host/pepper_file_message_filter.cc b/content/browser/renderer_host/pepper_file_message_filter.cc
index a86542ef14ea395ea2bd937b85457af794b932c4..6486a01dda20dfdcf8241def374824f7213882b2 100644
--- a/content/browser/renderer_host/pepper_file_message_filter.cc
+++ b/content/browser/renderer_host/pepper_file_message_filter.cc
@@ -23,6 +23,8 @@
 
 using content::BrowserThread;
 
+namespace {
+
 // Used to check if the renderer has permission for the requested operation.
 // TODO(viettrungluu): Verify these. They don't necessarily quite make sense,
 // but it seems to be approximately what the file system code does.
@@ -37,6 +39,28 @@ const int kWritePermissions = base::PLATFORM_FILE_OPEN |
                               base::PLATFORM_FILE_EXCLUSIVE_WRITE |
                               base::PLATFORM_FILE_WRITE_ATTRIBUTES;
 
+IPC::PlatformFileForTransit PlatformFileToPlatformFileForTransit(
+    base::ProcessHandle peer_handle,
+    base::PlatformFile file_handle,
+    base::PlatformFileError* error) {
+  IPC::PlatformFileForTransit file;
+#if defined(OS_WIN)
+  // Duplicate the file handle so that the renderer process can access the file.
+  if (!DuplicateHandle(GetCurrentProcess(), file_handle,
+                       peer_handle, &file, 0, false,
+                       DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
+    // file_handle is closed whether or not DuplicateHandle succeeds.
+    *error = base::PLATFORM_FILE_ERROR_ACCESS_DENIED;
+    file = INVALID_HANDLE_VALUE;
+  }
+#else
+  file = base::FileDescriptor(file_handle, true);
+#endif
+  return file;
+}
+
+}  // namespace
+
 PepperFileMessageFilter::PepperFileMessageFilter(int child_id)
         : child_id_(child_id),
           channel_(NULL) {
@@ -59,6 +83,8 @@ bool PepperFileMessageFilter::OnMessageReceived(const IPC::Message& message,
     IPC_MESSAGE_HANDLER(PepperFileMsg_CreateDir, OnCreateDir)
     IPC_MESSAGE_HANDLER(PepperFileMsg_QueryFile, OnQueryFile)
     IPC_MESSAGE_HANDLER(PepperFileMsg_GetDirContents, OnGetDirContents)
+    IPC_MESSAGE_HANDLER(PepperFileMsg_CreateTemporaryFile,
+                        OnCreateTemporaryFile)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP_EX()
   return handled;
@@ -110,18 +136,8 @@ void PepperFileMessageFilter::OnOpenFile(
     return;
   }
 
-#if defined(OS_WIN)
-  // Duplicate the file handle so that the renderer process can access the file.
-  if (!DuplicateHandle(GetCurrentProcess(), file_handle,
-                       peer_handle(), file, 0, false,
-                       DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
-    // file_handle is closed whether or not DuplicateHandle succeeds.
-    *error = base::PLATFORM_FILE_ERROR_ACCESS_DENIED;
-    *file = INVALID_HANDLE_VALUE;
-  }
-#else
-  *file = base::FileDescriptor(file_handle, true);
-#endif
+  *file = PlatformFileToPlatformFileForTransit(peer_handle(), file_handle,
+                                               error);
 }
 
 void PepperFileMessageFilter::OnRenameFile(
@@ -220,6 +236,43 @@ void PepperFileMessageFilter::OnGetDirContents(
   *error = base::PLATFORM_FILE_OK;
 }
 
+void PepperFileMessageFilter::OnCreateTemporaryFile(
+    base::PlatformFileError* error,
+    IPC::PlatformFileForTransit* file) {
+  *error = base::PLATFORM_FILE_ERROR_FAILED;
+  *file = IPC::InvalidPlatformFileForTransit();
+
+  ppapi::PepperFilePath dir_path(
+      ppapi::PepperFilePath::DOMAIN_MODULE_LOCAL, FilePath());
+  FilePath validated_dir_path = ValidateAndConvertPepperFilePath(
+      dir_path, kReadPermissions | kWritePermissions);
+  if (validated_dir_path.empty() ||
+      (!file_util::DirectoryExists(validated_dir_path) &&

[viettrungluu, 2012/06/11 17:31:46]

Why don't we have to do any of this for Open?

[yzshen1, 2012/06/12 23:53:06]

The folder of module-local root is not guaranteed to exist at this point, right?
Because the users of this method has no control over (or know about) where the
temp file locates, it is our responsibility to help them to make sure the folder
exists.

On the other hand, the users of Open can test whether the dir exists and (if
not) create it.

(But having to create the root directory explicitly does seem a little bit
weird. Do you suggest to treat Open-ing a file in the root directory specially?)

[viettrungluu, 2012/06/13 22:46:32]

Now that I think of it, I have no idea how anything at all works. Someone must
be ensuring that the directory exists -- could you figure out who?

[yzshen1, 2012/06/14 18:25:38]

The answer is no. :P
The pepper layer doesn't do anything to ensure that the module-local root
directory exists.

What I have tried:
I deleted the Pepper Data directory and then ran the youtube page.
At the beginning, Flash tried to open some files, while the module-local root
didn't exist. Some time later, it called CreateDirectory() (which created
directory recursively), the module-local root finally got created.

On 2012/06/13 22:46:32, viettrungluu wrote:

[viettrungluu, 2012/06/14 20:47:22]

Probably we (by which I mean you) should abstract out an
"EnsureModuleLocalRootExists" and call it for Open() as well. Could you do this
in a follow-up CL?

On 2012/06/14 18:25:38, yzshen1 wrote:

+       !file_util::CreateDirectory(validated_dir_path))) {
+    *error = base::PLATFORM_FILE_ERROR_ACCESS_DENIED;
+    return;
+  }
+
+  FilePath file_path;
+  if (!file_util::CreateTemporaryFileInDir(validated_dir_path, &file_path))
+    return;
+
+  base::PlatformFile file_handle = base::CreatePlatformFile(
+      file_path,
+      base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_EXCLUSIVE_READ |

[viettrungluu, 2012/06/11 17:31:46]

Sorry I didn't answer your question earlier; in general, locking is more
expensive than not locking, so we should probably avoid ..._EXCLUSIVE_... unless
there's a compelling reason for it.

[yzshen1, 2012/06/12 23:53:06]

Changed. Thanks for explanation.
On 2012/06/11 17:31:46, viettrungluu wrote:

+      base::PLATFORM_FILE_EXCLUSIVE_WRITE | base::PLATFORM_FILE_TEMPORARY |
+      base::PLATFORM_FILE_DELETE_ON_CLOSE,
+      NULL, error);
+
+  if (*error != base::PLATFORM_FILE_OK ||
+      file_handle == base::kInvalidPlatformFileValue) {

[viettrungluu, 2012/06/11 17:31:46]

Do you really think file_handle could be kInvalidPlatformFileValue? (If not,
CHECK or DCHECK.)

[yzshen1, 2012/06/12 23:53:06]

No. Changed.

On 2012/06/11 17:31:46, viettrungluu wrote:

+    return;
+  }
+
+  *file = PlatformFileToPlatformFileForTransit(peer_handle(), file_handle,
+                                               error);
+}
+
 FilePath PepperFileMessageFilter::ValidateAndConvertPepperFilePath(
     const ppapi::PepperFilePath& pepper_path, int flags) {
   FilePath file_path;  // Empty path returned on error.