Remove PlatforFile from fileapi/native_file_util

webkit/browser/fileapi/obfuscated_file_util.cc

Index: webkit/browser/fileapi/obfuscated_file_util.cc
diff --git a/webkit/browser/fileapi/obfuscated_file_util.cc b/webkit/browser/fileapi/obfuscated_file_util.cc
index 952645a2a42fabe099366c32b04c8734324834f9..24162f77c845edc9434bd8d07a1a5ba241664a1b 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,20 @@ base::File::Error ObfuscatedFileUtil::CreateFile(
     created = true;
   } else {
     if (base::PathExists(dest_local_path)) {
-      if (!base::DeleteFile(dest_local_path, true /* recursive */)) {
-        NOTREACHED();

[kinuko, 2014/03/24 09:56:58]

Why are you removing these NOTREACHED?

[rvargas (doing something else), 2014/03/24 23:02:48]

Because they are DCHECKing at the same time that handling the failure (frowned
upon by style), and not checking an actual precondition that can be verified as
it is outside of the control of the code (there are many reasons for the OS to
refuse to delete a file, or in general, succeed on any system call).

[kinuko, 2014/03/25 03:15:26]

I'm not original author of this code, and fine with deleting it in these
particular cases, but I think having NOTREACHED and also handling failure is not
that uncommon in our code base. (E.g. when we want to detect naive coding errors
in tests but still want to handle gracefully in prod case when there's a slight
chance that the case could occur (as in this case))

+      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;
+      error = NativeFileUtil::CreateOrOpen(dest_local_path, file_flags, &file);
+      if (error == base::File::FILE_OK) {
+        created = file.created();
+        *handle = file.TakePlatformFile();
+      }
       // If this succeeds, we must close handle on any subsequent error.
     } else {
       DCHECK(!file_flags);  // file_flags is only used by CreateOrOpen.
@@ -1101,7 +1105,6 @@ base::File::Error ObfuscatedFileUtil::CreateFile(
     return error;
 
   if (!created) {
-    NOTREACHED();
     if (handle) {
       DCHECK_NE(base::kInvalidPlatformFileValue, *handle);
       base::ClosePlatformFile(*handle);
@@ -1396,8 +1399,9 @@ base::File::Error ObfuscatedFileUtil::CreateOrOpenInternal(
     AllocateQuota(context, delta);
   }
 
-  error = NativeFileUtil::CreateOrOpen(
-      local_path, file_flags, file_handle, created);
+  // TODO(rvargas): make FileSystemFileUtil use base::File.
+  base::File file;
+  error = NativeFileUtil::CreateOrOpen(local_path, file_flags, &file);
   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.
@@ -1406,8 +1410,14 @@ base::File::Error ObfuscatedFileUtil::CreateOrOpenInternal(
     error = base::File::FILE_ERROR_FAILED;
   }
 
+  if (error != base::File::FILE_OK)
+    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));