diff options
23 files changed, 292 insertions, 243 deletions
diff --git a/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc b/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc index aa43ff7..c315d40 100644 --- a/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc +++ b/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc @@ -77,13 +77,13 @@ bool FileSystemProviderMountFunction::RunSync() { // It's an error if the file system Id is empty. if (params->options.file_system_id.empty()) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); + SetError(FileErrorToString(base::File::FILE_ERROR_INVALID_OPERATION)); return false; } // It's an error if the display name is empty. if (params->options.display_name.empty()) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); + SetError(FileErrorToString(base::File::FILE_ERROR_INVALID_OPERATION)); return false; } @@ -96,9 +96,10 @@ bool FileSystemProviderMountFunction::RunSync() { options.writable = params->options.writable; options.supports_notify_tag = params->options.supports_notify_tag; - // TODO(mtomasz): Pass more detailed errors, rather than just a bool. - if (!service->MountFileSystem(extension_id(), options)) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); + const base::File::Error result = + service->MountFileSystem(extension_id(), options); + if (result != base::File::FILE_OK) { + SetError(FileErrorToString(result)); return false; } @@ -113,11 +114,11 @@ bool FileSystemProviderUnmountFunction::RunSync() { Service* const service = Service::Get(GetProfile()); DCHECK(service); - if (!service->UnmountFileSystem(extension_id(), - params->options.file_system_id, - Service::UNMOUNT_REASON_USER)) { - // TODO(mtomasz): Pass more detailed errors, rather than just a bool. - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); + const base::File::Error result = + service->UnmountFileSystem(extension_id(), params->options.file_system_id, + Service::UNMOUNT_REASON_USER); + if (result != base::File::FILE_OK) { + SetError(FileErrorToString(result)); return false; } @@ -171,7 +172,7 @@ bool FileSystemProviderGetAllFunction::RunSync() { return true; } -bool FileSystemProviderNotifyFunction::RunSync() { +bool FileSystemProviderNotifyFunction::RunAsync() { using api::file_system_provider::Notify::Params; scoped_ptr<Params> params(Params::Create(*args_)); EXTENSION_FUNCTION_VALIDATE(params); @@ -183,25 +184,33 @@ bool FileSystemProviderNotifyFunction::RunSync() { service->GetProvidedFileSystem(extension_id(), params->options.file_system_id); if (!file_system) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); + SetError(FileErrorToString(base::File::FILE_ERROR_NOT_FOUND)); return false; } - if (!file_system->Notify( - base::FilePath::FromUTF8Unsafe(params->options.observed_path), - params->options.recursive, - ParseChangeType(params->options.change_type), - params->options.changes.get() - ? ParseChanges(*params->options.changes.get()) - : make_scoped_ptr(new ProvidedFileSystemObserver::Changes), - params->options.tag.get() ? *params->options.tag.get() : "")) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); - return false; - } + file_system->Notify( + base::FilePath::FromUTF8Unsafe(params->options.observed_path), + params->options.recursive, ParseChangeType(params->options.change_type), + params->options.changes.get() + ? ParseChanges(*params->options.changes.get()) + : make_scoped_ptr(new ProvidedFileSystemObserver::Changes), + params->options.tag.get() ? *params->options.tag.get() : "", + base::Bind(&FileSystemProviderNotifyFunction::OnNotifyCompleted, this)); return true; } +void FileSystemProviderNotifyFunction::OnNotifyCompleted( + base::File::Error result) { + if (result != base::File::FILE_OK) { + SetError(FileErrorToString(result)); + SendResponse(false); + return; + } + + SendResponse(true); +} + bool FileSystemProviderInternalUnmountRequestedSuccessFunction::RunWhenValid() { using api::file_system_provider_internal::UnmountRequestedSuccess::Params; scoped_ptr<Params> params(Params::Create(*args_)); diff --git a/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.h b/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.h index 5f5c8ad..23c783a 100644 --- a/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.h +++ b/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.h @@ -40,14 +40,18 @@ class FileSystemProviderGetAllFunction : public ChromeSyncExtensionFunction { virtual bool RunSync() override; }; -class FileSystemProviderNotifyFunction : public ChromeSyncExtensionFunction { +class FileSystemProviderNotifyFunction : public ChromeAsyncExtensionFunction { public: DECLARE_EXTENSION_FUNCTION("fileSystemProvider.notify", FILESYSTEMPROVIDER_NOTIFY) protected: virtual ~FileSystemProviderNotifyFunction() {} - virtual bool RunSync() override; + virtual bool RunAsync() override; + + private: + // Called when notifying is completed. + void OnNotifyCompleted(base::File::Error result); }; class FileSystemProviderInternalUnmountRequestedSuccessFunction diff --git a/chrome/browser/chromeos/extensions/file_system_provider/provider_function.cc b/chrome/browser/chromeos/extensions/file_system_provider/provider_function.cc index 3b24d086..3ad1db8 100644 --- a/chrome/browser/chromeos/extensions/file_system_provider/provider_function.cc +++ b/chrome/browser/chromeos/extensions/file_system_provider/provider_function.cc @@ -126,9 +126,10 @@ FileSystemProviderInternalFunction::FileSystemProviderInternalFunction() bool FileSystemProviderInternalFunction::RejectRequest( scoped_ptr<chromeos::file_system_provider::RequestValue> value, base::File::Error error) { - if (!request_manager_->RejectRequest(request_id_, value.Pass(), error)) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); - SendResponse(false); + const base::File::Error result = + request_manager_->RejectRequest(request_id_, value.Pass(), error); + if (result != base::File::FILE_OK) { + SetError(FileErrorToString(result)); return false; } @@ -138,9 +139,10 @@ bool FileSystemProviderInternalFunction::RejectRequest( bool FileSystemProviderInternalFunction::FulfillRequest( scoped_ptr<RequestValue> value, bool has_more) { - if (!request_manager_->FulfillRequest(request_id_, value.Pass(), has_more)) { - SetError(FileErrorToString(base::File::FILE_ERROR_SECURITY)); - SendResponse(false); + const base::File::Error result = + request_manager_->FulfillRequest(request_id_, value.Pass(), has_more); + if (result != base::File::FILE_OK) { + SetError(FileErrorToString(result)); return false; } @@ -150,10 +152,9 @@ bool FileSystemProviderInternalFunction::FulfillRequest( bool FileSystemProviderInternalFunction::RunSync() { DCHECK(args_); if (!Parse()) - return true; + return false; - SendResponse(RunWhenValid()); - return true; + return RunWhenValid(); } bool FileSystemProviderInternalFunction::Parse() { @@ -162,13 +163,11 @@ bool FileSystemProviderInternalFunction::Parse() { if (!args_->GetString(0, &file_system_id) || !args_->GetInteger(1, &request_id_)) { bad_message_ = true; - SendResponse(false); return false; } Service* service = Service::Get(GetProfile()); if (!service) { - SendResponse(false); return false; } @@ -176,7 +175,6 @@ bool FileSystemProviderInternalFunction::Parse() { service->GetProvidedFileSystem(extension_id(), file_system_id); if (!file_system) { SetError(FileErrorToString(base::File::FILE_ERROR_NOT_FOUND)); - SendResponse(false); return false; } diff --git a/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc b/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc index 3f415e26..cc676f8 100644 --- a/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc +++ b/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc @@ -367,14 +367,15 @@ void FakeProvidedFileSystem::RemoveObserver( observers_.RemoveObserver(observer); } -bool FakeProvidedFileSystem::Notify( +void FakeProvidedFileSystem::Notify( const base::FilePath& entry_path, bool recursive, storage::WatcherManager::ChangeType change_type, scoped_ptr<ProvidedFileSystemObserver::Changes> changes, - const std::string& tag) { + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback) { NOTREACHED(); - return false; + callback.Run(base::File::FILE_ERROR_SECURITY); } ProvidedFileSystemInterface* FakeProvidedFileSystem::Create( diff --git a/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h b/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h index 37f4bde..63ebc5d 100644 --- a/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h +++ b/chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h @@ -146,11 +146,13 @@ class FakeProvidedFileSystem : public ProvidedFileSystemInterface { virtual Watchers* GetWatchers() override; virtual void AddObserver(ProvidedFileSystemObserver* observer) override; virtual void RemoveObserver(ProvidedFileSystemObserver* observer) override; - virtual bool Notify(const base::FilePath& entry_path, - bool recursive, - storage::WatcherManager::ChangeType change_type, - scoped_ptr<ProvidedFileSystemObserver::Changes> changes, - const std::string& tag) override; + virtual void Notify( + const base::FilePath& entry_path, + bool recursive, + storage::WatcherManager::ChangeType change_type, + scoped_ptr<ProvidedFileSystemObserver::Changes> changes, + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback) override; virtual base::WeakPtr<ProvidedFileSystemInterface> GetWeakPtr() override; // Factory callback, to be used in Service::SetFileSystemFactory(). The diff --git a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc index d0bfc6d..dcf30d5 100644 --- a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc @@ -96,9 +96,9 @@ class FileSystemProviderFileStreamReader : public testing::Test { service->SetFileSystemFactoryForTesting( base::Bind(&FakeProvidedFileSystem::Create)); - const bool result = service->MountFileSystem( + const base::File::Error result = service->MountFileSystem( kExtensionId, MountOptions(kFileSystemId, "Testing File System")); - ASSERT_TRUE(result); + ASSERT_EQ(base::File::FILE_OK, result); FakeProvidedFileSystem* provided_file_system = static_cast<FakeProvidedFileSystem*>( service->GetProvidedFileSystem(kExtensionId, kFileSystemId)); diff --git a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer_unittest.cc b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer_unittest.cc index 8da0ba5..8d3282c 100644 --- a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer_unittest.cc @@ -80,9 +80,9 @@ class FileSystemProviderFileStreamWriter : public testing::Test { service->SetFileSystemFactoryForTesting( base::Bind(&FakeProvidedFileSystem::Create)); - const bool result = service->MountFileSystem( + const base::File::Error result = service->MountFileSystem( kExtensionId, MountOptions(kFileSystemId, "Testing File System")); - ASSERT_TRUE(result); + ASSERT_EQ(base::File::FILE_OK, result); provided_file_system_ = static_cast<FakeProvidedFileSystem*>( service->GetProvidedFileSystem(kExtensionId, kFileSystemId)); ASSERT_TRUE(provided_file_system_); diff --git a/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc b/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc index 06024b2..6e38f5f 100644 --- a/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc @@ -135,9 +135,9 @@ class FileSystemProviderProviderAsyncFileUtilTest : public testing::Test { service->SetFileSystemFactoryForTesting( base::Bind(&FakeProvidedFileSystem::Create)); - const bool result = service->MountFileSystem( + const base::File::Error result = service->MountFileSystem( kExtensionId, MountOptions(kFileSystemId, "Testing File System")); - ASSERT_TRUE(result); + ASSERT_EQ(base::File::FILE_OK, result); const ProvidedFileSystemInfo& file_system_info = service->GetProvidedFileSystem(kExtensionId, kFileSystemId) ->GetFileSystemInfo(); diff --git a/chrome/browser/chromeos/file_system_provider/mount_path_util_unittest.cc b/chrome/browser/chromeos/file_system_provider/mount_path_util_unittest.cc index cc2bf5d..e14f47e 100644 --- a/chrome/browser/chromeos/file_system_provider/mount_path_util_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/mount_path_util_unittest.cc @@ -127,9 +127,10 @@ TEST_F(FileSystemProviderMountPathUtilTest, IsFileSystemProviderLocalPath) { } TEST_F(FileSystemProviderMountPathUtilTest, Parser) { - const bool result = file_system_provider_service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName)); - ASSERT_TRUE(result); + const base::File::Error result = + file_system_provider_service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName)); + ASSERT_EQ(base::File::FILE_OK, result); const ProvidedFileSystemInfo file_system_info = file_system_provider_service_->GetProvidedFileSystem(kExtensionId, kFileSystemId) @@ -151,9 +152,10 @@ TEST_F(FileSystemProviderMountPathUtilTest, Parser) { } TEST_F(FileSystemProviderMountPathUtilTest, Parser_RootPath) { - const bool result = file_system_provider_service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName)); - ASSERT_TRUE(result); + const base::File::Error result = + file_system_provider_service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName)); + ASSERT_EQ(base::File::FILE_OK, result); const ProvidedFileSystemInfo file_system_info = file_system_provider_service_->GetProvidedFileSystem(kExtensionId, kFileSystemId) @@ -191,9 +193,10 @@ TEST_F(FileSystemProviderMountPathUtilTest, Parser_WrongUrl) { } TEST_F(FileSystemProviderMountPathUtilTest, Parser_IsolatedURL) { - const bool result = file_system_provider_service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName)); - ASSERT_TRUE(result); + const base::File::Error result = + file_system_provider_service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName)); + ASSERT_EQ(base::File::FILE_OK, result); const ProvidedFileSystemInfo file_system_info = file_system_provider_service_->GetProvidedFileSystem(kExtensionId, kFileSystemId) @@ -237,9 +240,10 @@ TEST_F(FileSystemProviderMountPathUtilTest, Parser_IsolatedURL) { } TEST_F(FileSystemProviderMountPathUtilTest, LocalPathParser) { - const bool result = file_system_provider_service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName)); - ASSERT_TRUE(result); + const base::File::Error result = + file_system_provider_service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName)); + ASSERT_EQ(base::File::FILE_OK, result); const ProvidedFileSystemInfo file_system_info = file_system_provider_service_->GetProvidedFileSystem(kExtensionId, kFileSystemId) @@ -261,9 +265,10 @@ TEST_F(FileSystemProviderMountPathUtilTest, LocalPathParser) { } TEST_F(FileSystemProviderMountPathUtilTest, LocalPathParser_RootPath) { - const bool result = file_system_provider_service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName)); - ASSERT_TRUE(result); + const base::File::Error result = + file_system_provider_service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName)); + ASSERT_EQ(base::File::FILE_OK, result); const ProvidedFileSystemInfo file_system_info = file_system_provider_service_->GetProvidedFileSystem(kExtensionId, kFileSystemId) diff --git a/chrome/browser/chromeos/file_system_provider/provided_file_system.cc b/chrome/browser/chromeos/file_system_provider/provided_file_system.cc index cdf2a46..c4e6cd3 100644 --- a/chrome/browser/chromeos/file_system_provider/provided_file_system.cc +++ b/chrome/browser/chromeos/file_system_provider/provided_file_system.cc @@ -501,34 +501,40 @@ void ProvidedFileSystem::RemoveObserver(ProvidedFileSystemObserver* observer) { observers_.RemoveObserver(observer); } -bool ProvidedFileSystem::Notify( +void ProvidedFileSystem::Notify( const base::FilePath& entry_path, bool recursive, storage::WatcherManager::ChangeType change_type, scoped_ptr<ProvidedFileSystemObserver::Changes> changes, - const std::string& tag) { + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback) { const WatcherKey key(entry_path, recursive); const auto& watcher_it = watchers_.find(key); - if (watcher_it == watchers_.end()) - return false; + if (watcher_it == watchers_.end()) { + callback.Run(base::File::FILE_ERROR_NOT_FOUND); + return; + } // The tag must be provided if and only if it's explicitly supported. - if (file_system_info_.supports_notify_tag() == tag.empty()) - return false; + if (file_system_info_.supports_notify_tag() == tag.empty()) { + callback.Run(base::File::FILE_ERROR_INVALID_OPERATION); + return; + } + + // It's illegal to provide a tag which is not unique. + if (!tag.empty() && tag == watcher_it->second.last_tag) { + callback.Run(base::File::FILE_ERROR_INVALID_OPERATION); + return; + } // The object is owned by AutoUpdated, so the reference is valid as long as // callbacks created with AutoUpdater::CreateCallback(). const ProvidedFileSystemObserver::Changes& changes_ref = *changes.get(); - scoped_refptr<AutoUpdater> auto_updater( - new AutoUpdater(base::Bind(&ProvidedFileSystem::OnNotifyCompleted, - weak_ptr_factory_.GetWeakPtr(), - entry_path, - recursive, - change_type, - base::Passed(&changes), - watcher_it->second.last_tag, - tag))); + scoped_refptr<AutoUpdater> auto_updater(new AutoUpdater( + base::Bind(&ProvidedFileSystem::OnNotifyCompleted, + weak_ptr_factory_.GetWeakPtr(), entry_path, recursive, + change_type, base::Passed(&changes), tag, callback))); // Call all notification callbacks (if any). for (const auto& subscriber_it : watcher_it->second.subscribers) { @@ -546,8 +552,6 @@ bool ProvidedFileSystem::Notify( change_type, changes_ref, auto_updater->CreateCallback())); - - return true; } base::WeakPtr<ProvidedFileSystemInterface> ProvidedFileSystem::GetWeakPtr() { @@ -607,23 +611,18 @@ void ProvidedFileSystem::OnNotifyCompleted( bool recursive, storage::WatcherManager::ChangeType change_type, scoped_ptr<ProvidedFileSystemObserver::Changes> /* changes */, - const std::string& last_tag, - const std::string& tag) { + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback) { const WatcherKey key(entry_path, recursive); const Watchers::iterator it = watchers_.find(key); // Check if the entry is still watched. - if (it == watchers_.end()) - return; - - // Another following notification finished earlier. - if (it->second.last_tag != last_tag) + if (it == watchers_.end()) { + callback.Run(base::File::FILE_ERROR_NOT_FOUND); return; + } - // It's illegal to provide a tag which is not unique. As for now only an error - // message is printed, but we may want to pass the error to the providing - // extension. TODO(mtomasz): Consider it. - if (!tag.empty() && tag == it->second.last_tag) - LOG(ERROR) << "Tag specified, but same as the previous one."; + // TODO(mtomasz): Add an async queue around notify and other watcher related + // methods so there is no race. it->second.last_tag = tag; @@ -643,6 +642,8 @@ void ProvidedFileSystem::OnNotifyCompleted( base::Bind(&EmptyStatusCallback)); } } + + callback.Run(base::File::FILE_OK); } } // namespace file_system_provider diff --git a/chrome/browser/chromeos/file_system_provider/provided_file_system.h b/chrome/browser/chromeos/file_system_provider/provided_file_system.h index 3f8b7a6..9c5dd3a 100644 --- a/chrome/browser/chromeos/file_system_provider/provided_file_system.h +++ b/chrome/browser/chromeos/file_system_provider/provided_file_system.h @@ -156,11 +156,13 @@ class ProvidedFileSystem : public ProvidedFileSystemInterface { virtual Watchers* GetWatchers() override; virtual void AddObserver(ProvidedFileSystemObserver* observer) override; virtual void RemoveObserver(ProvidedFileSystemObserver* observer) override; - virtual bool Notify(const base::FilePath& entry_path, - bool recursive, - storage::WatcherManager::ChangeType change_type, - scoped_ptr<ProvidedFileSystemObserver::Changes> changes, - const std::string& tag) override; + virtual void Notify( + const base::FilePath& entry_path, + bool recursive, + storage::WatcherManager::ChangeType change_type, + scoped_ptr<ProvidedFileSystemObserver::Changes> changes, + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback) override; virtual base::WeakPtr<ProvidedFileSystemInterface> GetWeakPtr() override; private: @@ -179,14 +181,14 @@ class ProvidedFileSystem : public ProvidedFileSystemInterface { base::File::Error result); // Called when all observers finished handling the change notification. It - // updates the tag from |last_tag| to |tag| for the entry at |entry_path|. + // updates the tag to |tag| for the entry at |entry_path|. void OnNotifyCompleted( const base::FilePath& entry_path, bool recursive, storage::WatcherManager::ChangeType change_type, scoped_ptr<ProvidedFileSystemObserver::Changes> changes, - const std::string& last_tag, - const std::string& tag); + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback); Profile* profile_; // Not owned. extensions::EventRouter* event_router_; // Not owned. May be NULL. diff --git a/chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h b/chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h index 4e8c4b8..9395a64 100644 --- a/chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h +++ b/chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h @@ -198,14 +198,17 @@ class ProvidedFileSystemInterface { const storage::AsyncFileUtil::StatusCallback& callback) = 0; // Notifies about changes related to the watcher within the file system. - // Invoked by the file system implementation. Returns false if the - // notification arguments are malformed or the entry is not watched anymore. + // Invoked by the file system implementation. Returns an error code via the + // callback if the notification arguments are malformed or the entry is not + // watched anymore. On success, returns base::File::FILE_OK. // TODO(mtomasz): Replace [entry_path, recursive] with a watcher id. - virtual bool Notify(const base::FilePath& entry_path, - bool recursive, - storage::WatcherManager::ChangeType change_type, - scoped_ptr<ProvidedFileSystemObserver::Changes> changes, - const std::string& tag) = 0; + virtual void Notify( + const base::FilePath& entry_path, + bool recursive, + storage::WatcherManager::ChangeType change_type, + scoped_ptr<ProvidedFileSystemObserver::Changes> changes, + const std::string& tag, + const storage::AsyncFileUtil::StatusCallback& callback) = 0; // Returns a provided file system info for this file system. virtual const ProvidedFileSystemInfo& GetFileSystemInfo() const = 0; diff --git a/chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc b/chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc index 905458d..649e1ac 100644 --- a/chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc @@ -757,12 +757,12 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) { const storage::WatcherManager::ChangeType change_type = storage::WatcherManager::CHANGED; const std::string tag = "hello-world"; - EXPECT_TRUE(provided_file_system_->Notify( - base::FilePath::FromUTF8Unsafe(kDirectoryPath), - false /* recursive */, - change_type, - make_scoped_ptr(new ProvidedFileSystemObserver::Changes), - tag)); + + Log log; + provided_file_system_->Notify( + base::FilePath::FromUTF8Unsafe(kDirectoryPath), false /* recursive */, + change_type, make_scoped_ptr(new ProvidedFileSystemObserver::Changes), + tag, base::Bind(&LogStatus, base::Unretained(&log))); // Confirm that the notification callback was called. ASSERT_EQ(1u, notification_log.size()); @@ -785,6 +785,9 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) { // Wait until all observers finish handling the notification. base::RunLoop().RunUntilIdle(); + ASSERT_EQ(1u, log.size()); + EXPECT_EQ(base::File::FILE_OK, log[0]); + // Confirm, that the watcher still exists, and that the tag is updated. ASSERT_EQ(1u, watchers->size()); EXPECT_EQ(tag, watchers->begin()->second.last_tag); @@ -798,13 +801,15 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) { storage::WatcherManager::DELETED; const ProvidedFileSystemObserver::Changes changes; const std::string tag = "chocolate-disco"; - EXPECT_TRUE(provided_file_system_->Notify( - base::FilePath::FromUTF8Unsafe(kDirectoryPath), - false /* recursive */, - change_type, - make_scoped_ptr(new ProvidedFileSystemObserver::Changes), - tag)); + + Log log; + provided_file_system_->Notify( + base::FilePath::FromUTF8Unsafe(kDirectoryPath), false /* recursive */, + change_type, make_scoped_ptr(new ProvidedFileSystemObserver::Changes), + tag, base::Bind(&LogStatus, base::Unretained(&log))); base::RunLoop().RunUntilIdle(); + ASSERT_EQ(1u, log.size()); + EXPECT_EQ(base::File::FILE_OK, log[0]); // Confirm that the notification callback was called. ASSERT_EQ(2u, notification_log.size()); diff --git a/chrome/browser/chromeos/file_system_provider/request_manager.cc b/chrome/browser/chromeos/file_system_provider/request_manager.cc index 214eab2..73bdbb3 100644 --- a/chrome/browser/chromeos/file_system_provider/request_manager.cc +++ b/chrome/browser/chromeos/file_system_provider/request_manager.cc @@ -119,13 +119,14 @@ int RequestManager::CreateRequest(RequestType type, return request_id; } -bool RequestManager::FulfillRequest(int request_id, - scoped_ptr<RequestValue> response, - bool has_more) { +base::File::Error RequestManager::FulfillRequest( + int request_id, + scoped_ptr<RequestValue> response, + bool has_more) { CHECK(response.get()); RequestMap::iterator request_it = requests_.find(request_id); if (request_it == requests_.end()) - return false; + return base::File::FILE_ERROR_NOT_FOUND; FOR_EACH_OBSERVER(Observer, observers_, @@ -141,16 +142,17 @@ bool RequestManager::FulfillRequest(int request_id, ResetTimer(request_id); } - return true; + return base::File::FILE_OK; } -bool RequestManager::RejectRequest(int request_id, - scoped_ptr<RequestValue> response, - base::File::Error error) { +base::File::Error RequestManager::RejectRequest( + int request_id, + scoped_ptr<RequestValue> response, + base::File::Error error) { CHECK(response.get()); RequestMap::iterator request_it = requests_.find(request_id); if (request_it == requests_.end()) - return false; + return base::File::FILE_ERROR_NOT_FOUND; FOR_EACH_OBSERVER(Observer, observers_, @@ -158,7 +160,7 @@ bool RequestManager::RejectRequest(int request_id, request_it->second->handler->OnError(request_id, response.Pass(), error); DestroyRequest(request_id); - return true; + return base::File::FILE_OK; } void RequestManager::SetTimeoutForTesting(const base::TimeDelta& timeout) { diff --git a/chrome/browser/chromeos/file_system_provider/request_manager.h b/chrome/browser/chromeos/file_system_provider/request_manager.h index 913408b..ccc87d8 100644 --- a/chrome/browser/chromeos/file_system_provider/request_manager.h +++ b/chrome/browser/chromeos/file_system_provider/request_manager.h @@ -113,17 +113,19 @@ class RequestManager { int CreateRequest(RequestType type, scoped_ptr<HandlerInterface> handler); // Handles successful response for the |request_id|. If |has_more| is false, - // then the request is disposed, after handling the |response|. On error, - // returns false, and the request is disposed. |response| must not be NULL. - bool FulfillRequest(int request_id, - scoped_ptr<RequestValue> response, - bool has_more); - - // Handles error response for the |request_id|. If handling the error fails, - // returns false. Always disposes the request. |response| must not be NULL. - bool RejectRequest(int request_id, - scoped_ptr<RequestValue> response, - base::File::Error error); + // then the request is disposed, after handling the |response|. On success, + // returns base::File::FILE_OK. Otherwise returns an error code. |response| + // must not be NULL. + base::File::Error FulfillRequest(int request_id, + scoped_ptr<RequestValue> response, + bool has_more); + + // Handles error response for the |request_id|. If handling the error + // succeeds, theen returns base::File::FILE_OK. Otherwise returns an error + // code. Always disposes the request. |response| must not be NULL. + base::File::Error RejectRequest(int request_id, + scoped_ptr<RequestValue> response, + base::File::Error error); // Sets a custom timeout for tests. The new timeout value will be applied to // new requests diff --git a/chrome/browser/chromeos/file_system_provider/request_manager_unittest.cc b/chrome/browser/chromeos/file_system_provider/request_manager_unittest.cc index b2363c3..4bb8339 100644 --- a/chrome/browser/chromeos/file_system_provider/request_manager_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/request_manager_unittest.cc @@ -377,9 +377,9 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndFulFill) { RequestValue::CreateForTesting("i-like-vanilla")); const bool has_more = false; - bool result = + const base::File::Error result = request_manager_->FulfillRequest(request_id, response.Pass(), has_more); - EXPECT_TRUE(result); + EXPECT_EQ(base::File::FILE_OK, result); ASSERT_EQ(1u, observer.fulfilled().size()); EXPECT_EQ(request_id, observer.fulfilled()[0].request_id()); @@ -402,19 +402,18 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndFulFill) { request_manager_->GetActiveRequestIds(); EXPECT_EQ(0u, active_request_ids.size()); - bool retry = request_manager_->FulfillRequest( + const base::File::Error retry = request_manager_->FulfillRequest( request_id, scoped_ptr<RequestValue>(new RequestValue), has_more); - EXPECT_FALSE(retry); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, retry); EXPECT_EQ(1u, observer.fulfilled().size()); } // Rejecting should also fail. { - bool retry = request_manager_->RejectRequest( - request_id, - scoped_ptr<RequestValue>(new RequestValue()), + const base::File::Error retry = request_manager_->RejectRequest( + request_id, scoped_ptr<RequestValue>(new RequestValue()), base::File::FILE_ERROR_FAILED); - EXPECT_FALSE(retry); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, retry); EXPECT_EQ(0u, observer.rejected().size()); } @@ -448,9 +447,9 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndFulFill_WithHasNext) { const bool has_more = true; - bool result = request_manager_->FulfillRequest( + const base::File::Error result = request_manager_->FulfillRequest( request_id, scoped_ptr<RequestValue>(new RequestValue), has_more); - EXPECT_TRUE(result); + EXPECT_EQ(base::File::FILE_OK, result); // Validate if the callback has correct arguments. ASSERT_EQ(1u, logger.success_events().size()); @@ -471,10 +470,10 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndFulFill_WithHasNext) { ASSERT_EQ(1u, active_request_ids.size()); EXPECT_EQ(request_id, active_request_ids[0]); - bool new_has_more = false; - bool retry = request_manager_->FulfillRequest( + const bool new_has_more = false; + const base::File::Error retry = request_manager_->FulfillRequest( request_id, scoped_ptr<RequestValue>(new RequestValue), new_has_more); - EXPECT_TRUE(retry); + EXPECT_EQ(base::File::FILE_OK, retry); ASSERT_EQ(2u, observer.fulfilled().size()); EXPECT_EQ(request_id, observer.fulfilled()[1].request_id()); @@ -488,10 +487,10 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndFulFill_WithHasNext) { request_manager_->GetActiveRequestIds(); EXPECT_EQ(0u, active_request_ids.size()); - bool new_has_more = false; - bool retry = request_manager_->FulfillRequest( + const bool new_has_more = false; + const base::File::Error retry = request_manager_->FulfillRequest( request_id, scoped_ptr<RequestValue>(new RequestValue), new_has_more); - EXPECT_FALSE(retry); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, retry); EXPECT_EQ(0u, observer.rejected().size()); } @@ -523,10 +522,10 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndReject) { ASSERT_EQ(1u, observer.executed().size()); EXPECT_EQ(request_id, observer.executed()[0].request_id()); - base::File::Error error = base::File::FILE_ERROR_NO_MEMORY; - bool result = request_manager_->RejectRequest( + const base::File::Error error = base::File::FILE_ERROR_NO_MEMORY; + const base::File::Error result = request_manager_->RejectRequest( request_id, scoped_ptr<RequestValue>(new RequestValue()), error); - EXPECT_TRUE(result); + EXPECT_EQ(base::File::FILE_OK, result); // Validate if the callback has correct arguments. ASSERT_EQ(1u, logger.error_events().size()); @@ -541,18 +540,18 @@ TEST_F(FileSystemProviderRequestManagerTest, CreateAndReject) { // Confirm, that the request is removed. Basically, fulfilling again for the // same request, should fail. { - bool has_more = false; - bool retry = request_manager_->FulfillRequest( + const bool has_more = false; + const base::File::Error retry = request_manager_->FulfillRequest( request_id, scoped_ptr<RequestValue>(new RequestValue), has_more); - EXPECT_FALSE(retry); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, retry); EXPECT_EQ(0u, observer.fulfilled().size()); } // Rejecting should also fail. { - bool retry = request_manager_->RejectRequest( + const base::File::Error retry = request_manager_->RejectRequest( request_id, scoped_ptr<RequestValue>(new RequestValue()), error); - EXPECT_FALSE(retry); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, retry); EXPECT_EQ(1u, observer.rejected().size()); } @@ -587,9 +586,9 @@ TEST_F(FileSystemProviderRequestManagerTest, const bool has_more = true; - const bool result = request_manager_->FulfillRequest( + const base::File::Error result = request_manager_->FulfillRequest( request_id + 1, scoped_ptr<RequestValue>(new RequestValue), has_more); - EXPECT_FALSE(result); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, result); // Callbacks should not be called. EXPECT_EQ(0u, logger.error_events().size()); @@ -600,9 +599,9 @@ TEST_F(FileSystemProviderRequestManagerTest, // Confirm, that the request hasn't been removed, by fulfilling it correctly. { - const bool retry = request_manager_->FulfillRequest( + const base::File::Error retry = request_manager_->FulfillRequest( request_id, scoped_ptr<RequestValue>(new RequestValue), has_more); - EXPECT_TRUE(retry); + EXPECT_EQ(base::File::FILE_OK, retry); EXPECT_EQ(1u, observer.fulfilled().size()); } @@ -631,10 +630,10 @@ TEST_F(FileSystemProviderRequestManagerTest, ASSERT_EQ(1u, observer.executed().size()); EXPECT_EQ(request_id, observer.executed()[0].request_id()); - base::File::Error error = base::File::FILE_ERROR_NO_MEMORY; - bool result = request_manager_->RejectRequest( + const base::File::Error error = base::File::FILE_ERROR_NO_MEMORY; + const base::File::Error result = request_manager_->RejectRequest( request_id + 1, scoped_ptr<RequestValue>(new RequestValue()), error); - EXPECT_FALSE(result); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, result); // Callbacks should not be called. EXPECT_EQ(0u, logger.error_events().size()); @@ -644,9 +643,9 @@ TEST_F(FileSystemProviderRequestManagerTest, // Confirm, that the request hasn't been removed, by rejecting it correctly. { - bool retry = request_manager_->RejectRequest( + const base::File::Error retry = request_manager_->RejectRequest( request_id, scoped_ptr<RequestValue>(new RequestValue()), error); - EXPECT_TRUE(retry); + EXPECT_EQ(base::File::FILE_OK, retry); EXPECT_EQ(1u, observer.rejected().size()); } diff --git a/chrome/browser/chromeos/file_system_provider/service.cc b/chrome/browser/chromeos/file_system_provider/service.cc index 2cd2467..ccba53b 100644 --- a/chrome/browser/chromeos/file_system_provider/service.cc +++ b/chrome/browser/chromeos/file_system_provider/service.cc @@ -60,9 +60,9 @@ Service::~Service() { const std::string extension_id = it->second->GetFileSystemInfo().extension_id(); ++it; - const bool unmount_result = UnmountFileSystem( + const base::File::Error unmount_result = UnmountFileSystem( extension_id, file_system_id, UNMOUNT_REASON_SHUTDOWN); - DCHECK(unmount_result); + DCHECK_EQ(base::File::FILE_OK, unmount_result); } DCHECK_EQ(0u, file_system_map_.size()); @@ -95,8 +95,8 @@ void Service::SetRegistryForTesting(scoped_ptr<RegistryInterface> registry) { registry_.reset(registry.release()); } -bool Service::MountFileSystem(const std::string& extension_id, - const MountOptions& options) { +base::File::Error Service::MountFileSystem(const std::string& extension_id, + const MountOptions& options) { DCHECK(thread_checker_.CalledOnValidThread()); // If already exists a file system provided by the same extension with this @@ -106,7 +106,7 @@ bool Service::MountFileSystem(const std::string& extension_id, observers_, OnProvidedFileSystemMount(ProvidedFileSystemInfo(), base::File::FILE_ERROR_EXISTS)); - return false; + return base::File::FILE_ERROR_EXISTS; } // Restrict number of file systems to prevent system abusing. @@ -116,7 +116,7 @@ bool Service::MountFileSystem(const std::string& extension_id, observers_, OnProvidedFileSystemMount(ProvidedFileSystemInfo(), base::File::FILE_ERROR_TOO_MANY_OPENED)); - return false; + return base::File::FILE_ERROR_TOO_MANY_OPENED; } storage::ExternalMountPoints* const mount_points = @@ -138,7 +138,7 @@ bool Service::MountFileSystem(const std::string& extension_id, observers_, OnProvidedFileSystemMount(ProvidedFileSystemInfo(), base::File::FILE_ERROR_INVALID_OPERATION)); - return false; + return base::File::FILE_ERROR_INVALID_OPERATION; } // Store the file system descriptor. Use the mount point name as the file @@ -165,12 +165,12 @@ bool Service::MountFileSystem(const std::string& extension_id, observers_, OnProvidedFileSystemMount(file_system_info, base::File::FILE_OK)); - return true; + return base::File::FILE_OK; } -bool Service::UnmountFileSystem(const std::string& extension_id, - const std::string& file_system_id, - UnmountReason reason) { +base::File::Error Service::UnmountFileSystem(const std::string& extension_id, + const std::string& file_system_id, + UnmountReason reason) { DCHECK(thread_checker_.CalledOnValidThread()); const ProvidedFileSystemMap::iterator file_system_it = @@ -182,7 +182,7 @@ bool Service::UnmountFileSystem(const std::string& extension_id, observers_, OnProvidedFileSystemUnmount(empty_file_system_info, base::File::FILE_ERROR_NOT_FOUND)); - return false; + return base::File::FILE_ERROR_NOT_FOUND; } storage::ExternalMountPoints* const mount_points = @@ -200,7 +200,7 @@ bool Service::UnmountFileSystem(const std::string& extension_id, observers_, OnProvidedFileSystemUnmount(file_system_info, base::File::FILE_ERROR_INVALID_OPERATION)); - return false; + return base::File::FILE_ERROR_INVALID_OPERATION; } FOR_EACH_OBSERVER( @@ -218,7 +218,7 @@ bool Service::UnmountFileSystem(const std::string& extension_id, delete file_system_it->second; file_system_map_.erase(file_system_it); - return true; + return base::File::FILE_OK; } bool Service::RequestUnmount(const std::string& extension_id, @@ -275,13 +275,12 @@ void Service::OnExtensionUnloaded( // by the UnmountFileSystem() call. ++it; if (file_system_info.extension_id() == extension->id()) { - const bool unmount_result = UnmountFileSystem( - file_system_info.extension_id(), - file_system_info.file_system_id(), + const base::File::Error unmount_result = UnmountFileSystem( + file_system_info.extension_id(), file_system_info.file_system_id(), reason == extensions::UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN ? UNMOUNT_REASON_SHUTDOWN : UNMOUNT_REASON_USER); - DCHECK(unmount_result); + DCHECK_EQ(base::File::FILE_OK, unmount_result); } } } @@ -292,9 +291,9 @@ void Service::OnExtensionLoaded(content::BrowserContext* browser_context, registry_->RestoreFileSystems(extension->id()); for (const auto& restored_file_system : *restored_file_systems) { - const bool result = MountFileSystem(restored_file_system.extension_id, - restored_file_system.options); - if (!result) { + const base::File::Error result = MountFileSystem( + restored_file_system.extension_id, restored_file_system.options); + if (result != base::File::FILE_OK) { LOG(ERROR) << "Failed to restore a provided file system from " << "registry: " << restored_file_system.extension_id << ", " << restored_file_system.options.file_system_id << ", " diff --git a/chrome/browser/chromeos/file_system_provider/service.h b/chrome/browser/chromeos/file_system_provider/service.h index e6eb3cf..5d6e979 100644 --- a/chrome/browser/chromeos/file_system_provider/service.h +++ b/chrome/browser/chromeos/file_system_provider/service.h @@ -82,15 +82,16 @@ class Service : public KeyedService, // Otherwise, only read-only operations are supported. If change notification // tags are supported, then |supports_notify_tag| must be true. Note, that // it is required in order to enable the internal cache. For success, returns - // true, otherwise false. - bool MountFileSystem(const std::string& extension_id, - const MountOptions& options); + // base::File::FILE_OK, otherwise an error code. + base::File::Error MountFileSystem(const std::string& extension_id, + const MountOptions& options); // Unmounts a file system with the specified |file_system_id| for the - // |extension_id|. For success returns true, otherwise false. - bool UnmountFileSystem(const std::string& extension_id, - const std::string& file_system_id, - UnmountReason reason); + // |extension_id|. For success returns base::File::FILE_OK, otherwise an error + // code. + base::File::Error UnmountFileSystem(const std::string& extension_id, + const std::string& file_system_id, + UnmountReason reason); // Requests unmounting of the file system. The callback is called when the // request is accepted or rejected, with an error code. Returns false if the diff --git a/chrome/browser/chromeos/file_system_provider/service_unittest.cc b/chrome/browser/chromeos/file_system_provider/service_unittest.cc index 372f61d..90bf652 100644 --- a/chrome/browser/chromeos/file_system_provider/service_unittest.cc +++ b/chrome/browser/chromeos/file_system_provider/service_unittest.cc @@ -216,8 +216,9 @@ TEST_F(FileSystemProviderServiceTest, MountFileSystem) { LoggingObserver observer; service_->AddObserver(&observer); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(1u, observer.mounts.size()); EXPECT_EQ(kExtensionId, observer.mounts[0].file_system_info().extension_id()); @@ -248,7 +249,8 @@ TEST_F(FileSystemProviderServiceTest, MountOptions options(kFileSystemId, kDisplayName); options.writable = true; options.supports_notify_tag = true; - EXPECT_TRUE(service_->MountFileSystem(kExtensionId, options)); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem(kExtensionId, options)); ASSERT_EQ(1u, observer.mounts.size()); EXPECT_TRUE(observer.mounts[0].file_system_info().writable()); @@ -265,10 +267,12 @@ TEST_F(FileSystemProviderServiceTest, MountFileSystem_UniqueIds) { LoggingObserver observer; service_->AddObserver(&observer); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); - EXPECT_FALSE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_ERROR_EXISTS, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(2u, observer.mounts.size()); EXPECT_EQ(base::File::FILE_OK, observer.mounts[0].error()); @@ -289,14 +293,16 @@ TEST_F(FileSystemProviderServiceTest, MountFileSystem_StressTest) { for (size_t i = 0; i < kMaxFileSystems; ++i) { const std::string file_system_id = std::string("test-") + base::IntToString(i); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(file_system_id, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(file_system_id, kDisplayName))); } ASSERT_EQ(kMaxFileSystems, observer.mounts.size()); // The next file system is out of limit, and registering it should fail. - EXPECT_FALSE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_ERROR_TOO_MANY_OPENED, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(kMaxFileSystems + 1, observer.mounts.size()); EXPECT_EQ(base::File::FILE_ERROR_TOO_MANY_OPENED, @@ -313,12 +319,14 @@ TEST_F(FileSystemProviderServiceTest, UnmountFileSystem) { LoggingObserver observer; service_->AddObserver(&observer); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(1u, observer.mounts.size()); - EXPECT_TRUE(service_->UnmountFileSystem( - kExtensionId, kFileSystemId, Service::UNMOUNT_REASON_USER)); + EXPECT_EQ(base::File::FILE_OK, + service_->UnmountFileSystem(kExtensionId, kFileSystemId, + Service::UNMOUNT_REASON_USER)); ASSERT_EQ(1u, observer.unmounts.size()); EXPECT_EQ(base::File::FILE_OK, observer.unmounts[0].error()); @@ -338,8 +346,9 @@ TEST_F(FileSystemProviderServiceTest, UnmountFileSystem_OnExtensionUnload) { LoggingObserver observer; service_->AddObserver(&observer); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(1u, observer.mounts.size()); // Directly call the observer's method. @@ -369,13 +378,15 @@ TEST_F(FileSystemProviderServiceTest, UnmountFileSystem_WrongExtensionId) { const std::string kWrongExtensionId = "helloworldhelloworldhelloworldhe"; - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(1u, observer.mounts.size()); ASSERT_EQ(1u, service_->GetProvidedFileSystemInfoList().size()); - EXPECT_FALSE(service_->UnmountFileSystem( - kWrongExtensionId, kFileSystemId, Service::UNMOUNT_REASON_USER)); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, + service_->UnmountFileSystem(kWrongExtensionId, kFileSystemId, + Service::UNMOUNT_REASON_USER)); ASSERT_EQ(1u, observer.unmounts.size()); EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, observer.unmounts[0].error()); ASSERT_EQ(1u, service_->GetProvidedFileSystemInfoList().size()); @@ -448,8 +459,9 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnMount) { EXPECT_FALSE(registry_->file_system_info()); EXPECT_FALSE(registry_->watchers()); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); ASSERT_EQ(1u, observer.mounts.size()); ASSERT_TRUE(registry_->file_system_info()); @@ -470,8 +482,9 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountOnShutdown) { { EXPECT_FALSE(registry_->file_system_info()); EXPECT_FALSE(registry_->watchers()); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); EXPECT_EQ(1u, observer.mounts.size()); EXPECT_TRUE(registry_->file_system_info()); @@ -479,8 +492,9 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountOnShutdown) { } { - EXPECT_TRUE(service_->UnmountFileSystem( - kExtensionId, kFileSystemId, Service::UNMOUNT_REASON_SHUTDOWN)); + EXPECT_EQ(base::File::FILE_OK, + service_->UnmountFileSystem(kExtensionId, kFileSystemId, + Service::UNMOUNT_REASON_SHUTDOWN)); EXPECT_EQ(1u, observer.unmounts.size()); EXPECT_TRUE(registry_->file_system_info()); @@ -497,8 +511,9 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountByUser) { { EXPECT_FALSE(registry_->file_system_info()); EXPECT_FALSE(registry_->watchers()); - EXPECT_TRUE(service_->MountFileSystem( - kExtensionId, MountOptions(kFileSystemId, kDisplayName))); + EXPECT_EQ(base::File::FILE_OK, + service_->MountFileSystem( + kExtensionId, MountOptions(kFileSystemId, kDisplayName))); EXPECT_EQ(1u, observer.mounts.size()); EXPECT_TRUE(registry_->file_system_info()); @@ -506,8 +521,9 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountByUser) { } { - EXPECT_TRUE(service_->UnmountFileSystem( - kExtensionId, kFileSystemId, Service::UNMOUNT_REASON_USER)); + EXPECT_EQ(base::File::FILE_OK, + service_->UnmountFileSystem(kExtensionId, kFileSystemId, + Service::UNMOUNT_REASON_USER)); EXPECT_EQ(1u, observer.unmounts.size()); EXPECT_FALSE(registry_->file_system_info()); diff --git a/chrome/test/data/extensions/api_test/file_system_provider/get_all/test.js b/chrome/test/data/extensions/api_test/file_system_provider/get_all/test.js index 0e48c51..d5a83aa 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/get_all/test.js +++ b/chrome/test/data/extensions/api_test/file_system_provider/get_all/test.js @@ -46,7 +46,7 @@ chrome.test.runTests([ function mountError() { chrome.fileSystemProvider.mount( {fileSystemId: '', displayName: ''}, - chrome.test.callbackFail('SECURITY', function() { + chrome.test.callbackFail('INVALID_OPERATION', function() { chrome.fileSystemProvider.getAll(chrome.test.callbackPass( function(fileSystems) { chrome.test.assertEq(0, fileSystems.length); diff --git a/chrome/test/data/extensions/api_test/file_system_provider/mount/test.js b/chrome/test/data/extensions/api_test/file_system_provider/mount/test.js index 25c6e04..81b6ae7 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/mount/test.js +++ b/chrome/test/data/extensions/api_test/file_system_provider/mount/test.js @@ -19,14 +19,14 @@ chrome.test.runTests([ function emptyDisplayName() { chrome.fileSystemProvider.mount( {fileSystemId: 'file-system-id', displayName: ''}, - chrome.test.callbackFail('SECURITY')); + chrome.test.callbackFail('INVALID_OPERATION')); }, // Verifies that mounting fails, when an empty string is provided as an Id function emptyFileSystemId() { chrome.fileSystemProvider.mount( {fileSystemId: '', displayName: 'File System Name'}, - chrome.test.callbackFail('SECURITY')); + chrome.test.callbackFail('INVALID_OPERATION')); }, // End to end test. Mounts a volume using fileSystemProvider.mount(), then @@ -97,7 +97,7 @@ chrome.test.runTests([ fileSystemId: 'over-the-limit-fs-id', displayName: 'Over The Limit File System' }, - chrome.test.callbackFail('SECURITY')); + chrome.test.callbackFail('TOO_MANY_OPENED')); } }; tryNextOne(); diff --git a/chrome/test/data/extensions/api_test/file_system_provider/notify/test.js b/chrome/test/data/extensions/api_test/file_system_provider/notify/test.js index f375401..0beaa1c 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/notify/test.js +++ b/chrome/test/data/extensions/api_test/file_system_provider/notify/test.js @@ -140,7 +140,7 @@ function runTests() { observedPath: fileEntry.fullPath, recursive: false, changeType: 'CHANGED', - }, chrome.test.callbackFail('SECURITY')); + }, chrome.test.callbackFail('INVALID_OPERATION')); }), function(error) { chrome.test.fail(error.name); @@ -165,7 +165,7 @@ function runTests() { recursive: true, changeType: 'CHANGED', tag: TESTING_ANOTHER_TAG, - }, chrome.test.callbackFail('SECURITY')); + }, chrome.test.callbackFail('NOT_FOUND')); })); }, @@ -223,7 +223,7 @@ function runTests() { recursive: false, changeType: 'CHANGED', tag: TESTING_ANOTHER_TAG - }, chrome.test.callbackFail('SECURITY')); + }, chrome.test.callbackFail('NOT_FOUND')); })); } ]); diff --git a/chrome/test/data/extensions/api_test/file_system_provider/unmount/test.js b/chrome/test/data/extensions/api_test/file_system_provider/unmount/test.js index 77e55a4..f6f9523 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/unmount/test.js +++ b/chrome/test/data/extensions/api_test/file_system_provider/unmount/test.js @@ -70,7 +70,7 @@ function runTests() { function unmountWrongId() { chrome.fileSystemProvider.unmount( {fileSystemId: 'wrong-fs-id'}, - chrome.test.callbackFail('SECURITY')); + chrome.test.callbackFail('NOT_FOUND')); }, // Tests if fileManagerPrivate.removeMount() for provided file systems emits |