Change some snapshot- or temporary-file related changes to use ScopedFile

- Remove FileSnapshotPolicy, and change
  FileSystemFileUtil::CreateSnapshotFile() to return webkit_blob::ScopedFile
  (if implementors want the backend to control the snapshot file's lifetime).
  This allows implementors to set arbitrary callbacks and
 also reduces the possibility of snapshot file leakage.

- Change DriveFileSyncService's temporary file handling to use
  webkit_blob::ScopedFile to simplify the codebase & reduce the
  possibility of temporary file leakage.

BUG=162598
TEST=existing tests

Review URL: https://codereview.chromium.org/14075016

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

10 files changed, 56 insertions, 81 deletions

diff --git a/webkit/fileapi/async_file_util.h b/webkit/fileapi/async_file_util.h
index 10b76af8..298a7a1 100644
--- a/webkit/fileapi/async_file_util.h
+++ b/webkit/fileapi/async_file_util.h
@@ -9,13 +9,16 @@
 #include "base/callback_forward.h"
 #include "base/files/file_util_proxy.h"
 #include "base/platform_file.h"
-#include "webkit/fileapi/file_snapshot_policy.h"
 #include "webkit/storage/webkit_storage_export.h"
 
 namespace base {
 class Time;
 }
 
+namespace webkit_blob {
+class ShareableFileReference;
+}
+
 namespace fileapi {
 
 class FileSystemOperationContext;
@@ -55,7 +58,8 @@ class WEBKIT_STORAGE_EXPORT AsyncFileUtil {
       void(base::PlatformFileError result,
            const base::PlatformFileInfo& file_info,
            const base::FilePath& platform_path,
-           SnapshotFilePolicy policy)> CreateSnapshotFileCallback;
+           const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref
+           )> CreateSnapshotFileCallback;
 
   AsyncFileUtil() {}
   virtual ~AsyncFileUtil() {}
@@ -297,9 +301,17 @@ class WEBKIT_STORAGE_EXPORT AsyncFileUtil {
   //
   // In the callback, it returns:
   // |file_info| is the metadata of the snapshot file created.
-  // |platform_path| is the path to the snapshot file created.
-  // |policy| should indicate the policy how the fileapi backend
-  // should handle the returned file.
+  // |platform_path| is the full absolute platform path to the snapshot
+  // file created.  If a file is not backed by a real local file in
+  // the implementor's FileSystem, the implementor must create a
+  // local snapshot file and return the path of the created file.
+  //
+  // If implementors creates a temporary file for snapshotting and wants
+  // FileAPI backend to take care of the lifetime of the file (so that
+  // it won't get deleted while JS layer has any references to the created
+  // File/Blob object), it should return non-empty |file_ref|.
+  // Via the |file_ref| implementors can schedule a file deletion
+  // or arbitrary callbacks when the last reference of File/Blob is dropped.
   //
   // LocalFileSystemOperation::CreateSnapshotFile calls this.
   //
diff --git a/webkit/fileapi/async_file_util_adapter.cc b/webkit/fileapi/async_file_util_adapter.cc
index eb30f3f..62fb835 100644
--- a/webkit/fileapi/async_file_util_adapter.cc
+++ b/webkit/fileapi/async_file_util_adapter.cc
@@ -7,19 +7,21 @@
 #include "base/bind.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task_runner_util.h"
+#include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_file_util.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/file_system_util.h"
 
-namespace fileapi {
-
 using base::Bind;
 using base::Callback;
 using base::Owned;
 using base::PlatformFileError;
 using base::Unretained;
+using webkit_blob::ShareableFileReference;
+
+namespace fileapi {
 
 namespace {
 
@@ -47,8 +49,7 @@ class EnsureFileExistsHelper {
 class GetFileInfoHelper {
  public:
   GetFileInfoHelper()
-      : error_(base::PLATFORM_FILE_OK),
-        snapshot_policy_(kSnapshotFileUnknown) {}
+      : error_(base::PLATFORM_FILE_OK) {}
 
   void GetFileInfo(FileSystemFileUtil* file_util,
                    FileSystemOperationContext* context,
@@ -59,8 +60,8 @@ class GetFileInfoHelper {
   void CreateSnapshotFile(FileSystemFileUtil* file_util,
                           FileSystemOperationContext* context,
                           const FileSystemURL& url) {
-    error_ = file_util->CreateSnapshotFile(
-        context, url, &file_info_, &platform_path_, &snapshot_policy_);
+    scoped_file_ = file_util->CreateSnapshotFile(
+        context, url, &error_, &file_info_, &platform_path_);
   }
 
   void ReplyFileInfo(const AsyncFileUtil::GetFileInfoCallback& callback) {
@@ -70,16 +71,16 @@ class GetFileInfoHelper {
 
   void ReplySnapshotFile(
       const AsyncFileUtil::CreateSnapshotFileCallback& callback) {
-    DCHECK(snapshot_policy_ != kSnapshotFileUnknown);
     if (!callback.is_null())
-      callback.Run(error_, file_info_, platform_path_, snapshot_policy_);
+      callback.Run(error_, file_info_, platform_path_,
+                   ShareableFileReference::GetOrCreate(scoped_file_.Pass()));
   }
 
  private:
   base::PlatformFileError error_;
   base::PlatformFileInfo file_info_;
   base::FilePath platform_path_;
-  SnapshotFilePolicy snapshot_policy_;
+  webkit_blob::ScopedFile scoped_file_;
   DISALLOW_COPY_AND_ASSIGN(GetFileInfoHelper);
 };
 
diff --git a/webkit/fileapi/file_snapshot_policy.h b/webkit/fileapi/file_snapshot_policy.h
deleted file mode 100644
index 7386684..0000000
--- a/webkit/fileapi/file_snapshot_policy.h
+++ /dev/null
@@ -1,26 +0,0 @@
-// 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.
-
-#ifndef WEBKIT_FILEAPI_FILE_SNAPSHOT_POLICY_H_
-#define WEBKIT_FILEAPI_FILE_SNAPSHOT_POLICY_H_
-
-namespace fileapi {
-
-// 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,
-};
-
-}  // namespace fileapi
-
-#endif  // WEBKIT_FILEAPI_FILE_SNAPSHOT_POLICY_H_
diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h
index 66b539b..5a094f1 100644
--- a/webkit/fileapi/file_system_file_util.h
+++ b/webkit/fileapi/file_system_file_util.h
@@ -8,7 +8,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/platform_file.h"
-#include "webkit/fileapi/file_snapshot_policy.h"
+#include "webkit/blob/scoped_file.h"
 #include "webkit/storage/webkit_storage_export.h"
 
 namespace base {
@@ -170,12 +170,12 @@ class WEBKIT_STORAGE_EXPORT FileSystemFileUtil {
   //
   // See header comments for AsyncFileUtil::CreateSnapshotFile() for
   // more details.
-  virtual base::PlatformFileError CreateSnapshotFile(
+  virtual webkit_blob::ScopedFile CreateSnapshotFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url,
+      base::PlatformFileError* error,
       base::PlatformFileInfo* file_info,
-      base::FilePath* platform_path,
-      SnapshotFilePolicy* policy) = 0;
+      base::FilePath* platform_path) = 0;
 
  protected:
   FileSystemFileUtil() {}
diff --git a/webkit/fileapi/local_file_system_operation.cc b/webkit/fileapi/local_file_system_operation.cc
index 1bd0e52..ce1378e 100644
--- a/webkit/fileapi/local_file_system_operation.cc
+++ b/webkit/fileapi/local_file_system_operation.cc
@@ -28,6 +28,7 @@
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/quota_types.h"
 
+using webkit_blob::ScopedFile;
 using webkit_blob::ShareableFileReference;
 
 namespace fileapi {
@@ -827,14 +828,7 @@ void LocalFileSystemOperation::DidCreateSnapshotFile(
     base::PlatformFileError result,
     const base::PlatformFileInfo& file_info,
     const base::FilePath& platform_path,
-    SnapshotFilePolicy snapshot_policy) {
-  scoped_refptr<ShareableFileReference> file_ref;
-  if (result == base::PLATFORM_FILE_OK &&
-      snapshot_policy == kSnapshotFileTemporary) {
-    file_ref = ShareableFileReference::GetOrCreate(
-        platform_path, ShareableFileReference::DELETE_ON_FINAL_RELEASE,
-        file_system_context()->task_runners()->file_task_runner());
-  }
+    const scoped_refptr<ShareableFileReference>& file_ref) {
   callback.Run(result, file_info, platform_path, file_ref);
 }
 
diff --git a/webkit/fileapi/local_file_system_operation.h b/webkit/fileapi/local_file_system_operation.h
index f3f7083..08365c9 100644
--- a/webkit/fileapi/local_file_system_operation.h
+++ b/webkit/fileapi/local_file_system_operation.h
@@ -9,7 +9,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "webkit/fileapi/file_snapshot_policy.h"
+#include "webkit/blob/scoped_file.h"
 #include "webkit/fileapi/file_system_operation.h"
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/file_writer_delegate.h"
@@ -281,10 +281,10 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
                    bool created);
   void DidCreateSnapshotFile(
       const SnapshotFileCallback& callback,
-      base::PlatformFileError rv,
+      base::PlatformFileError result,
       const base::PlatformFileInfo& file_info,
       const base::FilePath& platform_path,
-      SnapshotFilePolicy snapshot_policy);
+      const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref);
 
   // Checks the validity of a given |url| and populates |file_util| for |mode|.
   base::PlatformFileError SetUp(
diff --git a/webkit/fileapi/local_file_util.cc b/webkit/fileapi/local_file_util.cc
index 6e65518..4760423 100644
--- a/webkit/fileapi/local_file_util.cc
+++ b/webkit/fileapi/local_file_util.cc
@@ -246,21 +246,18 @@ PlatformFileError LocalFileUtil::DeleteDirectory(
   return NativeFileUtil::DeleteDirectory(file_path);
 }
 
-base::PlatformFileError LocalFileUtil::CreateSnapshotFile(
+webkit_blob::ScopedFile LocalFileUtil::CreateSnapshotFile(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
+    base::PlatformFileError* error,
     base::PlatformFileInfo* file_info,
-    base::FilePath* platform_path,
-    SnapshotFilePolicy* policy) {
-  DCHECK(policy);
+    base::FilePath* platform_path) {
   DCHECK(file_info);
   // We're just returning the local file information.
-  *policy = kSnapshotFileLocal;
-  base::PlatformFileError error =
-      GetFileInfo(context, url, file_info, platform_path);
-  if (error == base::PLATFORM_FILE_OK && file_info->is_directory)
-    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
-  return error;
+  *error = GetFileInfo(context, url, file_info, platform_path);
+  if (*error == base::PLATFORM_FILE_OK && file_info->is_directory)
+    *error = base::PLATFORM_FILE_ERROR_NOT_A_FILE;
+  return webkit_blob::ScopedFile();
 }
 
 }  // namespace fileapi
diff --git a/webkit/fileapi/local_file_util.h b/webkit/fileapi/local_file_util.h
index f7bc9f9..0321f2c6 100644
--- a/webkit/fileapi/local_file_util.h
+++ b/webkit/fileapi/local_file_util.h
@@ -82,12 +82,12 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE LocalFileUtil : public FileSystemFileUtil {
   virtual base::PlatformFileError DeleteDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual base::PlatformFileError CreateSnapshotFile(
+  virtual webkit_blob::ScopedFile CreateSnapshotFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url,
+      base::PlatformFileError* error,
       base::PlatformFileInfo* file_info,
-      base::FilePath* platform_path,
-      SnapshotFilePolicy* snapshot_policy) OVERRIDE;
+      base::FilePath* platform_path) OVERRIDE;
 
  private:
   // Given the filesystem url, produces a real, full local path for the
diff --git a/webkit/fileapi/obfuscated_file_util.cc b/webkit/fileapi/obfuscated_file_util.cc
index 84dd1f6..8103e62 100644
--- a/webkit/fileapi/obfuscated_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_util.cc
@@ -857,22 +857,19 @@ PlatformFileError ObfuscatedFileUtil::DeleteDirectory(
   return base::PLATFORM_FILE_OK;
 }
 
-base::PlatformFileError ObfuscatedFileUtil::CreateSnapshotFile(
+webkit_blob::ScopedFile ObfuscatedFileUtil::CreateSnapshotFile(
     FileSystemOperationContext* context,
     const FileSystemURL& url,
+    base::PlatformFileError* error,
     base::PlatformFileInfo* file_info,
-    base::FilePath* platform_path,
-    SnapshotFilePolicy* policy) {
-  DCHECK(policy);
+    base::FilePath* platform_path) {
   // We're just returning the local file information.
-  *policy = kSnapshotFileLocal;
-  base::PlatformFileError error = GetFileInfo(
-      context, url, file_info, platform_path);
-  if (error == base::PLATFORM_FILE_OK && file_info->is_directory) {
+  *error = GetFileInfo(context, url, file_info, platform_path);
+  if (*error == base::PLATFORM_FILE_OK && file_info->is_directory) {
     *file_info = base::PlatformFileInfo();
-    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
+    *error = base::PLATFORM_FILE_ERROR_NOT_A_FILE;
   }
-  return error;
+  return webkit_blob::ScopedFile();
 }
 
 scoped_ptr<FileSystemFileUtil::AbstractFileEnumerator>
diff --git a/webkit/fileapi/obfuscated_file_util.h b/webkit/fileapi/obfuscated_file_util.h
index 74c5f04..7f13995 100644
--- a/webkit/fileapi/obfuscated_file_util.h
+++ b/webkit/fileapi/obfuscated_file_util.h
@@ -112,12 +112,12 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
   virtual base::PlatformFileError DeleteDirectory(
       FileSystemOperationContext* context,
       const FileSystemURL& url) OVERRIDE;
-  virtual base::PlatformFileError CreateSnapshotFile(
+  virtual webkit_blob::ScopedFile CreateSnapshotFile(
       FileSystemOperationContext* context,
       const FileSystemURL& url,
+      base::PlatformFileError* error,
       base::PlatformFileInfo* file_info,
-      base::FilePath* platform_path,
-      SnapshotFilePolicy* policy) OVERRIDE;
+      base::FilePath* platform_path) OVERRIDE;
 
   // Same as the other CreateFileEnumerator, but with recursive support.
   scoped_ptr<AbstractFileEnumerator> CreateFileEnumerator(