Have media gallery (through native media file util) use MIME sniffer

chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc

Index: chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc
diff --git a/chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc b/chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc
index 45345e70ef8cdaac52c0e05c00cea7e9c4ca45fa..7adff7f35c3d27642c9b02cd8d1b37bbd8d0364f 100644
--- a/chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc
+++ b/chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc
@@ -12,12 +12,14 @@
 #include "chrome/browser/media_galleries/fileapi/media_path_filter.h"
 #include "chrome/browser/media_galleries/fileapi/mtp_device_async_delegate.h"
 #include "chrome/browser/media_galleries/fileapi/mtp_device_map_service.h"
+#include "chrome/browser/media_galleries/fileapi/native_media_file_util.h"
 #include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_task_runners.h"
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/isolated_context.h"
+#include "webkit/fileapi/native_file_util.h"
 
 using fileapi::FileSystemOperationContext;
 using fileapi::FileSystemURL;
@@ -67,6 +69,15 @@ void CreateSnapshotFileOnBlockingPool(
   }
 }
 
+void CheckMediaFile(
+    const base::FilePath& file_path,
+    base::PlatformFileError* error) {
+  *error = NativeMediaFileUtil::IsMediaFile(file_path);
+  if (*error == base::PLATFORM_FILE_ERROR_SECURITY)

[vandebo (ex-Chrome), 2013/04/30 21:20:36]

nit: {}'s because the body is multi line.

[Kevin Bailey, 2013/05/02 00:54:02]

Removed function.

+    // It exists but we reject it. Try to clean up.
+    fileapi::NativeFileUtil::DeleteFile(file_path);
+}
+
 }  // namespace
 
 DeviceMediaAsyncFileUtil::~DeviceMediaAsyncFileUtil() {
@@ -316,11 +327,36 @@ void DeviceMediaAsyncFileUtil::OnDidCreateSnapshotFile(
     const base::FilePath& platform_path) {
   if (callback.is_null())
     return;
-  callback.Run(base::PLATFORM_FILE_OK, file_info, platform_path,
-               ShareableFileReference::GetOrCreate(
-                   platform_path,
-                   ShareableFileReference::DELETE_ON_FINAL_RELEASE,
-                   media_task_runner));
+  base::PlatformFileError* error = new base::PlatformFileError;
+  media_task_runner->PostTaskAndReply(
+          FROM_HERE,
+          base::Bind(&CheckMediaFile, platform_path,
+                     base::Unretained(error)),
+          base::Bind(&DeviceMediaAsyncFileUtil::OnDidCheckMediaRunTask,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     callback, base::Unretained(media_task_runner),

[kinuko, 2013/05/01 02:03:46]

Please pass media_task_runner as a refptr, i.e.

make_scoped_refptr(media_task_runner)

so that the task can keep a ref to the runner

[Kevin Bailey, 2013/05/02 00:54:02]

Done. It's unfortunate that it doesn't do that automatically since it seems to
know that it's a reference counted object.

+                     file_info, platform_path, base::Owned(error)));

[kinuko, 2013/05/01 02:03:46]

nit: You can create a ShareableFileReference earlier in this method and pass the
ref (rather than a platform_path) to OnDidCheckMediaRunTask, and it'd be
slightly safer, i.e. has less possibility to leak a temporary file. You can
probably remove the line 78 (manual file deletion on failure) then.

And if you delete the line 78 I guess you can also remove the indirect
CheckMediaFile function? like:

media_task_runner->PostTaskAndReplyWith(
    FROM_HERE,
    base::Bind(&NativeMediaFileUtil::IsMediaFile, file_path),
    base::Bind(&DeviceMediaAsyncFileUtil::...., file_ref));

[Kevin Bailey, 2013/05/02 00:54:02]

I moved the GetOrCreate upstream but it seems like overkill. All I really want
is something that will remove the file if something fails. A ScopedFile would
suffice, however, ScopedFile isn't very friendly (copyable) and I didn't want to
go so far as to wrap it in a scoped_ptr. Also, the callback wants a
ShareableFileReference.

BTW, CreateSnapshotFileCallback can remove the FilePath parameter since it can
get it from the ShareableFileReference.

+}
+
+void DeviceMediaAsyncFileUtil::OnDidCheckMediaRunTask(
+    const AsyncFileUtil::CreateSnapshotFileCallback& callback,
+    base::SequencedTaskRunner* media_task_runner,
+    const base::PlatformFileInfo& file_info,
+    const base::FilePath& platform_path,
+    base::PlatformFileError* error) {
+  // really should be an assertion
+  if (callback.is_null())
+    return;
+  if (*error == base::PLATFORM_FILE_OK) {
+    callback.Run(base::PLATFORM_FILE_OK, file_info, platform_path,
+                 ShareableFileReference::GetOrCreate(
+                     platform_path,
+                     ShareableFileReference::DELETE_ON_FINAL_RELEASE,
+                     media_task_runner));
+  } else {
+    callback.Run(*error, file_info, platform_path,
+                 scoped_refptr<ShareableFileReference>());
+  }
 }
 
 void DeviceMediaAsyncFileUtil::OnCreateSnapshotFileError(