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

chrome/browser/media_galleries/fileapi/device_media_async_file_util.cc

 // Copyright (c) 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 "chrome/browser/media_galleries/fileapi/device_media_async_file_util.h"
 
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/single_thread_task_runner.h"
 #include "chrome/browser/media_galleries/fileapi/filtering_file_enumerator.h"
 #include "chrome/browser/media_galleries/fileapi/media_file_system_mount_point_provider.h"
 #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;
 using webkit_blob::ShareableFileReference;
 
 namespace chrome {
 
 namespace {
 
 const base::FilePath::CharType kDeviceMediaAsyncFileUtilTempDir[] =
@@ skipping 29 matching lines @@
       profile_path.Append(kDeviceMediaAsyncFileUtilTempDir);
   if (!file_util::CreateDirectory(isolated_media_file_system_dir_path) ||
       !file_util::CreateTemporaryFileInDir(isolated_media_file_system_dir_path,
                                            snapshot_file_path)) {
     LOG(WARNING) << "Could not create media snapshot file "
                  << isolated_media_file_system_dir_path.value();
     *snapshot_file_path = base::FilePath();
   }
 }
 
+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() {
 }
 
 // static
 DeviceMediaAsyncFileUtil* DeviceMediaAsyncFileUtil::Create(
     const base::FilePath& profile_path) {
   DCHECK(!profile_path.empty());
   return new DeviceMediaAsyncFileUtil(profile_path);
@@ skipping 229 matching lines @@
     callback.Run(error, AsyncFileUtil::EntryList(), false /*no more*/);
 }
 
 void DeviceMediaAsyncFileUtil::OnDidCreateSnapshotFile(
     const AsyncFileUtil::CreateSnapshotFileCallback& callback,
     base::SequencedTaskRunner* media_task_runner,
     const base::PlatformFileInfo& file_info,
     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(
     const AsyncFileUtil::CreateSnapshotFileCallback& callback,
     base::PlatformFileError error) {
   if (!callback.is_null())
     callback.Run(error, base::PlatformFileInfo(), base::FilePath(),
                  scoped_refptr<ShareableFileReference>());
 }
 
@@ skipping 18 matching lines @@
       base::Bind(&DeviceMediaAsyncFileUtil::OnDidCreateSnapshotFile,
                  weak_ptr_factory_.GetWeakPtr(),
                  callback,
                  make_scoped_refptr(context->task_runner())),
       base::Bind(&DeviceMediaAsyncFileUtil::OnCreateSnapshotFileError,
                  weak_ptr_factory_.GetWeakPtr(),
                  callback));
 }
 
 }  // namespace chrome