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

webkit/fileapi/media/native_media_file_util.cc

 // 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.
 
 #include "webkit/fileapi/media/native_media_file_util.h"
 
+#include "base/string_util.h"
+#include "googleurl/src/gurl.h"
+#include "net/base/mime_sniffer.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/media/media_path_filter.h"
 #include "webkit/fileapi/media/filtering_file_enumerator.h"
 #include "webkit/fileapi/native_file_util.h"
 
 using base::PlatformFile;
 using base::PlatformFileError;
 using base::PlatformFileInfo;
 
+namespace {
+
+// Modelled after ScopedFILEClose.
+class ScopedPlatformFileClose {
+ public:
+  void operator()(base::PlatformFile* ppf) const {
+    base::ClosePlatformFile(*ppf);
+  }
+};
+
+typedef scoped_ptr<base::PlatformFile,
+                   ScopedPlatformFileClose> ScopedPlatformFile;
+
+// Returns true if the current thread is IO thread.
+bool IsOnTaskRunnerThread(fileapi::FileSystemOperationContext* context) {
+  return context->file_system_context()->task_runners()->
+      io_task_runner()->RunsTasksOnCurrentThread();
+}
+
+}
+
 namespace fileapi {
 
 NativeMediaFileUtil::NativeMediaFileUtil() {
 }
 
 PlatformFileError NativeMediaFileUtil::CreateOrOpen(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
     int file_flags,
     PlatformFile* file_handle,
@@ skipping 181 matching lines @@
 
   if (!file_info.is_directory &&
       !context->media_path_filter()->Match(file_path)) {
     return failure_error;
   }
 
   *local_file_path = file_path;
   return base::PLATFORM_FILE_OK;
 }
 
+bool NativeMediaFileUtil::IsMediaFile(const base::FilePath& path) {
+  base::PlatformFile file_handle;
+  bool created;
+  base::PlatformFileError error;
+  error = NativeFileUtil::CreateOrOpen(path,
+                                       base::PLATFORM_FILE_OPEN |

[vandebo (ex-Chrome), 2013/03/20 20:03:49]

nit: technically line 248 and 249 are a single argument, so they should go on
one line if possible.  This is probably the most readable indention:
  base::PlatformFileError error = NativeFileUtil::CreateOrOpen(
      path, 
      base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, 
      &file_handle,
      &created);                       

This would also be acceptable.
  base::PlatformFileError error = NativeFileUtil::CreateOrOpen(
      path, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, &file_handle,
      &created);                       

[Kevin Bailey, 2013/03/21 15:53:19]

Done.

+                                       base::PLATFORM_FILE_READ,
+                                       &file_handle, &created);
+  if (error != base::PLATFORM_FILE_OK) {

[vandebo (ex-Chrome), 2013/03/20 20:03:49]

nit: Single line if's with single line bodies may omit the {}'s  This file seems
to do that, so for consistency they should be omitted.

[vandebo (ex-Chrome), 2013/03/20 20:03:49]

Hmm, I guess this function can fail for a reason other than not a media file. 
Maybe it should return base::PlatformFileError isntead of a bool. Thoughts?

[Kevin Bailey, 2013/03/21 15:53:19]

I glanced up to line 234 to see what style the file liked, but it lied. Done.

[Kevin Bailey, 2013/03/21 15:53:19]

Reason #27 why exceptions are better than return codes. This will work fine
until the function needs to return something else. As it is, the callers are a
bit simpler, at the expense of some extra conditions here.

+    return false;
+  }
+  ScopedPlatformFile scoped_platform_file(new base::PlatformFile(file_handle));
+  char buffer[net::kMaxBytesToSniff];
+  int64 len;
+  // Read as much as SniffMimeType() will bother looking at.
+  len = base::ReadPlatformFile(file_handle, 0, buffer, net::kMaxBytesToSniff);
+  if (len <= 0) {

[vandebo (ex-Chrome), 2013/03/20 20:03:49]

And here

[Kevin Bailey, 2013/03/21 15:53:19]

Done.

+    return false;
+  }
+  std::string mime_type;
+  if (!net::SniffMimeType(buffer, len, GURL("file://" + path.value()),
+                          "text/plain", &mime_type)) {
+    return false;
+  }
+  return StartsWithASCII(mime_type, "image/", true) ||
+         StartsWithASCII(mime_type, "audio/", true) ||
+         StartsWithASCII(mime_type, "video/", true);
+}
+
+base::PlatformFileError NativeMediaFileUtil::CreateSnapshotFile(
+    FileSystemOperationContext* context,
+    const FileSystemURL& url,
+    base::PlatformFileInfo* file_info,
+    base::FilePath* platform_path,
+    SnapshotFilePolicy* policy) {
+  DCHECK(policy);

[vandebo (ex-Chrome), 2013/03/20 20:03:49]

nit: this method doesn't actually care if policy for file_info are non-NULL. 
Generally we use DCHECKs for assumptions that the following code makes.  So just
leave it up to LocalFileUtil::CreateSnapshotFile to DCHECK those two parameters.

[Kevin Bailey, 2013/03/21 15:53:19]

Done.

+  DCHECK(file_info);
+  DCHECK(IsOnTaskRunnerThread(context));
+  base::PlatformFileError error = IsolatedFileUtil::CreateSnapshotFile(
+      context, url, file_info, platform_path, policy);
+  if (error != base::PLATFORM_FILE_OK)
+    return error;
+  if (!IsMediaFile(*platform_path))
+    return base::PLATFORM_FILE_ERROR_SECURITY;
+  return base::PLATFORM_FILE_OK;
+}
+
 }  // namespace fileapi