Implement SandboxFileWriter and rewrite FileWriterDelegate to use it

webkit/fileapi/file_system_operation.cc

 // Copyright (c) 2012 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/file_system_operation.h"
 
 #include "base/bind.h"
 #include "base/sequenced_task_runner.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/escape.h"
 #include "net/url_request/url_request_context.h"
 #include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_file_util_proxy.h"
 #include "webkit/fileapi/file_system_mount_point_provider.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_quota_util.h"
 #include "webkit/fileapi/file_system_types.h"
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/fileapi/file_writer_delegate.h"
+#include "webkit/fileapi/sandbox_file_writer.h"
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/quota_types.h"
 
 namespace fileapi {
 
 namespace {
 
 void GetMetadataForSnapshot(
     const FileSystemOperationInterface::SnapshotFileCallback& callback,
     base::PlatformFileError result,
@@ skipping 40 matching lines @@
   }
 }
 
 FileSystemOperation::TaskParamsForDidGetQuota::TaskParamsForDidGetQuota()
     : type(kFileSystemTypeUnknown) {
 }
 
 FileSystemOperation::TaskParamsForDidGetQuota::~TaskParamsForDidGetQuota() {}
 
 FileSystemOperation::~FileSystemOperation() {
-  if (file_writer_delegate_.get()) {
-    FileSystemOperationContext* c =
-        new FileSystemOperationContext(operation_context_);
-    base::FileUtilProxy::RelayClose(
-        file_system_context()->file_task_runner(),
-        base::Bind(&FileSystemFileUtil::Close,
-                   base::Unretained(src_util_),
-                   base::Owned(c)),
-        file_writer_delegate_->file(),
-        base::FileUtilProxy::StatusCallback());
-  }
 }
 
 void FileSystemOperation::CreateFile(const GURL& path_url,
                                      bool exclusive,
                                      const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationCreateFile));
 
   base::PlatformFileError result = SetUpFileSystemPath(
       path_url, &src_path_, &src_util_, PATH_FOR_CREATE);
   if (result != base::PLATFORM_FILE_OK) {
@@ skipping 178 matching lines @@
 
   base::PlatformFileError result = SetUpFileSystemPath(
       path_url, &src_path_, &src_util_, PATH_FOR_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result, 0, false);
     delete this;
     return;
   }
   DCHECK(blob_url.is_valid());
   file_writer_delegate_.reset(new FileWriterDelegate(
-          this, src_path_, offset));
+      this,
+      scoped_ptr<FileWriter>(
+          new SandboxFileWriter(file_system_context(), path_url, offset))));
   set_write_callback(callback);
   scoped_ptr<net::URLRequest> blob_request(
       new net::URLRequest(blob_url, file_writer_delegate_.get()));
   blob_request->set_context(url_request_context);
 
-  GetUsageAndQuotaThenRunTask(
-      src_path_.origin(), src_path_.type(),
-      base::Bind(&FileSystemOperation::DoWrite, weak_factory_.GetWeakPtr(),
-                 base::Passed(&blob_request)),
-      base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED, 0, true));
+  file_writer_delegate_->Start(blob_request.Pass());
 }
 
 void FileSystemOperation::Truncate(const GURL& path_url, int64 length,
                                    const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationTruncate));
 
   base::PlatformFileError result = SetUpFileSystemPath(
       path_url, &src_path_, &src_util_, PATH_FOR_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
@@ skipping 68 matching lines @@
       src_path_.origin(), src_path_.type(),
       base::Bind(&FileSystemOperation::DoOpenFile,
                  base::Unretained(deleter.release()), callback, file_flags),
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED,
                  base::kInvalidPlatformFileValue,
                  base::kNullProcessHandle));
 }
 
 // We can only get here on a write or truncate that's not yet completed.
 // We don't support cancelling any other operation at this time.
 void FileSystemOperation::Cancel(const StatusCallback& cancel_callback) {

[ericu, 2012/05/16 00:23:17]

TODO(ericu): Does cancel still work?

[kinuko, 2012/05/16 04:10:12]

At least the layout tests seem to work.

[ericu, 2012/05/17 00:04:13]

Sorry--that wasn't supposed to be sent.  I leave myself notes on what I'm still
verifying during code reviews.

That being said, the layout tests are unfortunately not a guarantee of
correctness.  It's really hard to test all possible cases of race conditions and
such during a cancel. It looks pretty good, subject to those additions I
suggest, though.

[kinuko, 2012/05/17 04:45:26]

Yup, totally agreed. I'm not assuming current tests would guarantee any
correctness.

   if (file_writer_delegate_.get()) {
     DCHECK_EQ(kOperationWrite, pending_operation_);
 
     // Writes are done without proxying through FileUtilProxy after the initial
     // opening of the PlatformFile.  All state changes are done on this thread,
     // so we're guaranteed to be able to shut down atomically.
     const bool delete_now = file_writer_delegate_->Cancel();
 
     if (!write_callback_.is_null()) {
       // Notify the failure status to the ongoing operation's callback.
@@ skipping 134 matching lines @@
 
 void FileSystemOperation::DoMove(const StatusCallback& callback) {
   FileSystemFileUtilProxy::Move(
       &operation_context_,
       src_util_, dest_util_,
       src_path_, dest_path_,
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
 }
 
-void FileSystemOperation::DoWrite(scoped_ptr<net::URLRequest> blob_request) {
-  int file_flags = base::PLATFORM_FILE_OPEN |
-                   base::PLATFORM_FILE_WRITE |
-                   base::PLATFORM_FILE_ASYNC;
-
-  // We may get deleted on the way so allocate a new operation context
-  // to keep it alive.
-  FileSystemOperationContext* write_context = new FileSystemOperationContext(
-      operation_context_);
-  FileSystemFileUtilProxy::CreateOrOpen(
-      write_context, src_util_, src_path_, file_flags,
-      base::Bind(&FileSystemOperation::OnFileOpenedForWrite,
-                 weak_factory_.GetWeakPtr(),
-                 base::Passed(&blob_request),
-                 base::Owned(write_context)));
-}
-
 void FileSystemOperation::DoTruncate(const StatusCallback& callback,
                                      int64 length) {
   FileSystemFileUtilProxy::Truncate(
       &operation_context_, src_util_, src_path_, length,
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
 }
 
 void FileSystemOperation::DoOpenFile(const OpenFileCallback& callback,
                                      int file_flags) {
@@ skipping 92 matching lines @@
 void FileSystemOperation::DidOpenFile(
     const OpenFileCallback& callback,
     base::PlatformFileError rv,
     base::PassPlatformFile file,
     bool unused) {
   if (rv == base::PLATFORM_FILE_OK)
     CHECK_NE(base::kNullProcessHandle, peer_handle_);
   callback.Run(rv, file.ReleaseValue(), peer_handle_);
 }
 
-void FileSystemOperation::OnFileOpenedForWrite(
-    scoped_ptr<net::URLRequest> blob_request,
-    FileSystemOperationContext* unused,
-    base::PlatformFileError rv,
-    base::PassPlatformFile file,
-    bool created) {
-  if (rv != base::PLATFORM_FILE_OK) {
-    if (!write_callback_.is_null())
-      write_callback_.Run(rv, 0, false);
-    delete this;
-    return;
-  }
-  file_writer_delegate_->Start(file.ReleaseValue(), blob_request.Pass());
-}
-
 base::PlatformFileError FileSystemOperation::SetUpFileSystemPath(
     const GURL& path_url,
     FileSystemPath* file_system_path,
     FileSystemFileUtil** file_util,
     SetUpPathMode mode) {
   DCHECK(file_system_path);
   GURL origin_url;
   FileSystemType type;
   FilePath cracked_path;
   if (!CrackFileSystemURL(path_url, &origin_url, &type, &cracked_path))
@@ skipping 47 matching lines @@
 }
 
 bool FileSystemOperation::SetPendingOperationType(OperationType type) {
   if (pending_operation_ != kOperationNone)
     return false;
   pending_operation_ = type;
   return true;
 }
 
 }  // namespace fileapi