Don't keep file system directories around if they contain unrelated files.

webkit/fileapi/obfuscated_file_util.cc

 // Copyright (c) 2012 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_util.h"
 
 #include <queue>
 #include <string>
 #include <vector>
 
@@ skipping 890 matching lines @@
     return FilePath();
   FilePath::StringType type_string = GetDirectoryNameForType(type);
   if (type_string.empty()) {
     LOG(WARNING) << "Unknown filesystem type requested:" << type;
 
     if (error_code)
       *error_code = base::PLATFORM_FILE_ERROR_INVALID_URL;
     return FilePath();
   }
   FilePath path = origin_dir.Append(type_string);
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
   if (!file_util::DirectoryExists(path) &&
       (!create || !file_util::CreateDirectory(path))) {
-    if (error_code) {
-      *error_code = create ?
+    error = create ?
           base::PLATFORM_FILE_ERROR_FAILED :
           base::PLATFORM_FILE_ERROR_NOT_FOUND;
-    }
-    return FilePath();
   }
 
   if (error_code)
-    *error_code = base::PLATFORM_FILE_OK;
+    *error_code = error;
   return path;
 }
 
 bool ObfuscatedFileUtil::DeleteDirectoryForOriginAndType(
     const GURL& origin, FileSystemType type) {
-  FilePath origin_type_path = GetDirectoryForOriginAndType(origin, type, false);
-  if (!file_util::PathExists(origin_type_path))
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath origin_type_path = GetDirectoryForOriginAndType(origin, type, false,
+                                                           &error);
+  if (origin_type_path.empty())
     return true;
 
-  // TODO(dmikurube): Consider the return value of DestroyDirectoryDatabase.
-  // We ignore its error now since 1) it doesn't matter the final result, and
-  // 2) it always returns false in Windows because of LevelDB's implementation.
-  // Information about failure would be useful for debugging.
-  DestroyDirectoryDatabase(origin, type);
-  if (!file_util::Delete(origin_type_path, true /* recursive */))
-    return false;
+  if (error != base::PLATFORM_FILE_ERROR_NOT_FOUND) {
+    // TODO(dmikurube): Consider the return value of DestroyDirectoryDatabase.
+    // We ignore its error now since 1) it doesn't matter the final result, and
+    // 2) it always returns false in Windows because of LevelDB's
+    // implementation.
+    // Information about failure would be useful for debugging.
+    DestroyDirectoryDatabase(origin, type);
+    if (!file_util::Delete(origin_type_path, true /* recursive */))
+      return false;
+  }
 
   FilePath origin_path = origin_type_path.DirName();
   DCHECK_EQ(origin_path.value(),
             GetDirectoryForOrigin(origin, false, NULL).value());
 
   // Delete the origin directory if the deleted one was the last remaining
-  // type for the origin.
-  if (file_util::Delete(origin_path, false /* recursive */)) {
+  // type for the origin, i.e. if the *other* type doesn't exist.
+  FileSystemType other_type = kFileSystemTypeUnknown;
+  switch (type) {
+    case kFileSystemTypeTemporary:
+      other_type = kFileSystemTypePersistent;
+      break;
+    case kFileSystemTypePersistent:
+      other_type = kFileSystemTypeTemporary;
+      break;
+    // These types shouldn't be used.
+    case kFileSystemTypeUnknown:
+    case kFileSystemTypeIsolated:
+    case kFileSystemTypeExternal:
+    case kFileSystemTypeTest:
+      NOTREACHED();
+  }
+  if (!file_util::DirectoryExists(
+          origin_path.Append(GetDirectoryNameForType(other_type)))) {
     InitOriginDatabase(false);
     if (origin_database_.get())
       origin_database_->RemovePathForOrigin(GetOriginIdentifierFromURL(origin));
+    if (!file_util::Delete(origin_path, true /* recursive */))
+      return false;
   }
 
   // At this point we are sure we had successfully deleted the origin/type
   // directory, so just returning true here.
   return true;
 }
 
 bool ObfuscatedFileUtil::MigrateFromOldSandbox(
     const GURL& origin_url, FileSystemType type, const FilePath& src_root) {
   if (!DestroyDirectoryDatabase(origin_url, type))
     return false;
-  FilePath dest_root = GetDirectoryForOriginAndType(origin_url, type, true);
-  if (dest_root.empty())
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath dest_root = GetDirectoryForOriginAndType(origin_url, type, true,
+                                                    &error);
+  if (error != base::PLATFORM_FILE_OK)
     return false;
   FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
       origin_url, type, true);
   if (!db)
     return false;
 
   file_util::FileEnumerator file_enum(src_root, true,
       static_cast<file_util::FileEnumerator::FileType>(
           file_util::FileEnumerator::FILES |
           file_util::FileEnumerator::DIRECTORIES));
@@ skipping 83 matching lines @@
     return true;
   }
   std::string key = GetOriginIdentifierFromURL(origin) + type_string;
   DirectoryMap::iterator iter = directories_.find(key);
   if (iter != directories_.end()) {
     FileSystemDirectoryDatabase* database = iter->second;
     directories_.erase(iter);
     delete database;
   }
 
-  FilePath path = GetDirectoryForOriginAndType(origin, type, false);
-  if (path.empty())
-    return true;
-  if (!file_util::DirectoryExists(path))
+  PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath path = GetDirectoryForOriginAndType(origin, type, false, &error);
+  if (path.empty() || error == base::PLATFORM_FILE_ERROR_NOT_FOUND)
     return true;
   return FileSystemDirectoryDatabase::DestroyDatabase(path);
 }
 
 // static
 int64 ObfuscatedFileUtil::ComputeFilePathCost(const FilePath& path) {
   return UsageForPath(VirtualPath::BaseName(path).value().size());
 }
 
 PlatformFileError ObfuscatedFileUtil::GetFileInfoInternal(
@@ skipping 50 matching lines @@
     FileSystemOperationContext* context,
     const FilePath& src_file_path,
     const GURL& dest_origin,
     FileSystemType dest_type,
     FileInfo* dest_file_info, int file_flags, PlatformFile* handle) {
   if (handle)
     *handle = base::kInvalidPlatformFileValue;
   FileSystemDirectoryDatabase* db = GetDirectoryDatabase(
       dest_origin, dest_type, true);
 
-  FilePath root = GetDirectoryForOriginAndType(dest_origin, dest_type, false);
-  if (root.empty())
+  PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath root = GetDirectoryForOriginAndType(dest_origin, dest_type, false,
+                                               &error);
+  if (error != base::PLATFORM_FILE_OK)
     return base::PLATFORM_FILE_ERROR_FAILED;

[kinuko, 2012/07/10 11:12:24]

Maybe we should just return the error?

[Bernhard Bauer, 2012/07/10 11:56:38]

Done.

 
   FilePath dest_local_path;
-  PlatformFileError error = GenerateNewLocalPath(
-      db, context, dest_origin, dest_type, &dest_local_path);
+  error = GenerateNewLocalPath(db, context, dest_origin, dest_type,
+                               &dest_local_path);
   if (error != base::PLATFORM_FILE_OK)
     return error;
 
   bool created = false;
   if (!src_file_path.empty()) {
     DCHECK(!file_flags);
     DCHECK(!handle);
     error = NativeFileUtil::CopyOrMoveFile(
         src_file_path, dest_local_path, true /* copy */);
     created = true;
@@ skipping 43 matching lines @@
     file_util::Delete(dest_local_path, false /* recursive */);
     return base::PLATFORM_FILE_ERROR_FAILED;
   }
   TouchDirectory(db, dest_file_info->parent_id);
 
   return base::PLATFORM_FILE_OK;
 }
 
 FilePath ObfuscatedFileUtil::DataPathToLocalPath(
     const GURL& origin, FileSystemType type, const FilePath& data_path) {
-  FilePath root = GetDirectoryForOriginAndType(origin, type, false);
-  if (root.empty())
-    return root;
+  PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath root = GetDirectoryForOriginAndType(origin, type, false, &error);
+  if (error != base::PLATFORM_FILE_OK)
+    return FilePath();
   return root.Append(data_path);
 }
 
 // TODO: How to do the whole validation-without-creation thing?  We may not have
 // quota even to create the database.  Ah, in that case don't even get here?
 // Still doesn't answer the quota issue, though.
 FileSystemDirectoryDatabase* ObfuscatedFileUtil::GetDirectoryDatabase(
     const GURL& origin, FileSystemType type, bool create) {
   std::string type_string = GetFileSystemTypeString(type);
   if (type_string.empty()) {
     LOG(WARNING) << "Unknown filesystem type requested:" << type;
     return NULL;
   }
   std::string key = GetOriginIdentifierFromURL(origin) + type_string;
   DirectoryMap::iterator iter = directories_.find(key);
   if (iter != directories_.end()) {
     MarkUsed();
     return iter->second;
   }
 
-  FilePath path = GetDirectoryForOriginAndType(origin, type, create);
-  if (path.empty())
+  PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath path = GetDirectoryForOriginAndType(origin, type, create, &error);
+  if (error != base::PLATFORM_FILE_OK)
     return NULL;
-  if (!file_util::DirectoryExists(path)) {
-    if (!file_util::CreateDirectory(path)) {
-      LOG(WARNING) << "Failed to origin+type directory: " << path.value();

[kinuko, 2012/07/10 11:12:24]

Can we preserve this WARNING when error != OK case?

[Bernhard Bauer, 2012/07/10 11:56:38]

Done.

-      return NULL;
-    }
-  }
   MarkUsed();
   FileSystemDirectoryDatabase* database = new FileSystemDirectoryDatabase(path);
   directories_[key] = database;
   return database;
 }
 
 FilePath ObfuscatedFileUtil::GetDirectoryForOrigin(
     const GURL& origin, bool create, base::PlatformFileError* error_code) {
   if (!InitOriginDatabase(create)) {
     if (error_code) {
@@ skipping 87 matching lines @@
     FileSystemDirectoryDatabase* db,
     FileSystemOperationContext* context,
     const GURL& origin,
     FileSystemType type,
     FilePath* local_path) {
   DCHECK(local_path);
   int64 number;
   if (!db || !db->GetNextInteger(&number))
     return base::PLATFORM_FILE_ERROR_FAILED;
 
+  PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath new_local_path = GetDirectoryForOriginAndType(origin, type, false,
+                                                         &error);
+  if (error != base::PLATFORM_FILE_OK)
+    return base::PLATFORM_FILE_ERROR_FAILED;
+
   // We use the third- and fourth-to-last digits as the directory.
   int64 directory_number = number % 10000 / 100;
-  FilePath new_local_path = GetDirectoryForOriginAndType(origin, type, false);
-  if (new_local_path.empty())
-    return base::PLATFORM_FILE_ERROR_FAILED;
-
   new_local_path = new_local_path.AppendASCII(
       StringPrintf("%02" PRId64, directory_number));
 
-  PlatformFileError error = NativeFileUtil::CreateDirectory(
+  error = NativeFileUtil::CreateDirectory(
       new_local_path, false /* exclusive */, false /* recursive */);
   if (error != base::PLATFORM_FILE_OK)
     return error;
 
   *local_path = new_local_path.AppendASCII(StringPrintf("%08" PRId64, number));
   return base::PLATFORM_FILE_OK;
 }
 
 }  // namespace fileapi