From 58ab102e8e56e847a33e5cd9ee8bcd885d1a2cce Mon Sep 17 00:00:00 2001 From: "erikwright@chromium.org" Date: Fri, 7 Sep 2012 13:19:37 +0000 Subject: Revert 155377 - Update callers of CreateFileSystemOperation so more detailed error codes can be returned. Where applicable, convert net errors to base platform errors. Compile error: ../../webkit/fileapi/local_file_system_test_helper.cc:180:76:error: too few arguments to function call, expected 2, have 1 file_system_context_->CreateFileSystemOperation(CreateURL(FilePath()))); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ../../webkit/fileapi/file_system_context.h:132:3: note: 'CreateFileSystemOperation' declared here FileSystemOperation* CreateFileSystemOperation( ^ http://build.chromium.org/p/chromium.memory/builders/Mac%20ASAN%20Builder/builds/4001/steps/compile/logs/stdio#error1 BUG=141617 TBR=tony@chromium.org Review URL: https://chromiumcodereview.appspot.com/10920087 TBR=calvinlo@chromium.org Review URL: https://chromiumcodereview.appspot.com/10928052 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@155379 0039d316-1c4b-4281-b951-d872f2087c98 --- .../api/downloads/downloads_api_unittest.cc | 3 +- content/browser/fileapi/fileapi_message_filter.cc | 88 +++++----------------- net/base/net_errors.cc | 6 +- webkit/fileapi/file_system_context.cc | 15 +--- webkit/fileapi/file_system_context.h | 6 +- webkit/fileapi/file_system_dir_url_request_job.cc | 23 ++---- webkit/fileapi/file_system_dir_url_request_job.h | 2 +- webkit/fileapi/file_system_file_stream_reader.cc | 7 +- webkit/fileapi/file_system_url_request_job.cc | 7 +- webkit/fileapi/isolated_file_util_unittest.cc | 6 +- .../media/native_media_file_util_unittest.cc | 2 +- webkit/fileapi/sandbox_file_stream_writer.cc | 6 +- webkit/tools/test_shell/simple_file_system.cc | 2 +- webkit/tools/test_shell/simple_file_writer.cc | 2 +- 14 files changed, 46 insertions(+), 129 deletions(-) diff --git a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc index bad27f0..cc692b4 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc +++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc @@ -722,8 +722,7 @@ class HTML5FileWriter { } fileapi::FileSystemOperation* operation() { - return fs_->CreateFileSystemOperation( - fileapi::FileSystemURL(GURL(root_)), NULL); + return fs_->CreateFileSystemOperation(fileapi::FileSystemURL(GURL(root_))); } void CreateFile() { diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc index 2237d55..3077956 100644 --- a/content/browser/fileapi/fileapi_message_filter.cc +++ b/content/browser/fileapi/fileapi_message_filter.cc @@ -138,10 +138,8 @@ void FileAPIMessageFilter::OnChannelClosing() { open_filesystem_urls_.begin(); iter != open_filesystem_urls_.end(); ++iter) { FileSystemURL url(*iter); - FileSystemOperation* operation = context_->CreateFileSystemOperation( - url, NULL); - if (operation) - operation->NotifyCloseFile(url); + FileSystemOperation* operation = context_->CreateFileSystemOperation(url); + operation->NotifyCloseFile(url); } } @@ -238,10 +236,7 @@ void FileAPIMessageFilter::OnMove( return; } - FileSystemOperation* operation = GetNewOperation(src_url, request_id); - if (!operation) - return; - operation->Move( + GetNewOperation(src_url, request_id)->Move( src_url, dest_url, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -258,10 +253,7 @@ void FileAPIMessageFilter::OnCopy( return; } - FileSystemOperation* operation = GetNewOperation(src_url, request_id); - if (!operation) - return - operation->Copy( + GetNewOperation(src_url, request_id)->Copy( src_url, dest_url, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -276,10 +268,7 @@ void FileAPIMessageFilter::OnRemove( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->Remove( + GetNewOperation(url, request_id)->Remove( url, recursive, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -294,10 +283,7 @@ void FileAPIMessageFilter::OnReadMetadata( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->GetMetadata( + GetNewOperation(url, request_id)->GetMetadata( url, base::Bind(&FileAPIMessageFilter::DidGetMetadata, this, request_id)); } @@ -313,15 +299,12 @@ void FileAPIMessageFilter::OnCreate( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; if (is_directory) { - operation->CreateDirectory( + GetNewOperation(url, request_id)->CreateDirectory( url, exclusive, recursive, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } else { - operation->CreateFile( + GetNewOperation(url, request_id)->CreateFile( url, exclusive, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -337,15 +320,12 @@ void FileAPIMessageFilter::OnExists( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; if (is_directory) { - operation->DirectoryExists( + GetNewOperation(url, request_id)->DirectoryExists( url, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } else { - operation->FileExists( + GetNewOperation(url, request_id)->FileExists( url, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -361,10 +341,7 @@ void FileAPIMessageFilter::OnReadDirectory( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->ReadDirectory( + GetNewOperation(url, request_id)->ReadDirectory( url, base::Bind(&FileAPIMessageFilter::DidReadDirectory, this, request_id)); } @@ -388,10 +365,7 @@ void FileAPIMessageFilter::OnWrite( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->Write( + GetNewOperation(url, request_id)->Write( request_context_, url, blob_url, offset, base::Bind(&FileAPIMessageFilter::DidWrite, this, request_id)); } @@ -407,10 +381,7 @@ void FileAPIMessageFilter::OnTruncate( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->Truncate( + GetNewOperation(url, request_id)->Truncate( url, length, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -428,10 +399,7 @@ void FileAPIMessageFilter::OnTouchFile( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->TouchFile( + GetNewOperation(url, request_id)->TouchFile( url, last_access_time, last_modified_time, base::Bind(&FileAPIMessageFilter::DidFinish, this, request_id)); } @@ -465,10 +433,7 @@ void FileAPIMessageFilter::OnOpenFile( return; } - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->OpenFile( + GetNewOperation(url, request_id)->OpenFile( url, file_flags, peer_handle(), base::Bind(&FileAPIMessageFilter::DidOpenFile, this, request_id, path)); } @@ -486,8 +451,7 @@ void FileAPIMessageFilter::OnNotifyCloseFile(const GURL& path) { // Do not use GetNewOperation() here, because NotifyCloseFile is a one-way // operation that does not have request_id by which we respond back. - FileSystemOperation* operation = context_->CreateFileSystemOperation( - url, NULL); + FileSystemOperation* operation = context_->CreateFileSystemOperation(url); if (operation) operation->NotifyCloseFile(url); } @@ -532,11 +496,9 @@ void FileAPIMessageFilter::OnSyncGetPlatformPath( // TODO(kinuko): this hack should go away once appropriate upload-stream // handling based on element types is supported. LocalFileSystemOperation* operation = - context_->CreateFileSystemOperation( - url, NULL)->AsLocalFileSystemOperation(); + context_->CreateFileSystemOperation(url)->AsLocalFileSystemOperation(); DCHECK(operation); - if (operation) - operation->SyncGetPlatformPath(url, platform_path); + operation->SyncGetPlatformPath(url, platform_path); } void FileAPIMessageFilter::OnCreateSnapshotFile( @@ -546,11 +508,7 @@ void FileAPIMessageFilter::OnCreateSnapshotFile( base::Callback register_file_callback = base::Bind(&FileAPIMessageFilter::RegisterFileAsBlob, this, blob_url, url.path()); - - FileSystemOperation* operation = GetNewOperation(url, request_id); - if (!operation) - return; - operation->CreateSnapshotFile( + GetNewOperation(url, request_id)->CreateSnapshotFile( url, base::Bind(&FileAPIMessageFilter::DidCreateSnapshot, this, request_id, register_file_callback)); @@ -837,14 +795,8 @@ bool FileAPIMessageFilter::HasPermissionsForFile( FileSystemOperation* FileAPIMessageFilter::GetNewOperation( const FileSystemURL& target_url, int request_id) { - base::PlatformFileError error_code; FileSystemOperation* operation = - context_->CreateFileSystemOperation(target_url, &error_code); - if (error_code != base::PLATFORM_FILE_OK) { - Send(new FileSystemMsg_DidFail(request_id, error_code)); - return NULL; - } - + context_->CreateFileSystemOperation(target_url); DCHECK(operation); operations_.AddWithID(operation, request_id); return operation; diff --git a/net/base/net_errors.cc b/net/base/net_errors.cc index 78c6e27..8ac6b89 100644 --- a/net/base/net_errors.cc +++ b/net/base/net_errors.cc @@ -49,12 +49,10 @@ Error PlatformFileErrorToNetError( switch (file_error) { case base::PLATFORM_FILE_OK: return net::OK; - case base::PLATFORM_FILE_ERROR_ACCESS_DENIED: - return net::ERR_ACCESS_DENIED; - case base::PLATFORM_FILE_ERROR_INVALID_URL: - return net::ERR_INVALID_URL; case base::PLATFORM_FILE_ERROR_NOT_FOUND: return net::ERR_FILE_NOT_FOUND; + case base::PLATFORM_FILE_ERROR_ACCESS_DENIED: + return net::ERR_ACCESS_DENIED; default: return net::ERR_FAILED; } diff --git a/webkit/fileapi/file_system_context.cc b/webkit/fileapi/file_system_context.cc index 4561515..4a9fe7a 100644 --- a/webkit/fileapi/file_system_context.cc +++ b/webkit/fileapi/file_system_context.cc @@ -202,22 +202,13 @@ void FileSystemContext::DeleteFileSystem( } FileSystemOperation* FileSystemContext::CreateFileSystemOperation( - const FileSystemURL& url, PlatformFileError* error_code) { - if (!url.is_valid()) { - if (error_code) - *error_code = base::PLATFORM_FILE_ERROR_INVALID_URL; + const FileSystemURL& url) { + if (!url.is_valid()) return NULL; - } FileSystemMountPointProvider* mount_point_provider = GetMountPointProvider(url.type()); - if (!mount_point_provider) { - if (error_code) - *error_code = base::PLATFORM_FILE_ERROR_FAILED; + if (!mount_point_provider) return NULL; - } - - if (error_code) - *error_code = base::PLATFORM_FILE_OK; return mount_point_provider->CreateFileSystemOperation(url, this); } diff --git a/webkit/fileapi/file_system_context.h b/webkit/fileapi/file_system_context.h index abd3e7d..dfb5664 100644 --- a/webkit/fileapi/file_system_context.h +++ b/webkit/fileapi/file_system_context.h @@ -50,7 +50,7 @@ class FILEAPI_EXPORT FileSystemContext DefaultContextDeleter> { public: // task_runners->file_task_runner() is used as default TaskRunner. - // Unless a MountPointProvider is overridden in CreateFileSystemOperation, + // Unless a MountPointProvired is override in CreateFileSystemOperation, // it is used for all file operations and file related meta operations. // The code assumes that // task_runners->file_task_runner()->RunsTasksOnCurrentThread() @@ -129,9 +129,7 @@ class FILEAPI_EXPORT FileSystemContext // and calling the provider's corresponding CreateFileSystemOperation method. // The resolved MountPointProvider could perform further specialization // depending on the filesystem type pointed by the |url|. - FileSystemOperation* CreateFileSystemOperation( - const FileSystemURL& url, - base::PlatformFileError* error_code); + FileSystemOperation* CreateFileSystemOperation(const FileSystemURL& url); // Creates new FileStreamReader instance to read a file pointed by the given // filesystem URL |url| starting from |offset|. diff --git a/webkit/fileapi/file_system_dir_url_request_job.cc b/webkit/fileapi/file_system_dir_url_request_job.cc index 1be9394..b0c9d8f 100644 --- a/webkit/fileapi/file_system_dir_url_request_job.cc +++ b/webkit/fileapi/file_system_dir_url_request_job.cc @@ -80,11 +80,10 @@ void FileSystemDirURLRequestJob::StartAsync() { if (!request_) return; url_ = FileSystemURL(request_->url()); - base::PlatformFileError error_code; - FileSystemOperation* operation = GetNewOperation(&error_code); - if (error_code != base::PLATFORM_FILE_OK) { + FileSystemOperation* operation = GetNewOperation(); + if (!operation) { NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, - net::PlatformFileErrorToNetError(error_code))); + net::ERR_INVALID_URL)); return; } operation->ReadDirectory( @@ -125,16 +124,7 @@ void FileSystemDirURLRequestJob::DidReadDirectory( } if (has_more) { - base::PlatformFileError error_code; - FileSystemOperation* operation = GetNewOperation(&error_code); - if (error_code != base::PLATFORM_FILE_OK) { - NotifyDone(URLRequestStatus( - URLRequestStatus::FAILED, - net::PlatformFileErrorToNetError(error_code))); - return; - } - - operation->ReadDirectory( + GetNewOperation()->ReadDirectory( url_, base::Bind(&FileSystemDirURLRequestJob::DidReadDirectory, this)); } else { @@ -143,9 +133,8 @@ void FileSystemDirURLRequestJob::DidReadDirectory( } } -FileSystemOperation* FileSystemDirURLRequestJob::GetNewOperation( - base::PlatformFileError* error_code) { - return file_system_context_->CreateFileSystemOperation(url_, error_code); +FileSystemOperation* FileSystemDirURLRequestJob::GetNewOperation() { + return file_system_context_->CreateFileSystemOperation(url_); } } // namespace fileapi diff --git a/webkit/fileapi/file_system_dir_url_request_job.h b/webkit/fileapi/file_system_dir_url_request_job.h index c5b4aca..84e67ec 100644 --- a/webkit/fileapi/file_system_dir_url_request_job.h +++ b/webkit/fileapi/file_system_dir_url_request_job.h @@ -52,7 +52,7 @@ class FILEAPI_EXPORT_PRIVATE FileSystemDirURLRequestJob void DidReadDirectory(base::PlatformFileError result, const std::vector& entries, bool has_more); - FileSystemOperation* GetNewOperation(base::PlatformFileError* error_code); + FileSystemOperation* GetNewOperation(); std::string data_; FileSystemURL url_; diff --git a/webkit/fileapi/file_system_file_stream_reader.cc b/webkit/fileapi/file_system_file_stream_reader.cc index ef0a915..d1e5220 100644 --- a/webkit/fileapi/file_system_file_stream_reader.cc +++ b/webkit/fileapi/file_system_file_stream_reader.cc @@ -53,11 +53,10 @@ int FileSystemFileStreamReader::Read( if (local_file_reader_.get()) return local_file_reader_->Read(buf, buf_len, callback); DCHECK(!has_pending_create_snapshot_); - base::PlatformFileError error_code; FileSystemOperation* operation = - file_system_context_->CreateFileSystemOperation(url_, &error_code); - if (error_code != base::PLATFORM_FILE_OK) - return net::PlatformFileErrorToNetError(error_code); + file_system_context_->CreateFileSystemOperation(url_); + if (!operation) + return net::ERR_INVALID_URL; has_pending_create_snapshot_ = true; operation->CreateSnapshotFile( url_, diff --git a/webkit/fileapi/file_system_url_request_job.cc b/webkit/fileapi/file_system_url_request_job.cc index 959c01b..c26a4d7 100644 --- a/webkit/fileapi/file_system_url_request_job.cc +++ b/webkit/fileapi/file_system_url_request_job.cc @@ -157,12 +157,11 @@ void FileSystemURLRequestJob::StartAsync() { return; DCHECK(!reader_.get()); url_ = FileSystemURL(request_->url()); - base::PlatformFileError error_code; FileSystemOperation* operation = - file_system_context_->CreateFileSystemOperation(url_, &error_code); - if (error_code != base::PLATFORM_FILE_OK) { + file_system_context_->CreateFileSystemOperation(url_); + if (!operation) { NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, - net::PlatformFileErrorToNetError(error_code))); + net::ERR_INVALID_URL)); return; } operation->GetMetadata( diff --git a/webkit/fileapi/isolated_file_util_unittest.cc b/webkit/fileapi/isolated_file_util_unittest.cc index f6ddaad..03953c8 100644 --- a/webkit/fileapi/isolated_file_util_unittest.cc +++ b/webkit/fileapi/isolated_file_util_unittest.cc @@ -304,11 +304,7 @@ TEST_F(IsolatedFileUtilTest, UnregisteredPathsTest) { ASSERT_FALSE(url.is_valid()); // We should not be able to create a new operation for an invalid URL. - base::PlatformFileError error_code; - FileSystemOperation* operation = - file_system_context()->CreateFileSystemOperation(url, &error_code); - ASSERT_EQ(NULL, operation); - ASSERT_EQ(base::PLATFORM_FILE_ERROR_INVALID_URL, error_code); + ASSERT_EQ(NULL, file_system_context()->CreateFileSystemOperation(url)); } } diff --git a/webkit/fileapi/media/native_media_file_util_unittest.cc b/webkit/fileapi/media/native_media_file_util_unittest.cc index f14c7aa..ab88b31 100644 --- a/webkit/fileapi/media/native_media_file_util_unittest.cc +++ b/webkit/fileapi/media/native_media_file_util_unittest.cc @@ -140,7 +140,7 @@ class NativeMediaFileUtilTest : public testing::Test { } FileSystemOperation* NewOperation(const FileSystemURL& url) { - return file_system_context_->CreateFileSystemOperation(url, NULL); + return file_system_context_->CreateFileSystemOperation(url); } private: diff --git a/webkit/fileapi/sandbox_file_stream_writer.cc b/webkit/fileapi/sandbox_file_stream_writer.cc index a12e00e..2dafb58 100644 --- a/webkit/fileapi/sandbox_file_stream_writer.cc +++ b/webkit/fileapi/sandbox_file_stream_writer.cc @@ -66,12 +66,8 @@ int SandboxFileStreamWriter::Write( if (local_file_writer_.get()) return WriteInternal(buf, buf_len, callback); - base::PlatformFileError error_code; FileSystemOperation* operation = - file_system_context_->CreateFileSystemOperation(url_, &error_code); - if (error_code != base::PLATFORM_FILE_OK) - return net::PlatformFileErrorToNetError(error_code); - + file_system_context_->CreateFileSystemOperation(url_); DCHECK(operation); net::CompletionCallback write_task = base::Bind(&SandboxFileStreamWriter::DidInitializeForWrite, diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc index e5434e9..441396d 100644 --- a/webkit/tools/test_shell/simple_file_system.cc +++ b/webkit/tools/test_shell/simple_file_system.cc @@ -271,7 +271,7 @@ bool SimpleFileSystem::HasFilePermission( FileSystemOperation* SimpleFileSystem::GetNewOperation( const fileapi::FileSystemURL& url) { - return file_system_context_->CreateFileSystemOperation(url, NULL); + return file_system_context_->CreateFileSystemOperation(url); } FileSystemOperation::StatusCallback diff --git a/webkit/tools/test_shell/simple_file_writer.cc b/webkit/tools/test_shell/simple_file_writer.cc index 4688924..3e6f4ae 100644 --- a/webkit/tools/test_shell/simple_file_writer.cc +++ b/webkit/tools/test_shell/simple_file_writer.cc @@ -93,7 +93,7 @@ class SimpleFileWriter::IOThreadProxy virtual ~IOThreadProxy() {} FileSystemOperation* GetNewOperation( const FileSystemURL& url) { - return file_system_context_->CreateFileSystemOperation(url, NULL); + return file_system_context_->CreateFileSystemOperation(url); } // Returns true if it is not writable. -- cgit v1.1