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..c2b6582e145dd63256e5c38f37140889a7ac54fd 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_system_file_util.cc
@@ -107,8 +107,16 @@ 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) {
+    context->file_system_context()->GetQuotaUtil(context->src_type())->
+        MarkDirtyOriginOnFileThread(context->src_origin_url(),

[ericu, 2011/08/19 04:05:56]

Marking the cache dirty won't help entirely solve the problem; we also need code
to remove the database entry at some point.  Please add a TODO.

[tzik, 2011/08/23 04:13:13]

Done.

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

I was talking about the entry in the FileSystemDirectoryDatabase, although yes,
we also need to tell the UsageTracker if it's caching this info.

Please add another TODO.

[tzik, 2011/08/24 06:31:59]

I see.
But, should we delete the database entry silently, when we lost the backing
file?
When the backing file was lost due to a error on native file system, the user
looks less unhappy if he can detect the fact.

Anyway, I'll add some comment.

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

Done.

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

It's really hard to say what will make the user happier.  In many cases these
files aren't even really visible to the user; they're silently used by some web
app as part of its manual cache management.  In others, the user might want to
know about the data loss, but isn't really technically savvy enough to
understand what's happened.  In any case, there's generally nothing the user can
do about it, so we're probably just better off cleaning up in almost all cases.
Thanks.

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

I agree that deleting files will work better in most case.
But,,, could we discuss some more in other place?

[ericu, 2011/08/31 03:51:14]

Yes, of course.

+                                    context->src_type());
+    LOG(WARNING) << "Lost a backing file.";
+    error = base::PLATFORM_FILE_ERROR_FAILED;
+  }
+  return error;
 }
 
 PlatformFileError ObfuscatedFileSystemFileUtil::EnsureFileExists(
@@ -331,6 +339,14 @@ 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)) {
+      context->file_system_context()->GetQuotaUtil(context->src_type())->
+          MarkDirtyOriginOnFileThread(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);
@@ -787,6 +803,14 @@ 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;
+      }
+      LOG(WARNING) << "A stray file detected";
+    }
+
     if (handle) {
       error = underlying_file_util_->CreateOrOpen(
         context, path, file_flags, handle, &created);
@@ -1032,14 +1056,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;
 }