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 52113d695ba4d74d0c4793872fc833600f0b8615..776bd8b3c2290eeccc93f512a55d1b3b56c4e535 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_system_file_util.cc
@@ -787,6 +787,15 @@ PlatformFileError ObfuscatedFileSystemFileUtil::CreateFile(
       context, source_path, path, true /* copy */);
     created = true;
   } else {
+    if (file_util::PathExists(path)) {
+      if (!file_util::Delete(path, false)) {
+        NOTREACHED();
+        return base::PLATFORM_FILE_ERROR_FAILED;
+      }
+      context->file_system_context()->RecalculateUsage(origin_url, type);

[ericu, 2011/08/17 00:54:08]

What's the purpose of this RecalculateUsage call?
If you're trying to update the quota usage, it's not necessary; the quota
doesn't include these stray files at all.

[kinuko, 2011/08/18 01:26:36]

Eric, this is one of the place we wondered if we'd better mark the usage cache
dirty before making any operations.

This inconsistency could happen in a situation like:
1. an app calls DeleteFile() to delete a file
2. first we delete the file entry from the database in OFSFU
3. then we mark the usage cache dirty, delete file, update the usage cache and
clear the dirty flag in QFU.

There's a slight chance chrome could crash between 2 and 3.  The FS
inconsistency could be easily and lazily fixed later (and this patch is doing
the fix), but in this case we cannot know if the usage cache has been already
updated or not, so we need to force recalculating it (that's why he calls
RecalculateUsage there).

But if we always mark the usage cache dirty before making any operations we will
no longer have such problems-- we can just recalculate the usage whenever the
cache is marked dirty at start-up time.

[tzik, 2011/08/18 08:02:26]

Thanks for explaining kinuko-san, it is what I wanted to say.

+      LOG(WARNING) << "A stray file detected";
+    }
+
     if (handle) {
       error = underlying_file_util_->CreateOrOpen(
         context, path, file_flags, handle, &created);
@@ -1032,14 +1041,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);

[ericu, 2011/08/17 00:54:08]

This part looks like a good idea.

-  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;
 }