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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: webkit/browser/fileapi/syncable/syncable_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/syncable/syncable_file_system_operation.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webkit/browser/fileapi/syncable/syncable_file_system_operation.cc
diff --git a/webkit/browser/fileapi/syncable/syncable_file_system_operation.cc b/webkit/browser/fileapi/syncable/syncable_file_system_operation.cc
index 0a91777f1e51f059ede5692ef0e65607933c6f66..4aee609adde015122b5efcda4607bfca6c018065 100644
--- a/webkit/browser/fileapi/syncable/syncable_file_system_operation.cc
+++ b/webkit/browser/fileapi/syncable/syncable_file_system_operation.cc
@@ -8,6 +8,7 @@
#include "webkit/browser/fileapi/file_system_context.h"
#include "webkit/browser/fileapi/file_system_operation_context.h"
#include "webkit/browser/fileapi/file_system_url.h"
+#include "webkit/browser/fileapi/local_file_system_operation.h"
#include "webkit/browser/fileapi/sandbox_mount_point_provider.h"
#include "webkit/browser/fileapi/syncable/local_file_sync_context.h"
#include "webkit/browser/fileapi/syncable/syncable_file_operation_runner.h"
@@ -33,9 +34,11 @@ void WriteCallbackAdapter(
class SyncableFileSystemOperation::QueueableTask
: public SyncableFileOperationRunner::Task {
public:
- QueueableTask(SyncableFileSystemOperation* operation,
+ QueueableTask(base::WeakPtr<SyncableFileSystemOperation> operation,
const base::Closure& task)
- : operation_(operation), task_(task) {}
+ : operation_(operation),
+ task_(task),
+ target_paths_(operation->target_paths_) {}
virtual ~QueueableTask() {
DCHECK(!operation_);
@@ -44,25 +47,25 @@ class SyncableFileSystemOperation::QueueableTask
virtual void Run() OVERRIDE {
DCHECK(!task_.is_null());
task_.Run();
- operation_ = NULL;
+ operation_.reset();
}
virtual void Cancel() OVERRIDE {
DCHECK(!task_.is_null());
- DCHECK(operation_);
- operation_->OnCancelled();
- task_.Reset(); // This will delete operation_.
- operation_ = NULL;
+ if (operation_)
+ operation_->OnCancelled();
+ task_.Reset();
+ operation_.reset();
}
- virtual std::vector<FileSystemURL>& target_paths() const OVERRIDE {
- DCHECK(operation_);
- return operation_->target_paths_;
+ virtual const std::vector<FileSystemURL>& target_paths() const OVERRIDE {
+ return target_paths_;
}
private:
- SyncableFileSystemOperation* operation_;
+ base::WeakPtr<SyncableFileSystemOperation> operation_;
base::Closure task_;
+ std::vector<FileSystemURL> target_paths_;
DISALLOW_COPY_AND_ASSIGN(QueueableTask);
};
@@ -74,18 +77,18 @@ void SyncableFileSystemOperation::CreateFile(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&FileSystemOperation::CreateFile,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
url, exclusive,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -96,22 +99,22 @@ void SyncableFileSystemOperation::CreateDirectory(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
if (!is_directory_operation_enabled_) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
+ callback.Run(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&FileSystemOperation::CreateDirectory,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
url, exclusive, recursive,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -121,18 +124,18 @@ void SyncableFileSystemOperation::Copy(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(dest_url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&FileSystemOperation::Copy,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
src_url, dest_url,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -142,7 +145,7 @@ void SyncableFileSystemOperation::Move(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
DCHECK(operation_runner_.get());
@@ -150,11 +153,11 @@ void SyncableFileSystemOperation::Move(
target_paths_.push_back(dest_url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&FileSystemOperation::Move,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
src_url, dest_url,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -162,55 +165,31 @@ void SyncableFileSystemOperation::DirectoryExists(
const FileSystemURL& url,
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
- if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
- return;
- }
NewOperation()->DirectoryExists(url, callback);
- delete this;
}
void SyncableFileSystemOperation::FileExists(
const FileSystemURL& url,
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
- if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
- return;
- }
NewOperation()->FileExists(url, callback);
- delete this;
}
void SyncableFileSystemOperation::GetMetadata(
const FileSystemURL& url,
const GetMetadataCallback& callback) {
DCHECK(CalledOnValidThread());
- if (!operation_runner_.get()) {
- callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND,
- base::PlatformFileInfo(),
- base::FilePath());
- delete this;
- return;
- }
NewOperation()->GetMetadata(url, callback);
- delete this;
}
void SyncableFileSystemOperation::ReadDirectory(
const FileSystemURL& url,
const ReadDirectoryCallback& callback) {
DCHECK(CalledOnValidThread());
- if (!operation_runner_.get()) {
- callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND, FileEntryList(), false);
- 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)
NewOperation()->ReadDirectory(url, callback);
- delete this;
}
void SyncableFileSystemOperation::Remove(
@@ -218,18 +197,18 @@ void SyncableFileSystemOperation::Remove(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&FileSystemOperation::Remove,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
url, recursive,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -242,17 +221,16 @@ void SyncableFileSystemOperation::Write(
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND, 0, true);
- delete this;
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(url);
completion_callback_ = base::Bind(&WriteCallbackAdapter, callback);
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
NewOperation()->GetWriteClosure(
url_request_context, url, blob_url, offset,
- base::Bind(&self::DidWrite, base::Owned(this), callback))));
+ base::Bind(&self::DidWrite, AsWeakPtr(), callback))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -261,18 +239,18 @@ void SyncableFileSystemOperation::Truncate(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&FileSystemOperation::Truncate,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
url, length,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -282,13 +260,8 @@ void SyncableFileSystemOperation::TouchFile(
const base::Time& last_modified_time,
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
- if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
- return;
- }
NewOperation()->TouchFile(url, last_access_time,
last_modified_time, callback);
- delete this;
}
void SyncableFileSystemOperation::OpenFile(
@@ -297,7 +270,6 @@ void SyncableFileSystemOperation::OpenFile(
base::ProcessHandle peer_handle,
const OpenFileCallback& callback) {
NOTREACHED();
- delete this;
}
void SyncableFileSystemOperation::Cancel(
@@ -311,16 +283,7 @@ void SyncableFileSystemOperation::CreateSnapshotFile(
const FileSystemURL& path,
const SnapshotFileCallback& callback) {
DCHECK(CalledOnValidThread());
- if (!operation_runner_.get()) {
- callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND,
- base::PlatformFileInfo(),
- base::FilePath(),
- NULL);
- delete this;
- return;
- }
NewOperation()->CreateSnapshotFile(path, callback);
- delete this;
}
void SyncableFileSystemOperation::CopyInForeignFile(
@@ -329,18 +292,18 @@ void SyncableFileSystemOperation::CopyInForeignFile(
const StatusCallback& callback) {
DCHECK(CalledOnValidThread());
if (!operation_runner_.get()) {
- AbortOperation(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND);
+ callback.Run(base::PLATFORM_FILE_ERROR_NOT_FOUND);
return;
}
DCHECK(operation_runner_.get());
target_paths_.push_back(dest_url);
completion_callback_ = callback;
scoped_ptr<SyncableFileOperationRunner::Task> task(new QueueableTask(
- this,
+ AsWeakPtr(),
base::Bind(&LocalFileSystemOperation::CopyInForeignFile,
- base::Unretained(NewOperation()),
+ NewOperation()->AsWeakPtr(),
src_local_disk_path, dest_url,
- base::Bind(&self::DidFinish, base::Owned(this)))));
+ base::Bind(&self::DidFinish, AsWeakPtr()))));
operation_runner_->PostOperationTask(task.Pass());
}
@@ -350,8 +313,7 @@ SyncableFileSystemOperation::SyncableFileSystemOperation(
scoped_ptr<FileSystemOperationContext> operation_context)
: LocalFileSystemOperation(url, file_system_context,
operation_context.Pass()),
- url_(url),
- inflight_operation_(NULL) {
+ url_(url) {
DCHECK(file_system_context);
if (!file_system_context->sync_context()) {
// Syncable FileSystem is opened in a file system context which doesn't
@@ -365,10 +327,10 @@ SyncableFileSystemOperation::SyncableFileSystemOperation(
LocalFileSystemOperation* SyncableFileSystemOperation::NewOperation() {
DCHECK(operation_context_);
- inflight_operation_ = new LocalFileSystemOperation(
- url_, file_system_context(), operation_context_.Pass());
+ inflight_operation_.reset(new LocalFileSystemOperation(
+ url_, file_system_context(), operation_context_.Pass()));
DCHECK(inflight_operation_);
- return inflight_operation_;
+ return inflight_operation_.get();
}
void SyncableFileSystemOperation::DidFinish(base::PlatformFileError status) {
@@ -397,15 +359,6 @@ void SyncableFileSystemOperation::DidWrite(
void SyncableFileSystemOperation::OnCancelled() {
DCHECK(!completion_callback_.is_null());
completion_callback_.Run(base::PLATFORM_FILE_ERROR_ABORT);
- delete inflight_operation_;
-}
-
-void SyncableFileSystemOperation::AbortOperation(
- const StatusCallback& callback,
- base::PlatformFileError error) {
- callback.Run(error);
- delete inflight_operation_;
- delete this;
}
} // namespace sync_file_system
« no previous file with comments | « webkit/browser/fileapi/syncable/syncable_file_system_operation.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698