From 132dc015c1928f080ec20a132d80d751a14e25c7 Mon Sep 17 00:00:00 2001 From: mtomasz Date: Thu, 19 Nov 2015 07:44:09 -0800 Subject: Add an option to specify requested fields for fetching metadata. Fetching metadata is heavy, and it may cause calling stat() even though it's not needed (is_directory is obtainable without stat()). This CL adds the option. Backends will be updated to reflect it separately. Also, as another step base::File::Info will be replaced with storage::Entry or something similar with scoped_ptr's as members, to avoid reading a member which was not requested in |fields|. TEST=All current tests pass. But not really used yet, as backends ignore it. BUG=558191 Review URL: https://codereview.chromium.org/1455403003 Cr-Commit-Position: refs/heads/master@{#360585} --- storage/browser/fileapi/async_file_util.h | 9 +++++---- storage/browser/fileapi/async_file_util_adapter.cc | 1 + storage/browser/fileapi/async_file_util_adapter.h | 1 + storage/browser/fileapi/copy_or_move_operation_delegate.cc | 5 +++-- storage/browser/fileapi/file_system_context.cc | 6 +++++- storage/browser/fileapi/file_system_dir_url_request_job.cc | 3 ++- storage/browser/fileapi/file_system_operation.h | 9 +++++++++ storage/browser/fileapi/file_system_operation_impl.cc | 12 +++++++----- storage/browser/fileapi/file_system_operation_impl.h | 1 + storage/browser/fileapi/file_system_operation_runner.cc | 8 ++++---- storage/browser/fileapi/file_system_operation_runner.h | 2 ++ storage/browser/fileapi/file_system_url_request_job.cc | 3 ++- 12 files changed, 42 insertions(+), 18 deletions(-) (limited to 'storage/browser') diff --git a/storage/browser/fileapi/async_file_util.h b/storage/browser/fileapi/async_file_util.h index eab6af7..799396b 100644 --- a/storage/browser/fileapi/async_file_util.h +++ b/storage/browser/fileapi/async_file_util.h @@ -80,6 +80,7 @@ class AsyncFileUtil { typedef base::Callback CopyFileProgressCallback; typedef FileSystemOperation::CopyOrMoveOption CopyOrMoveOption; + typedef FileSystemOperation::GetMetadataField GetMetadataField; // Creates an AsyncFileUtil instance which performs file operations on // local native file system. The created instance assumes @@ -149,10 +150,10 @@ class AsyncFileUtil { // - File::FILE_ERROR_NOT_FOUND if the file doesn't exist. // - Other error code if there was an error while retrieving the file info. // - virtual void GetFileInfo( - scoped_ptr context, - const FileSystemURL& url, - const GetFileInfoCallback& callback) = 0; + virtual void GetFileInfo(scoped_ptr context, + const FileSystemURL& url, + int fields, + const GetFileInfoCallback& callback) = 0; // Reads contents of a directory at |path|. // diff --git a/storage/browser/fileapi/async_file_util_adapter.cc b/storage/browser/fileapi/async_file_util_adapter.cc index 0de0f61..6a7664f 100644 --- a/storage/browser/fileapi/async_file_util_adapter.cc +++ b/storage/browser/fileapi/async_file_util_adapter.cc @@ -195,6 +195,7 @@ void AsyncFileUtilAdapter::CreateDirectory( void AsyncFileUtilAdapter::GetFileInfo( scoped_ptr context, const FileSystemURL& url, + int /* fields */, const GetFileInfoCallback& callback) { FileSystemOperationContext* context_ptr = context.release(); GetFileInfoHelper* helper = new GetFileInfoHelper; diff --git a/storage/browser/fileapi/async_file_util_adapter.h b/storage/browser/fileapi/async_file_util_adapter.h index 9c661a6..7a126b0 100644 --- a/storage/browser/fileapi/async_file_util_adapter.h +++ b/storage/browser/fileapi/async_file_util_adapter.h @@ -52,6 +52,7 @@ class STORAGE_EXPORT AsyncFileUtilAdapter const StatusCallback& callback) override; void GetFileInfo(scoped_ptr context, const FileSystemURL& url, + int /* fields */, const GetFileInfoCallback& callback) override; void ReadDirectory(scoped_ptr context, const FileSystemURL& url, diff --git a/storage/browser/fileapi/copy_or_move_operation_delegate.cc b/storage/browser/fileapi/copy_or_move_operation_delegate.cc index cf4692b..715999b 100644 --- a/storage/browser/fileapi/copy_or_move_operation_delegate.cc +++ b/storage/browser/fileapi/copy_or_move_operation_delegate.cc @@ -391,7 +391,8 @@ class StreamCopyOrMoveImpl // a directory. To check errors before destination file creation, // check metadata first. operation_runner_->GetMetadata( - src_url_, + src_url_, FileSystemOperation::GET_METADATA_FIELD_IS_DIRECTORY | + FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED, base::Bind(&StreamCopyOrMoveImpl::RunAfterGetMetadataForSource, weak_factory_.GetWeakPtr(), callback)); } @@ -884,7 +885,7 @@ void CopyOrMoveOperationDelegate::PostProcessDirectory( } operation_runner()->GetMetadata( - src_url, + src_url, FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED, base::Bind( &CopyOrMoveOperationDelegate::PostProcessDirectoryAfterGetMetadata, weak_factory_.GetWeakPtr(), src_url, callback)); diff --git a/storage/browser/fileapi/file_system_context.cc b/storage/browser/fileapi/file_system_context.cc index 7b280c8..8c099e7d 100644 --- a/storage/browser/fileapi/file_system_context.cc +++ b/storage/browser/fileapi/file_system_context.cc @@ -626,8 +626,12 @@ void FileSystemContext::DidOpenFileSystemForResolveURL( DCHECK(result); } + // TODO(mtomasz): Not all fields should be required for ResolveURL. operation_runner()->GetMetadata( - url, base::Bind(&DidGetMetadataForResolveURL, path, callback, info)); + url, FileSystemOperation::GET_METADATA_FIELD_IS_DIRECTORY | + FileSystemOperation::GET_METADATA_FIELD_SIZE | + FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED, + base::Bind(&DidGetMetadataForResolveURL, path, callback, info)); } } // namespace storage diff --git a/storage/browser/fileapi/file_system_dir_url_request_job.cc b/storage/browser/fileapi/file_system_dir_url_request_job.cc index 598b87a..c053773 100644 --- a/storage/browser/fileapi/file_system_dir_url_request_job.cc +++ b/storage/browser/fileapi/file_system_dir_url_request_job.cc @@ -157,7 +157,8 @@ void FileSystemDirURLRequestJob::GetMetadata(size_t index) { url_.path().Append(base::FilePath(entry.name))); DCHECK(url.is_valid()); file_system_context_->operation_runner()->GetMetadata( - url, + url, FileSystemOperation::GET_METADATA_FIELD_SIZE | + FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED, base::Bind(&FileSystemDirURLRequestJob::DidGetMetadata, this, index)); } diff --git a/storage/browser/fileapi/file_system_operation.h b/storage/browser/fileapi/file_system_operation.h index b9e6f95..8580b35 100644 --- a/storage/browser/fileapi/file_system_operation.h +++ b/storage/browser/fileapi/file_system_operation.h @@ -227,6 +227,14 @@ class FileSystemOperation { OPTION_PRESERVE_LAST_MODIFIED, }; + // Fields requested for the GetMetadata method. Used as a bitmask. + enum GetMetadataField { + GET_METADATA_FIELD_NONE = 0, + GET_METADATA_FIELD_SIZE = 1 << 0, + GET_METADATA_FIELD_IS_DIRECTORY = 1 << 1, + GET_METADATA_FIELD_LAST_MODIFIED = 1 << 2 + }; + // Used for Write(). typedef base::CallbackGetFileInfo( - operation_context_.Pass(), url, + operation_context_.Pass(), url, GET_METADATA_FIELD_IS_DIRECTORY, base::Bind(&FileSystemOperationImpl::DidDirectoryExists, weak_factory_.GetWeakPtr(), callback)); } @@ -140,15 +140,17 @@ void FileSystemOperationImpl::FileExists(const FileSystemURL& url, const StatusCallback& callback) { DCHECK(SetPendingOperationType(kOperationFileExists)); async_file_util_->GetFileInfo( - operation_context_.Pass(), url, + operation_context_.Pass(), url, GET_METADATA_FIELD_IS_DIRECTORY, base::Bind(&FileSystemOperationImpl::DidFileExists, weak_factory_.GetWeakPtr(), callback)); } -void FileSystemOperationImpl::GetMetadata( - const FileSystemURL& url, const GetMetadataCallback& callback) { +void FileSystemOperationImpl::GetMetadata(const FileSystemURL& url, + int fields, + const GetMetadataCallback& callback) { DCHECK(SetPendingOperationType(kOperationGetMetadata)); - async_file_util_->GetFileInfo(operation_context_.Pass(), url, callback); + async_file_util_->GetFileInfo(operation_context_.Pass(), url, fields, + callback); } void FileSystemOperationImpl::ReadDirectory( diff --git a/storage/browser/fileapi/file_system_operation_impl.h b/storage/browser/fileapi/file_system_operation_impl.h index 6f4b662..26a434e 100644 --- a/storage/browser/fileapi/file_system_operation_impl.h +++ b/storage/browser/fileapi/file_system_operation_impl.h @@ -53,6 +53,7 @@ class STORAGE_EXPORT FileSystemOperationImpl void FileExists(const FileSystemURL& url, const StatusCallback& callback) override; void GetMetadata(const FileSystemURL& url, + int fields, const GetMetadataCallback& callback) override; void ReadDirectory(const FileSystemURL& url, const ReadDirectoryCallback& callback) override; diff --git a/storage/browser/fileapi/file_system_operation_runner.cc b/storage/browser/fileapi/file_system_operation_runner.cc index 1648055..4b5d422 100644 --- a/storage/browser/fileapi/file_system_operation_runner.cc +++ b/storage/browser/fileapi/file_system_operation_runner.cc @@ -175,6 +175,7 @@ OperationID FileSystemOperationRunner::FileExists( OperationID FileSystemOperationRunner::GetMetadata( const FileSystemURL& url, + int fields, const GetMetadataCallback& callback) { base::File::Error error = base::File::FILE_OK; FileSystemOperation* operation = @@ -186,10 +187,9 @@ OperationID FileSystemOperationRunner::GetMetadata( return handle.id; } PrepareForRead(handle.id, url); - operation->GetMetadata( - url, - base::Bind(&FileSystemOperationRunner::DidGetMetadata, AsWeakPtr(), - handle, callback)); + operation->GetMetadata(url, fields, + base::Bind(&FileSystemOperationRunner::DidGetMetadata, + AsWeakPtr(), handle, callback)); return handle.id; } diff --git a/storage/browser/fileapi/file_system_operation_runner.h b/storage/browser/fileapi/file_system_operation_runner.h index 0493e4c..1421e1a 100644 --- a/storage/browser/fileapi/file_system_operation_runner.h +++ b/storage/browser/fileapi/file_system_operation_runner.h @@ -48,6 +48,7 @@ class STORAGE_EXPORT FileSystemOperationRunner typedef FileSystemOperation::CopyFileProgressCallback CopyFileProgressCallback; typedef FileSystemOperation::CopyOrMoveOption CopyOrMoveOption; + typedef FileSystemOperation::GetMetadataField GetMetadataField; typedef int OperationID; @@ -98,6 +99,7 @@ class STORAGE_EXPORT FileSystemOperationRunner // Gets the metadata of a file or directory at |url|. OperationID GetMetadata(const FileSystemURL& url, + int fields, const GetMetadataCallback& callback); // Reads contents of a directory at |url|. diff --git a/storage/browser/fileapi/file_system_url_request_job.cc b/storage/browser/fileapi/file_system_url_request_job.cc index 6c33fc8..8edcb3d 100644 --- a/storage/browser/fileapi/file_system_url_request_job.cc +++ b/storage/browser/fileapi/file_system_url_request_job.cc @@ -165,7 +165,8 @@ void FileSystemURLRequestJob::StartAsync() { return; } file_system_context_->operation_runner()->GetMetadata( - url_, + url_, FileSystemOperation::GET_METADATA_FIELD_IS_DIRECTORY | + FileSystemOperation::GET_METADATA_FIELD_SIZE, base::Bind(&FileSystemURLRequestJob::DidGetMetadata, weak_factory_.GetWeakPtr())); } -- cgit v1.1