Introduce CreateNestedOperation and cleanup memory hacks around recursive operation

This removes some weird memory hack I introduced in my last changes to split recursive-Copy/Move into multiple tasks.
- Removes LocalFileSystemOperation::termination_callback
- Cleans up nested sub-operation creation code

BUG=176444
TEST=existing tests

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

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

11 files changed, 133 insertions, 250 deletions

diff --git a/webkit/fileapi/cross_operation_delegate.cc b/webkit/fileapi/cross_operation_delegate.cc
index e2989f2..69621c6 100644
--- a/webkit/fileapi/cross_operation_delegate.cc
+++ b/webkit/fileapi/cross_operation_delegate.cc
@@ -14,18 +14,19 @@
 namespace fileapi {
 
 CrossOperationDelegate::CrossOperationDelegate(
+    FileSystemContext* file_system_context,
     LocalFileSystemOperation* src_root_operation,
-    LocalFileSystemOperation* dest_root_operation,
+    scoped_ptr<LocalFileSystemOperation> dest_root_operation,
     const FileSystemURL& src_root,
     const FileSystemURL& dest_root,
     OperationType operation_type,
     const StatusCallback& callback)
-    : RecursiveOperationDelegate(src_root_operation),
+    : RecursiveOperationDelegate(file_system_context, src_root_operation),
       src_root_(src_root),
       dest_root_(dest_root),
       operation_type_(operation_type),
       callback_(callback),
-      dest_root_operation_(dest_root_operation) {
+      dest_root_operation_(dest_root_operation.Pass()) {
   same_file_system_ = AreSameFileSystem(src_root_, dest_root_);
 }
 
@@ -66,16 +67,12 @@ void CrossOperationDelegate::ProcessFile(const FileSystemURL& src_url,
 void CrossOperationDelegate::ProcessDirectory(const FileSystemURL& src_url,
                                               const StatusCallback& callback) {
   FileSystemURL dest_url = CreateDestURL(src_url);
-  LocalFileSystemOperation* dest_operation = NewDestOperation(dest_url);
-  if (!dest_operation) {
-    return;
-  }
 
   // If operation_type == Move we may need to record directories and
   // restore directory timestamps in the end, though it may have
   // negative performance impact.
   // See http://crbug.com/171284 for more details.
-  dest_operation->CreateDirectory(
+  NewDestOperation()->CreateDirectory(
       dest_url, false /* exclusive */, false /* recursive */, callback);
 }
 
@@ -90,10 +87,7 @@ void CrossOperationDelegate::DidTryCopyOrMoveFile(
   // The src_root_ looks to be a directory.
   // Try removing the dest_root_ to see if it exists and/or it is an
   // empty directory.
-  LocalFileSystemOperation* dest_operation = NewDestOperation(dest_root_);
-  if (!dest_operation)
-    return;
-  dest_operation->RemoveDirectory(
+  NewDestOperation()->RemoveDirectory(
       dest_root_, base::Bind(&CrossOperationDelegate::DidTryRemoveDestRoot,
                              AsWeakPtr()));
 }
@@ -123,16 +117,12 @@ void CrossOperationDelegate::CopyOrMoveFile(
     const FileSystemURL& src,
     const FileSystemURL& dest,
     const StatusCallback& callback) {
-  LocalFileSystemOperation* src_operation = NewSourceOperation(src);
-  if (!src_operation)
-    return;
-
   // Same filesystem case.
   if (same_file_system_) {
     if (operation_type_ == OPERATION_MOVE)
-      src_operation->MoveFileLocal(src, dest, callback);
+      NewSourceOperation()->MoveFileLocal(src, dest, callback);
     else
-      src_operation->CopyFileLocal(src, dest, callback);
+      NewSourceOperation()->CopyFileLocal(src, dest, callback);
     return;
   }
 
@@ -142,7 +132,7 @@ void CrossOperationDelegate::CopyOrMoveFile(
   StatusCallback copy_callback =
       base::Bind(&CrossOperationDelegate::DidFinishCopy, AsWeakPtr(),
                  src, callback);
-  src_operation->CreateSnapshotFile(
+  NewSourceOperation()->CreateSnapshotFile(
       src, base::Bind(&CrossOperationDelegate::DidCreateSnapshot, AsWeakPtr(),
                       dest, copy_callback));
 }
@@ -164,10 +154,7 @@ void CrossOperationDelegate::DidCreateSnapshot(
   // TODO(kinuko): Otherwise create a FileStreamReader to perform a copy/move.
   DCHECK(!platform_path.empty());
 
-  LocalFileSystemOperation* dest_operation = NewDestOperation(dest);
-  if (!dest_operation)
-    return;
-  dest_operation->CopyInForeignFile(platform_path, dest, callback);
+  NewDestOperation()->CopyInForeignFile(platform_path, dest, callback);
 }
 
 void CrossOperationDelegate::DidFinishCopy(
@@ -183,10 +170,7 @@ void CrossOperationDelegate::DidFinishCopy(
   DCHECK_EQ(OPERATION_MOVE, operation_type_);
 
   // Remove the source for finalizing move operation.
-  LocalFileSystemOperation* src_operation = NewSourceOperation(src);
-  if (!src_operation)
-    return;
-  src_operation->Remove(
+  NewSourceOperation()->Remove(
       src, true /* recursive */,
       base::Bind(&CrossOperationDelegate::DidRemoveSourceForMove,
                  AsWeakPtr(), callback));
@@ -214,40 +198,14 @@ FileSystemURL CrossOperationDelegate::CreateDestURL(
       relative);
 }
 
-LocalFileSystemOperation* CrossOperationDelegate::NewSourceOperation(
-    const FileSystemURL& url) {
-  base::PlatformFileError error = base::PLATFORM_FILE_OK;
-  LocalFileSystemOperation* operation =
-      RecursiveOperationDelegate::NewOperation(url, &error);
-  if (!operation) {
-    DCHECK_NE(base::PLATFORM_FILE_OK, error);
-    callback_.Run(error);
-    return NULL;
-  }
-  return operation;
+LocalFileSystemOperation* CrossOperationDelegate::NewSourceOperation() {
+  return NewNestedOperation();
 }
 
-LocalFileSystemOperation* CrossOperationDelegate::NewDestOperation(
-    const FileSystemURL& url) {
+LocalFileSystemOperation* CrossOperationDelegate::NewDestOperation() {
   if (same_file_system_)
-    return NewSourceOperation(url);
-
-  base::PlatformFileError error = base::PLATFORM_FILE_OK;
-  FileSystemOperation* operation = file_system_context()->
-      CreateFileSystemOperation(url, &error);
-  if (!operation) {
-    DCHECK_NE(base::PLATFORM_FILE_OK, error);
-    callback_.Run(error);
-    return NULL;
-  }
-  LocalFileSystemOperation* local_operation =
-      operation->AsLocalFileSystemOperation();
-  DCHECK(local_operation);
-
-  // Let the new operation inherit from the root operation.
-  local_operation->set_overriding_operation_context(
-      dest_root_operation_->operation_context());
-  return local_operation;
+    return NewSourceOperation();
+  return dest_root_operation_->CreateNestedOperation();
 }
 
 }  // namespace fileapi
diff --git a/webkit/fileapi/cross_operation_delegate.h b/webkit/fileapi/cross_operation_delegate.h
index 5209045..6426971 100644
--- a/webkit/fileapi/cross_operation_delegate.h
+++ b/webkit/fileapi/cross_operation_delegate.h
@@ -28,8 +28,9 @@ class CrossOperationDelegate
   };
 
   CrossOperationDelegate(
+      FileSystemContext* file_system_context,
       LocalFileSystemOperation* src_root_operation,
-      LocalFileSystemOperation* dest_root_operation,
+      scoped_ptr<LocalFileSystemOperation> dest_root_operation,
       const FileSystemURL& src_root,
       const FileSystemURL& dest_root,
       OperationType operation_type,
@@ -83,8 +84,8 @@ class CrossOperationDelegate
   //   separate FileSystemOperationContext, so it creates a new operation
   //   which inherits context from dest_root_operation_.
   //
-  LocalFileSystemOperation* NewSourceOperation(const FileSystemURL& url);
-  LocalFileSystemOperation* NewDestOperation(const FileSystemURL& url);
+  LocalFileSystemOperation* NewSourceOperation();
+  LocalFileSystemOperation* NewDestOperation();
 
   FileSystemURL src_root_;
   FileSystemURL dest_root_;
diff --git a/webkit/fileapi/local_file_system_operation.cc b/webkit/fileapi/local_file_system_operation.cc
index 2a7f471..c5482f6 100644
--- a/webkit/fileapi/local_file_system_operation.cc
+++ b/webkit/fileapi/local_file_system_operation.cc
@@ -37,8 +37,6 @@ LocalFileSystemOperation::~LocalFileSystemOperation() {
     operation_context()->update_observers()->Notify(
         &FileUpdateObserver::OnEndUpdate, MakeTuple(write_target_url_));
   }
-  if (!termination_callback_.is_null())
-    termination_callback_.Run();
 }
 
 void LocalFileSystemOperation::CreateFile(const FileSystemURL& url,
@@ -83,40 +81,37 @@ void LocalFileSystemOperation::Copy(const FileSystemURL& src_url,
                                     const FileSystemURL& dest_url,
                                     const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationCopy));
+  scoped_ptr<LocalFileSystemOperation> deleter(this);
 
   // Setting up this (source) operation.
   base::PlatformFileError result = SetUp(src_url, OPERATION_MODE_READ);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
-    delete this;
     return;
   }
 
   // Setting up a new operation for dest.
-  base::PlatformFileError error = base::PLATFORM_FILE_OK;
   FileSystemOperation* operation =
-      file_system_context()->CreateFileSystemOperation(dest_url, &error);
-  if (error != base::PLATFORM_FILE_OK) {
-    DCHECK(!operation);
-    callback.Run(error);
-    delete this;
+      file_system_context()->CreateFileSystemOperation(dest_url, &result);
+  if (result != base::PLATFORM_FILE_OK) {
+    callback.Run(result);
     return;
   }
-  LocalFileSystemOperation* dest_operation =
-      operation->AsLocalFileSystemOperation();
+  scoped_ptr<LocalFileSystemOperation> dest_operation(
+      operation->AsLocalFileSystemOperation());
   DCHECK(dest_operation);
   result = dest_operation->SetUp(dest_url, OPERATION_MODE_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
-    delete dest_operation;
-    delete this;
     return;
   }
 
   DCHECK(!recursive_operation_delegate_);
   recursive_operation_delegate_.reset(
       new CrossOperationDelegate(
-          this, dest_operation, src_url, dest_url,
+          file_system_context(),
+          deleter.release(), dest_operation.Pass(),
+          src_url, dest_url,
           CrossOperationDelegate::OPERATION_COPY,
           base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
                      base::Unretained(this), callback)));
@@ -127,12 +122,12 @@ void LocalFileSystemOperation::Move(const FileSystemURL& src_url,
                                     const FileSystemURL& dest_url,
                                     const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationMove));
+  scoped_ptr<LocalFileSystemOperation> deleter(this);
 
   // Setting up this (source) operation.
   base::PlatformFileError result = SetUp(src_url, OPERATION_MODE_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
-    delete this;
     return;
   }
 
@@ -143,25 +138,23 @@ void LocalFileSystemOperation::Move(const FileSystemURL& src_url,
   if (error != base::PLATFORM_FILE_OK) {
     DCHECK(!operation);
     callback.Run(error);
-    delete this;
     return;
   }
-  LocalFileSystemOperation* dest_operation =
-      operation->AsLocalFileSystemOperation();
+  scoped_ptr<LocalFileSystemOperation> dest_operation(
+      operation->AsLocalFileSystemOperation());
   DCHECK(dest_operation);
-
   result = dest_operation->SetUp(dest_url, OPERATION_MODE_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
-    delete dest_operation;
-    delete this;
     return;
   }
 
   DCHECK(!recursive_operation_delegate_);
   recursive_operation_delegate_.reset(
       new CrossOperationDelegate(
-          this, dest_operation, src_url, dest_url,
+          file_system_context(),
+          deleter.release(), dest_operation.Pass(),
+          src_url, dest_url,
           CrossOperationDelegate::OPERATION_MOVE,
           base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
                      base::Unretained(this), callback)));
@@ -251,6 +244,7 @@ void LocalFileSystemOperation::Remove(const FileSystemURL& url,
   DCHECK(!recursive_operation_delegate_);
   recursive_operation_delegate_.reset(
       new RemoveOperationDelegate(
+          file_system_context(),
           this, url,
           base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
                      base::Unretained(this), callback)));
@@ -430,6 +424,14 @@ void LocalFileSystemOperation::CreateSnapshotFile(
                  base::Owned(this), callback));
 }
 
+LocalFileSystemOperation* LocalFileSystemOperation::CreateNestedOperation() {
+  LocalFileSystemOperation* operation = new LocalFileSystemOperation(
+      file_system_context(),
+      make_scoped_ptr(new FileSystemOperationContext(file_system_context())));
+  operation->parent_operation_ = this;
+  return operation;
+}
+
 void LocalFileSystemOperation::CopyInForeignFile(
     const base::FilePath& src_local_disk_file_path,
     const FileSystemURL& dest_url,
@@ -537,7 +539,7 @@ LocalFileSystemOperation::LocalFileSystemOperation(
     : file_system_context_(file_system_context),
       operation_context_(operation_context.Pass()),
       async_file_util_(NULL),
-      overriding_operation_context_(NULL),
+      parent_operation_(NULL),
       peer_handle_(base::kNullProcessHandle),
       pending_operation_(kOperationNone),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
@@ -844,9 +846,8 @@ base::PlatformFileError LocalFileSystemOperation::SetUp(
     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 notifications.
-  if (overriding_operation_context_)
+  // it has the parent operation) we skip duplicated notifications.
+  if (parent_operation_)
     return base::PLATFORM_FILE_OK;
 
   switch (mode) {
diff --git a/webkit/fileapi/local_file_system_operation.h b/webkit/fileapi/local_file_system_operation.h
index 5950d07..1bfa32e 100644
--- a/webkit/fileapi/local_file_system_operation.h
+++ b/webkit/fileapi/local_file_system_operation.h
@@ -82,6 +82,11 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
       const FileSystemURL& path,
       const SnapshotFileCallback& callback) OVERRIDE;
 
+  // Creates a nestable operation that inherits operation context
+  // from this operation.  The operation created by this method have to
+  // be die before this operation goes away.
+  virtual LocalFileSystemOperation* CreateNestedOperation();
+
   // Copies in a single file from a different filesystem.
   //
   // This returns:
@@ -186,8 +191,8 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   }
 
   FileSystemOperationContext* operation_context() const {
-    if (overriding_operation_context_)
-      return overriding_operation_context_;
+    if (parent_operation_)
+      return parent_operation_->operation_context();
     return operation_context_.get();
   }
 
@@ -302,31 +307,16 @@ 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_;
   AsyncFileUtil* async_file_util_;  // Not owned.
 
-  FileSystemOperationContext* overriding_operation_context_;
-
-  // A callback that is called when this instance goes away.
-  base::Closure termination_callback_;
+  // If this operation is created as a sub-operation for nested operation,
+  // this holds non-null value and points to the parent operation.
+  // TODO(kinuko): Cleanup this when we finish cleaning up the
+  // FileSystemOperation lifetime issue.
+  LocalFileSystemOperation* parent_operation_;
 
   // These are all used only by Write().
   friend class FileWriterDelegate;
diff --git a/webkit/fileapi/recursive_operation_delegate.cc b/webkit/fileapi/recursive_operation_delegate.cc
index 733bc7c..5ec0899 100644
--- a/webkit/fileapi/recursive_operation_delegate.cc
+++ b/webkit/fileapi/recursive_operation_delegate.cc
@@ -17,12 +17,15 @@ const int kMaxInflightOperations = 5;
 }
 
 RecursiveOperationDelegate::RecursiveOperationDelegate(
-    LocalFileSystemOperation* original_operation)
-    : original_operation_(original_operation),
+    FileSystemContext* file_system_context,
+    LocalFileSystemOperation* operation)
+    : file_system_context_(file_system_context),
+      operation_(operation),
       inflight_operations_(0) {
 }
 
-RecursiveOperationDelegate::~RecursiveOperationDelegate() {}
+RecursiveOperationDelegate::~RecursiveOperationDelegate() {
+}
 
 void RecursiveOperationDelegate::StartRecursiveOperation(
     const FileSystemURL& root,
@@ -32,36 +35,8 @@ void RecursiveOperationDelegate::StartRecursiveOperation(
   ProcessNextDirectory();
 }
 
-LocalFileSystemOperation* RecursiveOperationDelegate::NewOperation(
-    const FileSystemURL& url,
-    base::PlatformFileError* error_out) {
-  base::PlatformFileError error = base::PLATFORM_FILE_OK;
-  FileSystemOperation* operation = original_operation_->file_system_context()->
-      CreateFileSystemOperation(url, &error);
-  if (error != base::PLATFORM_FILE_OK) {
-    if (error_out)
-      *error_out = 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());
-  if (error_out)
-    *error_out = base::PLATFORM_FILE_OK;
-  return local_operation;
-}
-
-FileSystemContext* RecursiveOperationDelegate::file_system_context() {
-  return original_operation_->file_system_context();
-}
-
-const FileSystemContext* RecursiveOperationDelegate::file_system_context()
-    const {
-  return original_operation_->file_system_context();
+LocalFileSystemOperation* RecursiveOperationDelegate::NewNestedOperation() {
+  return operation_->CreateNestedOperation();
 }
 
 void RecursiveOperationDelegate::ProcessNextDirectory() {
@@ -116,12 +91,7 @@ void RecursiveOperationDelegate::DidProcessDirectory(
     callback_.Run(error);
     return;
   }
-  LocalFileSystemOperation* operation = NewOperation(url, &error);
-  if (!operation) {
-    callback_.Run(error);
-    return;
-  }
-  operation->ReadDirectory(
+  NewNestedOperation()->ReadDirectory(
       url, base::Bind(&RecursiveOperationDelegate::DidReadDirectory,
                       AsWeakPtr(), url));
 }
@@ -143,7 +113,7 @@ void RecursiveOperationDelegate::DidReadDirectory(
     return;
   }
   for (size_t i = 0; i < entries.size(); i++) {
-    FileSystemURL url = file_system_context()->CreateCrackedFileSystemURL(
+    FileSystemURL url = file_system_context_->CreateCrackedFileSystemURL(
         parent.origin(),
         parent.mount_type(),
         parent.virtual_path().Append(entries[i].name));
diff --git a/webkit/fileapi/recursive_operation_delegate.h b/webkit/fileapi/recursive_operation_delegate.h
index 8c2a36f..99d0bc0 100644
--- a/webkit/fileapi/recursive_operation_delegate.h
+++ b/webkit/fileapi/recursive_operation_delegate.h
@@ -16,25 +16,26 @@
 namespace fileapi {
 
 class FileSystemContext;
-class LocalFileSystemOperation;
 
 // A base class for recursive operation delegates.
 // This also provides some convenient default implementations for subclasses
-// like StartRecursiveOperation() and  NewOperation().
+// like StartRecursiveOperation() and  NewNestedOperation().
 //
 // In short, each subclass should override ProcessFile and ProcessDirectory
 // to process a directory or a file. To start the recursive operation it
 // should also call StartRecursiveOperation.
 //
-// Each subclass can call NewOperation to create a new file system operation
-// to perform a sub-operations, e.g. can call RemoveFile for recursive Remove.
+// Each subclass can call NewNestedOperation to create a new file system
+// operation to perform a sub-operations, e.g. can call RemoveFile for
+// recursive Remove.
 class RecursiveOperationDelegate
     : public base::SupportsWeakPtr<RecursiveOperationDelegate> {
  public:
   typedef FileSystemOperation::StatusCallback StatusCallback;
   typedef FileSystemOperation::FileEntryList FileEntryList;
 
-  RecursiveOperationDelegate(LocalFileSystemOperation* original_operation);
+  RecursiveOperationDelegate(FileSystemContext* file_system_context,
+                             LocalFileSystemOperation* operation);
   virtual ~RecursiveOperationDelegate();
 
   // This is called when the consumer of this instance starts a non-recursive
@@ -65,14 +66,13 @@ class RecursiveOperationDelegate
   void StartRecursiveOperation(const FileSystemURL& root,
                                const StatusCallback& callback);
 
-  // Returns new sub-operation for a given |url|.
-  // This may return NULL if any error has happened.
-  // If non-null |error| is given it is set to the error code.
-  LocalFileSystemOperation* NewOperation(const FileSystemURL& url,
-                                         base::PlatformFileError* error);
+  // Returns new nested sub-operation.
+  LocalFileSystemOperation* NewNestedOperation();
 
-  FileSystemContext* file_system_context();
-  const FileSystemContext* file_system_context() const;
+  FileSystemContext* file_system_context() { return file_system_context_; }
+  const FileSystemContext* file_system_context() const {
+    return file_system_context_;
+  }
 
  private:
   void ProcessNextDirectory();
@@ -88,7 +88,8 @@ class RecursiveOperationDelegate
   void DidTryProcessFile(base::PlatformFileError previous_error,
                          base::PlatformFileError error);
 
-  LocalFileSystemOperation* original_operation_;
+  FileSystemContext* file_system_context_;
+  LocalFileSystemOperation* operation_;
   StatusCallback callback_;
   std::queue<FileSystemURL> pending_directories_;
   std::queue<FileSystemURL> pending_files_;
diff --git a/webkit/fileapi/remove_operation_delegate.cc b/webkit/fileapi/remove_operation_delegate.cc
index 5504025..50a0157 100644
--- a/webkit/fileapi/remove_operation_delegate.cc
+++ b/webkit/fileapi/remove_operation_delegate.cc
@@ -12,10 +12,11 @@
 namespace fileapi {
 
 RemoveOperationDelegate::RemoveOperationDelegate(
-    LocalFileSystemOperation* original_operation,
+    FileSystemContext* file_system_context,
+    LocalFileSystemOperation* operation,
     const FileSystemURL& url,
     const StatusCallback& callback)
-    : RecursiveOperationDelegate(original_operation),
+    : RecursiveOperationDelegate(file_system_context, operation),
       url_(url),
       callback_(callback) {
 }
@@ -23,13 +24,7 @@ RemoveOperationDelegate::RemoveOperationDelegate(
 RemoveOperationDelegate::~RemoveOperationDelegate() {}
 
 void RemoveOperationDelegate::Run() {
-  base::PlatformFileError error;
-  LocalFileSystemOperation* operation = NewOperation(url_, &error);
-  if (!operation) {
-    callback_.Run(error);
-    return;
-  }
-  operation->RemoveFile(url_, base::Bind(
+  NewNestedOperation()->RemoveFile(url_, base::Bind(
       &RemoveOperationDelegate::DidTryRemoveFile, AsWeakPtr()));
 }
 
@@ -41,18 +36,12 @@ void RemoveOperationDelegate::RunRecursively() {
 
 void RemoveOperationDelegate::ProcessFile(const FileSystemURL& url,
                                           const StatusCallback& callback) {
-  base::PlatformFileError error;
-  LocalFileSystemOperation* operation = NewOperation(url, &error);
-  if (!operation) {
-    callback.Run(error);
-    return;
-  }
   if (to_remove_directories_.size() == 1u &&
       to_remove_directories_.top() == url) {
     // We seem to have been re-directed from ProcessDirectory.
     to_remove_directories_.pop();
   }
-  operation->RemoveFile(url, base::Bind(
+  NewNestedOperation()->RemoveFile(url, base::Bind(
       &RemoveOperationDelegate::DidRemoveFile, AsWeakPtr(), callback));
 }
 
@@ -69,12 +58,7 @@ void RemoveOperationDelegate::DidTryRemoveFile(
     callback_.Run(error);
     return;
   }
-  LocalFileSystemOperation* operation = NewOperation(url_, &error);
-  if (!operation) {
-    callback_.Run(error);
-    return;
-  }
-  operation->RemoveDirectory(url_, callback_);
+  NewNestedOperation()->RemoveDirectory(url_, callback_);
 }
 
 void RemoveOperationDelegate::DidRemoveFile(const StatusCallback& callback,
@@ -95,12 +79,7 @@ void RemoveOperationDelegate::RemoveNextDirectory(
   }
   FileSystemURL url = to_remove_directories_.top();
   to_remove_directories_.pop();
-  LocalFileSystemOperation* operation = NewOperation(url, &error);
-  if (!operation) {
-    callback_.Run(error);
-    return;
-  }
-  operation->RemoveDirectory(url, base::Bind(
+  NewNestedOperation()->RemoveDirectory(url, base::Bind(
       &RemoveOperationDelegate::RemoveNextDirectory,
       AsWeakPtr()));
 }
diff --git a/webkit/fileapi/remove_operation_delegate.h b/webkit/fileapi/remove_operation_delegate.h
index 2a296c7..20bdd23 100644
--- a/webkit/fileapi/remove_operation_delegate.h
+++ b/webkit/fileapi/remove_operation_delegate.h
@@ -15,7 +15,8 @@ class RemoveOperationDelegate
     : public RecursiveOperationDelegate,
       public base::SupportsWeakPtr<RemoveOperationDelegate> {
  public:
-  RemoveOperationDelegate(LocalFileSystemOperation* original_operation,
+  RemoveOperationDelegate(FileSystemContext* file_system_context,
+                          LocalFileSystemOperation* operation,
                           const FileSystemURL& url,
                           const StatusCallback& callback);
   virtual ~RemoveOperationDelegate();
diff --git a/webkit/fileapi/sandbox_mount_point_provider.cc b/webkit/fileapi/sandbox_mount_point_provider.cc
index bc005c6..5113b3c 100644
--- a/webkit/fileapi/sandbox_mount_point_provider.cc
+++ b/webkit/fileapi/sandbox_mount_point_provider.cc
@@ -281,8 +281,7 @@ FileSystemOperation* SandboxMountPointProvider::CreateFileSystemOperation(
     operation_context->set_change_observers(syncable_change_observers_);
     operation_context->set_access_observers(access_observers_);
     return new sync_file_system::SyncableFileSystemOperation(
-        context,
-        new LocalFileSystemOperation(context, operation_context.Pass()));
+        context, operation_context.Pass());
   }
 
   // For regular sandboxed types.
diff --git a/webkit/fileapi/syncable/syncable_file_system_operation.cc b/webkit/fileapi/syncable/syncable_file_system_operation.cc
index 3d90af7..5ed679b 100644
--- a/webkit/fileapi/syncable/syncable_file_system_operation.cc
+++ b/webkit/fileapi/syncable/syncable_file_system_operation.cc
@@ -7,13 +7,14 @@
 #include "base/logging.h"
 #include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_url.h"
-#include "webkit/fileapi/local_file_system_operation.h"
 #include "webkit/fileapi/sandbox_mount_point_provider.h"
 #include "webkit/fileapi/syncable/local_file_sync_context.h"
 #include "webkit/fileapi/syncable/syncable_file_operation_runner.h"
 
 using fileapi::FileSystemURL;
+using fileapi::FileSystemOperationContext;
 using fileapi::LocalFileSystemOperation;
 
 namespace sync_file_system {
@@ -81,7 +82,7 @@ void SyncableFileSystemOperation::CreateFile(
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
       base::Bind(&FileSystemOperation::CreateFile,
-                 base::Unretained(file_system_operation_),
+                 base::Unretained(NewOperation()),
                  url, exclusive,
                  base::Bind(&self::DidFinish, base::Owned(this)))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -107,7 +108,7 @@ void SyncableFileSystemOperation::CreateDirectory(
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
       base::Bind(&FileSystemOperation::CreateDirectory,
-                 base::Unretained(file_system_operation_),
+                 base::Unretained(NewOperation()),
                  url, exclusive, recursive,
                  base::Bind(&self::DidFinish, base::Owned(this)))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -128,7 +129,7 @@ void SyncableFileSystemOperation::Copy(
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
       base::Bind(&FileSystemOperation::Copy,
-                 base::Unretained(file_system_operation_),
+                 base::Unretained(NewOperation()),
                  src_url, dest_url,
                  base::Bind(&self::DidFinish, base::Owned(this)))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -150,7 +151,7 @@ void SyncableFileSystemOperation::Move(
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
       base::Bind(&FileSystemOperation::Move,
-                 base::Unretained(file_system_operation_),
+                 base::Unretained(NewOperation()),
                  src_url, dest_url,
                  base::Bind(&self::DidFinish, base::Owned(this)))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -164,7 +165,7 @@ void SyncableFileSystemOperation::DirectoryExists(
     AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
     return;
   }
-  file_system_operation_->DirectoryExists(url, callback);
+  NewOperation()->DirectoryExists(url, callback);
   delete this;
 }
 
@@ -176,7 +177,7 @@ void SyncableFileSystemOperation::FileExists(
     AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
     return;
   }
-  file_system_operation_->FileExists(url, callback);
+  NewOperation()->FileExists(url, callback);
   delete this;
 }
 
@@ -187,11 +188,10 @@ void SyncableFileSystemOperation::GetMetadata(
   if (!operation_runner_) {
     callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND,
                  base::PlatformFileInfo(), base::FilePath());
-    delete file_system_operation_;
     delete this;
     return;
   }
-  file_system_operation_->GetMetadata(url, callback);
+  NewOperation()->GetMetadata(url, callback);
   delete this;
 }
 
@@ -201,14 +201,13 @@ void SyncableFileSystemOperation::ReadDirectory(
   DCHECK(CalledOnValidThread());
   if (!operation_runner_) {
     callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND, FileEntryList(), false);
-    delete file_system_operation_;
     delete this;
     return;
   }
   // This is a read operation and there'd be no hard to let it go even if
   // directory operation is disabled. (And we should allow this if it's made
   // on the root directory)
-  file_system_operation_->ReadDirectory(url, callback);
+  NewOperation()->ReadDirectory(url, callback);
   delete this;
 }
 
@@ -226,7 +225,7 @@ void SyncableFileSystemOperation::Remove(
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
       base::Bind(&FileSystemOperation::Remove,
-                 base::Unretained(file_system_operation_),
+                 base::Unretained(NewOperation()),
                  url, recursive,
                  base::Bind(&self::DidFinish, base::Owned(this)))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -241,7 +240,6 @@ void SyncableFileSystemOperation::Write(
   DCHECK(CalledOnValidThread());
   if (!operation_runner_) {
     callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND, 0, true);
-    delete file_system_operation_;
     delete this;
     return;
   }
@@ -250,7 +248,7 @@ void SyncableFileSystemOperation::Write(
   completion_callback_ = base::Bind(&WriteCallbackAdapter, callback);
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
-      file_system_operation_->GetWriteClosure(
+      NewOperation()->GetWriteClosure(
           url_request_context, url, blob_url, offset,
           base::Bind(&self::DidWrite, base::Owned(this), callback))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -270,7 +268,7 @@ void SyncableFileSystemOperation::Truncate(
   scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
       this,
       base::Bind(&FileSystemOperation::Truncate,
-                 base::Unretained(file_system_operation_),
+                 base::Unretained(NewOperation()),
                  url, length,
                  base::Bind(&self::DidFinish, base::Owned(this)))));
   operation_runner_->PostOperationTask(task.Pass());
@@ -286,8 +284,8 @@ void SyncableFileSystemOperation::TouchFile(
     AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
     return;
   }
-  file_system_operation_->TouchFile(url, last_access_time,
-                                    last_modified_time, callback);
+  NewOperation()->TouchFile(url, last_access_time,
+                            last_modified_time, callback);
   delete this;
 }
 
@@ -309,22 +307,9 @@ void SyncableFileSystemOperation::NotifyCloseFile(
 void SyncableFileSystemOperation::Cancel(
     const StatusCallback& cancel_callback) {
   DCHECK(CalledOnValidThread());
-  DCHECK(file_system_operation_);
+  DCHECK(inflight_operation_);
   completion_callback_ = cancel_callback;
-  file_system_operation_->Cancel(
-      base::Bind(&self::DidFinish, base::Owned(this)));
-}
-
-LocalFileSystemOperation*
-SyncableFileSystemOperation::AsLocalFileSystemOperation() {
-  // 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_;
+  NewOperation()->Cancel(base::Bind(&self::DidFinish, base::Owned(this)));
 }
 
 void SyncableFileSystemOperation::CreateSnapshotFile(
@@ -334,21 +319,20 @@ void SyncableFileSystemOperation::CreateSnapshotFile(
   if (!operation_runner_) {
     callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND,
                  base::PlatformFileInfo(), base::FilePath(), NULL);
-    delete file_system_operation_;
     delete this;
     return;
   }
-  file_system_operation_->CreateSnapshotFile(path, callback);
+  NewOperation()->CreateSnapshotFile(path, callback);
   delete this;
 }
 
 SyncableFileSystemOperation::SyncableFileSystemOperation(
     fileapi::FileSystemContext* file_system_context,
-    fileapi::FileSystemOperation* file_system_operation) {
+    scoped_ptr<FileSystemOperationContext> operation_context)
+    : LocalFileSystemOperation(file_system_context,
+                               operation_context.Pass()),
+      inflight_operation_(NULL) {
   DCHECK(file_system_context);
-  DCHECK(file_system_operation);
-  file_system_operation_ = file_system_operation->AsLocalFileSystemOperation();
-  DCHECK(file_system_operation_);
   if (!file_system_context->sync_context()) {
     // Syncable FileSystem is opened in a file system context which doesn't
     // support (or is not initialized for) the API.
@@ -360,6 +344,15 @@ SyncableFileSystemOperation::SyncableFileSystemOperation(
       is_sync_directory_operation_enabled();
 }
 
+LocalFileSystemOperation* SyncableFileSystemOperation::NewOperation() {
+  DCHECK(operation_context_);
+  inflight_operation_ = new LocalFileSystemOperation(
+      file_system_context(),
+      operation_context_.Pass());
+  DCHECK(inflight_operation_);
+  return inflight_operation_;
+}
+
 void SyncableFileSystemOperation::DidFinish(base::PlatformFileError status) {
   DCHECK(CalledOnValidThread());
   DCHECK(!completion_callback_.is_null());
@@ -386,18 +379,14 @@ void SyncableFileSystemOperation::DidWrite(
 void SyncableFileSystemOperation::OnCancelled() {
   DCHECK(!completion_callback_.is_null());
   completion_callback_.Run(base::PLATFORM_FILE_ERROR_ABORT);
-  delete file_system_operation_;
+  delete inflight_operation_;
 }
 
 void SyncableFileSystemOperation::AbortOperation(
     const StatusCallback& callback,
     base::PlatformFileError error) {
   callback.Run(error);
-  delete file_system_operation_;
-  delete this;
-}
-
-void SyncableFileSystemOperation::Destruct() {
+  delete inflight_operation_;
   delete this;
 }
 
diff --git a/webkit/fileapi/syncable/syncable_file_system_operation.h b/webkit/fileapi/syncable/syncable_file_system_operation.h
index 9631c72..537c61e 100644
--- a/webkit/fileapi/syncable/syncable_file_system_operation.h
+++ b/webkit/fileapi/syncable/syncable_file_system_operation.h
@@ -12,23 +12,22 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
-#include "webkit/fileapi/file_system_operation.h"
+#include "webkit/fileapi/local_file_system_operation.h"
 #include "webkit/storage/webkit_storage_export.h"
 
 namespace fileapi {
 class FileSystemContext;
-class FileSystemOperation;
+class FileSystemOperationContext;
 class SandboxMountPointProvider;
 }
 
 namespace sync_file_system {
 
-class FileSystemContext;
 class SyncableFileOperationRunner;
 
 // A wrapper class of LocalFileSystemOperation for syncable file system.
 class WEBKIT_STORAGE_EXPORT SyncableFileSystemOperation
-    : public NON_EXPORTED_BASE(fileapi::FileSystemOperation),
+    : public fileapi::LocalFileSystemOperation,
       public base::NonThreadSafe {
  public:
   virtual ~SyncableFileSystemOperation();
@@ -74,8 +73,6 @@ class WEBKIT_STORAGE_EXPORT SyncableFileSystemOperation
                         const OpenFileCallback& callback) OVERRIDE;
   virtual void NotifyCloseFile(const fileapi::FileSystemURL& url) OVERRIDE;
   virtual void Cancel(const StatusCallback& cancel_callback) OVERRIDE;
-  virtual fileapi::LocalFileSystemOperation*
-      AsLocalFileSystemOperation() OVERRIDE;
   virtual void CreateSnapshotFile(
       const fileapi::FileSystemURL& path,
       const SnapshotFileCallback& callback) OVERRIDE;
@@ -86,9 +83,11 @@ class WEBKIT_STORAGE_EXPORT SyncableFileSystemOperation
 
   // Only MountPointProviders can create a new operation directly.
   friend class fileapi::SandboxMountPointProvider;
+  friend class SandboxMountPointProvider;
   SyncableFileSystemOperation(
       fileapi::FileSystemContext* file_system_context,
-      fileapi::FileSystemOperation* file_system_operation);
+      scoped_ptr<fileapi::FileSystemOperationContext> operation_context);
+  fileapi::LocalFileSystemOperation* NewOperation();
 
   void DidFinish(base::PlatformFileError status);
   void DidWrite(const WriteCallback& callback,
@@ -100,13 +99,8 @@ class WEBKIT_STORAGE_EXPORT SyncableFileSystemOperation
   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_;
-  fileapi::LocalFileSystemOperation* file_system_operation_;
+  fileapi::LocalFileSystemOperation* inflight_operation_;
   std::vector<fileapi::FileSystemURL> target_paths_;
 
   StatusCallback completion_callback_;