Simplify directory path accounting.

webkit/fileapi/file_system_file_util.cc

Index: webkit/fileapi/file_system_file_util.cc
diff --git a/webkit/fileapi/file_system_file_util.cc b/webkit/fileapi/file_system_file_util.cc
index 3600a8fd414dc8a13e47e7c19c159f87dd4a439f..0b5833408eec2b2485f6057f392c0e2c9c63bfc3 100644
--- a/webkit/fileapi/file_system_file_util.cc
+++ b/webkit/fileapi/file_system_file_util.cc
@@ -136,13 +136,10 @@ PlatformFileError FileSystemFileUtil::ReadDirectory(
 }
 
 PlatformFileError FileSystemFileUtil::CreateDirectory(
-    FileSystemOperationContext* fs_context,
+    FileSystemOperationContext* unused,
     const FilePath& file_path,
     bool exclusive,
     bool recursive) {
-  if (fs_context->do_not_write_actually())
-    return base::PLATFORM_FILE_OK;
-
   // If parent dir of file doesn't exist.
   if (!recursive && !file_util::PathExists(file_path.DirName()))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
@@ -253,21 +250,26 @@ FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
      (context->src_type() == context->dest_type());
   FileSystemFileUtil* dest_util = context->dest_file_system_file_util();
   DCHECK(dest_util);
-  if (same_file_system)
+  scoped_ptr<FileSystemOperationContext> local_dest_context;
+  FileSystemOperationContext* dest_context = NULL;
+  if (same_file_system) {
+    dest_context = context;
     DCHECK(context->src_file_system_file_util() ==
          context->dest_file_system_file_util());
-  // All the single-path virtual FSFU methods expect the context information
-  // to be in the src_* variables, not the dest_* variables, so we have to
-  // make a new context if we want to call them on the dest_file_path.
-  scoped_ptr<FileSystemOperationContext> dest_context(
-      context->CreateInheritedContextForDest());
+  } else {
+    local_dest_context.reset(context->CreateInheritedContextForDest());
+    // All the single-path virtual FSFU methods expect the context information
+    // to be in the src_* variables, not the dest_* variables, so we have to
+    // make a new context if we want to call them on the dest_file_path.
+    dest_context = local_dest_context.get();
+  }
 
   // Exits earlier if the source path does not exist.
   if (!PathExists(context, src_file_path))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   // The parent of the |dest_file_path| does not exist.
-  if (!ParentExists(dest_context.get(), dest_util, dest_file_path))
+  if (!ParentExists(dest_context, dest_util, dest_file_path))
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
 
   // It is an error to try to copy/move an entry into its child.
@@ -275,14 +277,14 @@ FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
     return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
 
   // Now it is ok to return if the |dest_file_path| does not exist.
-  if (!dest_util->PathExists(dest_context.get(), dest_file_path))
+  if (!dest_util->PathExists(dest_context, dest_file_path))
     return base::PLATFORM_FILE_OK;
 
   // |src_file_path| exists and is a directory.
   // |dest_file_path| exists and is a file.
   bool src_is_directory = DirectoryExists(context, src_file_path);
   bool dest_is_directory =
-      dest_util->DirectoryExists(dest_context.get(), dest_file_path);
+      dest_util->DirectoryExists(dest_context, dest_file_path);
   if (src_is_directory && !dest_is_directory)
     return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
 
@@ -301,14 +303,16 @@ FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
     // the file_util's Copy or Move method doesn't perform overwrite
     // on all platforms, so we delete the destination directory here.
     // TODO(kinuko): may be better to change the file_util::{Copy,Move}.
-    PlatformFileError error = dest_util->Delete(
-        dest_context.get(), dest_file_path, false /* recursive */);
-    context->ImportAllowedBytesGrowth(*dest_context);
-    if (base::PLATFORM_FILE_OK != error) {
-      if (!dest_util->IsDirectoryEmpty(dest_context.get(), dest_file_path))
+    if (base::PLATFORM_FILE_OK !=
+        dest_util->Delete(dest_context, dest_file_path,
+                          false /* recursive */)) {
+      if (!dest_util->IsDirectoryEmpty(dest_context, dest_file_path))
         return base::PLATFORM_FILE_ERROR_NOT_EMPTY;
       return base::PLATFORM_FILE_ERROR_FAILED;
     }
+    // Reflect changes in usage back to the original context.
+    if (!same_file_system)
+      context->set_allowed_bytes_growth(dest_context->allowed_bytes_growth());
   }
   return base::PLATFORM_FILE_OK;
 }
@@ -342,6 +346,9 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
       const FilePath& dest_file_path,
       bool copy) {
   FileSystemFileUtil* dest_util = context->dest_file_system_file_util();
+  // All the single-path virtual FSFU methods expect the context information to
+  // be in the src_* variables, not the dest_* variables, so we have to make a
+  // new context if we want to call them on the dest_file_path.
   scoped_ptr<FileSystemOperationContext> dest_context(
       context->CreateInheritedContextForDest());
 
@@ -356,7 +363,6 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
   if (!dest_util->DirectoryExists(dest_context.get(), dest_file_path)) {
     PlatformFileError error = dest_util->CreateDirectory(dest_context.get(),
         dest_file_path, false, false);
-    context->ImportAllowedBytesGrowth(*dest_context);
     if (error != base::PLATFORM_FILE_OK)
       return error;
   }
@@ -369,21 +375,13 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
     src_file_path.AppendRelativePath(src_file_path_each, &dest_file_path_each);
 
     if (file_enum->IsDirectory()) {
-      scoped_ptr<FileSystemOperationContext> new_directory_context(
-          dest_context->CreateInheritedContextWithNewVirtualPaths(
-              dest_file_path_each, FilePath()));
-      PlatformFileError error = dest_util->CreateDirectory(
-          new_directory_context.get(), dest_file_path_each, false, false);
-      context->ImportAllowedBytesGrowth(*new_directory_context);
+      PlatformFileError error = dest_util->CreateDirectory(dest_context.get(),
+          dest_file_path_each, false, false);

[Dai Mikurube (NOT FULLTIME), 2011/08/18 12:00:10]

The dest_context should be reflected back in the while loop since
CopyOrMoveFileHelper uses the original context.  Without reflecting back here,
CopyOrMoveFileHelper succeeds even if CreateDirectory uses up the quota.

I guess reflecting back just after operations with copied contexts is safer...

[ericu, 2011/08/18 22:30:36]

Ah, good catch--thanks!

Fixed.

       if (error != base::PLATFORM_FILE_OK)
         return error;
     } else {
-      scoped_ptr<FileSystemOperationContext> copy_context(
-          context->CreateInheritedContextWithNewVirtualPaths(
-              src_file_path_each, dest_file_path_each));
       PlatformFileError error = CopyOrMoveFileHelper(
-          copy_context.get(), src_file_path_each, dest_file_path_each, copy);
-      context->ImportAllowedBytesGrowth(*copy_context);
+          context, src_file_path_each, dest_file_path_each, copy);
       if (error != base::PLATFORM_FILE_OK)
         return error;
     }
@@ -394,6 +392,9 @@ PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
     if (error != base::PLATFORM_FILE_OK)
       return error;
   }
+
+  // Reflect changes in usage back to the original context.
+  context->set_allowed_bytes_growth(dest_context->allowed_bytes_growth());
   return base::PLATFORM_FILE_OK;
 }
 
@@ -469,12 +470,7 @@ PlatformFileError FileSystemFileUtil::DeleteDirectoryRecursive(
       directories.push(file_path_each);
     } else {
       // DeleteFile here is the virtual overridden member function.
-      scoped_ptr<FileSystemOperationContext> inherited_context(
-          context->CreateInheritedContextWithNewVirtualPaths(
-              file_path_each, FilePath()));
-      PlatformFileError error =
-          DeleteFile(inherited_context.get(), file_path_each);
-      context->ImportAllowedBytesGrowth(*inherited_context);
+      PlatformFileError error = DeleteFile(context, file_path_each);

[Dai Mikurube (NOT FULLTIME), 2011/08/18 12:00:10]

file_path_each is better to be passed as src_virtual_path in the context.  (Not
required if we remove *_virtual_path from FSOC in this change.)

[ericu, 2011/08/18 22:30:36]

*_virtual_path is unused in all Delete/Remove calls, so it's not necessary here.
 Let's just remove it all.  If it's OK with you, though, I'd like to do that in
a separate CL to keep this one from getting too big.

       if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND)
         return base::PLATFORM_FILE_ERROR_FAILED;
       else if (error != base::PLATFORM_FILE_OK)
@@ -483,12 +479,7 @@ PlatformFileError FileSystemFileUtil::DeleteDirectoryRecursive(
   }
 
   while (!directories.empty()) {
-    scoped_ptr<FileSystemOperationContext> inherited_context(
-        context->CreateInheritedContextWithNewVirtualPaths(
-            directories.top(), FilePath()));
-    PlatformFileError error =
-        DeleteSingleDirectory(inherited_context.get(), directories.top());
-    context->ImportAllowedBytesGrowth(*inherited_context);
+    PlatformFileError error = DeleteSingleDirectory(context, directories.top());
     if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND)
       return base::PLATFORM_FILE_ERROR_FAILED;
     else if (error != base::PLATFORM_FILE_OK)