Handle inconsistency between DB and Files

webkit/fileapi/obfuscated_file_system_file_util.cc

Index: webkit/fileapi/obfuscated_file_system_file_util.cc
diff --git a/webkit/fileapi/obfuscated_file_system_file_util.cc b/webkit/fileapi/obfuscated_file_system_file_util.cc
index dc6b70e80461d2299a24a73e14a8aae02f51cbcc..8526d9f9d0f209276321d74ef8ef9dd1c3361814 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_system_file_util.cc
@@ -157,8 +157,17 @@ PlatformFileError ObfuscatedFileSystemFileUtil::CreateOrOpen(
     return base::PLATFORM_FILE_ERROR_NOT_A_FILE;
   FilePath data_path = DataPathToLocalPath(context->src_origin_url(),
     context->src_type(), file_info.data_path);
-  return underlying_file_util_->CreateOrOpen(
+  base::PlatformFileError error = underlying_file_util_->CreateOrOpen(
       context, data_path, file_flags, file_handle, created);
+  if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND) {
+    // TODO(tzik): Also invalidate on-memory usage cache in UsageTracker.
+    context->file_system_context()->GetQuotaUtil(context->src_type())->
+        InvalidateUsageCache(context->src_origin_url(),
+                             context->src_type());
+    LOG(WARNING) << "Lost a backing file.";
+    error = base::PLATFORM_FILE_ERROR_FAILED;
+  }
+  return error;
 }
 
 PlatformFileError ObfuscatedFileSystemFileUtil::EnsureFileExists(
@@ -385,6 +394,15 @@ PlatformFileError ObfuscatedFileSystemFileUtil::CopyOrMoveFile(
   if (copy) {
     FilePath src_data_path = DataPathToLocalPath(context->src_origin_url(),
       context->src_type(), src_file_info.data_path);
+    if (!underlying_file_util_->PathExists(context, src_data_path)) {
+      // TODO(tzik): Also invalidate on-memory usage cache in UsageTracker.
+      context->file_system_context()->GetQuotaUtil(context->src_type())->
+          InvalidateUsageCache(context->src_origin_url(),
+                               context->src_type());
+      LOG(WARNING) << "Lost a backing file.";
+      return base::PLATFORM_FILE_ERROR_FAILED;
+    }
+
     if (overwrite) {
       FilePath dest_data_path = DataPathToLocalPath(context->src_origin_url(),
         context->src_type(), dest_file_info.data_path);
@@ -865,6 +883,14 @@ PlatformFileError ObfuscatedFileSystemFileUtil::CreateFile(
       context, source_path, path, true /* copy */);
     created = true;
   } else {
+    if (file_util::PathExists(path)) {

[ericu, 2011/08/24 03:43:50]

This should probably use underlying_file_util_, not file_util.  However, in the
case of quota_file_util, which is generally what's under there, it might falsely
lower the quota.  Dai, Kinuko: what do you think about this?

[tzik, 2011/08/29 06:54:13]

We use underlying_file_util_ in the next patch.
It makes the usage lower than actual usage until next recalculation, maybe until
next boot.

[kinuko, 2011/08/29 07:45:37]

Maybe we can explicitly call underlying_file_util_->GetLocalFilePath() to get a
platform path and call file_util methods?

[tzik, 2011/08/29 08:34:15]

Done.

[ericu, 2011/08/29 22:48:20]

Thanks, and Kinuko, that was a good idea.
I've just noticed another inconsistency, though, which dates from when I wrote
this.  GetDirectoryForOriginAndType always returns a real OS path, and I've
mixed that in with calls to underlying_file_util as if the paths are in the same
namespace.  I don't see an obvious fix for that right now, but then nothing's
actually broken yet, just brittle.  Taiju, would you mind putting in this
comment for me?

[at line 864] TODO(ericu): local_path is an OS path; underlying_file_util_ isn't
guaranteed to understand OS paths.

Thanks.

[tzik, 2011/08/31 02:40:05]

Done.

+      if (!file_util::Delete(path, false)) {
+        NOTREACHED();
+        return base::PLATFORM_FILE_ERROR_FAILED;
+      }
+      LOG(WARNING) << "A stray file detected";
+    }
+
     if (handle) {
       error = underlying_file_util_->CreateOrOpen(
         context, path, file_flags, handle, &created);
@@ -1111,14 +1137,26 @@ FilePath ObfuscatedFileSystemFileUtil::GetDirectoryForOrigin(
     return FilePath();
   FilePath directory_name;
   std::string id = GetOriginIdentifierFromURL(origin);
-  if (!create && !origin_database_->HasOriginPath(id))
+
+  bool exists_in_db = origin_database_->HasOriginPath(id);
+  if (!exists_in_db && !create)
     return FilePath();
   if (!origin_database_->GetPathForOrigin(id, &directory_name))
     return FilePath();
+
   FilePath path = file_system_directory_.Append(directory_name);
-  if (!file_util::DirectoryExists(path) &&
-      (!create || !file_util::CreateDirectory(path)))
-    return FilePath();
+  bool exists_in_fs = file_util::DirectoryExists(path);
+  if (!exists_in_db && exists_in_fs) {
+    if (!file_util::Delete(path, true))
+      return FilePath();
+    exists_in_fs = false;
+  }
+
+  if (!exists_in_fs) {
+    if (!create || !file_util::CreateDirectory(path))
+      return FilePath();
+  }
+
   return path;
 }