diff options
7 files changed, 98 insertions, 70 deletions
diff --git a/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.cc b/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.cc index b36285e..f80e275 100644 --- a/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.cc +++ b/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.cc @@ -326,26 +326,6 @@ void FileBrowserHandlerInternalSelectFileFunction::OnFilePathSelected( external_backend->GetVirtualPath(full_path, &file_definition.virtual_path); DCHECK(!file_definition.virtual_path.empty()); - file_manager::util::ConvertFileDefinitionToEntryDefinition( - GetProfile(), - extension_id(), - file_definition, - base::Bind( - &FileBrowserHandlerInternalSelectFileFunction::GrantPermissions, - this, - full_path, - file_definition)); -} - -void FileBrowserHandlerInternalSelectFileFunction::GrantPermissions( - const base::FilePath& full_path, - const FileDefinition& file_definition, - const EntryDefinition& entry_definition) { - fileapi::ExternalFileSystemBackend* external_backend = - file_manager::util::GetFileSystemContextForRenderViewHost( - GetProfile(), render_view_host())->external_backend(); - DCHECK(external_backend); - // Grant access to this particular file to target extension. This will // ensure that the target extension can access only this FS entry and // prevent from traversing FS hierarchy upward. @@ -356,6 +336,17 @@ void FileBrowserHandlerInternalSelectFileFunction::GrantPermissions( content::ChildProcessSecurityPolicy::GetInstance()->GrantCreateReadWriteFile( render_view_host()->GetProcess()->GetID(), full_path); + file_manager::util::ConvertFileDefinitionToEntryDefinition( + GetProfile(), + extension_id(), + file_definition, + base::Bind( + &FileBrowserHandlerInternalSelectFileFunction::RespondEntryDefinition, + this)); +} + +void FileBrowserHandlerInternalSelectFileFunction::RespondEntryDefinition( + const EntryDefinition& entry_definition) { Respond(entry_definition, true); } diff --git a/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.h b/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.h index 5b78194..472782f 100644 --- a/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.h +++ b/chrome/browser/chromeos/extensions/file_manager/file_browser_handler_api.h @@ -113,11 +113,8 @@ class FileBrowserHandlerInternalSelectFileFunction virtual bool RunImpl() OVERRIDE; private: - // Grants file access permissions for the created file to the caller. - // Inside this method, - void GrantPermissions( - const base::FilePath& full_path, - const file_manager::util::FileDefinition& file_definition, + // Respond to the API with selected entry definition. + void RespondEntryDefinition( const file_manager::util::EntryDefinition& entry_definition); // Creates dictionary value that will be used to as the extension function's diff --git a/chrome/browser/chromeos/file_manager/fileapi_util.cc b/chrome/browser/chromeos/file_manager/fileapi_util.cc index 4f7860a..31340de 100644 --- a/chrome/browser/chromeos/file_manager/fileapi_util.cc +++ b/chrome/browser/chromeos/file_manager/fileapi_util.cc @@ -71,9 +71,10 @@ class FileDefinitionListConverter { // Then, calls OnIteratorConverted with the created entry definition. void OnResolvedURL(scoped_ptr<FileDefinitionListConverter> self_deleter, FileDefinitionList::const_iterator iterator, - const GURL& root_url, - const std::string& name, - base::File::Error error); + base::File::Error error, + const fileapi::FileSystemInfo& info, + const base::FilePath& file_path, + fileapi::FileSystemContext::ResolvedEntryType type); // Called when the iterator is converted. Adds the |entry_definition| to // |results_| and calls ConvertNextIterator() for the next element. @@ -132,16 +133,6 @@ void FileDefinitionListConverter::ConvertNextIterator( return; } - fileapi::ExternalFileSystemBackend* backend = - file_system_context_->external_backend(); - if (!backend) { - OnIteratorConverted(self_deleter.Pass(), - iterator, - CreateEntryDefinitionWithError( - base::File::FILE_ERROR_INVALID_OPERATION)); - return; - } - fileapi::FileSystemURL url = file_system_context_->CreateCrackedFileSystemURL( extensions::Extension::GetBaseURLFromExtensionId(extension_id_), fileapi::kFileSystemTypeExternal, @@ -150,13 +141,8 @@ void FileDefinitionListConverter::ConvertNextIterator( // The converter object will be deleted if the callback is not called because // of shutdown during ResolveURL(). - // - // TODO(mtomasz, nhiroki): Call FileSystemContext::ResolveURL() directly, - // after removing redundant thread restrictions in there. - backend->ResolveURL( + file_system_context_->ResolveURL( url, - // Not sandboxed file systems are not creatable via ResolveURL(). - fileapi::OPEN_FILE_SYSTEM_FAIL_IF_NONEXISTENT, base::Bind(&FileDefinitionListConverter::OnResolvedURL, base::Unretained(this), base::Passed(&self_deleter), @@ -166,21 +152,32 @@ void FileDefinitionListConverter::ConvertNextIterator( void FileDefinitionListConverter::OnResolvedURL( scoped_ptr<FileDefinitionListConverter> self_deleter, FileDefinitionList::const_iterator iterator, - const GURL& root_url, - const std::string& name, - base::File::Error error) { + base::File::Error error, + const fileapi::FileSystemInfo& info, + const base::FilePath& file_path, + fileapi::FileSystemContext::ResolvedEntryType type) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (error != base::File::FILE_OK) { + OnIteratorConverted(self_deleter.Pass(), + iterator, + CreateEntryDefinitionWithError(error)); + return; + } + EntryDefinition entry_definition; - entry_definition.file_system_root_url = root_url.spec(); - entry_definition.file_system_name = name; + entry_definition.file_system_root_url = info.root_url.spec(); + entry_definition.file_system_name = info.name; + DCHECK(type == fileapi::FileSystemContext::RESOLVED_ENTRY_NOT_FOUND || + iterator->is_directory == + (type == fileapi::FileSystemContext::RESOLVED_ENTRY_DIRECTORY)); entry_definition.is_directory = iterator->is_directory; - entry_definition.error = error; + entry_definition.error = base::File::FILE_OK; // Construct a target Entry.fullPath value from the virtual path and the // root URL. Eg. Downloads/A/b.txt -> A/b.txt. const base::FilePath root_virtual_path = - file_system_context_->CrackURL(root_url).virtual_path(); + file_system_context_->CrackURL(info.root_url).virtual_path(); DCHECK(root_virtual_path == iterator->virtual_path || root_virtual_path.IsParent(iterator->virtual_path)); base::FilePath full_path; diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc index 9e2372e..b900124 100644 --- a/content/browser/fileapi/fileapi_message_filter.cc +++ b/content/browser/fileapi/fileapi_message_filter.cc @@ -771,16 +771,22 @@ void FileAPIMessageFilter::DidOpenFileSystem(int request_id, // For OpenFileSystem we do not create a new operation, so no unregister here. } -void FileAPIMessageFilter::DidResolveURL(int request_id, - base::File::Error result, - const fileapi::FileSystemInfo& info, - const base::FilePath& file_path, - bool is_directory) { +void FileAPIMessageFilter::DidResolveURL( + int request_id, + base::File::Error result, + const fileapi::FileSystemInfo& info, + const base::FilePath& file_path, + fileapi::FileSystemContext::ResolvedEntryType type) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + if (result == base::File::FILE_OK && + type == fileapi::FileSystemContext::RESOLVED_ENTRY_NOT_FOUND) + result = base::File::FILE_ERROR_NOT_FOUND; + if (result == base::File::FILE_OK) { DCHECK(info.root_url.is_valid()); Send(new FileSystemMsg_DidResolveURL( - request_id, info, file_path, is_directory)); + request_id, info, file_path, + type == fileapi::FileSystemContext::RESOLVED_ENTRY_DIRECTORY)); } else { Send(new FileSystemMsg_DidFail(request_id, result)); } diff --git a/content/browser/fileapi/fileapi_message_filter.h b/content/browser/fileapi/fileapi_message_filter.h index 98a7230..158eb6c 100644 --- a/content/browser/fileapi/fileapi_message_filter.h +++ b/content/browser/fileapi/fileapi_message_filter.h @@ -19,6 +19,7 @@ #include "content/browser/streams/stream_context.h" #include "content/common/content_export.h" #include "content/public/browser/browser_message_filter.h" +#include "webkit/browser/fileapi/file_system_context.h" #include "webkit/browser/fileapi/file_system_operation_runner.h" #include "webkit/common/blob/blob_data.h" #include "webkit/common/fileapi/file_system_types.h" @@ -33,7 +34,6 @@ class Time; namespace fileapi { class FileSystemURL; -class FileSystemContext; class FileSystemOperationRunner; struct DirectoryEntry; struct FileSystemInfo; @@ -187,7 +187,7 @@ class CONTENT_EXPORT FileAPIMessageFilter : public BrowserMessageFilter { base::File::Error result, const fileapi::FileSystemInfo& info, const base::FilePath& file_path, - bool is_directory); + fileapi::FileSystemContext::ResolvedEntryType type); void DidDeleteFileSystem(int request_id, base::File::Error result); void DidCreateSnapshot( diff --git a/webkit/browser/fileapi/file_system_context.cc b/webkit/browser/fileapi/file_system_context.cc index f6e7df3..847a68b 100644 --- a/webkit/browser/fileapi/file_system_context.cc +++ b/webkit/browser/fileapi/file_system_context.cc @@ -51,10 +51,29 @@ void DidGetMetadataForResolveURL( base::File::Error error, const base::File::Info& file_info) { if (error != base::File::FILE_OK) { - callback.Run(error, FileSystemInfo(), base::FilePath(), false); + if (error == base::File::FILE_ERROR_NOT_FOUND) { + callback.Run(base::File::FILE_OK, info, path, + FileSystemContext::RESOLVED_ENTRY_NOT_FOUND); + } else { + callback.Run(error, FileSystemInfo(), base::FilePath(), + FileSystemContext::RESOLVED_ENTRY_NOT_FOUND); + } return; } - callback.Run(error, info, path, file_info.is_directory); + callback.Run(error, info, path, file_info.is_directory ? + FileSystemContext::RESOLVED_ENTRY_DIRECTORY : + FileSystemContext::RESOLVED_ENTRY_FILE); +} + +void RelayResolveURLCallback( + scoped_refptr<base::MessageLoopProxy> message_loop, + const FileSystemContext::ResolveURLCallback& callback, + base::File::Error result, + const FileSystemInfo& info, + const base::FilePath& file_path, + FileSystemContext::ResolvedEntryType type) { + message_loop->PostTask( + FROM_HERE, base::Bind(callback, result, info, file_path, type)); } } // namespace @@ -322,15 +341,24 @@ void FileSystemContext::OpenFileSystem( void FileSystemContext::ResolveURL( const FileSystemURL& url, const ResolveURLCallback& callback) { - // TODO(nhiroki, kinuko): Remove this thread restriction, so it can be called - // on either UI or IO thread. - DCHECK(io_task_runner_->RunsTasksOnCurrentThread()); DCHECK(!callback.is_null()); + // If not on IO thread, forward before passing the task to the backend. + if (!io_task_runner_->RunsTasksOnCurrentThread()) { + ResolveURLCallback relay_callback = + base::Bind(&RelayResolveURLCallback, + base::MessageLoopProxy::current(), callback); + io_task_runner_->PostTask( + FROM_HERE, + base::Bind(&FileSystemContext::ResolveURL, this, url, relay_callback)); + return; + } + FileSystemBackend* backend = GetFileSystemBackend(url.type()); if (!backend) { callback.Run(base::File::FILE_ERROR_SECURITY, - FileSystemInfo(), base::FilePath(), false); + FileSystemInfo(), base::FilePath(), + FileSystemContext::RESOLVED_ENTRY_NOT_FOUND); return; } @@ -560,7 +588,8 @@ void FileSystemContext::DidOpenFileSystemForResolveURL( DCHECK(io_task_runner_->RunsTasksOnCurrentThread()); if (error != base::File::FILE_OK) { - callback.Run(error, FileSystemInfo(), base::FilePath(), false); + callback.Run(error, FileSystemInfo(), base::FilePath(), + FileSystemContext::RESOLVED_ENTRY_NOT_FOUND); return; } diff --git a/webkit/browser/fileapi/file_system_context.h b/webkit/browser/fileapi/file_system_context.h index ba91af7..6e53981 100644 --- a/webkit/browser/fileapi/file_system_context.h +++ b/webkit/browser/fileapi/file_system_context.h @@ -188,10 +188,15 @@ class WEBKIT_STORAGE_BROWSER_EXPORT FileSystemContext OpenFileSystemCallback; // Used for ResolveURL. + enum ResolvedEntryType { + RESOLVED_ENTRY_FILE, + RESOLVED_ENTRY_DIRECTORY, + RESOLVED_ENTRY_NOT_FOUND, + }; typedef base::Callback<void(base::File::Error result, const FileSystemInfo& info, const base::FilePath& file_path, - bool is_directory)> ResolveURLCallback; + ResolvedEntryType type)> ResolveURLCallback; // Used for DeleteFileSystem and OpenPluginPrivateFileSystem. typedef base::Callback<void(base::File::Error result)> StatusCallback; @@ -207,9 +212,12 @@ class WEBKIT_STORAGE_BROWSER_EXPORT FileSystemContext OpenFileSystemMode mode, const OpenFileSystemCallback& callback); - // Opens the filesystem for the given |url| as read-only, and then checks the - // existence of the file entry referred by the URL. This should be called on - // the IO thread. + // Opens the filesystem for the given |url| as read-only, if the filesystem + // backend referred by the URL allows opening by resolveURL. Otherwise it + // fails with FILE_ERROR_SECURITY. The entry pointed by the URL can be + // absent; in that case RESOLVED_ENTRY_NOT_FOUND type is returned to the + // callback for indicating the absence. Can be called from any thread with + // a message loop. |callback| is invoked on the caller thread. void ResolveURL( const FileSystemURL& url, const ResolveURLCallback& callback); |