Bind: Merge FileUtilProxy and FileSystemFileUtilProxy: Delete/Touch/Truncate/Copy/Move

webkit/fileapi/file_system_operation.cc

Index: webkit/fileapi/file_system_operation.cc
diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index c651322ff2ac5b3fd50a59bc2df5159441d0c19a..90626af1f34671d5830402480a25316decdfb5f6 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -78,7 +78,7 @@ FileSystemOperation::~FileSystemOperation() {
                    base::Unretained(operation_context_.src_file_util()),
                    base::Unretained(&operation_context_)),
         file_writer_delegate_->file(),
-        FileSystemFileUtilProxy::StatusCallback());
+        base::FileUtilProxy::StatusCallback());
 }
 
 void FileSystemOperation::OpenFileSystem(
@@ -114,15 +114,14 @@ void FileSystemOperation::CreateFile(const GURL& path,
     delete this;
     return;
   }
-  exclusive_ = exclusive;
-
   GetUsageAndQuotaThenCallback(
       operation_context_.src_origin_url(),
       base::Bind(&FileSystemOperation::DelayedCreateFileForQuota,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(), exclusive));
 }
 
 void FileSystemOperation::DelayedCreateFileForQuota(
+    bool exclusive,
     quota::QuotaStatusCode status, int64 usage, int64 quota) {
   operation_context_.set_allowed_bytes_growth(quota - usage);
 
@@ -136,8 +135,8 @@ void FileSystemOperation::DelayedCreateFileForQuota(
       proxy_,
       src_virtual_path_,
       base::Bind(
-          exclusive_ ? &FileSystemOperation::DidEnsureFileExistsExclusive
-                     : &FileSystemOperation::DidEnsureFileExistsNonExclusive,
+          exclusive ? &FileSystemOperation::DidEnsureFileExistsExclusive
+                    : &FileSystemOperation::DidEnsureFileExistsNonExclusive,
           weak_factory_.GetWeakPtr()));
 }
 
@@ -152,16 +151,14 @@ void FileSystemOperation::CreateDirectory(const GURL& path,
     delete this;
     return;
   }
-  exclusive_ = exclusive;
-  recursive_ = recursive;
-
   GetUsageAndQuotaThenCallback(
       operation_context_.src_origin_url(),
       base::Bind(&FileSystemOperation::DelayedCreateDirectoryForQuota,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(), exclusive, recursive));
 }
 
 void FileSystemOperation::DelayedCreateDirectoryForQuota(
+    bool exclusive, bool recursive,
     quota::QuotaStatusCode status, int64 usage, int64 quota) {
   operation_context_.set_allowed_bytes_growth(quota - usage);
 
@@ -170,9 +167,12 @@ void FileSystemOperation::DelayedCreateDirectoryForQuota(
       operation_context_.src_origin_url(),
       operation_context_.src_type()));
 
-  FileSystemFileUtilProxy::CreateDirectory(
-      operation_context_, proxy_, src_virtual_path_, exclusive_,
-      recursive_,
+  base::FileUtilProxy::RelayFileTask(
+      proxy_, FROM_HERE,
+      base::Bind(&FileSystemFileUtil::CreateDirectory,
+                 base::Unretained(operation_context_.src_file_util()),
+                 base::Unretained(&operation_context_),

[willchan no longer on Chromium, 2011/11/01 23:56:48]

Sorry, I'm not familiar with the code. It's not obvious to me that this is
correct. What prevents operation_context_ from going away? I think this
FileSystemOperation is supposed to be self-deleting, right? That said, I don't
know when it gets deleted, and it looks like
FileSystemOperation::DidFinishOperation() uses a weak pointer, so that makes me
a bit worried. Is base::Unretained() correct here? Is there someone else who can
look at this to doublecheck? Adam Klein perhaps?

[kinuko, 2011/11/02 08:04:28]

Right, it's self-deleting and WeakPtr makes things confusing.  I converted some
'delete this' with base::Owned(this) and removed WeakPtr usage.  Verified that
valgrind says ok in my local env.  Hoping that Eric, Taiju or Adam can
doublecheck the changes under fileapi.

+                 src_virtual_path_, exclusive, recursive),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  weak_factory_.GetWeakPtr()));
 }
@@ -204,9 +204,12 @@ void FileSystemOperation::DelayedCopyForQuota(quota::QuotaStatusCode status,
       operation_context_.dest_origin_url(),
       operation_context_.dest_type()));
 
-  FileSystemFileUtilProxy::Copy(
-      operation_context_, proxy_, src_virtual_path_,
-      dest_virtual_path_,
+  base::FileUtilProxy::RelayFileTask(
+      proxy_, FROM_HERE,
+      base::Bind(&FileSystemFileUtil::Copy,
+                 base::Unretained(operation_context_.src_file_util()),
+                 base::Unretained(&operation_context_),
+                 src_virtual_path_, dest_virtual_path_),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  weak_factory_.GetWeakPtr()));
 }
@@ -238,9 +241,12 @@ void FileSystemOperation::DelayedMoveForQuota(quota::QuotaStatusCode status,
       operation_context_.dest_origin_url(),
       operation_context_.dest_type()));
 
-  FileSystemFileUtilProxy::Move(
-      operation_context_, proxy_, src_virtual_path_,
-      dest_virtual_path_,
+  base::FileUtilProxy::RelayFileTask(
+      proxy_, FROM_HERE,
+      base::Bind(&FileSystemFileUtil::Move,
+                 base::Unretained(operation_context_.src_file_util()),
+                 base::Unretained(&operation_context_),
+                 src_virtual_path_, dest_virtual_path_),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  weak_factory_.GetWeakPtr()));
 }
@@ -319,8 +325,12 @@ void FileSystemOperation::Remove(const GURL& path, bool recursive) {
     return;
   }
 
-  FileSystemFileUtilProxy::Delete(
-      operation_context_, proxy_, src_virtual_path_, recursive,
+  base::FileUtilProxy::RelayFileTask(
+      proxy_, FROM_HERE,
+      base::Bind(&FileSystemFileUtil::Delete,
+                 base::Unretained(operation_context_.src_file_util()),
+                 base::Unretained(&operation_context_),
+                 src_virtual_path_, recursive),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  weak_factory_.GetWeakPtr()));
 }
@@ -386,15 +396,14 @@ void FileSystemOperation::Truncate(const GURL& path, int64 length) {
     delete this;
     return;
   }
-  length_ = length;
-
   GetUsageAndQuotaThenCallback(
       operation_context_.src_origin_url(),
       base::Bind(&FileSystemOperation::DelayedTruncateForQuota,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(), length));
 }
 
-void FileSystemOperation::DelayedTruncateForQuota(quota::QuotaStatusCode status,
+void FileSystemOperation::DelayedTruncateForQuota(int64 length,
+                                                  quota::QuotaStatusCode status,
                                                   int64 usage, int64 quota) {
   operation_context_.set_allowed_bytes_growth(quota - usage);
 
@@ -403,8 +412,12 @@ void FileSystemOperation::DelayedTruncateForQuota(quota::QuotaStatusCode status,
       operation_context_.src_origin_url(),
       operation_context_.src_type()));
 
-  FileSystemFileUtilProxy::Truncate(
-      operation_context_, proxy_, src_virtual_path_, length_,
+  base::FileUtilProxy::RelayFileTask(
+      proxy_, FROM_HERE,
+      base::Bind(&FileSystemFileUtil::Truncate,
+                 base::Unretained(operation_context_.src_file_util()),
+                 base::Unretained(&operation_context_),
+                 src_virtual_path_, length),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  weak_factory_.GetWeakPtr()));
 }
@@ -421,9 +434,12 @@ void FileSystemOperation::TouchFile(const GURL& path,
     return;
   }
 
-  FileSystemFileUtilProxy::Touch(
-      operation_context_, proxy_, src_virtual_path_,
-      last_access_time, last_modified_time,
+  base::FileUtilProxy::RelayFileTask(
+      proxy_, FROM_HERE,
+      base::Bind(&FileSystemFileUtil::Touch,
+                 base::Unretained(operation_context_.src_file_util()),
+                 base::Unretained(&operation_context_),
+                 src_virtual_path_, last_access_time, last_modified_time),
       base::Bind(&FileSystemOperation::DidTouchFile,
                  weak_factory_.GetWeakPtr()));
 }
@@ -459,15 +475,14 @@ void FileSystemOperation::OpenFile(const GURL& path,
       return;
     }
   }
-  file_flags_ = file_flags;
-
   GetUsageAndQuotaThenCallback(
       operation_context_.src_origin_url(),
       base::Bind(&FileSystemOperation::DelayedOpenFileForQuota,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(), file_flags));
 }
 
-void FileSystemOperation::DelayedOpenFileForQuota(quota::QuotaStatusCode status,
+void FileSystemOperation::DelayedOpenFileForQuota(int file_flags,
+                                                  quota::QuotaStatusCode status,
                                                   int64 usage, int64 quota) {
   operation_context_.set_allowed_bytes_growth(quota - usage);
 
@@ -482,7 +497,7 @@ void FileSystemOperation::DelayedOpenFileForQuota(quota::QuotaStatusCode status,
                  base::Unretained(operation_context_.src_file_util()),
                  base::Unretained(&operation_context_),
                  src_virtual_path_,
-                 file_flags_),
+                 file_flags),
       base::Bind(&FileSystemFileUtil::Close,
                  base::Unretained(operation_context_.src_file_util()),
                  base::Unretained(&operation_context_)),