diff options
| -rw-r--r-- | chrome/browser/file_system/browser_file_system_callback_dispatcher.cc | 60 | ||||
| -rw-r--r-- | chrome/browser/file_system/browser_file_system_callback_dispatcher.h | 35 | ||||
| -rw-r--r-- | chrome/browser/file_system/file_system_dispatcher_host.cc | 56 | ||||
| -rw-r--r-- | chrome/browser/file_system/file_system_dispatcher_host.h | 5 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 2 | ||||
| -rw-r--r-- | webkit/fileapi/file_system_operation.cc | 34 | ||||
| -rw-r--r-- | webkit/fileapi/file_system_operation.h | 16 | ||||
| -rw-r--r-- | webkit/fileapi/file_system_operation_unittest.cc | 274 | ||||
| -rw-r--r-- | webkit/fileapi/sandboxed_file_system_operation.cc | 49 | ||||
| -rw-r--r-- | webkit/fileapi/sandboxed_file_system_operation.h | 4 | ||||
| -rw-r--r-- | webkit/tools/test_shell/simple_file_system.cc | 24 | ||||
| -rw-r--r-- | webkit/tools/test_shell/simple_file_writer.cc | 32 |
12 files changed, 256 insertions, 335 deletions
diff --git a/chrome/browser/file_system/browser_file_system_callback_dispatcher.cc b/chrome/browser/file_system/browser_file_system_callback_dispatcher.cc deleted file mode 100644 index 810bf3f..0000000 --- a/chrome/browser/file_system/browser_file_system_callback_dispatcher.cc +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) 2010 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 "chrome/browser/file_system/browser_file_system_callback_dispatcher.h" - -#include "chrome/browser/file_system/file_system_dispatcher_host.h" -#include "chrome/common/render_messages.h" - -BrowserFileSystemCallbackDispatcher::BrowserFileSystemCallbackDispatcher( - FileSystemDispatcherHost* dispatcher_host, int request_id) - : dispatcher_host_(dispatcher_host), - request_id_(request_id) { - DCHECK(dispatcher_host_); -} - -BrowserFileSystemCallbackDispatcher::~BrowserFileSystemCallbackDispatcher() {} - -void BrowserFileSystemCallbackDispatcher::DidSucceed() { - dispatcher_host_->Send(new ViewMsg_FileSystem_DidSucceed(request_id_)); - dispatcher_host_->RemoveCompletedOperation(request_id_); -} - -void BrowserFileSystemCallbackDispatcher::DidReadMetadata( - const base::PlatformFileInfo& info) { - dispatcher_host_->Send(new ViewMsg_FileSystem_DidReadMetadata( - request_id_, info)); - dispatcher_host_->RemoveCompletedOperation(request_id_); -} - -void BrowserFileSystemCallbackDispatcher::DidReadDirectory( - const std::vector<base::FileUtilProxy::Entry>& entries, bool has_more) { - dispatcher_host_->Send(new ViewMsg_FileSystem_DidReadDirectory( - request_id_, entries, has_more)); - dispatcher_host_->RemoveCompletedOperation(request_id_); -} - -void BrowserFileSystemCallbackDispatcher::DidOpenFileSystem( - const std::string& name, const FilePath& path) { - dispatcher_host_->Send( - new ViewMsg_OpenFileSystemRequest_Complete( - request_id_, !path.empty(), name, path)); - dispatcher_host_->RemoveCompletedOperation(request_id_); -} - -void BrowserFileSystemCallbackDispatcher::DidFail( - base::PlatformFileError error_code) { - dispatcher_host_->Send(new ViewMsg_FileSystem_DidFail( - request_id_, error_code)); - dispatcher_host_->RemoveCompletedOperation(request_id_); -} - -void BrowserFileSystemCallbackDispatcher::DidWrite( - int64 bytes, - bool complete) { - dispatcher_host_->Send(new ViewMsg_FileSystem_DidWrite( - request_id_, bytes, complete)); - if (complete) - dispatcher_host_->RemoveCompletedOperation(request_id_); -} diff --git a/chrome/browser/file_system/browser_file_system_callback_dispatcher.h b/chrome/browser/file_system/browser_file_system_callback_dispatcher.h deleted file mode 100644 index 7e85b41..0000000 --- a/chrome/browser/file_system/browser_file_system_callback_dispatcher.h +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) 2010 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. - -#ifndef CHROME_BROWSER_FILE_SYSTEM_BROWSER_FILE_SYSTEM_CALLBACK_DISPATCHER_H_ -#define CHROME_BROWSER_FILE_SYSTEM_BROWSER_FILE_SYSTEM_CALLBACK_DISPATCHER_H_ - -#include "webkit/fileapi/file_system_callback_dispatcher.h" - -class FileSystemDispatcherHost; - -class BrowserFileSystemCallbackDispatcher - : public fileapi::FileSystemCallbackDispatcher { - public: - BrowserFileSystemCallbackDispatcher(FileSystemDispatcherHost* dispatcher_host, - int request_id); - virtual ~BrowserFileSystemCallbackDispatcher(); - - // FileSystemCallbackDispatcher implementation. - virtual void DidSucceed(); - virtual void DidReadMetadata(const base::PlatformFileInfo& file_info); - virtual void DidReadDirectory( - const std::vector<base::FileUtilProxy::Entry>& entries, - bool has_more); - virtual void DidOpenFileSystem(const std::string& name, - const FilePath& root_path); - virtual void DidFail(base::PlatformFileError error_code); - virtual void DidWrite(int64 bytes, bool complete); - - private: - scoped_refptr<FileSystemDispatcherHost> dispatcher_host_; - int request_id_; -}; - -#endif // CHROME_BROWSER_FILE_SYSTEM_BROWSER_FILE_SYSTEM_CALLBACK_DISPATCHER_H_ diff --git a/chrome/browser/file_system/file_system_dispatcher_host.cc b/chrome/browser/file_system/file_system_dispatcher_host.cc index 2668a78..6807f28 100644 --- a/chrome/browser/file_system/file_system_dispatcher_host.cc +++ b/chrome/browser/file_system/file_system_dispatcher_host.cc @@ -10,7 +10,6 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_thread.h" #include "chrome/browser/content_settings/host_content_settings_map.h" -#include "chrome/browser/file_system/browser_file_system_callback_dispatcher.h" #include "chrome/browser/file_system/browser_file_system_context.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/profile.h" @@ -20,14 +19,67 @@ #include "chrome/common/render_messages_params.h" #include "googleurl/src/gurl.h" #include "net/url_request/url_request_context.h" +#include "webkit/fileapi/file_system_callback_dispatcher.h" #include "webkit/fileapi/file_system_operation.h" #include "webkit/fileapi/file_system_path_manager.h" #include "webkit/fileapi/file_system_quota_manager.h" #include "webkit/fileapi/sandboxed_file_system_operation.h" +using fileapi::FileSystemCallbackDispatcher; using fileapi::FileSystemQuotaManager; using fileapi::SandboxedFileSystemOperation; +class BrowserFileSystemCallbackDispatcher + : public FileSystemCallbackDispatcher { + public: + BrowserFileSystemCallbackDispatcher( + FileSystemDispatcherHost* dispatcher_host, int request_id) + : dispatcher_host_(dispatcher_host), + request_id_(request_id) { + DCHECK(dispatcher_host_); + } + + virtual ~BrowserFileSystemCallbackDispatcher() { + dispatcher_host_->UnregisterOperation(request_id_); + } + + virtual void DidSucceed() { + dispatcher_host_->Send(new ViewMsg_FileSystem_DidSucceed(request_id_)); + } + + virtual void DidReadMetadata(const base::PlatformFileInfo& info) { + dispatcher_host_->Send(new ViewMsg_FileSystem_DidReadMetadata( + request_id_, info)); + } + + virtual void DidReadDirectory( + const std::vector<base::FileUtilProxy::Entry>& entries, bool has_more) { + dispatcher_host_->Send(new ViewMsg_FileSystem_DidReadDirectory( + request_id_, entries, has_more)); + } + + virtual void DidOpenFileSystem(const std::string& name, + const FilePath& path) { + dispatcher_host_->Send( + new ViewMsg_OpenFileSystemRequest_Complete( + request_id_, !path.empty(), name, path)); + } + + virtual void DidFail(base::PlatformFileError error_code) { + dispatcher_host_->Send(new ViewMsg_FileSystem_DidFail( + request_id_, error_code)); + } + + virtual void DidWrite(int64 bytes, bool complete) { + dispatcher_host_->Send(new ViewMsg_FileSystem_DidWrite( + request_id_, bytes, complete)); + } + + private: + scoped_refptr<FileSystemDispatcherHost> dispatcher_host_; + int request_id_; +}; + FileSystemDispatcherHost::FileSystemDispatcherHost( IPC::Message::Sender* sender, Profile* profile) : message_sender_(sender), @@ -217,7 +269,7 @@ SandboxedFileSystemOperation* FileSystemDispatcherHost::GetNewOperation( return operation; } -void FileSystemDispatcherHost::RemoveCompletedOperation(int request_id) { +void FileSystemDispatcherHost::UnregisterOperation(int request_id) { DCHECK(operations_.Lookup(request_id)); operations_.Remove(request_id); } diff --git a/chrome/browser/file_system/file_system_dispatcher_host.h b/chrome/browser/file_system/file_system_dispatcher_host.h index 2cfe4f6..fdbe8f6 100644 --- a/chrome/browser/file_system/file_system_dispatcher_host.h +++ b/chrome/browser/file_system/file_system_dispatcher_host.h @@ -79,7 +79,7 @@ class FileSystemDispatcherHost const base::Time& last_modified_time); void OnCancel(int request_id, int request_to_cancel); void Send(IPC::Message* message); - void RemoveCompletedOperation(int request_id); + void UnregisterOperation(int request_id); private: // Creates a new SandboxedFileSystemOperation. @@ -99,8 +99,7 @@ class FileSystemDispatcherHost scoped_refptr<HostContentSettingsMap> host_content_settings_map_; // Keeps ongoing file system operations. - typedef IDMap<fileapi::SandboxedFileSystemOperation, IDMapOwnPointer> - OperationsMap; + typedef IDMap<fileapi::SandboxedFileSystemOperation> OperationsMap; OperationsMap operations_; // This holds the URLRequestContextGetter until Init() can be called from the diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index a5f6f16..b3d8577 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1656,8 +1656,6 @@ 'browser/file_path_watcher_win.cc', 'browser/file_select_helper.cc', 'browser/file_select_helper.h', - 'browser/file_system/browser_file_system_callback_dispatcher.cc', - 'browser/file_system/browser_file_system_callback_dispatcher.h', 'browser/file_system/browser_file_system_context.cc', 'browser/file_system/browser_file_system_context.h', 'browser/file_system/file_system_dispatcher_host.cc', diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc index fe07758..97e2bfe 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/webkit/fileapi/file_system_operation.cc @@ -16,8 +16,7 @@ FileSystemOperation::FileSystemOperation( scoped_refptr<base::MessageLoopProxy> proxy) : proxy_(proxy), dispatcher_(dispatcher), - callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), - cancel_operation_(NULL) { + callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { DCHECK(dispatcher); #ifndef NDEBUG pending_operation_ = kOperationNone; @@ -159,6 +158,7 @@ void FileSystemOperation::OnFileOpenedForWrite( bool created) { if (base::PLATFORM_FILE_OK != rv) { dispatcher_->DidFail(rv); + delete this; return; } file_writer_delegate_->Start(file.ReleaseValue(), blob_request_.get()); @@ -189,7 +189,8 @@ void FileSystemOperation::TouchFile(const FilePath& path, // 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(FileSystemOperation* cancel_operation) { +void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation_ptr) { + scoped_ptr<FileSystemOperation> cancel_operation(cancel_operation_ptr); if (file_writer_delegate_.get()) { #ifndef NDEBUG DCHECK(kOperationWrite == pending_operation_); @@ -203,9 +204,9 @@ void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation) { // This halts any calls to file_writer_delegate_ from blob_request_. blob_request_->Cancel(); - // This deletes us, and by proxy deletes file_writer_delegate_ if any. dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT); - cancel_operation->dispatcher_->DidSucceed(); + cancel_operation->dispatcher()->DidSucceed(); + delete this; } else { #ifndef NDEBUG DCHECK(kOperationTruncate == pending_operation_); @@ -214,15 +215,17 @@ void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation) { // since it's been proxied to another thread. We need to save the // cancel_operation so that when the truncate returns, it can see that it's // been cancelled, report it, and report that the cancel has succeeded. - cancel_operation_ = cancel_operation; + DCHECK(!cancel_operation_.get()); + cancel_operation_.swap(cancel_operation); } } void FileSystemOperation::DidEnsureFileExistsExclusive( base::PlatformFileError rv, bool created) { - if (rv == base::PLATFORM_FILE_OK && !created) + if (rv == base::PLATFORM_FILE_OK && !created) { dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_EXISTS); - else + delete this; + } else DidFinishFileOperation(rv); } @@ -233,19 +236,19 @@ void FileSystemOperation::DidEnsureFileExistsNonExclusive( void FileSystemOperation::DidFinishFileOperation( base::PlatformFileError rv) { - if (cancel_operation_) { + if (cancel_operation_.get()) { #ifndef NDEBUG DCHECK(kOperationTruncate == pending_operation_); #endif - FileSystemOperation *cancel_op = cancel_operation_; - // This call deletes us, so we have to extract cancel_op first. + dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT); - cancel_op->dispatcher_->DidSucceed(); + cancel_operation_->dispatcher()->DidSucceed(); } else if (rv == base::PLATFORM_FILE_OK) { dispatcher_->DidSucceed(); } else { dispatcher_->DidFail(rv); } + delete this; } void FileSystemOperation::DidDirectoryExists( @@ -259,6 +262,7 @@ void FileSystemOperation::DidDirectoryExists( } else { dispatcher_->DidFail(rv); } + delete this; } void FileSystemOperation::DidFileExists( @@ -272,6 +276,7 @@ void FileSystemOperation::DidFileExists( } else { dispatcher_->DidFail(rv); } + delete this; } void FileSystemOperation::DidGetMetadata( @@ -281,6 +286,7 @@ void FileSystemOperation::DidGetMetadata( dispatcher_->DidReadMetadata(file_info); else dispatcher_->DidFail(rv); + delete this; } void FileSystemOperation::DidReadDirectory( @@ -290,6 +296,7 @@ void FileSystemOperation::DidReadDirectory( dispatcher_->DidReadDirectory(entries, false /* has_more */); else dispatcher_->DidFail(rv); + delete this; } void FileSystemOperation::DidWrite( @@ -300,6 +307,8 @@ void FileSystemOperation::DidWrite( dispatcher_->DidWrite(bytes, complete); else dispatcher_->DidFail(rv); + if (complete || rv != base::PLATFORM_FILE_OK) + delete this; } void FileSystemOperation::DidTouchFile(base::PlatformFileError rv) { @@ -307,6 +316,7 @@ void FileSystemOperation::DidTouchFile(base::PlatformFileError rv) { dispatcher_->DidSucceed(); else dispatcher_->DidFail(rv); + delete this; } } // namespace fileapi diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h index 6614b5e..e260e24 100644 --- a/webkit/fileapi/file_system_operation.h +++ b/webkit/fileapi/file_system_operation.h @@ -36,8 +36,11 @@ class FileWriterDelegate; // Only one method(CreateFile, CreateDirectory, Copy, Move, DirectoryExists, // GetMetadata, ReadDirectory and Remove) may be called during the lifetime of // this object and it should be called no more than once. +// This class is self-destructed and an instance automatically gets deleted +// when its operation is finished. class FileSystemOperation { public: + // |dispatcher| will be owned by this class. FileSystemOperation(FileSystemCallbackDispatcher* dispatcher, scoped_refptr<base::MessageLoopProxy> proxy); virtual ~FileSystemOperation(); @@ -65,15 +68,16 @@ class FileSystemOperation { virtual void Remove(const FilePath& path, bool recursive); - virtual void Write( - scoped_refptr<URLRequestContext> url_request_context, - const FilePath& path, const GURL& blob_url, int64 offset); + virtual void Write(scoped_refptr<URLRequestContext> url_request_context, + const FilePath& path, + const GURL& blob_url, + int64 offset); virtual void Truncate(const FilePath& path, int64 length); virtual void TouchFile(const FilePath& path, - const base::Time& last_access_time, - const base::Time& last_modified_time); + const base::Time& last_access_time, + const base::Time& last_modified_time); // Try to cancel the current operation [we support cancelling write or // truncate only]. Report failure for the current operation, then tell the @@ -154,7 +158,7 @@ class FileSystemOperation { friend class FileWriterDelegate; scoped_ptr<FileWriterDelegate> file_writer_delegate_; scoped_ptr<net::URLRequest> blob_request_; - FileSystemOperation* cancel_operation_; + scoped_ptr<FileSystemOperation> cancel_operation_; DISALLOW_COPY_AND_ASSIGN(FileSystemOperation); }; diff --git a/webkit/fileapi/file_system_operation_unittest.cc b/webkit/fileapi/file_system_operation_unittest.cc index 370d95c..dffcc33 100644 --- a/webkit/fileapi/file_system_operation_unittest.cc +++ b/webkit/fileapi/file_system_operation_unittest.cc @@ -14,40 +14,69 @@ namespace fileapi { -const int kInvalidRequestId = -1; -const int kFileOperationStatusNotSet = 0; -const int kFileOperationSucceeded = 1; - -static int last_request_id = -1; +const int kFileOperationStatusNotSet = 1; +const int kFileOperationSucceeded = 0; static bool FileExists(FilePath path) { return file_util::PathExists(path) && !file_util::DirectoryExists(path); } -class MockDispatcher : public FileSystemCallbackDispatcher { +class MockDispatcher; + +class FileSystemOperationTest : public testing::Test { public: - MockDispatcher(int request_id) - : status_(kFileOperationStatusNotSet), - request_id_(request_id) { + FileSystemOperationTest() + : status_(kFileOperationStatusNotSet) { + base_.CreateUniqueTempDir(); + EXPECT_TRUE(base_.IsValid()); } + FileSystemOperation* operation(); + + void set_status(int status) { status_ = status; } + int status() const { return status_; } + void set_info(const base::PlatformFileInfo& info) { info_ = info; } + const base::PlatformFileInfo& info() const { return info_; } + void set_entries(const std::vector<base::FileUtilProxy::Entry>& entries) { + entries_ = entries; + } + const std::vector<base::FileUtilProxy::Entry>& entries() const { + return entries_; + } + + protected: + // Common temp base for nondestructive uses. + ScopedTempDir base_; + + // For post-operation status. + int status_; + base::PlatformFileInfo info_; + std::vector<base::FileUtilProxy::Entry> entries_; + + DISALLOW_COPY_AND_ASSIGN(FileSystemOperationTest); +}; + +class MockDispatcher : public FileSystemCallbackDispatcher { + public: + MockDispatcher(FileSystemOperationTest* test) : test_(test) { } + virtual void DidFail(base::PlatformFileError status) { - status_ = status; + test_->set_status(status); } virtual void DidSucceed() { - status_ = kFileOperationSucceeded; + test_->set_status(kFileOperationSucceeded); } virtual void DidReadMetadata(const base::PlatformFileInfo& info) { - info_ = info; - status_ = kFileOperationSucceeded; + test_->set_info(info); + test_->set_status(kFileOperationSucceeded); } virtual void DidReadDirectory( const std::vector<base::FileUtilProxy::Entry>& entries, bool /* has_more */) { - entries_ = entries; + test_->set_entries(entries); } virtual void DidOpenFileSystem(const std::string&, const FilePath&) { @@ -58,58 +87,22 @@ class MockDispatcher : public FileSystemCallbackDispatcher { NOTREACHED(); } - // Helpers for testing. - int status() const { return status_; } - int request_id() const { return request_id_; } - const base::PlatformFileInfo& info() const { return info_; } - const std::vector<base::FileUtilProxy::Entry>& entries() const { - return entries_; - } - private: - int status_; - int request_id_; - base::PlatformFileInfo info_; - std::vector<base::FileUtilProxy::Entry> entries_; + FileSystemOperationTest* test_; }; -class FileSystemOperationTest : public testing::Test { - public: - FileSystemOperationTest() - : request_id_(kInvalidRequestId), - operation_(NULL) { - base_.CreateUniqueTempDir(); - EXPECT_TRUE(base_.IsValid()); - } - - FileSystemOperation* operation() { - request_id_ = ++last_request_id; - mock_dispatcher_ = new MockDispatcher(request_id_); - operation_.reset(new FileSystemOperation( - mock_dispatcher_, base::MessageLoopProxy::CreateForCurrentThread())); - return operation_.get(); - } - - protected: - // Common temp base for nondestructive uses. - ScopedTempDir base_; - - int request_id_; - scoped_ptr<FileSystemOperation> operation_; - - // Owned by |operation_|. - MockDispatcher* mock_dispatcher_; - - DISALLOW_COPY_AND_ASSIGN(FileSystemOperationTest); -}; +FileSystemOperation* FileSystemOperationTest::operation() { + return new FileSystemOperation( + new MockDispatcher(this), + base::MessageLoopProxy::CreateForCurrentThread()); +} TEST_F(FileSystemOperationTest, TestMoveFailureSrcDoesntExist) { FilePath src(base_.path().Append(FILE_PATH_LITERAL("a"))); FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); operation()->Move(src, dest); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) { @@ -121,9 +114,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) { &dest_dir_path)); operation()->Move(src_dir.path(), dest_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status()); } TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) { @@ -138,9 +129,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) { operation()->Move(src_dir.path(), dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) { @@ -155,8 +144,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) { operation()->Move(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); } TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) { @@ -171,9 +159,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) { operation()->Move(src_file, dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); } TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) { @@ -186,8 +172,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) { operation()->Move(src_dir.path(), nonexisting_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndOverwrite) { @@ -203,9 +188,8 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndOverwrite) { operation()->Move(src_file, dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) { @@ -220,9 +204,8 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) { operation()->Move(src_file, dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) { @@ -234,8 +217,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) { operation()->Move(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); // Make sure we've overwritten but not moved the source under the |dest_dir|. @@ -254,8 +236,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndNew) { operation()->Move(src_dir.path(), dest_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); EXPECT_TRUE(file_util::DirectoryExists(dest_dir_path)); } @@ -271,8 +252,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) { operation()->Move(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName()))); } @@ -281,8 +261,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) { FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); operation()->Copy(src, dest); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) { @@ -294,9 +273,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) { &dest_dir_path)); operation()->Copy(src_dir.path(), dest_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status()); } TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) { @@ -311,9 +288,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) { operation()->Copy(src_dir.path(), dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) { @@ -328,8 +303,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) { operation()->Copy(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); } TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) { @@ -344,9 +318,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) { operation()->Copy(src_file, dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); } TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) { @@ -362,8 +334,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) { operation()->Copy(src_dir, nonexisting_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndOverwrite) { @@ -379,9 +350,8 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndOverwrite) { operation()->Copy(src_file, dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) { @@ -396,9 +366,8 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) { operation()->Copy(src_file, dest_file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) { @@ -410,8 +379,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) { operation()->Copy(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); // Make sure we've overwritten but not copied the source under the |dest_dir|. EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path())); @@ -429,8 +397,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndNew) { operation()->Copy(src_dir.path(), dest_dir); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir)); } @@ -445,8 +412,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirRecursive) { operation()->Copy(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName()))); } @@ -459,8 +425,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileFailure) { file_util::CreateTemporaryFileInDir(dir.path(), &file); operation()->CreateFile(file, true); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileExists) { @@ -472,9 +437,8 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileExists) { operation()->CreateFile(file, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(file)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestCreateFileSuccessExclusive) { @@ -484,9 +448,8 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessExclusive) { FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist")); operation()->CreateFile(file, true); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(file)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileDoesntExist) { @@ -496,8 +459,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileDoesntExist) { FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist")); operation()->CreateFile(file, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); } TEST_F(FileSystemOperationTest, @@ -509,8 +471,7 @@ TEST_F(FileSystemOperationTest, FILE_PATH_LITERAL("FileDoesntExist")); operation()->CreateDirectory(nonexisting_file, false, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) { @@ -519,8 +480,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) { ASSERT_TRUE(src_dir.CreateUniqueTempDir()); operation()->CreateDirectory(src_dir.path(), true, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } TEST_F(FileSystemOperationTest, TestCreateDirFailureFileExists) { @@ -531,8 +491,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureFileExists) { file_util::CreateTemporaryFileInDir(dir.path(), &file); operation()->CreateDirectory(file, true, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } TEST_F(FileSystemOperationTest, TestCreateDirSuccess) { @@ -541,17 +500,15 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) { ASSERT_TRUE(dir.CreateUniqueTempDir()); operation()->CreateDirectory(dir.path(), false, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); // Dir doesn't exist. FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); operation()->CreateDirectory(nonexisting_dir_path, false, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { @@ -561,9 +518,8 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { operation()->CreateDirectory(nonexisting_dir_path, true, false); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path)); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } TEST_F(FileSystemOperationTest, TestExistsAndMetadataFailure) { @@ -571,18 +527,16 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataFailure) { FILE_PATH_LITERAL("nonexistingdir"))); operation()->GetMetadata(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); operation()->FileExists(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); file_util::EnsureEndsWithSeparator(&nonexisting_dir_path); operation()->DirectoryExists(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { @@ -591,27 +545,23 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { operation()->DirectoryExists(dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); operation()->GetMetadata(dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_TRUE(mock_dispatcher_->info().is_directory); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); + EXPECT_TRUE(info().is_directory); FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); operation()->FileExists(file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); operation()->GetMetadata(file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_FALSE(mock_dispatcher_->info().is_directory); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); + EXPECT_FALSE(info().is_directory); } TEST_F(FileSystemOperationTest, TestReadDirFailure) { @@ -621,8 +571,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) { file_util::EnsureEndsWithSeparator(&nonexisting_dir_path); operation()->ReadDirectory(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); // File exists. ScopedTempDir dir; @@ -632,8 +581,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) { operation()->ReadDirectory(file); MessageLoop::current()->RunAllPending(); // TODO(kkanetkar) crbug.com/54309 to change the error code. - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } TEST_F(FileSystemOperationTest, TestReadDirSuccess) { @@ -651,17 +599,16 @@ TEST_F(FileSystemOperationTest, TestReadDirSuccess) { operation()->ReadDirectory(parent_dir.path()); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationStatusNotSet, mock_dispatcher_->status()); - EXPECT_EQ(2u, mock_dispatcher_->entries().size()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationStatusNotSet, status()); + EXPECT_EQ(2u, entries().size()); - for (size_t i = 0; i < mock_dispatcher_->entries().size(); ++i) { - if (mock_dispatcher_->entries()[i].is_directory) { + for (size_t i = 0; i < entries().size(); ++i) { + if (entries()[i].is_directory) { EXPECT_EQ(child_dir.BaseName().value(), - mock_dispatcher_->entries()[i].name); + entries()[i].name); } else { EXPECT_EQ(child_file.BaseName().value(), - mock_dispatcher_->entries()[i].name); + entries()[i].name); } } } @@ -674,8 +621,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { operation()->Remove(nonexisting, false /* recursive */); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); // It's an error to try to remove a non-empty directory if recursive flag // is false. @@ -694,8 +640,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { operation()->Remove(parent_dir.path(), false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, - mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + status()); } TEST_F(FileSystemOperationTest, TestRemoveSuccess) { @@ -705,9 +650,8 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { operation()->Remove(empty_dir.path(), false /* recursive */); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(empty_dir.path())); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); // Removing a non-empty directory with recursive flag == true should be ok. // parent_dir @@ -724,9 +668,8 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { operation()->Remove(parent_dir.path(), true /* recursive */); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); + EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(parent_dir.path())); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); } // TODO(ericu): Add tests for Write, Cancel. @@ -745,17 +688,15 @@ TEST_F(FileSystemOperationTest, TestTruncate) { // Check that its length is the size of the data written. operation()->GetMetadata(file); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_FALSE(mock_dispatcher_->info().is_directory); - EXPECT_EQ(data_size, mock_dispatcher_->info().size); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); + EXPECT_FALSE(info().is_directory); + EXPECT_EQ(data_size, info().size); // Extend the file by truncating it. int length = 17; operation()->Truncate(file, length); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); // Check that its length is now 17 and that it's all zeroes after the test // data. @@ -776,8 +717,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) { length = 3; operation()->Truncate(file, length); MessageLoop::current()->RunAllPending(); - EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status()); - EXPECT_EQ(request_id_, mock_dispatcher_->request_id()); + EXPECT_EQ(kFileOperationSucceeded, status()); // Check that its length is now 3 and that it contains only bits of test data. EXPECT_TRUE(file_util::GetFileInfo(file, &info)); diff --git a/webkit/fileapi/sandboxed_file_system_operation.cc b/webkit/fileapi/sandboxed_file_system_operation.cc index dfc2884..7f9dd38 100644 --- a/webkit/fileapi/sandboxed_file_system_operation.cc +++ b/webkit/fileapi/sandboxed_file_system_operation.cc @@ -38,15 +38,19 @@ void SandboxedFileSystemOperation::OpenFileSystem( void SandboxedFileSystemOperation::CreateFile( const FilePath& path, bool exclusive) { - if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) + if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) { + delete this; return; + } FileSystemOperation::CreateFile(path, exclusive); } void SandboxedFileSystemOperation::CreateDirectory( const FilePath& path, bool exclusive, bool recursive) { - if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) + if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) { + delete this; return; + } FileSystemOperation::CreateDirectory(path, exclusive, recursive); } @@ -54,8 +58,10 @@ void SandboxedFileSystemOperation::Copy( const FilePath& src_path, const FilePath& dest_path) { if (!VerifyFileSystemPathForRead(src_path) || !VerifyFileSystemPathForWrite(dest_path, true /* create */, - FileSystemQuotaManager::kUnknownSize)) + FileSystemQuotaManager::kUnknownSize)) { + delete this; return; + } FileSystemOperation::Copy(src_path, dest_path); } @@ -63,39 +69,51 @@ void SandboxedFileSystemOperation::Move( const FilePath& src_path, const FilePath& dest_path) { if (!VerifyFileSystemPathForRead(src_path) || !VerifyFileSystemPathForWrite(dest_path, true /* create */, - FileSystemQuotaManager::kUnknownSize)) + FileSystemQuotaManager::kUnknownSize)) { + delete this; return; + } FileSystemOperation::Move(src_path, dest_path); } void SandboxedFileSystemOperation::DirectoryExists(const FilePath& path) { - if (!VerifyFileSystemPathForRead(path)) + if (!VerifyFileSystemPathForRead(path)) { + delete this; return; + } FileSystemOperation::DirectoryExists(path); } void SandboxedFileSystemOperation::FileExists(const FilePath& path) { - if (!VerifyFileSystemPathForRead(path)) + if (!VerifyFileSystemPathForRead(path)) { + delete this; return; + } FileSystemOperation::FileExists(path); } void SandboxedFileSystemOperation::GetMetadata(const FilePath& path) { - if (!VerifyFileSystemPathForRead(path)) + if (!VerifyFileSystemPathForRead(path)) { + delete this; return; + } FileSystemOperation::GetMetadata(path); } void SandboxedFileSystemOperation::ReadDirectory(const FilePath& path) { - if (!VerifyFileSystemPathForRead(path)) + if (!VerifyFileSystemPathForRead(path)) { + delete this; return; + } FileSystemOperation::ReadDirectory(path); } void SandboxedFileSystemOperation::Remove( const FilePath& path, bool recursive) { - if (!VerifyFileSystemPathForWrite(path, false /* create */, 0)) + if (!VerifyFileSystemPathForWrite(path, false /* create */, 0)) { + delete this; return; + } FileSystemOperation::Remove(path, recursive); } @@ -103,15 +121,19 @@ void SandboxedFileSystemOperation::Write( scoped_refptr<URLRequestContext> url_request_context, const FilePath& path, const GURL& blob_url, int64 offset) { if (!VerifyFileSystemPathForWrite(path, true /* create */, - FileSystemQuotaManager::kUnknownSize)) + FileSystemQuotaManager::kUnknownSize)) { + delete this; return; + } FileSystemOperation::Write(url_request_context, path, blob_url, offset); } void SandboxedFileSystemOperation::Truncate( const FilePath& path, int64 length) { - if (!VerifyFileSystemPathForWrite(path, false /* create */, 0)) + if (!VerifyFileSystemPathForWrite(path, false /* create */, 0)) { + delete this; return; + } FileSystemOperation::Truncate(path, length); } @@ -119,8 +141,10 @@ void SandboxedFileSystemOperation::TouchFile( const FilePath& path, const base::Time& last_access_time, const base::Time& last_modified_time) { - if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) + if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) { + delete this; return; + } FileSystemOperation::TouchFile(path, last_access_time, last_modified_time); } @@ -128,6 +152,7 @@ void SandboxedFileSystemOperation::DidGetRootPath( bool success, const FilePath& path, const std::string& name) { DCHECK(success || path.empty()); dispatcher()->DidOpenFileSystem(name, path); + delete this; } bool SandboxedFileSystemOperation::VerifyFileSystemPathForRead( diff --git a/webkit/fileapi/sandboxed_file_system_operation.h b/webkit/fileapi/sandboxed_file_system_operation.h index 18d47f7..8794a9d 100644 --- a/webkit/fileapi/sandboxed_file_system_operation.h +++ b/webkit/fileapi/sandboxed_file_system_operation.h @@ -71,6 +71,8 @@ class SandboxedFileSystemOperation : public FileSystemOperation { // Returns true if the given |path| is a valid FileSystem path. // Otherwise it calls dispatcher's DidFail method with // PLATFORM_FILE_ERROR_SECURITY and returns false. + // (Note: this doesn't delete this when it calls DidFail and returns false; + // it's the caller's responsibility.) bool VerifyFileSystemPathForRead(const FilePath& path); // Checks the validity of a given |path| for writing. @@ -85,6 +87,8 @@ class SandboxedFileSystemOperation : public FileSystemOperation { // If |create| flag is true this also checks if the |path| contains // any restricted names and chars. If it does, the call fires dispatcher's // DidFail with PLATFORM_FILE_ERROR_SECURITY and returns false. + // (Note: this doesn't delete this when it calls DidFail and returns false; + // it's the caller's responsibility.) bool VerifyFileSystemPathForWrite(const FilePath& path, bool create, int64 growth); diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc index 29ea45e..94269b9 100644 --- a/webkit/tools/test_shell/simple_file_system.cc +++ b/webkit/tools/test_shell/simple_file_system.cc @@ -54,17 +54,11 @@ class SimpleFileSystemCallbackDispatcher } ~SimpleFileSystemCallbackDispatcher() { - DCHECK(!operation_.get()); - } - - void set_operation(SandboxedFileSystemOperation* operation) { - operation_.reset(operation); } virtual void DidSucceed() { - if (file_system_) - callbacks_->didSucceed(); - RemoveOperation(); + DCHECK(file_system_); + callbacks_->didSucceed(); } virtual void DidReadMetadata(const base::PlatformFileInfo& info) { @@ -75,7 +69,6 @@ class SimpleFileSystemCallbackDispatcher web_file_info.type = info.is_directory ? WebFileInfo::TypeDirectory : WebFileInfo::TypeFile; callbacks_->didReadMetadata(web_file_info); - RemoveOperation(); } virtual void DidReadDirectory( @@ -93,7 +86,6 @@ class SimpleFileSystemCallbackDispatcher WebVector<WebKit::WebFileSystemEntry> web_entries = web_entries_vector; callbacks_->didReadDirectory(web_entries, has_more); - RemoveOperation(); } virtual void DidOpenFileSystem( @@ -104,14 +96,12 @@ class SimpleFileSystemCallbackDispatcher else callbacks_->didOpenFileSystem( UTF8ToUTF16(name), webkit_glue::FilePathToWebString(path)); - RemoveOperation(); } virtual void DidFail(base::PlatformFileError error_code) { DCHECK(file_system_); callbacks_->didFail( webkit_glue::PlatformFileErrorToWebFileError(error_code)); - RemoveOperation(); } virtual void DidWrite(int64, bool) { @@ -119,17 +109,8 @@ class SimpleFileSystemCallbackDispatcher } private: - void RemoveOperation() { - // We need to make sure operation_ is null when we delete the operation - // (which in turn deletes this dispatcher instance). - scoped_ptr<SandboxedFileSystemOperation> operation; - operation.swap(operation_); - operation.reset(); - } - WeakPtr<SimpleFileSystem> file_system_; WebFileSystemCallbacks* callbacks_; - scoped_ptr<SandboxedFileSystemOperation> operation_; }; } // namespace @@ -262,6 +243,5 @@ SandboxedFileSystemOperation* SimpleFileSystem::GetNewOperation( SandboxedFileSystemOperation* operation = new SandboxedFileSystemOperation( dispatcher, base::MessageLoopProxy::CreateForCurrentThread(), sandboxed_context_.get()); - dispatcher->set_operation(operation); return operation; } diff --git a/webkit/tools/test_shell/simple_file_writer.cc b/webkit/tools/test_shell/simple_file_writer.cc index bb4758f..ad6109a 100644 --- a/webkit/tools/test_shell/simple_file_writer.cc +++ b/webkit/tools/test_shell/simple_file_writer.cc @@ -28,7 +28,8 @@ class SimpleFileWriter::IOThreadProxy : public base::RefCountedThreadSafe<SimpleFileWriter::IOThreadProxy> { public: explicit IOThreadProxy(const base::WeakPtr<SimpleFileWriter>& simple_writer) - : simple_writer_(simple_writer) { + : simple_writer_(simple_writer), + operation_(NULL) { // The IO thread needs to be running for this class to work. SimpleResourceLoaderBridge::EnsureIOThread(); io_thread_ = SimpleResourceLoaderBridge::GetIoThread(); @@ -44,8 +45,8 @@ class SimpleFileWriter::IOThreadProxy this, &IOThreadProxy::Truncate, path, offset)); return; } - DCHECK(!operation_.get()); - operation_.reset(GetNewOperation()); + DCHECK(!operation_); + operation_ = GetNewOperation(); operation_->Truncate(path, offset); } @@ -56,8 +57,8 @@ class SimpleFileWriter::IOThreadProxy return; } DCHECK(request_context_); - DCHECK(!operation_.get()); - operation_.reset(GetNewOperation()); + DCHECK(!operation_); + operation_ = GetNewOperation(); operation_->Write(request_context_, path, blob_url, offset); } @@ -67,12 +68,11 @@ class SimpleFileWriter::IOThreadProxy this, &IOThreadProxy::Cancel)); return; } - if (!operation_.get()) { + if (!operation_) { DidFail(base::PLATFORM_FILE_ERROR_INVALID_OPERATION); return; } - cancel_operation_.reset(GetNewOperation()); - operation_->Cancel(cancel_operation_.get()); + operation_->Cancel(GetNewOperation()); } private: @@ -82,6 +82,10 @@ class SimpleFileWriter::IOThreadProxy explicit CallbackDispatcher(IOThreadProxy* proxy) : proxy_(proxy) { } + ~CallbackDispatcher() { + proxy_->ClearOperation(); + } + virtual void DidSucceed() { proxy_->DidSucceed(); } @@ -119,7 +123,6 @@ class SimpleFileWriter::IOThreadProxy void DidSucceed() { if (!main_thread_->BelongsToCurrentThread()) { - operation_.reset(); main_thread_->PostTask(FROM_HERE, NewRunnableMethod( this, &IOThreadProxy::DidSucceed)); return; @@ -130,7 +133,6 @@ class SimpleFileWriter::IOThreadProxy void DidFail(base::PlatformFileError error_code) { if (!main_thread_->BelongsToCurrentThread()) { - operation_.reset(); main_thread_->PostTask(FROM_HERE, NewRunnableMethod( this, &IOThreadProxy::DidFail, error_code)); return; @@ -141,8 +143,6 @@ class SimpleFileWriter::IOThreadProxy void DidWrite(int64 bytes, bool complete) { if (!main_thread_->BelongsToCurrentThread()) { - if (complete) - operation_.reset(); main_thread_->PostTask(FROM_HERE, NewRunnableMethod( this, &IOThreadProxy::DidWrite, bytes, complete)); return; @@ -151,6 +151,11 @@ class SimpleFileWriter::IOThreadProxy simple_writer_->DidWrite(bytes, complete); } + void ClearOperation() { + DCHECK(io_thread_->BelongsToCurrentThread()); + operation_ = NULL; + } + scoped_refptr<base::MessageLoopProxy> io_thread_; scoped_refptr<base::MessageLoopProxy> main_thread_; @@ -158,8 +163,7 @@ class SimpleFileWriter::IOThreadProxy base::WeakPtr<SimpleFileWriter> simple_writer_; // Only used on the io thread. - scoped_ptr<FileSystemOperation> operation_; - scoped_ptr<FileSystemOperation> cancel_operation_; + FileSystemOperation* operation_; }; |