Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(172)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: webkit/browser/fileapi/local_file_system_operation.cc

Issue 16413007: Make FileSystemOperation NOT self-destruct (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: fix browser_tests Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webkit/browser/fileapi/local_file_system_operation.h ('k') | webkit/browser/fileapi/syncable/syncable_file_system_operation.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webkit/browser/fileapi/local_file_system_operation.cc
diff --git a/webkit/browser/fileapi/local_file_system_operation.cc b/webkit/browser/fileapi/local_file_system_operation.cc
index c14a3e4d191584b9f255b1561865c38bc19fcc82..a79b4799dba4e463050a8931c09db8c9b7fc3c95 100644
--- a/webkit/browser/fileapi/local_file_system_operation.cc
+++ b/webkit/browser/fileapi/local_file_system_operation.cc
@@ -45,8 +45,7 @@ LocalFileSystemOperation::LocalFileSystemOperation(
operation_context_(operation_context.Pass()),
async_file_util_(NULL),
peer_handle_(base::kNullProcessHandle),
- pending_operation_(kOperationNone),
- weak_factory_(this) {
+ pending_operation_(kOperationNone) {
DCHECK(operation_context_.get());
operation_context_->DetachUserDataThread();
async_file_util_ = file_system_context_->GetAsyncFileUtil(url.type());
@@ -63,7 +62,7 @@ void LocalFileSystemOperation::CreateFile(const FileSystemURL& url,
GetUsageAndQuotaThenRunTask(
url,
base::Bind(&LocalFileSystemOperation::DoCreateFile,
- base::Unretained(this), url, callback, exclusive),
+ AsWeakPtr(), url, callback, exclusive),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
}
@@ -75,7 +74,7 @@ void LocalFileSystemOperation::CreateDirectory(const FileSystemURL& url,
GetUsageAndQuotaThenRunTask(
url,
base::Bind(&LocalFileSystemOperation::DoCreateDirectory,
- base::Unretained(this), url, callback, exclusive, recursive),
+ AsWeakPtr(), url, callback, exclusive, recursive),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
}
@@ -89,8 +88,8 @@ void LocalFileSystemOperation::Copy(const FileSystemURL& src_url,
file_system_context(),
src_url, dest_url,
CopyOrMoveOperationDelegate::OPERATION_COPY,
- base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
- base::Unretained(this), callback)));
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback)));
recursive_operation_delegate_->RunRecursively();
}
@@ -104,8 +103,8 @@ void LocalFileSystemOperation::Move(const FileSystemURL& src_url,
file_system_context(),
src_url, dest_url,
CopyOrMoveOperationDelegate::OPERATION_MOVE,
- base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
- base::Unretained(this), callback)));
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback)));
recursive_operation_delegate_->RunRecursively();
}
@@ -113,36 +112,31 @@ void LocalFileSystemOperation::DirectoryExists(const FileSystemURL& url,
const StatusCallback& callback) {
DCHECK(SetPendingOperationType(kOperationDirectoryExists));
async_file_util_->GetFileInfo(
- operation_context(), url,
+ operation_context_.Pass(), url,
base::Bind(&LocalFileSystemOperation::DidDirectoryExists,
- base::Owned(this), callback));
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::FileExists(const FileSystemURL& url,
const StatusCallback& callback) {
DCHECK(SetPendingOperationType(kOperationFileExists));
async_file_util_->GetFileInfo(
- operation_context(), url,
+ operation_context_.Pass(), url,
base::Bind(&LocalFileSystemOperation::DidFileExists,
- base::Owned(this), callback));
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::GetMetadata(
const FileSystemURL& url, const GetMetadataCallback& callback) {
DCHECK(SetPendingOperationType(kOperationGetMetadata));
- async_file_util_->GetFileInfo(
- operation_context(), url,
- base::Bind(&LocalFileSystemOperation::DidGetMetadata,
- base::Owned(this), callback));
+ async_file_util_->GetFileInfo(operation_context_.Pass(), url, callback);
}
void LocalFileSystemOperation::ReadDirectory(
const FileSystemURL& url, const ReadDirectoryCallback& callback) {
DCHECK(SetPendingOperationType(kOperationReadDirectory));
async_file_util_->ReadDirectory(
- operation_context(), url,
- base::Bind(&LocalFileSystemOperation::DidReadDirectory,
- base::Owned(this), callback));
+ operation_context_.Pass(), url, callback);
}
void LocalFileSystemOperation::Remove(const FileSystemURL& url,
@@ -153,8 +147,8 @@ void LocalFileSystemOperation::Remove(const FileSystemURL& url,
recursive_operation_delegate_.reset(
new RemoveOperationDelegate(
file_system_context(), url,
- base::Bind(&LocalFileSystemOperation::DidFinishDelegatedOperation,
- base::Unretained(this), callback)));
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback)));
if (recursive)
recursive_operation_delegate_->RunRecursively();
else
@@ -176,7 +170,7 @@ void LocalFileSystemOperation::Truncate(const FileSystemURL& url, int64 length,
GetUsageAndQuotaThenRunTask(
url,
base::Bind(&LocalFileSystemOperation::DoTruncate,
- base::Unretained(this), url, callback, length),
+ AsWeakPtr(), url, callback, length),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
}
@@ -186,10 +180,10 @@ void LocalFileSystemOperation::TouchFile(const FileSystemURL& url,
const StatusCallback& callback) {
DCHECK(SetPendingOperationType(kOperationTouchFile));
async_file_util_->Touch(
- operation_context(), url,
+ operation_context_.Pass(), url,
last_access_time, last_modified_time,
- base::Bind(&LocalFileSystemOperation::DidTouchFile,
- base::Owned(this), callback));
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::OpenFile(const FileSystemURL& url,
@@ -197,8 +191,6 @@ void LocalFileSystemOperation::OpenFile(const FileSystemURL& url,
base::ProcessHandle peer_handle,
const OpenFileCallback& callback) {
DCHECK(SetPendingOperationType(kOperationOpenFile));
- scoped_ptr<LocalFileSystemOperation> deleter(this);
-
peer_handle_ = peer_handle;
if (file_flags & (
@@ -213,7 +205,7 @@ void LocalFileSystemOperation::OpenFile(const FileSystemURL& url,
GetUsageAndQuotaThenRunTask(
url,
base::Bind(&LocalFileSystemOperation::DoOpenFile,
- base::Unretained(deleter.release()),
+ AsWeakPtr(),
url, callback, file_flags),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED,
base::kInvalidPlatformFileValue,
@@ -224,33 +216,17 @@ void LocalFileSystemOperation::OpenFile(const FileSystemURL& url,
// 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 LocalFileSystemOperation::Cancel(const StatusCallback& cancel_callback) {
- if (file_writer_delegate_) {
- 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();
+ DCHECK(cancel_callback_.is_null());
+ cancel_callback_ = cancel_callback;
- if (!write_callback_.is_null()) {
- // Notify the failure status to the ongoing operation's callback.
- write_callback_.Run(base::PLATFORM_FILE_ERROR_ABORT, 0, false);
- }
- cancel_callback.Run(base::PLATFORM_FILE_OK);
- write_callback_.Reset();
-
- if (delete_now) {
- delete this;
- return;
- }
+ if (file_writer_delegate_.get()) {
+ DCHECK_EQ(kOperationWrite, pending_operation_);
+ // This will call DidWrite() with ABORT status code.
+ file_writer_delegate_->Cancel();
} else {
+ // For truncate we have no way to cancel the inflight operation (for now).
+ // Let it just run and dispatch cancel callback later.
DCHECK_EQ(kOperationTruncate, pending_operation_);
- // We're cancelling a truncate operation, but we can't actually stop it
- // since it's been proxied to another thread. We need to save the
- // cancel_callback so that when the truncate returns, it can see that it's
- // been cancelled, report it, and report that the cancel has succeeded.
- DCHECK(cancel_callback_.is_null());
- cancel_callback_ = cancel_callback;
}
}
@@ -259,15 +235,14 @@ LocalFileSystemOperation::AsLocalFileSystemOperation() {
return this;
}
-void LocalFileSystemOperation::SyncGetPlatformPath(const FileSystemURL& url,
- base::FilePath* platform_path) {
+void LocalFileSystemOperation::SyncGetPlatformPath(
+ const FileSystemURL& url,
+ base::FilePath* platform_path) {
DCHECK(SetPendingOperationType(kOperationGetLocalPath));
FileSystemFileUtil* file_util = file_system_context()->GetFileUtil(
url.type());
DCHECK(file_util);
- file_util->GetLocalFilePath(operation_context(), url, platform_path);
-
- delete this;
+ file_util->GetLocalFilePath(operation_context_.get(), url, platform_path);
}
void LocalFileSystemOperation::CreateSnapshotFile(
@@ -275,9 +250,7 @@ void LocalFileSystemOperation::CreateSnapshotFile(
const SnapshotFileCallback& callback) {
DCHECK(SetPendingOperationType(kOperationCreateSnapshotFile));
async_file_util_->CreateSnapshotFile(
- operation_context(), url,
- base::Bind(&LocalFileSystemOperation::DidCreateSnapshotFile,
- base::Owned(this), callback));
+ operation_context_.Pass(), url, callback);
}
void LocalFileSystemOperation::CopyInForeignFile(
@@ -288,7 +261,7 @@ void LocalFileSystemOperation::CopyInForeignFile(
GetUsageAndQuotaThenRunTask(
dest_url,
base::Bind(&LocalFileSystemOperation::DoCopyInForeignFile,
- base::Unretained(this), src_local_disk_file_path, dest_url,
+ AsWeakPtr(), src_local_disk_file_path, dest_url,
callback),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
}
@@ -298,9 +271,9 @@ void LocalFileSystemOperation::RemoveFile(
const StatusCallback& callback) {
DCHECK(SetPendingOperationType(kOperationRemove));
async_file_util_->DeleteFile(
- operation_context(), url,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ operation_context_.Pass(), url,
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::RemoveDirectory(
@@ -308,9 +281,9 @@ void LocalFileSystemOperation::RemoveDirectory(
const StatusCallback& callback) {
DCHECK(SetPendingOperationType(kOperationRemove));
async_file_util_->DeleteDirectory(
- operation_context(), url,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ operation_context_.Pass(), url,
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::CopyFileLocal(
@@ -322,7 +295,7 @@ void LocalFileSystemOperation::CopyFileLocal(
GetUsageAndQuotaThenRunTask(
dest_url,
base::Bind(&LocalFileSystemOperation::DoCopyFileLocal,
- base::Unretained(this), src_url, dest_url, callback),
+ AsWeakPtr(), src_url, dest_url, callback),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
}
@@ -335,7 +308,7 @@ void LocalFileSystemOperation::MoveFileLocal(
GetUsageAndQuotaThenRunTask(
dest_url,
base::Bind(&LocalFileSystemOperation::DoMoveFileLocal,
- base::Unretained(this), src_url, dest_url, callback),
+ AsWeakPtr(), src_url, dest_url, callback),
base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
}
@@ -349,7 +322,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;
}
@@ -360,7 +333,7 @@ void LocalFileSystemOperation::GetUsageAndQuotaThenRunTask(
url.origin(),
FileSystemTypeToQuotaStorageType(url.type()),
base::Bind(&LocalFileSystemOperation::DidGetUsageAndQuotaAndRunTask,
- weak_factory_.GetWeakPtr(), task, error_callback));
+ AsWeakPtr(), task, error_callback));
}
void LocalFileSystemOperation::DidGetUsageAndQuotaAndRunTask(
@@ -374,7 +347,7 @@ void LocalFileSystemOperation::DidGetUsageAndQuotaAndRunTask(
return;
}
- operation_context()->set_allowed_bytes_growth(quota - usage);
+ operation_context_->set_allowed_bytes_growth(quota - usage);
task.Run();
}
@@ -391,17 +364,16 @@ base::Closure LocalFileSystemOperation::GetWriteClosure(
if (!writer) {
// Write is not supported.
return base::Bind(&LocalFileSystemOperation::DidFailWrite,
- base::Owned(this), callback,
+ AsWeakPtr(), callback,
base::PLATFORM_FILE_ERROR_SECURITY);
}
DCHECK(blob_url.is_valid());
file_writer_delegate_.reset(new FileWriterDelegate(
- base::Bind(&LocalFileSystemOperation::DidWrite,
- weak_factory_.GetWeakPtr(), url),
+ base::Bind(&LocalFileSystemOperation::DidWrite, AsWeakPtr(),
+ url, callback),
writer.Pass()));
- write_callback_ = callback;
scoped_ptr<net::URLRequest> blob_request(url_request_context->CreateRequest(
blob_url, file_writer_delegate_.get()));
@@ -421,12 +393,12 @@ void LocalFileSystemOperation::DoCreateFile(
const StatusCallback& callback,
bool exclusive) {
async_file_util_->EnsureFileExists(
- operation_context(), url,
+ operation_context_.Pass(), url,
base::Bind(
exclusive ?
&LocalFileSystemOperation::DidEnsureFileExistsExclusive :
&LocalFileSystemOperation::DidEnsureFileExistsNonExclusive,
- base::Owned(this), callback));
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DoCreateDirectory(
@@ -434,10 +406,10 @@ void LocalFileSystemOperation::DoCreateDirectory(
const StatusCallback& callback,
bool exclusive, bool recursive) {
async_file_util_->CreateDirectory(
- operation_context(),
+ operation_context_.Pass(),
url, exclusive, recursive,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DoCopyFileLocal(
@@ -445,9 +417,9 @@ void LocalFileSystemOperation::DoCopyFileLocal(
const FileSystemURL& dest_url,
const StatusCallback& callback) {
async_file_util_->CopyFileLocal(
- operation_context(), src_url, dest_url,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ operation_context_.Pass(), src_url, dest_url,
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DoMoveFileLocal(
@@ -455,9 +427,9 @@ void LocalFileSystemOperation::DoMoveFileLocal(
const FileSystemURL& dest_url,
const StatusCallback& callback) {
async_file_util_->MoveFileLocal(
- operation_context(), src_url, dest_url,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ operation_context_.Pass(), src_url, dest_url,
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DoCopyInForeignFile(
@@ -465,28 +437,28 @@ void LocalFileSystemOperation::DoCopyInForeignFile(
const FileSystemURL& dest_url,
const StatusCallback& callback) {
async_file_util_->CopyInForeignFile(
- operation_context(),
+ operation_context_.Pass(),
src_local_disk_file_path, dest_url,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DoTruncate(const FileSystemURL& url,
const StatusCallback& callback,
int64 length) {
async_file_util_->Truncate(
- operation_context(), url, length,
- base::Bind(&LocalFileSystemOperation::DidFinishFileOperation,
- base::Owned(this), callback));
+ operation_context_.Pass(), url, length,
+ base::Bind(&LocalFileSystemOperation::DidFinishOperation,
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DoOpenFile(const FileSystemURL& url,
const OpenFileCallback& callback,
int file_flags) {
async_file_util_->CreateOrOpen(
- operation_context(), url, file_flags,
+ operation_context_.Pass(), url, file_flags,
base::Bind(&LocalFileSystemOperation::DidOpenFile,
- base::Owned(this), callback));
+ AsWeakPtr(), callback));
}
void LocalFileSystemOperation::DidEnsureFileExistsExclusive(
@@ -495,39 +467,30 @@ void LocalFileSystemOperation::DidEnsureFileExistsExclusive(
if (rv == base::PLATFORM_FILE_OK && !created) {
callback.Run(base::PLATFORM_FILE_ERROR_EXISTS);
} else {
- DidFinishFileOperation(callback, rv);
+ DidFinishOperation(callback, rv);
}
}
void LocalFileSystemOperation::DidEnsureFileExistsNonExclusive(
const StatusCallback& callback,
base::PlatformFileError rv, bool /* created */) {
- DidFinishFileOperation(callback, rv);
+ DidFinishOperation(callback, rv);
}
-void LocalFileSystemOperation::DidFinishFileOperation(
+void LocalFileSystemOperation::DidFinishOperation(
const StatusCallback& callback,
base::PlatformFileError rv) {
if (!cancel_callback_.is_null()) {
DCHECK_EQ(kOperationTruncate, pending_operation_);
+ StatusCallback cancel_callback = cancel_callback_;
callback.Run(base::PLATFORM_FILE_ERROR_ABORT);
- cancel_callback_.Run(base::PLATFORM_FILE_OK);
- cancel_callback_.Reset();
+ cancel_callback.Run(base::PLATFORM_FILE_OK);
} else {
callback.Run(rv);
}
}
-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,
@@ -548,49 +511,24 @@ void LocalFileSystemOperation::DidFileExists(
callback.Run(rv);
}
-void LocalFileSystemOperation::DidGetMetadata(
- const GetMetadataCallback& callback,
- base::PlatformFileError rv,
- const base::PlatformFileInfo& file_info,
- const base::FilePath& platform_path) {
- callback.Run(rv, file_info, platform_path);
-}
-
-void LocalFileSystemOperation::DidReadDirectory(
- const ReadDirectoryCallback& callback,
- base::PlatformFileError rv,
- const std::vector<DirectoryEntry>& entries,
- bool has_more) {
- callback.Run(rv, entries, has_more);
-}
-
void LocalFileSystemOperation::DidWrite(
const FileSystemURL& url,
+ const WriteCallback& write_callback,
base::PlatformFileError rv,
int64 bytes,
FileWriterDelegate::WriteProgressStatus write_status) {
- if (write_callback_.is_null()) {
- // If cancelled, callback is already invoked and set to null in Cancel().
- // We must not call it twice. Just shut down this operation object.
- delete this;
- return;
- }
-
const bool complete = (
write_status != FileWriterDelegate::SUCCESS_IO_PENDING);
if (complete && write_status != FileWriterDelegate::ERROR_WRITE_NOT_STARTED) {
- operation_context()->change_observers()->Notify(
+ DCHECK(operation_context_);
+ operation_context_->change_observers()->Notify(
&FileChangeObserver::OnModifyFile, MakeTuple(url));
}
- write_callback_.Run(rv, bytes, complete);
- if (complete || rv != base::PLATFORM_FILE_OK)
- delete this;
-}
-
-void LocalFileSystemOperation::DidTouchFile(const StatusCallback& callback,
- base::PlatformFileError rv) {
- callback.Run(rv);
+ StatusCallback cancel_callback = cancel_callback_;
+ write_callback.Run(rv, bytes, complete);
+ if (!cancel_callback.is_null())
+ cancel_callback.Run(base::PLATFORM_FILE_OK);
}
void LocalFileSystemOperation::DidOpenFile(
@@ -605,15 +543,6 @@ void LocalFileSystemOperation::DidOpenFile(
peer_handle_);
}
-void LocalFileSystemOperation::DidCreateSnapshotFile(
- const SnapshotFileCallback& callback,
- base::PlatformFileError result,
- const base::PlatformFileInfo& file_info,
- const base::FilePath& platform_path,
- const scoped_refptr<ShareableFileReference>& file_ref) {
- callback.Run(result, file_info, platform_path, file_ref);
-}
-
bool LocalFileSystemOperation::SetPendingOperationType(OperationType type) {
if (pending_operation_ != kOperationNone)
return false;
« no previous file with comments | « webkit/browser/fileapi/local_file_system_operation.h ('k') | webkit/browser/fileapi/syncable/syncable_file_system_operation.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698