Handle inconsistency between DB and Files

webkit/fileapi/obfuscated_file_system_file_util.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/fileapi/obfuscated_file_system_file_util.h"
 
 #include <queue>
 #include <vector>
 
 #include "base/file_util.h"
@@ skipping 89 matching lines @@
 
   FileInfo file_info;
   if (!db->GetFileInfo(file_id, &file_info)) {
     NOTREACHED();
     return base::PLATFORM_FILE_ERROR_FAILED;
   }
   if (file_info.is_directory())
     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(
     FileSystemOperationContext* context,
     const FilePath& virtual_path,
     bool* created) {
   FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
       context->src_origin_url(), context->src_type(), true);
   if (!db)
     return base::PLATFORM_FILE_ERROR_FAILED;
@@ skipping 202 matching lines @@
    *  transaction:
    *    Remove source entry.
    *    Point target entry to source entry's backing file.
    *  Delete target entry's old backing file
    * Move-without-overwrite
    *  Just update metadata
    */
   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);
       return underlying_file_util_->CopyOrMoveFile(context,
           src_data_path, dest_data_path, copy);
     } else {
       FileId dest_parent_id;
       if (!db->GetFileWithPath(dest_file_path.DirName(), &dest_parent_id)) {
         NOTREACHED();  // We shouldn't be called in this case.
         return base::PLATFORM_FILE_ERROR_NOT_FOUND;
@@ skipping 436 matching lines @@
     return base::PLATFORM_FILE_ERROR_FAILED;
   PlatformFileError error;
   bool created = false;
   if (!source_path.empty()) {
     DCHECK(!file_flags);
     DCHECK(!handle);
     error = underlying_file_util_->CopyOrMoveFile(
       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);
       // If this succeeds, we must close handle on any subsequent error.
     } else {
       DCHECK(!file_flags);  // file_flags is only used by CreateOrOpen.
       error = underlying_file_util_->EnsureFileExists(
           context, path, &created);
     }
   }
@@ skipping 225 matching lines @@
   directories_[key] = database;
   return database;
 }
 
 FilePath ObfuscatedFileSystemFileUtil::GetDirectoryForOrigin(
     const GURL& origin, bool create) {
   if (!InitOriginDatabase(create))
     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;
 }
 
 void ObfuscatedFileSystemFileUtil::MarkUsed() {
   if (timer_.IsRunning())
     timer_.Reset();
   else
     timer_.Start(base::TimeDelta::FromSeconds(kFlushDelaySeconds), this,
       &ObfuscatedFileSystemFileUtil::DropDatabases);
 }
@@ skipping 40 matching lines @@
       return false;
     }
     origin_database_.reset(
         new FileSystemOriginDatabase(
             file_system_directory_.AppendASCII(kOriginDatabaseName)));
   }
   return true;
 }
 
 }  // namespace fileapi