Create ShareableFileReference on IO thread

* ShareableFileReference is not thread-safe, we should make sure we only handle it on IO thread
* FileUtil implementation no longer needs to directly handle ShareableFileReference

BUG=140508


Review URL: https://chromiumcodereview.appspot.com/10834167

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149992 0039d316-1c4b-4281-b951-d872f2087c98

webkit/fileapi/file_system_file_util.h

@@ -46,6 +46,20 @@ class FILEAPI_EXPORT FileSystemFileUtil {
     virtual bool IsDirectory() = 0;
   };
 
+  // A policy flag for CreateSnapshotFile.
+  enum SnapshotFilePolicy {
+    kSnapshotFileUnknown,
+
+    // The implementation just uses the local file as the snapshot file.
+    // The FileAPI backend does nothing on the returned file.
+    kSnapshotFileLocal,
+
+    // The implementation returns a temporary file as the snapshot file.
+    // The FileAPI backend takes care of the lifetime of the returned file
+    // and will delete when the last reference of the file is dropped.
+    kSnapshotFileTemporary,
+  };
+
   class EmptyFileEnumerator : public AbstractFileEnumerator {
     virtual FilePath Next() OVERRIDE { return FilePath(); }
     virtual int64 Size() OVERRIDE { return 0; }
@@ -183,20 +197,16 @@ class FILEAPI_EXPORT FileSystemFileUtil {
   // temporary snapshot file which holds the file data and return
   // the metadata of the temporary file.
   //
-  // |result| is the return code of the operation.
   // |file_info| is the metadata of the snapshot file created.
   // |platform_path| is the path to the snapshot file created.
-  //
-  // The implementation can optionally return a file reference
-  // to let the fileapi backend manage the lifetime of the returned
-  // snapshot file. Otherwise it is ok to return NULL.
-  // Please see the comment for ShareableFileReference for details.
-  virtual scoped_refptr<webkit_blob::ShareableFileReference>
-      CreateSnapshotFile(FileSystemOperationContext* context,
-                         const FileSystemURL& url,
-                         base::PlatformFileError* result,
-                         base::PlatformFileInfo* file_info,
-                         FilePath* platform_path) = 0;
+  // |policy| should indicate the policy how the fileapi backend
+  // should handle the returned file.
+  virtual base::PlatformFileError CreateSnapshotFile(
+      FileSystemOperationContext* context,
+      const FileSystemURL& url,
+      base::PlatformFileInfo* file_info,
+      FilePath* platform_path,
+      SnapshotFilePolicy* policy) = 0;
 
  protected:
   FileSystemFileUtil() {}

webkit/fileapi/file_system_file_util_proxy.cc

@@ -47,7 +47,9 @@ class EnsureFileExistsHelper {
 
 class GetFileInfoHelper {
  public:
-  GetFileInfoHelper() : error_(base::PLATFORM_FILE_OK) {}
+  GetFileInfoHelper()
+      : error_(base::PLATFORM_FILE_OK),
+        snapshot_policy_(FileSystemFileUtil::kSnapshotFileUnknown) {}
 
   void GetFileInfo(FileSystemFileUtil* file_util,
                    FileSystemOperationContext* context,
@@ -58,8 +60,8 @@ class GetFileInfoHelper {
   void CreateSnapshotFile(FileSystemFileUtil* file_util,
                           FileSystemOperationContext* context,
                           const FileSystemURL& url) {
-    file_ref_ = file_util->CreateSnapshotFile(
-        context, url, &error_, &file_info_, &platform_path_);
+    error_ = file_util->CreateSnapshotFile(
+        context, url, &file_info_, &platform_path_, &snapshot_policy_);
   }
 
   void ReplyFileInfo(const Proxy::GetFileInfoCallback& callback) {
@@ -68,15 +70,16 @@ class GetFileInfoHelper {
   }
 
   void ReplySnapshotFile(const Proxy::SnapshotFileCallback& callback) {
+    DCHECK(snapshot_policy_ != FileSystemFileUtil::kSnapshotFileUnknown);
     if (!callback.is_null())
-      callback.Run(error_, file_info_, platform_path_, file_ref_);
+      callback.Run(error_, file_info_, platform_path_, snapshot_policy_);
   }
 
  private:
   base::PlatformFileError error_;
   base::PlatformFileInfo file_info_;
   FilePath platform_path_;
-  scoped_refptr<webkit_blob::ShareableFileReference> file_ref_;
+  FileSystemFileUtil::SnapshotFilePolicy snapshot_policy_;
   DISALLOW_COPY_AND_ASSIGN(GetFileInfoHelper);
 };
 

webkit/fileapi/file_system_file_util_proxy.h

@@ -13,6 +13,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/platform_file.h"
 #include "base/tracked_objects.h"
+#include "webkit/fileapi/file_system_file_util.h"
 #include "webkit/fileapi/file_system_operation_interface.h"
 
 namespace fileapi {
@@ -39,11 +40,16 @@ class FileSystemFileUtilProxy {
   typedef base::Callback<void(PlatformFileError status,
                               bool created)> EnsureFileExistsCallback;
   typedef FileSystemOperationInterface::GetMetadataCallback GetFileInfoCallback;
-  typedef FileSystemOperationInterface::SnapshotFileCallback
-      SnapshotFileCallback;
   typedef FileSystemOperationInterface::ReadDirectoryCallback
       ReadDirectoryCallback;
 
+  typedef base::Callback<
+      void(base::PlatformFileError result,
+           const base::PlatformFileInfo& file_info,
+           const FilePath& platform_path,
+           FileSystemFileUtil::SnapshotFilePolicy snapshot_policy)>
+      SnapshotFileCallback;
+
   // Deletes a file or a directory on the given context's file_task_runner.
   // It is an error to delete a non-empty directory with recursive=false.
   static bool Delete(

webkit/fileapi/file_util_helper.cc

@@ -6,7 +6,6 @@
 
 #include <stack>
 
-#include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_system_file_util.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_url.h"
@@ -17,6 +16,18 @@ namespace fileapi {
 
 namespace {
 
+// A helper class to delete a temporary file.
+class ScopedFileDeleter {
+ public:
+  explicit ScopedFileDeleter(const FilePath& path) : path_(path) {}
+  ~ScopedFileDeleter() {
+    file_util::Delete(path_, false /* recursive */);
+  }
+
+ private:
+  FilePath path_;
+};
+
 // A helper class for cross-FileUtil Copy/Move operations.
 class CrossFileUtilHelper {
  public:
@@ -250,14 +261,17 @@ PlatformFileError CrossFileUtilHelper::CopyOrMoveFile(
   // Resolve the src_url's underlying file path.
   base::PlatformFileInfo file_info;
   FilePath platform_file_path;
-  PlatformFileError error = base::PLATFORM_FILE_OK;
+  FileSystemFileUtil::SnapshotFilePolicy snapshot_policy;
 
-  scoped_refptr<webkit_blob::ShareableFileReference> file_ref =
-      src_util_->CreateSnapshotFile(context_, src_url,
-                                    &error, &file_info, &platform_file_path);
+  PlatformFileError error = src_util_->CreateSnapshotFile(
+      context_, src_url, &file_info, &platform_file_path, &snapshot_policy);
   if (error != base::PLATFORM_FILE_OK)
     return error;
 
+  scoped_ptr<ScopedFileDeleter> file_deleter;
+  if (snapshot_policy == FileSystemFileUtil::kSnapshotFileTemporary)
+    file_deleter.reset(new ScopedFileDeleter(platform_file_path));
+
   // Call CopyInForeignFile() on the dest_util_ with the resolved source path
   // to perform limited cross-FileSystemFileUtil copy/move.
   error = dest_util_->CopyInForeignFile(

webkit/fileapi/local_file_system_operation.cc

@@ -24,6 +24,8 @@
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/quota_types.h"
 
+using webkit_blob::ShareableFileReference;
+
 namespace fileapi {
 
 class LocalFileSystemOperation::ScopedQuotaNotifier {
@@ -676,7 +678,14 @@ void LocalFileSystemOperation::DidCreateSnapshotFile(
     base::PlatformFileError result,
     const base::PlatformFileInfo& file_info,
     const FilePath& platform_path,
-    const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref) {
+    FileSystemFileUtil::SnapshotFilePolicy snapshot_policy) {
+  scoped_refptr<ShareableFileReference> file_ref;
+  if (result == base::PLATFORM_FILE_OK &&
+      snapshot_policy == FileSystemFileUtil::kSnapshotFileTemporary) {
+    file_ref = ShareableFileReference::GetOrCreate(
+        platform_path, ShareableFileReference::DELETE_ON_FINAL_RELEASE,
+        file_system_context()->file_task_runner());
+  }
   callback.Run(result, file_info, platform_path, file_ref);
 }
 

webkit/fileapi/local_file_system_operation.h

@@ -226,7 +226,7 @@ class FILEAPI_EXPORT LocalFileSystemOperation
       base::PlatformFileError rv,
       const base::PlatformFileInfo& file_info,
       const FilePath& platform_path,
-      const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref);
+      FileSystemFileUtil::SnapshotFilePolicy snapshot_policy);
 
   // Checks the validity of a given |url| and populates |file_util| for |mode|.
   base::PlatformFileError SetUp(

webkit/fileapi/local_file_util.cc

@@ -269,18 +269,16 @@ PlatformFileError LocalFileUtil::DeleteSingleDirectory(
   return NativeFileUtil::DeleteSingleDirectory(file_path);
 }
 
-scoped_refptr<webkit_blob::ShareableFileReference>
-LocalFileUtil::CreateSnapshotFile(
+base::PlatformFileError LocalFileUtil::CreateSnapshotFile(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
-    base::PlatformFileError* result,
     base::PlatformFileInfo* file_info,
-    FilePath* platform_path) {
-  DCHECK(result);
-  *result = GetFileInfo(context, url, file_info, platform_path);
-  // We don't want the third party to delete our local file, so just returning
-  // NULL.
-  return NULL;
+    FilePath* platform_path,
+    SnapshotFilePolicy* policy) {
+  DCHECK(policy);
+  // We're just returning the local file information.
+  *policy = kSnapshotFileLocal;
+  return GetFileInfo(context, url, file_info, platform_path);
 }
 
 }  // namespace fileapi

webkit/fileapi/local_file_util.h

@@ -103,12 +103,12 @@ class FILEAPI_EXPORT_PRIVATE LocalFileUtil : public FileSystemFileUtil {
   virtual PlatformFileError DeleteSingleDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual scoped_refptr<webkit_blob::ShareableFileReference>
-      CreateSnapshotFile(FileSystemOperationContext* context,
-                         const FileSystemURL& url,
-                         base::PlatformFileError* result,
-                         base::PlatformFileInfo* file_info,
-                         FilePath* platform_path) OVERRIDE;
+  virtual PlatformFileError CreateSnapshotFile(
+      FileSystemOperationContext* context,
+      const FileSystemURL& url,
+      base::PlatformFileInfo* file_info,
+      FilePath* platform_path,
+      SnapshotFilePolicy* snapshot_policy) OVERRIDE;
 
  private:
   // Given the filesystem url, produces a real, full local path for the

webkit/fileapi/media/device_media_file_util.cc

@@ -7,7 +7,6 @@
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop_proxy.h"
-#include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/isolated_context.h"
@@ -16,7 +15,6 @@
 
 using base::PlatformFileError;
 using base::PlatformFileInfo;
-using webkit_blob::ShareableFileReference;
 
 namespace fileapi {
 
@@ -156,47 +154,44 @@ PlatformFileError DeviceMediaFileUtil::DeleteSingleDirectory(
   return base::PLATFORM_FILE_ERROR_SECURITY;
 }
 
-scoped_refptr<ShareableFileReference> DeviceMediaFileUtil::CreateSnapshotFile(
+base::PlatformFileError DeviceMediaFileUtil::CreateSnapshotFile(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
-    base::PlatformFileError* result,
     base::PlatformFileInfo* file_info,
-    FilePath* local_path) {
-  DCHECK(result);
+    FilePath* local_path,
+    SnapshotFilePolicy* snapshot_policy) {
   DCHECK(file_info);
   DCHECK(local_path);
+  DCHECK(snapshot_policy);
 
-  scoped_refptr<ShareableFileReference> file_ref;
-  if (!context->media_device()) {
-    *result = base::PLATFORM_FILE_ERROR_NOT_FOUND;
-    return file_ref;
-  }
+  if (!context->media_device())
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
-  *result = base::PLATFORM_FILE_ERROR_FAILED;
+  // We return a temporary file as a snapshot.
+  *snapshot_policy = FileSystemFileUtil::kSnapshotFileTemporary;
 
   // Create a temp file in "profile_path_/kDeviceMediaFileUtilTempDir".
   FilePath isolated_media_file_system_dir_path =
       profile_path_.Append(kDeviceMediaFileUtilTempDir);
   bool dir_exists = file_util::DirectoryExists(
       isolated_media_file_system_dir_path);
-  if (!dir_exists) {
-    if (!file_util::CreateDirectory(isolated_media_file_system_dir_path))
-      return file_ref;
+  if (!dir_exists &&
+      !file_util::CreateDirectory(isolated_media_file_system_dir_path)) {
+    LOG(WARNING) << "Could not create a directory for media snapshot file "
+                 << isolated_media_file_system_dir_path.value();
+    return base::PLATFORM_FILE_ERROR_FAILED;
   }
 
   bool file_created = file_util::CreateTemporaryFileInDir(
       isolated_media_file_system_dir_path, local_path);
-  if (!file_created)
-    return file_ref;
-
-  *result = context->media_device()->CreateSnapshotFile(url.path(), *local_path,
-                                                        file_info);
-  if (*result == base::PLATFORM_FILE_OK) {
-    file_ref = ShareableFileReference::GetOrCreate(
-        *local_path, ShareableFileReference::DELETE_ON_FINAL_RELEASE,
-        context->file_task_runner());
+  if (!file_created) {
+    LOG(WARNING) << "Could not create a temporary file for media snapshot in "
+                 << isolated_media_file_system_dir_path.value();
+    return base::PLATFORM_FILE_ERROR_FAILED;
   }
-  return file_ref;
+
+  return context->media_device()->CreateSnapshotFile(
+      url.path(), *local_path, file_info);
 }
 
 }  // namespace fileapi

webkit/fileapi/media/device_media_file_util.h

@@ -87,12 +87,12 @@ class FILEAPI_EXPORT_PRIVATE DeviceMediaFileUtil : public FileSystemFileUtil {
   virtual base::PlatformFileError DeleteSingleDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual scoped_refptr<webkit_blob::ShareableFileReference>
-      CreateSnapshotFile(FileSystemOperationContext* context,
-                         const FileSystemURL& url,
-                         base::PlatformFileError* result,
-                         base::PlatformFileInfo* file_info,
-                         FilePath* platform_path) OVERRIDE;
+  virtual base::PlatformFileError CreateSnapshotFile(
+      FileSystemOperationContext* context,
+      const FileSystemURL& url,
+      base::PlatformFileInfo* file_info,
+      FilePath* platform_path,
+      SnapshotFilePolicy* policy) OVERRIDE;
 
  private:
   // Profile path

webkit/fileapi/obfuscated_file_util.cc

@@ -892,16 +892,16 @@ PlatformFileError ObfuscatedFileUtil::DeleteSingleDirectory(
   return base::PLATFORM_FILE_OK;
 }
 
-scoped_refptr<webkit_blob::ShareableFileReference>
-ObfuscatedFileUtil::CreateSnapshotFile(
+base::PlatformFileError ObfuscatedFileUtil::CreateSnapshotFile(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
-    base::PlatformFileError* result,
     base::PlatformFileInfo* file_info,
-    FilePath* platform_path) {
-  DCHECK(result);
-  *result = GetFileInfo(context, url, file_info, platform_path);
-  return NULL;
+    FilePath* platform_path,
+    SnapshotFilePolicy* policy) {
+  DCHECK(policy);
+  // We're just returning the local file information.
+  *policy = kSnapshotFileLocal;
+  return GetFileInfo(context, url, file_info, platform_path);
 }
 
 FilePath ObfuscatedFileUtil::GetDirectoryForOriginAndType(

webkit/fileapi/obfuscated_file_util.h

@@ -122,12 +122,12 @@ class FILEAPI_EXPORT_PRIVATE ObfuscatedFileUtil : public FileSystemFileUtil {
   virtual base::PlatformFileError DeleteSingleDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual scoped_refptr<webkit_blob::ShareableFileReference>
-      CreateSnapshotFile(FileSystemOperationContext* context,
-                         const FileSystemURL& url,
-                         base::PlatformFileError* result,
-                         base::PlatformFileInfo* file_info,
-                         FilePath* platform_path) OVERRIDE;
+  virtual base::PlatformFileError CreateSnapshotFile(
+      FileSystemOperationContext* context,
+      const FileSystemURL& url,
+      base::PlatformFileInfo* file_info,
+      FilePath* platform_path,
+      SnapshotFilePolicy* policy) OVERRIDE;
 
   // Gets the topmost directory specific to this origin and type.  This will
   // contain both the directory database's files and all the backing file