Remove PlatforFile from fileapi/native_file_util

BUG=322664

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

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

5 files changed, 61 insertions, 62 deletions

diff --git a/webkit/browser/fileapi/local_file_util.cc b/webkit/browser/fileapi/local_file_util.cc
index 032244f..90d45e3 100644
--- a/webkit/browser/fileapi/local_file_util.cc
+++ b/webkit/browser/fileapi/local_file_util.cc
@@ -89,13 +89,20 @@ base::File::Error LocalFileUtil::CreateOrOpen(
   if (base::IsLink(file_path))
     return base::File::FILE_ERROR_NOT_FOUND;
 
-  return NativeFileUtil::CreateOrOpen(file_path, file_flags, file_handle,
-                                      created);
+  // TODO(rvargas): make FileSystemFileUtil use base::File.
+  base::File file = NativeFileUtil::CreateOrOpen(file_path, file_flags);
+  if (!file.IsValid())
+    return file.error_details();
+
+  *created = file.created();
+  *file_handle = file.TakePlatformFile();
+  return base::File::FILE_OK;
 }
 
 base::File::Error LocalFileUtil::Close(FileSystemOperationContext* context,
                                        base::PlatformFile file) {
-  return NativeFileUtil::Close(file);
+  base::File auto_closed(file);
+  return base::File::FILE_OK;
 }
 
 base::File::Error LocalFileUtil::EnsureFileExists(
diff --git a/webkit/browser/fileapi/native_file_util.cc b/webkit/browser/fileapi/native_file_util.cc
index 8f4560b..c022994 100644
--- a/webkit/browser/fileapi/native_file_util.cc
+++ b/webkit/browser/fileapi/native_file_util.cc
@@ -121,27 +121,18 @@ NativeFileUtil::CopyOrMoveMode NativeFileUtil::CopyOrMoveModeForDestination(
   return MOVE;
 }
 
-base::File::Error NativeFileUtil::CreateOrOpen(
-    const base::FilePath& path, int file_flags,
-    PlatformFile* file_handle, bool* created) {
+base::File NativeFileUtil::CreateOrOpen(const base::FilePath& path,
+                                        int file_flags) {
   if (!base::DirectoryExists(path.DirName())) {
     // If its parent does not exist, should return NOT_FOUND error.
-    return base::File::FILE_ERROR_NOT_FOUND;
+    return base::File(base::File::FILE_ERROR_NOT_FOUND);
   }
-  if (base::DirectoryExists(path))
-    return base::File::FILE_ERROR_NOT_A_FILE;
 
-  // TODO(rvargas): convert this code to use base::File.
-  base::PlatformFileError error_code = base::PLATFORM_FILE_OK;
-  *file_handle = base::CreatePlatformFile(path, file_flags,
-                                          created, &error_code);
-  return static_cast<base::File::Error>(error_code);
-}
+  // TODO(rvargas): Check |file_flags| instead. See bug 356358.
+  if (base::DirectoryExists(path))
+    return base::File(base::File::FILE_ERROR_NOT_A_FILE);
 
-base::File::Error NativeFileUtil::Close(PlatformFile file_handle) {
-  if (!base::ClosePlatformFile(file_handle))
-    return base::File::FILE_ERROR_FAILED;
-  return base::File::FILE_OK;
+  return base::File(path, file_flags);
 }
 
 base::File::Error NativeFileUtil::EnsureFileExists(
diff --git a/webkit/browser/fileapi/native_file_util.h b/webkit/browser/fileapi/native_file_util.h
index 7069c02..c6aeec4 100644
--- a/webkit/browser/fileapi/native_file_util.h
+++ b/webkit/browser/fileapi/native_file_util.h
@@ -9,7 +9,6 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util_proxy.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/platform_file.h"
 #include "webkit/browser/fileapi/file_system_file_util.h"
 #include "webkit/browser/webkit_storage_browser_export.h"
 
@@ -39,12 +38,8 @@ class WEBKIT_STORAGE_BROWSER_EXPORT_PRIVATE NativeFileUtil {
   static CopyOrMoveMode CopyOrMoveModeForDestination(
       const FileSystemURL& dest_url, bool copy);
 
-  static base::File::Error CreateOrOpen(
-      const base::FilePath& path,
-      int file_flags,
-      base::PlatformFile* file_handle,
-      bool* created);
-  static base::File::Error Close(base::PlatformFile file);
+  static base::File CreateOrOpen(const base::FilePath& path,
+                                 int file_flags);
   static base::File::Error EnsureFileExists(const base::FilePath& path,
                                             bool* created);
   static base::File::Error CreateDirectory(const base::FilePath& path,
@@ -59,7 +54,7 @@ class WEBKIT_STORAGE_BROWSER_EXPORT_PRIVATE NativeFileUtil {
                                  const base::Time& last_access_time,
                                  const base::Time& last_modified_time);
   static base::File::Error Truncate(const base::FilePath& path,
-                                          int64 length);
+                                    int64 length);
   static bool PathExists(const base::FilePath& path);
   static bool DirectoryExists(const base::FilePath& path);
   static base::File::Error CopyOrMoveFile(
diff --git a/webkit/browser/fileapi/native_file_util_unittest.cc b/webkit/browser/fileapi/native_file_util_unittest.cc
index cdf088f..6850921 100644
--- a/webkit/browser/fileapi/native_file_util_unittest.cc
+++ b/webkit/browser/fileapi/native_file_util_unittest.cc
@@ -5,9 +5,9 @@
 #include <set>
 
 #include "base/file_util.h"
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/platform_file.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/browser/fileapi/native_file_util.h"
 
@@ -49,28 +49,21 @@ class NativeFileUtilTest : public testing::Test {
 
 TEST_F(NativeFileUtilTest, CreateCloseAndDeleteFile) {
   base::FilePath file_name = Path("test_file");
-  base::PlatformFile file_handle;
-  bool created = false;
-  int flags = base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC;
-  ASSERT_EQ(base::File::FILE_OK,
-            NativeFileUtil::CreateOrOpen(file_name,
-                                         base::PLATFORM_FILE_CREATE | flags,
-                                         &file_handle, &created));
-  ASSERT_TRUE(created);
+  int flags = base::File::FLAG_WRITE | base::File::FLAG_ASYNC;
+  base::File file =
+      NativeFileUtil::CreateOrOpen(file_name, base::File::FLAG_CREATE | flags);
+  ASSERT_TRUE(file.IsValid());
+  ASSERT_TRUE(file.created());
 
   EXPECT_TRUE(base::PathExists(file_name));
   EXPECT_TRUE(NativeFileUtil::PathExists(file_name));
   EXPECT_EQ(0, GetSize(file_name));
-  EXPECT_NE(base::kInvalidPlatformFileValue, file_handle);
+  file.Close();
 
-  ASSERT_EQ(base::File::FILE_OK, NativeFileUtil::Close(file_handle));
-
-  ASSERT_EQ(base::File::FILE_OK,
-            NativeFileUtil::CreateOrOpen(file_name,
-                                         base::PLATFORM_FILE_OPEN | flags,
-                                         &file_handle, &created));
-  ASSERT_FALSE(created);
-  ASSERT_EQ(base::File::FILE_OK, NativeFileUtil::Close(file_handle));
+  file = NativeFileUtil::CreateOrOpen(file_name, base::File::FLAG_OPEN | flags);
+  ASSERT_TRUE(file.IsValid());
+  ASSERT_FALSE(file.created());
+  file.Close();
 
   ASSERT_EQ(base::File::FILE_OK,
             NativeFileUtil::DeleteFile(file_name));
diff --git a/webkit/browser/fileapi/obfuscated_file_util.cc b/webkit/browser/fileapi/obfuscated_file_util.cc
index 952645a..d720ca0 100644
--- a/webkit/browser/fileapi/obfuscated_file_util.cc
+++ b/webkit/browser/fileapi/obfuscated_file_util.cc
@@ -292,7 +292,8 @@ base::File::Error ObfuscatedFileUtil::CreateOrOpen(
 base::File::Error ObfuscatedFileUtil::Close(
     FileSystemOperationContext* context,
     base::PlatformFile file) {
-  return NativeFileUtil::Close(file);
+  base::File auto_closed(file);
+  return base::File::FILE_OK;
 }
 
 base::File::Error ObfuscatedFileUtil::EnsureFileExists(
@@ -1080,17 +1081,23 @@ base::File::Error ObfuscatedFileUtil::CreateFile(
     created = true;
   } else {
     if (base::PathExists(dest_local_path)) {
-      if (!base::DeleteFile(dest_local_path, true /* recursive */)) {
-        NOTREACHED();
+      if (!base::DeleteFile(dest_local_path, true /* recursive */))
         return base::File::FILE_ERROR_FAILED;
-      }
       LOG(WARNING) << "A stray file detected";
       InvalidateUsageCache(context, dest_url.origin(), dest_url.type());
     }
 
     if (handle) {
-      error = NativeFileUtil::CreateOrOpen(
-          dest_local_path, file_flags, handle, &created);
+      // TODO(rvargas): Remove PlatformFile from this code.
+      base::File file =
+          NativeFileUtil::CreateOrOpen(dest_local_path, file_flags);
+      if (file.IsValid()) {
+        created = file.created();
+        *handle = file.TakePlatformFile();
+        error = base::File::FILE_OK;
+      } else {
+        error = file.error_details();
+      }
       // If this succeeds, we must close handle on any subsequent error.
     } else {
       DCHECK(!file_flags);  // file_flags is only used by CreateOrOpen.
@@ -1101,7 +1108,6 @@ base::File::Error ObfuscatedFileUtil::CreateFile(
     return error;
 
   if (!created) {
-    NOTREACHED();
     if (handle) {
       DCHECK_NE(base::kInvalidPlatformFileValue, *handle);
       base::ClosePlatformFile(*handle);
@@ -1396,23 +1402,30 @@ base::File::Error ObfuscatedFileUtil::CreateOrOpenInternal(
     AllocateQuota(context, delta);
   }
 
-  error = NativeFileUtil::CreateOrOpen(
-      local_path, file_flags, file_handle, created);
-  if (error == base::File::FILE_ERROR_NOT_FOUND) {
-    // TODO(tzik): Also invalidate on-memory usage cache in UsageTracker.
-    // TODO(tzik): Delete database entry after ensuring the file lost.
-    InvalidateUsageCache(context, url.origin(), url.type());
-    LOG(WARNING) << "Lost a backing file.";
-    error = base::File::FILE_ERROR_FAILED;
+  // TODO(rvargas): make FileSystemFileUtil use base::File.
+  base::File file = NativeFileUtil::CreateOrOpen(local_path, file_flags);
+  if (!file.IsValid()) {
+    error = file.error_details();
+    if (error == base::File::FILE_ERROR_NOT_FOUND) {
+      // TODO(tzik): Also invalidate on-memory usage cache in UsageTracker.
+      // TODO(tzik): Delete database entry after ensuring the file lost.
+      InvalidateUsageCache(context, url.origin(), url.type());
+      LOG(WARNING) << "Lost a backing file.";
+      error = base::File::FILE_ERROR_FAILED;
+    }
+    return error;
   }
 
+  *created = file.created();
+  *file_handle = file.TakePlatformFile();
+
   // If truncating we need to update the usage.
-  if (error == base::File::FILE_OK && delta) {
+  if (delta) {
     UpdateUsage(context, url, delta);
     context->change_observers()->Notify(
         &FileChangeObserver::OnModifyFile, MakeTuple(url));
   }
-  return error;
+  return base::File::FILE_OK;
 }
 
 bool ObfuscatedFileUtil::HasIsolatedStorage(const GURL& origin) {