From 01ce105b980dc83da154ca5de76a8e414d80ada1 Mon Sep 17 00:00:00 2001 From: "kinuko@chromium.org" Date: Tue, 8 Nov 2011 09:04:16 +0000 Subject: Retry: Merge FileUtilProxy and FileSystemFileUtilProxy using PostTaskAndReply: CreateOrOpen/Close Original reviewed issue: http://codereview.chromium.org/8424006/ BUG=none TEST=test_shell_tests:*FileSystem* Review URL: http://codereview.chromium.org/8499005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108991 0039d316-1c4b-4281-b951-d872f2087c98 --- webkit/fileapi/file_system_file_util_proxy.cc | 83 ------------------- webkit/fileapi/file_system_file_util_proxy.h | 18 ----- webkit/fileapi/file_system_operation.cc | 110 ++++++++++++++++++-------- webkit/fileapi/file_system_operation.h | 1 + 4 files changed, 78 insertions(+), 134 deletions(-) (limited to 'webkit') diff --git a/webkit/fileapi/file_system_file_util_proxy.cc b/webkit/fileapi/file_system_file_util_proxy.cc index 01af4b9..6c54cdc 100644 --- a/webkit/fileapi/file_system_file_util_proxy.cc +++ b/webkit/fileapi/file_system_file_util_proxy.cc @@ -71,51 +71,6 @@ class MessageLoopRelay fileapi::FileSystemFileUtil* file_util_; }; -class RelayCreateOrOpen : public MessageLoopRelay { - public: - RelayCreateOrOpen( - const fileapi::FileSystemOperationContext& context, - scoped_refptr message_loop_proxy, - const FilePath& file_path, - int file_flags, - const fileapi::FileSystemFileUtilProxy::CreateOrOpenCallback& callback) - : MessageLoopRelay(context), - message_loop_proxy_(message_loop_proxy), - file_path_(file_path), - file_flags_(file_flags), - callback_(callback), - file_handle_(base::kInvalidPlatformFileValue), - created_(false) { - DCHECK_EQ(false, callback.is_null()); - } - - protected: - virtual ~RelayCreateOrOpen() { - if (file_handle_ != base::kInvalidPlatformFileValue) - fileapi::FileSystemFileUtilProxy::Close( - *context(), message_loop_proxy_, file_handle_, - fileapi::FileSystemFileUtilProxy::StatusCallback()); - } - - virtual void RunWork() { - set_error_code(file_util()->CreateOrOpen( - context(), file_path_, file_flags_, &file_handle_, &created_)); - } - - virtual void RunCallback() { - callback_.Run(error_code(), base::PassPlatformFile(&file_handle_), - created_); - } - - private: - scoped_refptr message_loop_proxy_; - FilePath file_path_; - int file_flags_; - fileapi::FileSystemFileUtilProxy::CreateOrOpenCallback callback_; - base::PlatformFile file_handle_; - bool created_; -}; - class RelayWithStatusCallback : public MessageLoopRelay { public: RelayWithStatusCallback( @@ -137,24 +92,6 @@ class RelayWithStatusCallback : public MessageLoopRelay { fileapi::FileSystemFileUtilProxy::StatusCallback callback_; }; -class RelayClose : public RelayWithStatusCallback { - public: - RelayClose(const fileapi::FileSystemOperationContext& context, - base::PlatformFile file_handle, - const fileapi::FileSystemFileUtilProxy::StatusCallback& callback) - : RelayWithStatusCallback(context, callback), - file_handle_(file_handle) { - } - - protected: - virtual void RunWork() { - set_error_code(file_util()->Close(context(), file_handle_)); - } - - private: - base::PlatformFile file_handle_; -}; - class RelayEnsureFileExists : public MessageLoopRelay { public: RelayEnsureFileExists( @@ -425,26 +362,6 @@ bool Start(const tracked_objects::Location& from_here, namespace fileapi { // static -bool FileSystemFileUtilProxy::CreateOrOpen( - const FileSystemOperationContext& context, - scoped_refptr message_loop_proxy, - const FilePath& file_path, int file_flags, - const CreateOrOpenCallback& callback) { - return Start(FROM_HERE, message_loop_proxy, new RelayCreateOrOpen(context, - message_loop_proxy, file_path, file_flags, callback)); -} - -// static -bool FileSystemFileUtilProxy::Close( - const FileSystemOperationContext& context, - scoped_refptr message_loop_proxy, - base::PlatformFile file_handle, - const StatusCallback& callback) { - return Start(FROM_HERE, message_loop_proxy, - new RelayClose(context, file_handle, callback)); -} - -// static bool FileSystemFileUtilProxy::EnsureFileExists( const FileSystemOperationContext& context, scoped_refptr message_loop_proxy, diff --git a/webkit/fileapi/file_system_file_util_proxy.h b/webkit/fileapi/file_system_file_util_proxy.h index a3e85e5..b6a9ada 100644 --- a/webkit/fileapi/file_system_file_util_proxy.h +++ b/webkit/fileapi/file_system_file_util_proxy.h @@ -36,7 +36,6 @@ class FileSystemFileUtilProxy { typedef base::FileUtilProxy::Entry Entry; typedef base::FileUtilProxy::StatusCallback StatusCallback; - typedef base::FileUtilProxy::CreateOrOpenCallback CreateOrOpenCallback; typedef base::Callback EnsureFileExistsCallback; @@ -52,23 +51,6 @@ class FileSystemFileUtilProxy { const FilePath& /* local_path, where possible */ >::Type GetLocalPathCallback; - // Creates or opens a file with the given flags. It is invalid to pass NULL - // for the callback. - // If PLATFORM_FILE_CREATE is set in |file_flags| it always tries to create - // a new file at the given |file_path| and calls back with - // PLATFORM_FILE_ERROR_FILE_EXISTS if the |file_path| already exists. - static bool CreateOrOpen(const FileSystemOperationContext& context, - scoped_refptr message_loop_proxy, - const FilePath& file_path, - int file_flags, - const CreateOrOpenCallback& callback); - - // Close the given file handle. - static bool Close(const FileSystemOperationContext& context, - scoped_refptr message_loop_proxy, - PlatformFile, - const StatusCallback& callback); - // Ensures that the given |file_path| exist. This creates a empty new file // at |file_path| if the |file_path| does not exist. // If a new file han not existed and is created at the |file_path|, diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc index 7c504a6..c65aee7 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/webkit/fileapi/file_system_operation.cc @@ -71,16 +71,22 @@ FileSystemOperation::FileSystemOperation( } FileSystemOperation::~FileSystemOperation() { - if (file_writer_delegate_.get()) - FileSystemFileUtilProxy::Close( - operation_context_, proxy_, file_writer_delegate_->file(), + if (file_writer_delegate_.get()) { + FileSystemOperationContext* c = + new FileSystemOperationContext(operation_context_); + base::FileUtilProxy::RelayClose( + proxy_, + base::Bind(&FileSystemFileUtil::Close, + base::Unretained(c->src_file_util()), + base::Owned(c)), + file_writer_delegate_->file(), FileSystemFileUtilProxy::StatusCallback()); + } } void FileSystemOperation::OpenFileSystem( const GURL& origin_url, fileapi::FileSystemType type, bool create) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = static_cast( kOperationOpenFileSystem); @@ -104,7 +110,6 @@ void FileSystemOperation::OpenFileSystem( void FileSystemOperation::CreateFile(const GURL& path, bool exclusive) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationCreateFile; #endif @@ -143,7 +148,6 @@ void FileSystemOperation::CreateDirectory(const GURL& path, bool exclusive, bool recursive) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationCreateDirectory; #endif @@ -179,7 +183,6 @@ void FileSystemOperation::DelayedCreateDirectoryForQuota( void FileSystemOperation::Copy(const GURL& src_path, const GURL& dest_path) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationCopy; #endif @@ -214,7 +217,6 @@ void FileSystemOperation::DelayedCopyForQuota(quota::QuotaStatusCode status, void FileSystemOperation::Move(const GURL& src_path, const GURL& dest_path) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationMove; #endif @@ -248,7 +250,6 @@ void FileSystemOperation::DelayedMoveForQuota(quota::QuotaStatusCode status, void FileSystemOperation::DirectoryExists(const GURL& path) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationDirectoryExists; #endif @@ -265,7 +266,6 @@ void FileSystemOperation::DirectoryExists(const GURL& path) { void FileSystemOperation::FileExists(const GURL& path) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationFileExists; #endif @@ -282,7 +282,6 @@ void FileSystemOperation::FileExists(const GURL& path) { void FileSystemOperation::GetMetadata(const GURL& path) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationGetMetadata; #endif @@ -299,7 +298,6 @@ void FileSystemOperation::GetMetadata(const GURL& path) { void FileSystemOperation::ReadDirectory(const GURL& path) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationReadDirectory; #endif @@ -316,7 +314,6 @@ void FileSystemOperation::ReadDirectory(const GURL& path) { void FileSystemOperation::Remove(const GURL& path, bool recursive) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationRemove; #endif @@ -337,7 +334,6 @@ void FileSystemOperation::Write( const GURL& blob_url, int64 offset) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationWrite; #endif @@ -366,19 +362,25 @@ void FileSystemOperation::DelayedWriteForQuota(quota::QuotaStatusCode status, operation_context_.src_origin_url(), operation_context_.src_type())); - FileSystemFileUtilProxy::CreateOrOpen( - operation_context_, + int file_flags = base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_WRITE | + base::PLATFORM_FILE_ASYNC; + + base::FileUtilProxy::RelayCreateOrOpen( proxy_, - src_virtual_path_, - base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE | - base::PLATFORM_FILE_ASYNC, + base::Bind(&FileSystemFileUtil::CreateOrOpen, + base::Unretained(operation_context_.src_file_util()), + base::Unretained(&operation_context_), + src_virtual_path_, file_flags), + base::Bind(&FileSystemFileUtil::Close, + base::Unretained(operation_context_.src_file_util()), + base::Unretained(&operation_context_)), base::Bind(&FileSystemOperation::OnFileOpenedForWrite, weak_factory_.GetWeakPtr())); } void FileSystemOperation::Truncate(const GURL& path, int64 length) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationTruncate; #endif @@ -413,7 +415,6 @@ void FileSystemOperation::TouchFile(const GURL& path, const base::Time& last_access_time, const base::Time& last_modified_time) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationTouchFile; #endif @@ -433,7 +434,6 @@ void FileSystemOperation::OpenFile(const GURL& path, int file_flags, base::ProcessHandle peer_handle) { #ifndef NDEBUG - DCHECK(dispatcher_.get()); DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationOpenFile; #endif @@ -478,8 +478,15 @@ void FileSystemOperation::DelayedOpenFileForQuota(quota::QuotaStatusCode status, operation_context_.src_origin_url(), operation_context_.src_type())); - FileSystemFileUtilProxy::CreateOrOpen( - operation_context_, proxy_, src_virtual_path_, file_flags_, + base::FileUtilProxy::RelayCreateOrOpen( + proxy_, + base::Bind(&FileSystemFileUtil::CreateOrOpen, + base::Unretained(operation_context_.src_file_util()), + base::Unretained(&operation_context_), + src_virtual_path_, file_flags_), + base::Bind(&FileSystemFileUtil::Close, + base::Unretained(operation_context_.src_file_util()), + base::Unretained(&operation_context_)), base::Bind(&FileSystemOperation::DidOpenFile, weak_factory_.GetWeakPtr())); } @@ -518,9 +525,10 @@ void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation_ptr) { // This halts any calls to file_writer_delegate_ from blob_request_. blob_request_->Cancel(); - dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT); + if (dispatcher_.get()) + dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT); cancel_operation->dispatcher_->DidSucceed(); - delete this; + dispatcher_.reset(); } else { #ifndef NDEBUG DCHECK(kOperationTruncate == pending_operation_); @@ -568,14 +576,16 @@ void FileSystemOperation::DidGetRootPath( operation_context_.src_origin_url(), operation_context_.src_type()); } - dispatcher_->DidOpenFileSystem(name, result); + if (dispatcher_.get()) + dispatcher_->DidOpenFileSystem(name, result); delete this; } void FileSystemOperation::DidEnsureFileExistsExclusive( base::PlatformFileError rv, bool created) { if (rv == base::PLATFORM_FILE_OK && !created) { - dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_EXISTS); + if (dispatcher_.get()) + dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_EXISTS); delete this; } else { DidFinishFileOperation(rv); @@ -594,12 +604,14 @@ void FileSystemOperation::DidFinishFileOperation( DCHECK(kOperationTruncate == pending_operation_); #endif - dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT); + if (dispatcher_.get()) + dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT); cancel_operation_->dispatcher_->DidSucceed(); - } else if (rv == base::PLATFORM_FILE_OK) { - dispatcher_->DidSucceed(); - } else { - dispatcher_->DidFail(rv); + } else if (dispatcher_.get()) { + if (rv == base::PLATFORM_FILE_OK) + dispatcher_->DidSucceed(); + else + dispatcher_->DidFail(rv); } delete this; } @@ -608,6 +620,10 @@ void FileSystemOperation::DidDirectoryExists( base::PlatformFileError rv, const base::PlatformFileInfo& file_info, const FilePath& unused) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) { if (file_info.is_directory) dispatcher_->DidSucceed(); @@ -623,6 +639,10 @@ void FileSystemOperation::DidFileExists( base::PlatformFileError rv, const base::PlatformFileInfo& file_info, const FilePath& unused) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) { if (file_info.is_directory) dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_NOT_A_FILE); @@ -638,6 +658,10 @@ void FileSystemOperation::DidGetMetadata( base::PlatformFileError rv, const base::PlatformFileInfo& file_info, const FilePath& platform_path) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) dispatcher_->DidReadMetadata(file_info, platform_path); else @@ -648,6 +672,10 @@ void FileSystemOperation::DidGetMetadata( void FileSystemOperation::DidReadDirectory( base::PlatformFileError rv, const std::vector& entries) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) dispatcher_->DidReadDirectory(entries, false /* has_more */); @@ -660,6 +688,10 @@ void FileSystemOperation::DidWrite( base::PlatformFileError rv, int64 bytes, bool complete) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) dispatcher_->DidWrite(bytes, complete); else @@ -669,6 +701,10 @@ void FileSystemOperation::DidWrite( } void FileSystemOperation::DidTouchFile(base::PlatformFileError rv) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) dispatcher_->DidSucceed(); else @@ -680,6 +716,10 @@ void FileSystemOperation::DidOpenFile( base::PlatformFileError rv, base::PassPlatformFile file, bool unused) { + if (!dispatcher_.get()) { + delete this; + return; + } if (rv == base::PLATFORM_FILE_OK) dispatcher_->DidOpenFile(file.ReleaseValue(), peer_handle_); else @@ -691,6 +731,10 @@ void FileSystemOperation::OnFileOpenedForWrite( base::PlatformFileError rv, base::PassPlatformFile file, bool created) { + if (!dispatcher_.get()) { + delete this; + return; + } if (base::PLATFORM_FILE_OK != rv) { dispatcher_->DidFail(rv); delete this; diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h index ccfb033..0846ca6 100644 --- a/webkit/fileapi/file_system_operation.h +++ b/webkit/fileapi/file_system_operation.h @@ -251,6 +251,7 @@ class FileSystemOperation { // Proxy for calling file_util_proxy methods. scoped_refptr proxy_; + // This can be NULL if the operation is cancelled on the way. scoped_ptr dispatcher_; FileSystemOperationContext operation_context_; -- cgit v1.1