[fsp] Fix error codes.

This patch cleans up returned error codes. And now:
- base::File::FILE_ERROR_SECURITY is for policy errors, or not handled events.
- base::File::FILE_ERROR_ACCESS_DENIED for all writing operations.
- base::File::FILE_ERROR_INVALID_OPERATION for invalid arguments to operations
    or operations which are read-only but not yet implemented.

TEST=browser_tests, unit_tests: *FileSystemProvider*
BUG=373151

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273178 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/file_system_provider/fake_provided_file_system.cc

@@ -146,12 +146,19 @@ void FakeProvidedFileSystem::OpenFile(const base::FilePath& file_path,
                                       OpenFileMode mode,
                                       bool create,
                                       const OpenFileCallback& callback) {
-  if (file_path.AsUTF8Unsafe() != "/hello.txt" ||
-      mode == OPEN_FILE_MODE_WRITE || create) {
+  if (mode == OPEN_FILE_MODE_WRITE || create) {
+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   0 /* file_handle */,
+                   base::File::FILE_ERROR_ACCESS_DENIED));
+  }
+
+  if (file_path.AsUTF8Unsafe() != "/hello.txt") {
     base::MessageLoopProxy::current()->PostTask(
         FROM_HERE,
         base::Bind(
-            callback, 0 /* file_handle */, base::File::FILE_ERROR_SECURITY));
+            callback, 0 /* file_handle */, base::File::FILE_ERROR_NOT_FOUND));
     return;
   }
 
@@ -192,7 +199,7 @@ void FakeProvidedFileSystem::ReadFile(
         base::Bind(callback,
                    0 /* chunk_length */,
                    false /* has_next */,
-                   base::File::FILE_ERROR_SECURITY));
+                   base::File::FILE_ERROR_INVALID_OPERATION));
     return;
   }
 

chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc

@@ -170,7 +170,7 @@ TEST_F(FileSystemProviderFileStreamReader, Read_WrongFile) {
   base::RunLoop().RunUntilIdle();
 
   ASSERT_EQ(1u, logger.results().size());
-  EXPECT_EQ(net::ERR_FAILED, logger.results()[0]);
+  EXPECT_EQ(net::ERR_FILE_NOT_FOUND, logger.results()[0]);
 }
 
 TEST_F(FileSystemProviderFileStreamReader, Read_InChunks) {
@@ -288,7 +288,7 @@ TEST_F(FileSystemProviderFileStreamReader, GetLength_WrongFile) {
   base::RunLoop().RunUntilIdle();
 
   ASSERT_EQ(1u, logger.results().size());
-  EXPECT_EQ(net::ERR_FAILED, logger.results()[0]);
+  EXPECT_EQ(net::ERR_FILE_NOT_FOUND, logger.results()[0]);
 }
 
 }  // namespace file_system_provider

chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util.cc

@@ -29,7 +29,7 @@ void GetFileInfoOnUIThread(
     const fileapi::AsyncFileUtil::GetFileInfoCallback& callback) {
   util::FileSystemURLParser parser(url);
   if (!parser.Parse()) {
-    callback.Run(base::File::FILE_ERROR_NOT_FOUND, base::File::Info());
+    callback.Run(base::File::FILE_ERROR_INVALID_OPERATION, base::File::Info());
     return;
   }
 
@@ -51,7 +51,7 @@ void ReadDirectoryOnUIThread(
     const fileapi::AsyncFileUtil::ReadDirectoryCallback& callback) {
   util::FileSystemURLParser parser(url);
   if (!parser.Parse()) {
-    callback.Run(base::File::FILE_ERROR_NOT_FOUND,
+    callback.Run(base::File::FILE_ERROR_INVALID_OPERATION,
                  fileapi::AsyncFileUtil::EntryList(),
                  false /* has_more */);
     return;
@@ -87,12 +87,14 @@ void ProviderAsyncFileUtil::CreateOrOpen(
       (file_flags & base::File::FLAG_OPEN_ALWAYS) ||
       (file_flags & base::File::FLAG_CREATE_ALWAYS) ||
       (file_flags & base::File::FLAG_OPEN_TRUNCATED)) {
-    callback.Run(base::File(base::File::FILE_ERROR_SECURITY), base::Closure());
+    callback.Run(base::File(base::File::FILE_ERROR_ACCESS_DENIED),
+                 base::Closure());
     return;
   }
 
   NOTIMPLEMENTED();
-  callback.Run(base::File(base::File::FILE_ERROR_NOT_FOUND), base::Closure());
+  callback.Run(base::File(base::File::FILE_ERROR_INVALID_OPERATION),
+               base::Closure());
 }
 
 void ProviderAsyncFileUtil::EnsureFileExists(
@@ -100,7 +102,7 @@ void ProviderAsyncFileUtil::EnsureFileExists(
     const fileapi::FileSystemURL& url,
     const EnsureFileExistsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY, false /* created */);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED, false /* created */);
 }
 
 void ProviderAsyncFileUtil::CreateDirectory(
@@ -110,7 +112,7 @@ void ProviderAsyncFileUtil::CreateDirectory(
     bool recursive,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::GetFileInfo(
@@ -146,7 +148,7 @@ void ProviderAsyncFileUtil::Touch(
     const base::Time& last_modified_time,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::Truncate(
@@ -155,7 +157,7 @@ void ProviderAsyncFileUtil::Truncate(
     int64 length,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::CopyFileLocal(
@@ -166,7 +168,7 @@ void ProviderAsyncFileUtil::CopyFileLocal(
     const CopyFileProgressCallback& progress_callback,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::MoveFileLocal(
@@ -176,7 +178,7 @@ void ProviderAsyncFileUtil::MoveFileLocal(
     CopyOrMoveOption option,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::CopyInForeignFile(
@@ -185,7 +187,7 @@ void ProviderAsyncFileUtil::CopyInForeignFile(
     const fileapi::FileSystemURL& dest_url,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::DeleteFile(
@@ -193,7 +195,7 @@ void ProviderAsyncFileUtil::DeleteFile(
     const fileapi::FileSystemURL& url,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::DeleteDirectory(
@@ -201,7 +203,7 @@ void ProviderAsyncFileUtil::DeleteDirectory(
     const fileapi::FileSystemURL& url,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::DeleteRecursively(
@@ -209,7 +211,7 @@ void ProviderAsyncFileUtil::DeleteRecursively(
     const fileapi::FileSystemURL& url,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(base::File::FILE_ERROR_SECURITY);
+  callback.Run(base::File::FILE_ERROR_ACCESS_DENIED);
 }
 
 void ProviderAsyncFileUtil::CreateSnapshotFile(
@@ -218,7 +220,7 @@ void ProviderAsyncFileUtil::CreateSnapshotFile(
     const CreateSnapshotFileCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   NOTIMPLEMENTED();
-  callback.Run(base::File::FILE_ERROR_NOT_FOUND,
+  callback.Run(base::File::FILE_ERROR_INVALID_OPERATION,
                base::File::Info(),
                base::FilePath(),
                scoped_refptr<webkit_blob::ShareableFileReference>());

chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc

@@ -117,8 +117,8 @@ KeyedService* CreateService(content::BrowserContext* context) {
 // Tests in this file are very lightweight and just test integration between
 // AsyncFileUtil and ProvideFileSystemInterface. Currently it tests if not
 // implemented operations return a correct error code. For not allowed
-// operations it is FILE_ERROR_SECURITY, and for not implemented the error is
-// FILE_ERROR_NOT_FOUND.
+// operations it is FILE_ERROR_ACCESS_DENIED, and for not implemented the error
+// is FILE_ERROR_INVALID_OPERATION.
 class FileSystemProviderProviderAsyncFileUtilTest : public testing::Test {
  protected:
   FileSystemProviderProviderAsyncFileUtilTest() {}
@@ -191,7 +191,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_Create) {
       base::Bind(&EventLogger::OnCreateOrOpen, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_CreateAlways) {
@@ -204,7 +204,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_CreateAlways) {
       base::Bind(&EventLogger::OnCreateOrOpen, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_OpenAlways) {
@@ -217,7 +217,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_OpenAlways) {
       base::Bind(&EventLogger::OnCreateOrOpen, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest,
@@ -231,7 +231,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest,
       base::Bind(&EventLogger::OnCreateOrOpen, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_Open) {
@@ -244,7 +244,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateOrOpen_Open) {
       base::Bind(&EventLogger::OnCreateOrOpen, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_INVALID_OPERATION, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, EnsureFileExists) {
@@ -256,7 +256,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, EnsureFileExists) {
       base::Bind(&EventLogger::OnEnsureFileExists, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateDirectory) {
@@ -270,7 +270,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateDirectory) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, GetFileInfo) {
@@ -310,7 +310,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, Touch) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, Truncate) {
@@ -323,7 +323,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, Truncate) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CopyFileLocal) {
@@ -338,7 +338,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CopyFileLocal) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, MoveFileLocal) {
@@ -352,7 +352,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, MoveFileLocal) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CopyInForeignFile) {
@@ -365,7 +365,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CopyInForeignFile) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, DeleteFile) {
@@ -377,7 +377,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, DeleteFile) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, DeleteDirectory) {
@@ -389,7 +389,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, DeleteDirectory) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, DeleteRecursively) {
@@ -401,7 +401,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, DeleteRecursively) {
       base::Bind(&EventLogger::OnStatus, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_SECURITY, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED, *logger.error());
 }
 
 TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateSnapshotFile) {
@@ -413,7 +413,7 @@ TEST_F(FileSystemProviderProviderAsyncFileUtilTest, CreateSnapshotFile) {
       base::Bind(&EventLogger::OnCreateSnapshotFile, logger.GetWeakPtr()));
 
   ASSERT_TRUE(logger.error());
-  EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, *logger.error());
+  EXPECT_EQ(base::File::FILE_ERROR_INVALID_OPERATION, *logger.error());
 }
 
 }  // namespace file_system_provider