Simplify directory path accounting.

webkit/fileapi/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/file_system_file_util.h"
 
 #include <stack>
 #include <vector>
 
 #include "base/file_util_proxy.h"
@@ skipping 118 matching lines @@
     // http://code.google.com/p/chromium-os/issues/detail?id=15948
     // This currently just prevents a file from showing up at all
     // if it's a link, hence preventing arbitary 'read' exploits.
     if (!file_util::IsLink(file_path.Append(entry.name)))
       entries->push_back(entry);
   }
   return base::PLATFORM_FILE_OK;
 }
 
 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;
 
   bool path_exists = file_util::PathExists(file_path);
   if (exclusive && path_exists)
     return base::PLATFORM_FILE_ERROR_EXISTS;
 
   // If file exists at the path.
   if (path_exists && !file_util::DirectoryExists(file_path))
@@ skipping 90 matching lines @@
 PlatformFileError
 FileSystemFileUtil::PerformCommonCheckAndPreparationForMoveAndCopy(
     FileSystemOperationContext* context,
     const FilePath& src_file_path,
     const FilePath& dest_file_path) {
   bool same_file_system =
      (context->src_origin_url() == context->dest_origin_url()) &&
      (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.
   if (same_file_system && src_file_path.IsParent(dest_file_path))
     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;
 
   // |src_file_path| exists and is a file.
   // |dest_file_path| exists and is a directory.
   if (!src_is_directory && dest_is_directory)
     return base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
 
   // It is an error to copy/move an entry into the same path.
   if (same_file_system && (src_file_path.value() == dest_file_path.value()))
     return base::PLATFORM_FILE_ERROR_EXISTS;
 
   if (dest_is_directory) {
     // It is an error to copy/move an entry to a non-empty directory.
     // Otherwise the copy/move attempt must overwrite the destination, but
     // 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;
 }
 
 PlatformFileError FileSystemFileUtil::CopyOrMoveFile(
       FileSystemOperationContext* unused,
       const FilePath& src_file_path,
       const FilePath& dest_file_path,
       bool copy) {
   if (copy) {
@@ skipping 13 matching lines @@
       const FilePath& dest_file_path) {
   return CopyOrMoveFile(context, src_file_path, dest_file_path, true);
 }
 
 PlatformFileError FileSystemFileUtil::CopyOrMoveDirectory(
       FileSystemOperationContext* context,
       const FilePath& src_file_path,
       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());
 
   // Re-check PerformCommonCheckAndPreparationForMoveAndCopy() by DCHECK.
   DCHECK(DirectoryExists(context, src_file_path));
   DCHECK(ParentExists(dest_context.get(), dest_util, dest_file_path));
   DCHECK(!dest_util->PathExists(dest_context.get(), dest_file_path));
   if ((context->src_origin_url() == context->dest_origin_url()) &&
       (context->src_type() == context->dest_type()))
     DCHECK(!src_file_path.IsParent(dest_file_path));
 
   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;
   }
 
   scoped_ptr<AbstractFileEnumerator> file_enum(
       CreateFileEnumerator(context, src_file_path));
   FilePath src_file_path_each;
   while (!(src_file_path_each = file_enum->Next()).empty()) {
     FilePath dest_file_path_each(dest_file_path);
     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;
     }
   }
 
   if (!copy) {
     PlatformFileError error = Delete(context, src_file_path, true);
     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;
 }
 
 PlatformFileError FileSystemFileUtil::CopyOrMoveFileHelper(
     FileSystemOperationContext* context,
     const FilePath& src_file_path,
     const FilePath& dest_file_path,
     bool copy) {
   // CopyOrMoveFile here is the virtual overridden member function.
   if ((context->src_origin_url() == context->dest_origin_url()) &&
@@ skipping 55 matching lines @@
   scoped_ptr<AbstractFileEnumerator> file_enum(
       CreateFileEnumerator(context, file_path));
   FilePath file_path_each;
 
   std::stack<FilePath> directories;
   while (!(file_path_each = file_enum->Next()).empty()) {
     if (file_enum->IsDirectory()) {
       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)
         return error;
     }
   }
 
   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)
       return error;
     directories.pop();
   }
   return DeleteSingleDirectory(context, file_path);
 }
 
 bool FileSystemFileUtil::PathExists(
@@ skipping 46 matching lines @@
 FileSystemFileUtil::CreateFileEnumerator(
     FileSystemOperationContext* unused,
     const FilePath& root_path) {
   return new FileSystemFileEnumerator(
       root_path, true, static_cast<file_util::FileEnumerator::FileType>(
       file_util::FileEnumerator::FILES |
       file_util::FileEnumerator::DIRECTORIES));
 }
 
 }  // namespace fileapi