2nd try: FileAPI: Split recursive remove into multiple tasks.

Original review page: https://codereview.chromium.org/12018017/

While this change makes each recursive delete run slower, concurrent jobs can run without waiting for the entire recursive job to finish.

Recursive remove end-to-end (200 files, 120 dirs, 4-level tree, total 100MB):
Before this change: Ave: 72.94 msec, Stddev: 3.571 msec
After this change: Ave:129.54 msec, Stddev: 25.368 msec

It takes 1.76 times slower than before by average (we can probably do more optimization as this implementation doesn't care much about it), while another concurrent file task called immediately after the delete could finish in 1-5 msec (while back then it needed to wait for 70-80 msec).

BUG=146215
TEST=content_unittests:.*File.*,content_browsertests:FileSystemLayoutTests.OpRemove


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

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

11 files changed, 424 insertions, 52 deletions

diff --git a/webkit/fileapi/file_system_file_util_proxy.cc b/webkit/fileapi/file_system_file_util_proxy.cc
index b9ce096..d7caf8e 100644
--- a/webkit/fileapi/file_system_file_util_proxy.cc
+++ b/webkit/fileapi/file_system_file_util_proxy.cc
@@ -108,15 +108,28 @@ class ReadDirectoryHelper {
 }  // namespace
 
 // static
-bool FileSystemFileUtilProxy::Delete(
+bool FileSystemFileUtilProxy::DeleteFile(
+    FileSystemOperationContext* context,
+    FileSystemFileUtil* file_util,
+    const FileSystemURL& url,
+    const StatusCallback& callback) {
+  return base::PostTaskAndReplyWithResult(
+      context->task_runner(), FROM_HERE,
+      Bind(&FileSystemFileUtil::DeleteFile, Unretained(file_util),
+           context, url),
+      callback);
+}
+
+// static
+bool FileSystemFileUtilProxy::DeleteDirectory(
     FileSystemOperationContext* context,
     FileSystemFileUtil* file_util,
     const FileSystemURL& url,
-    bool recursive,
     const StatusCallback& callback) {
   return base::PostTaskAndReplyWithResult(
       context->task_runner(), FROM_HERE,
-      Bind(&FileUtilHelper::Delete, context, file_util, url, recursive),
+      Bind(&FileSystemFileUtil::DeleteDirectory, Unretained(file_util),
+           context, url),
       callback);
 }
 
diff --git a/webkit/fileapi/file_system_file_util_proxy.h b/webkit/fileapi/file_system_file_util_proxy.h
index 903b821..560ed6d 100644
--- a/webkit/fileapi/file_system_file_util_proxy.h
+++ b/webkit/fileapi/file_system_file_util_proxy.h
@@ -41,13 +41,18 @@ class FileSystemFileUtilProxy {
            FileSystemFileUtil::SnapshotFilePolicy snapshot_policy)>
       SnapshotFileCallback;
 
-  // Deletes a file or a directory on the given context's task_runner.
-  // It is an error to delete a non-empty directory with recursive=false.
-  static bool Delete(
+  // Deletes a file on the given context's task_runner.
+  static bool DeleteFile(
+      FileSystemOperationContext* context,
+      FileSystemFileUtil* file_util,
+      const FileSystemURL& url,
+      const StatusCallback& callback);
+
+  // Deletes a directory on the given context's task_runner.
+  static bool DeleteDirectory(
       FileSystemOperationContext* context,
       FileSystemFileUtil* file_util,
       const FileSystemURL& url,
-      bool recursive,
       const StatusCallback& callback);
 
   // Creates or opens a file with the given flags by calling |file_util|'s
diff --git a/webkit/fileapi/local_file_system_operation.cc b/webkit/fileapi/local_file_system_operation.cc
index 1aa7071..dc0281a 100644
--- a/webkit/fileapi/local_file_system_operation.cc
+++ b/webkit/fileapi/local_file_system_operation.cc
@@ -20,6 +20,7 @@
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/fileapi/file_writer_delegate.h"
+#include "webkit/fileapi/remove_operation_delegate.h"
 #include "webkit/fileapi/sandbox_file_stream_writer.h"
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/quota_types.h"
@@ -44,6 +45,8 @@ bool IsCrossOperationAllowed(FileSystemType src_type,
 
 }  // namespace
 
+// LocalFileSystemOperation::ScopedUpdateNotifier -----------------------------
+
 class LocalFileSystemOperation::ScopedUpdateNotifier {
  public:
   ScopedUpdateNotifier(FileSystemOperationContext* operation_context,
@@ -71,7 +74,11 @@ LocalFileSystemOperation::ScopedUpdateNotifier::~ScopedUpdateNotifier() {
       &FileUpdateObserver::OnEndUpdate, MakeTuple(url_));
 }
 
+// LocalFileSystemOperation ---------------------------------------------------
+
 LocalFileSystemOperation::~LocalFileSystemOperation() {
+  if (!termination_callback_.is_null())
+    termination_callback_.Run();
 }
 
 void LocalFileSystemOperation::CreateFile(const FileSystemURL& url,
@@ -187,7 +194,7 @@ void LocalFileSystemOperation::DirectoryExists(const FileSystemURL& url,
   }
 
   FileSystemFileUtilProxy::GetFileInfo(
-      operation_context_.get(), src_util_, url,
+      operation_context(), src_util_, url,
       base::Bind(&LocalFileSystemOperation::DidDirectoryExists,
                  base::Owned(this), callback));
 }
@@ -204,7 +211,7 @@ void LocalFileSystemOperation::FileExists(const FileSystemURL& url,
   }
 
   FileSystemFileUtilProxy::GetFileInfo(
-      operation_context_.get(), src_util_, url,
+      operation_context(), src_util_, url,
       base::Bind(&LocalFileSystemOperation::DidFileExists,
                  base::Owned(this), callback));
 }
@@ -221,7 +228,7 @@ void LocalFileSystemOperation::GetMetadata(
   }
 
   FileSystemFileUtilProxy::GetFileInfo(
-      operation_context_.get(), src_util_, url,
+      operation_context(), src_util_, url,
       base::Bind(&LocalFileSystemOperation::DidGetMetadata,
                  base::Owned(this), callback));
 }
@@ -238,7 +245,7 @@ void LocalFileSystemOperation::ReadDirectory(
   }
 
   FileSystemFileUtilProxy::ReadDirectory(
-      operation_context_.get(), src_util_, url,
+      operation_context(), src_util_, url,
       base::Bind(&LocalFileSystemOperation::DidReadDirectory,
                  base::Owned(this), callback));
 }
@@ -247,18 +254,14 @@ void LocalFileSystemOperation::Remove(const FileSystemURL& url,
                                       bool recursive,
                                       const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationRemove));
-
-  base::PlatformFileError result = SetUp(url, &src_util_, SETUP_FOR_WRITE);
-  if (result != base::PLATFORM_FILE_OK) {
-    callback.Run(result);
-    delete this;
-    return;
-  }
-
-  FileSystemFileUtilProxy::Delete(
-      operation_context_.get(), src_util_, url, recursive,
-      base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
-                 base::Owned(this), callback));
+  DCHECK(!remove_operation_delegate_);
+  remove_operation_delegate_.reset(new RemoveOperationDelegate(
+      this, base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
+                       base::Unretained(this), callback)));
+  if (recursive)
+    remove_operation_delegate_->RunRecursively(url);
+  else
+    remove_operation_delegate_->Run(url);
 }
 
 void LocalFileSystemOperation::Write(
@@ -301,7 +304,7 @@ void LocalFileSystemOperation::TouchFile(const FileSystemURL& url,
   }
 
   FileSystemFileUtilProxy::Touch(
-      operation_context_.get(), src_util_, url,
+      operation_context(), src_util_, url,
       last_access_time, last_modified_time,
       base::Bind(&LocalFileSystemOperation::DidTouchFile,
                  base::Owned(this), callback));
@@ -405,7 +408,7 @@ void LocalFileSystemOperation::SyncGetPlatformPath(const FileSystemURL& url,
     return;
   }
 
-  src_util_->GetLocalFilePath(operation_context_.get(), url, platform_path);
+  src_util_->GetLocalFilePath(operation_context(), url, platform_path);
 
   delete this;
 }
@@ -423,7 +426,7 @@ void LocalFileSystemOperation::CreateSnapshotFile(
   }
 
   FileSystemFileUtilProxy::CreateSnapshotFile(
-      operation_context_.get(), src_util_, url,
+      operation_context(), src_util_, url,
       base::Bind(&LocalFileSystemOperation::DidCreateSnapshotFile,
                  base::Owned(this), callback));
 }
@@ -450,6 +453,40 @@ void LocalFileSystemOperation::CopyInForeignFile(
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
 }
 
+void LocalFileSystemOperation::RemoveFile(
+    const FileSystemURL& url,
+    const StatusCallback& callback) {
+  DCHECK(SetPendingOperationType(kOperationRemove));
+  base::PlatformFileError result = SetUp(url, &src_util_, SETUP_FOR_WRITE);
+  if (result != base::PLATFORM_FILE_OK) {
+    callback.Run(result);
+    delete this;
+    return;
+  }
+
+  FileSystemFileUtilProxy::DeleteFile(
+      operation_context(), src_util_, url,
+      base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
+                 base::Owned(this), callback));
+}
+
+void LocalFileSystemOperation::RemoveDirectory(
+    const FileSystemURL& url,
+    const StatusCallback& callback) {
+  DCHECK(SetPendingOperationType(kOperationRemove));
+  base::PlatformFileError result = SetUp(url, &src_util_, SETUP_FOR_WRITE);
+  if (result != base::PLATFORM_FILE_OK) {
+    callback.Run(result);
+    delete this;
+    return;
+  }
+
+  FileSystemFileUtilProxy::DeleteDirectory(
+      operation_context(), src_util_, url,
+      base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
+                 base::Owned(this), callback));
+}
+
 LocalFileSystemOperation::LocalFileSystemOperation(
     FileSystemContext* file_system_context,
     scoped_ptr<FileSystemOperationContext> operation_context)
@@ -457,6 +494,7 @@ LocalFileSystemOperation::LocalFileSystemOperation(
       operation_context_(operation_context.Pass()),
       src_util_(NULL),
       dest_util_(NULL),
+      overriding_operation_context_(NULL),
       is_cross_operation_(false),
       peer_handle_(base::kNullProcessHandle),
       pending_operation_(kOperationNone),
@@ -474,7 +512,7 @@ void LocalFileSystemOperation::GetUsageAndQuotaThenRunTask(
       !file_system_context()->GetQuotaUtil(url.type())) {
     // If we don't have the quota manager or the requested filesystem type
     // does not support quota, we should be able to let it go.
-    operation_context_->set_allowed_bytes_growth(kint64max);
+    operation_context()->set_allowed_bytes_growth(kint64max);
     task.Run();
     return;
   }
@@ -499,7 +537,7 @@ void LocalFileSystemOperation::DidGetUsageAndQuotaAndRunTask(
     return;
   }
 
-  operation_context_->set_allowed_bytes_growth(quota - usage);
+  operation_context()->set_allowed_bytes_growth(quota - usage);
   task.Run();
 }
 
@@ -556,7 +594,7 @@ void LocalFileSystemOperation::DoCreateFile(
     const StatusCallback& callback,
     bool exclusive) {
   FileSystemFileUtilProxy::EnsureFileExists(
-      operation_context_.get(),
+      operation_context(),
       src_util_, url,
       base::Bind(
           exclusive ?
@@ -570,7 +608,7 @@ void LocalFileSystemOperation::DoCreateDirectory(
     const StatusCallback& callback,
     bool exclusive, bool recursive) {
   FileSystemFileUtilProxy::CreateDirectory(
-      operation_context_.get(),
+      operation_context(),
       src_util_, url, exclusive, recursive,
       base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
@@ -580,7 +618,7 @@ void LocalFileSystemOperation::DoCopy(const FileSystemURL& src_url,
                                       const FileSystemURL& dest_url,
                                       const StatusCallback& callback) {
   FileSystemFileUtilProxy::Copy(
-      operation_context_.get(),
+      operation_context(),
       src_util_, dest_util_,
       src_url, dest_url,
       base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
@@ -592,7 +630,7 @@ void LocalFileSystemOperation::DoCopyInForeignFile(
     const FileSystemURL& dest_url,
     const StatusCallback& callback) {
   FileSystemFileUtilProxy::CopyInForeignFile(
-      operation_context_.get(),
+      operation_context(),
       dest_util_,
       src_local_disk_file_path, dest_url,
       base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
@@ -603,7 +641,7 @@ void LocalFileSystemOperation::DoMove(const FileSystemURL& src_url,
                                       const FileSystemURL& dest_url,
                                       const StatusCallback& callback) {
   FileSystemFileUtilProxy::Move(
-      operation_context_.get(),
+      operation_context(),
       src_util_, dest_util_,
       src_url, dest_url,
       base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
@@ -614,7 +652,7 @@ void LocalFileSystemOperation::DoTruncate(const FileSystemURL& url,
                                           const StatusCallback& callback,
                                           int64 length) {
   FileSystemFileUtilProxy::Truncate(
-      operation_context_.get(), src_util_, url, length,
+      operation_context(), src_util_, url, length,
       base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
 }
@@ -623,7 +661,7 @@ void LocalFileSystemOperation::DoOpenFile(const FileSystemURL& url,
                                           const OpenFileCallback& callback,
                                           int file_flags) {
   FileSystemFileUtilProxy::CreateOrOpen(
-      operation_context_.get(), src_util_, url, file_flags,
+      operation_context(), src_util_, url, file_flags,
       base::Bind(&LocalFileSystemOperation::DidOpenFile,
                  base::Owned(this), callback));
 }
@@ -658,6 +696,15 @@ void LocalFileSystemOperation::DidFinishFileOperation(
   }
 }
 
+void LocalFileSystemOperation::DidFinishDelegatedOperation(
+    const StatusCallback& callback,
+    base::PlatformFileError rv) {
+  // The callback might be held by the delegate and Owned() may not work,
+  // so just explicitly delete this now.
+  callback.Run(rv);
+  delete this;
+}
+
 void LocalFileSystemOperation::DidDirectoryExists(
     const StatusCallback& callback,
     base::PlatformFileError rv,
@@ -709,7 +756,7 @@ void LocalFileSystemOperation::DidWrite(
   const bool complete = (
       write_status != FileWriterDelegate::SUCCESS_IO_PENDING);
   if (complete && write_status != FileWriterDelegate::ERROR_WRITE_NOT_STARTED) {
-    operation_context_->change_observers()->Notify(
+    operation_context()->change_observers()->Notify(
         &FileChangeObserver::OnModifyFile, MakeTuple(url));
   }
 
@@ -761,23 +808,29 @@ base::PlatformFileError LocalFileSystemOperation::SetUp(
       mode != SETUP_FOR_READ)
     return base::PLATFORM_FILE_ERROR_SECURITY;
 
-  if (!file_system_context()->GetMountPointProvider(
-          url.type())->IsAccessAllowed(url))
-    return base::PLATFORM_FILE_ERROR_SECURITY;
-
   DCHECK(file_util);
   if (!*file_util)
     *file_util = file_system_context()->GetFileUtil(url.type());
   if (!*file_util)
     return base::PLATFORM_FILE_ERROR_SECURITY;
 
+  // If this operation is created for recursive sub-operations (i.e.
+  // operation context is overridden from another operation) we skip
+  // some duplicated security checks.
+  if (overriding_operation_context_)
+    return base::PLATFORM_FILE_OK;
+
+  if (!file_system_context()->GetMountPointProvider(
+          url.type())->IsAccessAllowed(url))
+    return base::PLATFORM_FILE_ERROR_SECURITY;
+
   if (mode == SETUP_FOR_READ) {
     // TODO(kinuko): This doesn't work well for cross-filesystem operation
     // in the current architecture since the operation context (thus the
     // observers) is configured for the destination URL while this method
     // could be called for both src and dest URL.
     if (!is_cross_operation_) {
-      operation_context_->access_observers()->Notify(
+      operation_context()->access_observers()->Notify(
           &FileAccessObserver::OnAccess, MakeTuple(url));
     }
     return base::PLATFORM_FILE_OK;
@@ -786,7 +839,7 @@ base::PlatformFileError LocalFileSystemOperation::SetUp(
   DCHECK(mode == SETUP_FOR_WRITE || mode == SETUP_FOR_CREATE);
 
   scoped_update_notifiers_.push_back(new ScopedUpdateNotifier(
-      operation_context_.get(), url));
+      operation_context(), url));
 
   // Any write access is disallowed on the root path.
   if (url.path().value().length() == 0 ||
diff --git a/webkit/fileapi/local_file_system_operation.h b/webkit/fileapi/local_file_system_operation.h
index e80e91f..fafaca6 100644
--- a/webkit/fileapi/local_file_system_operation.h
+++ b/webkit/fileapi/local_file_system_operation.h
@@ -25,6 +25,7 @@ class CrosMountPointProvider;
 namespace fileapi {
 
 class FileSystemContext;
+class RemoveOperationDelegate;
 
 // FileSystemOperation implementation for local file systems.
 class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
@@ -78,10 +79,39 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
       const FileSystemURL& path,
       const SnapshotFileCallback& callback) OVERRIDE;
 
+  // Copies in a single file from a different filesystem.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |src_file_path|
+  //   or the parent directory of |dest_url| does not exist.
+  // - PLATFORM_FILE_ERROR_INVALID_OPERATION if |dest_url| exists and
+  //   is not a file.
+  // - PLATFORM_FILE_ERROR_FAILED if |dest_url| does not exist and
+  //   its parent path is a file.
+  //
   void CopyInForeignFile(const FilePath& src_local_disk_path,
                          const FileSystemURL& dest_url,
                          const StatusCallback& callback);
 
+  // Removes a single file.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |url| does not exist.
+  // - PLATFORM_FILE_ERROR_NOT_A_FILE if |url| is not a file.
+  //
+  void RemoveFile(const FileSystemURL& url,
+                  const StatusCallback& callback);
+
+  // Removes a single empty directory.
+  //
+  // This returns:
+  // - PLATFORM_FILE_ERROR_NOT_FOUND if |url| does not exist.
+  // - PLATFORM_FILE_ERROR_NOT_A_DIRECTORY if |url| is not a directory.
+  // - PLATFORM_FILE_ERROR_NOT_EMPTY if |url| is not empty.
+  //
+  void RemoveDirectory(const FileSystemURL& url,
+                       const StatusCallback& callback);
+
   // Synchronously gets the platform path for the given |url|.
   void SyncGetPlatformPath(const FileSystemURL& url, FilePath* platform_path);
 
@@ -108,6 +138,7 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   friend class FileSystemQuotaTest;
   friend class LocalFileSystemTestOriginHelper;
 
+  friend class RemoveOperationDelegate;
   friend class SyncableFileSystemOperation;
 
   LocalFileSystemOperation(
@@ -119,6 +150,8 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   }
 
   FileSystemOperationContext* operation_context() const {
+    if (overriding_operation_context_)
+      return overriding_operation_context_;
     return operation_context_.get();
   }
 
@@ -187,6 +220,10 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   void DidFinishFileOperation(const StatusCallback& callback,
                               base::PlatformFileError rv);
 
+  // Generic callback when we delegated the operation.
+  void DidFinishDelegatedOperation(const StatusCallback& callback,
+                                   base::PlatformFileError rv);
+
   void DidDirectoryExists(const StatusCallback& callback,
                           base::PlatformFileError rv,
                           const base::PlatformFileInfo& file_info,
@@ -230,12 +267,33 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   // Returns false if there's another inflight pending operation.
   bool SetPendingOperationType(OperationType type);
 
+  // Overrides this operation's operation context by given |context|.
+  // This operation won't own |context| and the |context| needs to outlive
+  // this operation.
+  //
+  // Called only from operation delegates when they create sub-operations
+  // for performing a recursive operation.
+  void set_overriding_operation_context(FileSystemOperationContext* context) {
+    overriding_operation_context_ = context;
+  }
+
+  // Sets a termination callback which is called when this instance goes away
+  // (indicates the operation is finished).
+  void set_termination_callback(const base::Closure& closure) {
+    termination_callback_ = closure;
+  }
+
   scoped_refptr<FileSystemContext> file_system_context_;
 
   scoped_ptr<FileSystemOperationContext> operation_context_;
   FileSystemFileUtil* src_util_;  // Not owned.
   FileSystemFileUtil* dest_util_;  // Not owned.
 
+  FileSystemOperationContext* overriding_operation_context_;
+
+  // A callback that is called when this instance goes away.
+  base::Closure termination_callback_;
+
   // Indicates if this operation is for cross filesystem operation or not.
   // TODO(kinuko): This should be cleaned up.
   bool is_cross_operation_;
@@ -248,6 +306,8 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   friend class FileWriterDelegate;
   scoped_ptr<FileWriterDelegate> file_writer_delegate_;
 
+  scoped_ptr<RemoveOperationDelegate> remove_operation_delegate_;
+
   // write_callback is kept in this class for so that we can dispatch it when
   // the operation is cancelled. calcel_callback is kept for canceling a
   // Truncate() operation. We can't actually stop Truncate in another thread;
diff --git a/webkit/fileapi/local_file_system_operation_unittest.cc b/webkit/fileapi/local_file_system_operation_unittest.cc
index 604f91d..d170b7c 100644
--- a/webkit/fileapi/local_file_system_operation_unittest.cc
+++ b/webkit/fileapi/local_file_system_operation_unittest.cc
@@ -1046,9 +1046,6 @@ TEST_F(LocalFileSystemOperationTest, TestRemoveSuccess) {
   EXPECT_EQ(base::PLATFORM_FILE_OK, status());
   EXPECT_FALSE(DirectoryExists(parent_dir_path));
 
-  // Remove is not a 'read' access.
-  EXPECT_EQ(0, quota_manager_proxy()->notify_storage_accessed_count());
-
   EXPECT_EQ(2, change_observer()->get_and_reset_remove_directory_count());
   EXPECT_EQ(1, change_observer()->get_and_reset_remove_file_count());
   EXPECT_TRUE(change_observer()->HasNoChange());
diff --git a/webkit/fileapi/obfuscated_file_util.cc b/webkit/fileapi/obfuscated_file_util.cc
index 4e743c6..9bd64bc 100644
--- a/webkit/fileapi/obfuscated_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_util.cc
@@ -813,10 +813,8 @@ PlatformFileError ObfuscatedFileUtil::DeleteFile(
       error != base::PLATFORM_FILE_OK)
     return error;
 
-  if (file_info.is_directory()) {
-    NOTREACHED();
-    return base::PLATFORM_FILE_ERROR_FAILED;
-  }
+  if (file_info.is_directory())
+    return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
 
   int64 growth = -UsageForPath(file_info.name.size()) - platform_file_info.size;
   AllocateQuota(context, growth);
diff --git a/webkit/fileapi/remove_operation_delegate.cc b/webkit/fileapi/remove_operation_delegate.cc
new file mode 100644
index 0000000..3dafbdb
--- /dev/null
+++ b/webkit/fileapi/remove_operation_delegate.cc
@@ -0,0 +1,166 @@
+// 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 "webkit/fileapi/remove_operation_delegate.h"
+
+#include "base/bind.h"
+#include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_operation_context.h"
+#include "webkit/fileapi/local_file_system_operation.h"
+
+namespace fileapi {
+
+RemoveOperationDelegate::RemoveOperationDelegate(
+    LocalFileSystemOperation* original_operation,
+    const StatusCallback& callback)
+    : original_operation_(original_operation),
+      callback_(callback),
+      inflight_operations_(0) {
+}
+
+RemoveOperationDelegate::~RemoveOperationDelegate() {}
+
+void RemoveOperationDelegate::Run(const FileSystemURL& url) {
+  LocalFileSystemOperation* operation = NewOperation(url);
+  if (!operation)
+    return;
+  operation->RemoveFile(url, base::Bind(
+      &RemoveOperationDelegate::DidTryRemoveFile, AsWeakPtr(), url));
+}
+
+void RemoveOperationDelegate::RunRecursively(const FileSystemURL& url) {
+  DCHECK(pending_directories_.empty());
+  pending_directories_.push(url);
+  ProcessNextDirectory(base::PLATFORM_FILE_OK);
+}
+
+void RemoveOperationDelegate::DidTryRemoveFile(
+    const FileSystemURL& url,
+    base::PlatformFileError error) {
+  if (error == base::PLATFORM_FILE_OK ||
+      error != base::PLATFORM_FILE_ERROR_NOT_A_FILE) {
+    callback_.Run(error);
+    return;
+  }
+  LocalFileSystemOperation* operation = NewOperation(url);
+  if (!operation)
+    return;
+  operation->RemoveDirectory(url, callback_);
+}
+
+void RemoveOperationDelegate::ProcessNextDirectory(
+    base::PlatformFileError error) {
+  if (error != base::PLATFORM_FILE_OK) {
+    callback_.Run(error);
+    return;
+  }
+  if (inflight_operations_ > 0)
+    return;
+  if (pending_directories_.empty()) {
+    RemoveNextDirectory(error);
+    return;
+  }
+  FileSystemURL url = pending_directories_.front();
+  pending_directories_.pop();
+  LocalFileSystemOperation* operation = NewOperation(url);
+  if (!operation)
+    return;
+  inflight_operations_++;
+  operation->ReadDirectory(
+      url, base::Bind(&RemoveOperationDelegate::DidReadDirectory,
+                      AsWeakPtr(), url));
+}
+
+void RemoveOperationDelegate::DidReadDirectory(
+    const FileSystemURL& parent,
+    base::PlatformFileError error,
+    const FileEntryList& entries,
+    bool has_more) {
+  if (error != base::PLATFORM_FILE_OK) {
+    if (error == base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY) {
+      // The given path may have been a file, so try RemoveFile.
+      inflight_operations_--;
+      DCHECK_GE(inflight_operations_, 0);
+      RemoveFile(parent);
+      return;
+    }
+    callback_.Run(error);
+    return;
+  }
+  for (size_t i = 0; i < entries.size(); i++) {
+    FileSystemURL url = parent.WithPath(parent.path().Append(entries[i].name));
+    if (entries[i].is_directory) {
+      pending_directories_.push(url);
+      continue;
+    }
+    RemoveFile(url);
+  }
+  if (has_more)
+    return;
+
+  to_remove_directories_.push(parent);
+  inflight_operations_--;
+  DCHECK_GE(inflight_operations_, 0);
+  ProcessNextDirectory(base::PLATFORM_FILE_OK);
+}
+
+void RemoveOperationDelegate::RemoveFile(const FileSystemURL& url) {
+  LocalFileSystemOperation* operation = NewOperation(url);
+  if (!operation)
+    return;
+  inflight_operations_++;
+  operation->RemoveFile(url, base::Bind(
+      &RemoveOperationDelegate::DidRemoveFile, AsWeakPtr()));
+}
+
+void RemoveOperationDelegate::DidRemoveFile(base::PlatformFileError error) {
+  inflight_operations_--;
+  DCHECK_GE(inflight_operations_, 0);
+  if (error != base::PLATFORM_FILE_OK &&
+      error != base::PLATFORM_FILE_ERROR_NOT_FOUND) {
+    callback_.Run(error);
+    return;
+  }
+  ProcessNextDirectory(error);
+}
+
+void RemoveOperationDelegate::RemoveNextDirectory(
+    base::PlatformFileError error) {
+  DCHECK_EQ(0, inflight_operations_);
+  DCHECK(pending_directories_.empty());
+  if (error != base::PLATFORM_FILE_OK ||
+      to_remove_directories_.empty()) {
+    callback_.Run(error);
+    return;
+  }
+  FileSystemURL url = to_remove_directories_.top();
+  to_remove_directories_.pop();
+  LocalFileSystemOperation* operation = NewOperation(url);
+  if (!operation)
+    return;
+  operation->RemoveDirectory(url, base::Bind(
+      &RemoveOperationDelegate::RemoveNextDirectory,
+      AsWeakPtr()));
+}
+
+LocalFileSystemOperation* RemoveOperationDelegate::NewOperation(
+    const FileSystemURL& url) {
+  base::PlatformFileError error;
+  FileSystemOperation* operation = original_operation_->file_system_context()->
+      CreateFileSystemOperation(url, &error);
+  if (error != base::PLATFORM_FILE_OK) {
+    callback_.Run(error);
+    return NULL;
+  }
+  LocalFileSystemOperation* local_operation =
+      operation->AsLocalFileSystemOperation();
+  DCHECK(local_operation);
+
+  // Let the new operation inherit from the original operation.
+  local_operation->set_overriding_operation_context(
+      original_operation_->operation_context());
+  return local_operation;
+}
+
+}  // namespace fileapi
diff --git a/webkit/fileapi/remove_operation_delegate.h b/webkit/fileapi/remove_operation_delegate.h
new file mode 100644
index 0000000..43ea9d2
--- /dev/null
+++ b/webkit/fileapi/remove_operation_delegate.h
@@ -0,0 +1,64 @@
+// 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_REMOVE_OPERATION_DELEGATE_H_
+#define WEBKIT_FILEAPI_REMOVE_OPERATION_DELEGATE_H_
+
+#include <queue>
+#include <stack>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "webkit/fileapi/file_system_operation.h"
+#include "webkit/fileapi/file_system_url.h"
+
+namespace fileapi {
+
+class FileSystemURL;
+class LocalFileSystemOperation;
+
+class RemoveOperationDelegate
+    : public base::SupportsWeakPtr<RemoveOperationDelegate> {
+ public:
+  typedef FileSystemOperation::StatusCallback StatusCallback;
+  typedef FileSystemOperation::FileEntryList FileEntryList;
+
+  RemoveOperationDelegate(LocalFileSystemOperation* original_operation,
+                          const StatusCallback& callback);
+  virtual ~RemoveOperationDelegate();
+
+  void Run(const FileSystemURL& url);
+  void RunRecursively(const FileSystemURL& url);
+
+ private:
+  void ProcessNextDirectory(base::PlatformFileError error);
+  void DidTryRemoveFile(
+      const FileSystemURL& url,
+      base::PlatformFileError error);
+  void DidReadDirectory(
+      const FileSystemURL& parent,
+      base::PlatformFileError error,
+      const FileEntryList& entries,
+      bool has_more);
+  void RemoveFile(const FileSystemURL& url);
+  void DidRemoveFile(base::PlatformFileError error);
+  void RemoveNextDirectory(base::PlatformFileError error);
+
+  LocalFileSystemOperation* NewOperation(const FileSystemURL& url);
+
+  LocalFileSystemOperation* original_operation_;
+  StatusCallback callback_;
+
+  std::queue<FileSystemURL> pending_directories_;
+  std::stack<FileSystemURL> to_remove_directories_;
+
+  int inflight_operations_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoveOperationDelegate);
+};
+
+}  // namespace fileapi
+
+#endif  // WEBKIT_FILEAPI_REMOVE_OPERATION_DELEGATE_H_
diff --git a/webkit/fileapi/syncable/syncable_file_system_operation.cc b/webkit/fileapi/syncable/syncable_file_system_operation.cc
index 8954f26..0387535 100644
--- a/webkit/fileapi/syncable/syncable_file_system_operation.cc
+++ b/webkit/fileapi/syncable/syncable_file_system_operation.cc
@@ -314,8 +314,14 @@ void SyncableFileSystemOperation::Cancel(
 
 LocalFileSystemOperation*
 SyncableFileSystemOperation::AsLocalFileSystemOperation() {
-  NOTREACHED();
-  return NULL;
+  // This must be called for nested sub-task operations
+  // where we don't need extra task queueing.
+  // Just return the internal file_system_operation_ and let this
+  // instance goes away when the operation dies.
+  file_system_operation_->set_termination_callback(
+      base::Bind(&SyncableFileSystemOperation::Destruct,
+                 base::Unretained(this)));
+  return file_system_operation_;
 }
 
 void SyncableFileSystemOperation::CreateSnapshotFile(
@@ -388,4 +394,8 @@ void SyncableFileSystemOperation::AbortOperation(
   delete this;
 }
 
+void SyncableFileSystemOperation::Destruct() {
+  delete this;
+}
+
 }  // namespace fileapi
diff --git a/webkit/fileapi/syncable/syncable_file_system_operation.h b/webkit/fileapi/syncable/syncable_file_system_operation.h
index 20a297f..2a87192 100644
--- a/webkit/fileapi/syncable/syncable_file_system_operation.h
+++ b/webkit/fileapi/syncable/syncable_file_system_operation.h
@@ -89,10 +89,14 @@ class WEBKIT_STORAGE_EXPORT SyncableFileSystemOperation
                 bool complete);
 
   void OnCancelled();
-
   void AbortOperation(const StatusCallback& callback,
                       base::PlatformFileError error);
 
+  // Just destruct this; used when we 9simply delegate the operation
+  // to the owning file_system_operation_.
+  // (See the comment at AsLocalFileSystemOperation())
+  void Destruct();
+
   base::WeakPtr<SyncableFileOperationRunner> operation_runner_;
   LocalFileSystemOperation* file_system_operation_;
   std::vector<FileSystemURL> target_paths_;
diff --git a/webkit/fileapi/webkit_fileapi.gypi b/webkit/fileapi/webkit_fileapi.gypi
index 324f9e5..ec9f7ad 100644
--- a/webkit/fileapi/webkit_fileapi.gypi
+++ b/webkit/fileapi/webkit_fileapi.gypi
@@ -79,6 +79,8 @@
       '../fileapi/obfuscated_file_util.cc',
       '../fileapi/obfuscated_file_util.h',
       '../fileapi/remote_file_system_proxy.h',
+      '../fileapi/remove_operation_delegate.cc',
+      '../fileapi/remove_operation_delegate.h',
       '../fileapi/sandbox_file_stream_writer.cc',
       '../fileapi/sandbox_file_stream_writer.h',
       '../fileapi/sandbox_mount_point_provider.cc',