Commit 07aa825a authored by kinaba@chromium.org's avatar kinaba@chromium.org

Rename "CheckWritable" to "PrepareForWritableApp" to reflect the actual behavior.

The existing implementation in fact does not check whether the file is
writable or not (see below for the detail.)
Since the current behavior itself is fine for the use cases,
this CL renames the relevant functions, and refactors the implementation
so that the intent is clearer.

Existing implementation tries to open a file with CREATE|READ|WRITE flags
and verifies that the result is either FILE_OK or ALREADY_EXISTS.

This _succeeds_ when read-only file already existed at the location
because ALREADY_EXISTS error precedes write failure.
Considering the fact, it is essentially equivalent to open with OPEN_ALWAYS|READ
and comparing the result just against FILE_OK.

BUG=367028

Review URL: https://codereview.chromium.org/306073004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274485 0039d316-1c4b-4281-b951-d872f2087c98
parent 6b7cc120
...@@ -46,7 +46,7 @@ namespace app_runtime = apps::api::app_runtime; ...@@ -46,7 +46,7 @@ namespace app_runtime = apps::api::app_runtime;
using apps::file_handler_util::GrantedFileEntry; using apps::file_handler_util::GrantedFileEntry;
using content::BrowserThread; using content::BrowserThread;
using extensions::app_file_handler_util::CheckWritableFiles; using extensions::app_file_handler_util::PrepareFilesForWritableApp;
using extensions::app_file_handler_util::FileHandlerForId; using extensions::app_file_handler_util::FileHandlerForId;
using extensions::app_file_handler_util::FileHandlerCanHandleFile; using extensions::app_file_handler_util::FileHandlerCanHandleFile;
using extensions::app_file_handler_util::FirstFileHandlerForFile; using extensions::app_file_handler_util::FirstFileHandlerForFile;
...@@ -114,7 +114,7 @@ class PlatformAppPathLauncher ...@@ -114,7 +114,7 @@ class PlatformAppPathLauncher
if (HasFileSystemWritePermission(extension_)) { if (HasFileSystemWritePermission(extension_)) {
std::vector<base::FilePath> paths; std::vector<base::FilePath> paths;
paths.push_back(file_path_); paths.push_back(file_path_);
CheckWritableFiles( PrepareFilesForWritableApp(
paths, paths,
profile_, profile_,
false, false,
......
...@@ -171,7 +171,7 @@ void IsNonNativeLocalPathDirectory( ...@@ -171,7 +171,7 @@ void IsNonNativeLocalPathDirectory(
base::Bind(&BoolCallbackAsFileErrorCallback, callback)); base::Bind(&BoolCallbackAsFileErrorCallback, callback));
} }
void PrepareNonNativeLocalPathWritableFile( void PrepareNonNativeLocalFileForWritableApp(
Profile* profile, Profile* profile,
const base::FilePath& path, const base::FilePath& path,
const base::Callback<void(bool)>& callback) { const base::Callback<void(bool)>& callback) {
......
...@@ -40,9 +40,10 @@ void IsNonNativeLocalPathDirectory( ...@@ -40,9 +40,10 @@ void IsNonNativeLocalPathDirectory(
const base::FilePath& path, const base::FilePath& path,
const base::Callback<void(bool)>& callback); const base::Callback<void(bool)>& callback);
// Prepares a writable file at |path|, i.e., creates a file there if it didn't // Ensures a file exists at |path|, i.e., it does nothing if a file is already
// exist, and asynchronously sends to |callback| whether it succeeded. // present, or creates a file there if it isn't, and asynchronously sends to
void PrepareNonNativeLocalPathWritableFile( // |callback| whether it succeeded.
void PrepareNonNativeLocalFileForWritableApp(
Profile* profile, Profile* profile,
const base::FilePath& path, const base::FilePath& path,
const base::Callback<void(bool)>& callback); const base::Callback<void(bool)>& callback);
......
...@@ -77,12 +77,9 @@ bool DoCheckWritableFile(const base::FilePath& path, bool is_directory) { ...@@ -77,12 +77,9 @@ bool DoCheckWritableFile(const base::FilePath& path, bool is_directory) {
return base::DirectoryExists(path); return base::DirectoryExists(path);
// Create the file if it doesn't already exist. // Create the file if it doesn't already exist.
int creation_flags = base::File::FLAG_CREATE | base::File::FLAG_READ | int creation_flags = base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_READ;
base::File::FLAG_WRITE;
base::File file(path, creation_flags); base::File file(path, creation_flags);
if (file.IsValid()) return file.IsValid();
return true;
return file.error_details() == base::File::FILE_ERROR_EXISTS;
} }
// Checks whether a list of paths are all OK for writing and calls a provided // Checks whether a list of paths are all OK for writing and calls a provided
...@@ -155,7 +152,7 @@ void WritableFileChecker::Check() { ...@@ -155,7 +152,7 @@ void WritableFileChecker::Check() {
base::Bind(&WritableFileChecker::NonNativeLocalPathCheckDone, base::Bind(&WritableFileChecker::NonNativeLocalPathCheckDone,
this, *it)); this, *it));
} else { } else {
file_manager::util::PrepareNonNativeLocalPathWritableFile( file_manager::util::PrepareNonNativeLocalFileForWritableApp(
profile_, profile_,
*it, *it,
base::Bind(&WritableFileChecker::NonNativeLocalPathCheckDone, base::Bind(&WritableFileChecker::NonNativeLocalPathCheckDone,
...@@ -320,7 +317,7 @@ GrantedFileEntry CreateFileEntry( ...@@ -320,7 +317,7 @@ GrantedFileEntry CreateFileEntry(
return result; return result;
} }
void CheckWritableFiles( void PrepareFilesForWritableApp(
const std::vector<base::FilePath>& paths, const std::vector<base::FilePath>& paths,
Profile* profile, Profile* profile,
bool is_directory, bool is_directory,
......
...@@ -66,7 +66,12 @@ apps::file_handler_util::GrantedFileEntry CreateFileEntry( ...@@ -66,7 +66,12 @@ apps::file_handler_util::GrantedFileEntry CreateFileEntry(
const base::FilePath& path, const base::FilePath& path,
bool is_directory); bool is_directory);
void CheckWritableFiles( // When |is_directory| is true, it verifies that directories exist at each of
// the |paths| and calls back to |on_success| or otherwise to |on_failure|.
// When |is_directory| is false, it ensures regular files exists (not links and
// directories) at the |paths|, creating files if needed, and calls back to
// |on_success| or to |on_failure| depending on the result.
void PrepareFilesForWritableApp(
const std::vector<base::FilePath>& paths, const std::vector<base::FilePath>& paths,
Profile* profile, Profile* profile,
bool is_directory, bool is_directory,
......
...@@ -283,10 +283,10 @@ FileSystemEntryFunction::FileSystemEntryFunction() ...@@ -283,10 +283,10 @@ FileSystemEntryFunction::FileSystemEntryFunction()
is_directory_(false), is_directory_(false),
response_(NULL) {} response_(NULL) {}
void FileSystemEntryFunction::CheckWritableFiles( void FileSystemEntryFunction::PrepareFilesForWritableApp(
const std::vector<base::FilePath>& paths) { const std::vector<base::FilePath>& paths) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
app_file_handler_util::CheckWritableFiles( app_file_handler_util::PrepareFilesForWritableApp(
paths, paths,
GetProfile(), GetProfile(),
is_directory_, is_directory_,
...@@ -392,7 +392,7 @@ void FileSystemGetWritableEntryFunction::CheckPermissionAndSendResponse() { ...@@ -392,7 +392,7 @@ void FileSystemGetWritableEntryFunction::CheckPermissionAndSendResponse() {
} }
std::vector<base::FilePath> paths; std::vector<base::FilePath> paths;
paths.push_back(path_); paths.push_back(path_);
CheckWritableFiles(paths); PrepareFilesForWritableApp(paths);
} }
void FileSystemGetWritableEntryFunction::SetIsDirectoryOnFileThread() { void FileSystemGetWritableEntryFunction::SetIsDirectoryOnFileThread() {
...@@ -770,7 +770,7 @@ void FileSystemChooseEntryFunction::ConfirmDirectoryAccessOnFileThread( ...@@ -770,7 +770,7 @@ void FileSystemChooseEntryFunction::ConfirmDirectoryAccessOnFileThread(
void FileSystemChooseEntryFunction::OnDirectoryAccessConfirmed( void FileSystemChooseEntryFunction::OnDirectoryAccessConfirmed(
const std::vector<base::FilePath>& paths) { const std::vector<base::FilePath>& paths) {
if (app_file_handler_util::HasFileSystemWritePermission(extension_)) { if (app_file_handler_util::HasFileSystemWritePermission(extension_)) {
CheckWritableFiles(paths); PrepareFilesForWritableApp(paths);
return; return;
} }
......
...@@ -53,7 +53,7 @@ class FileSystemEntryFunction : public ChromeAsyncExtensionFunction { ...@@ -53,7 +53,7 @@ class FileSystemEntryFunction : public ChromeAsyncExtensionFunction {
// will ensure the files exist, creating them if necessary, and also check // will ensure the files exist, creating them if necessary, and also check
// that none of the files are links. If it succeeds it proceeds to // that none of the files are links. If it succeeds it proceeds to
// RegisterFileSystemsAndSendResponse, otherwise to HandleWritableFileError. // RegisterFileSystemsAndSendResponse, otherwise to HandleWritableFileError.
void CheckWritableFiles(const std::vector<base::FilePath>& path); void PrepareFilesForWritableApp(const std::vector<base::FilePath>& path);
// This will finish the choose file process. This is either called directly // This will finish the choose file process. This is either called directly
// from FilesSelected, or from WritableFileChecker. It is called on the UI // from FilesSelected, or from WritableFileChecker. It is called on the UI
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment