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 769 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;
+      }
+      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);
       // 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);

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