Quota calculation is wrong in cross-filesystem operation

webkit/fileapi/local_file_system_operation.cc

Index: webkit/fileapi/local_file_system_operation.cc
diff --git a/webkit/fileapi/local_file_system_operation.cc b/webkit/fileapi/local_file_system_operation.cc
index 3d9498b36c5682395b485cc3c8eb8fd6c5b71277..ad9a7fb0f459f2b3160367df3d0a056ecbdb2c2c 100644
--- a/webkit/fileapi/local_file_system_operation.cc
+++ b/webkit/fileapi/local_file_system_operation.cc
@@ -36,7 +36,7 @@ bool IsMediaFileSystemType(FileSystemType type) {
 }
 
 bool IsCrossOperationAllowed(FileSystemType src_type,
-                                                FileSystemType dest_type) {
+                             FileSystemType dest_type) {
   // If two types are supposed to run on different task runners we should not
   // allow cross FileUtil operations at this layer.
   return IsMediaFileSystemType(src_type) == IsMediaFileSystemType(dest_type);
@@ -116,6 +116,7 @@ void LocalFileSystemOperation::Copy(const FileSystemURL& src_url,
                                     const FileSystemURL& dest_url,
                                     const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationCopy));
+  is_cross_operation_ = (src_url.type() != dest_url.type());
 
   base::PlatformFileError result = SetUp(src_url, &src_util_, SETUP_FOR_READ);
   if (result == base::PLATFORM_FILE_OK)
@@ -141,8 +142,19 @@ void LocalFileSystemOperation::Move(const FileSystemURL& src_url,
                                     const FileSystemURL& dest_url,
                                     const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationMove));
+  is_cross_operation_ = (src_url.type() != dest_url.type());
+
   scoped_ptr<LocalFileSystemOperation> deleter(this);
 
+  // Temporarily disables cross-filesystem move.
+  // TODO(kinuko,tzik,kinaba): This special handling must be removed once
+  // we support saner cross-filesystem operation.
+  // (See http://crbug.com/130055)

[tzik, 2012/11/30 07:00:44]

Could you update the issue?

+  if (is_cross_operation_) {
+    callback.Run(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
+    return;
+  }
+
   base::PlatformFileError result = SetUp(src_url, &src_util_, SETUP_FOR_WRITE);
   if (result == base::PLATFORM_FILE_OK)
     result = SetUp(dest_url, &dest_util_, SETUP_FOR_CREATE);
@@ -155,17 +167,6 @@ void LocalFileSystemOperation::Move(const FileSystemURL& src_url,
     return;
   }
 
-  // Temporarily disables cross-filesystem move for sandbox filesystems.
-  // TODO(kinuko,tzik,kinaba): This special handling must be removed once
-  // we support saner cross-filesystem operation.
-  // (See http://crbug.com/130055)
-  if (src_url.type() != dest_url.type() &&
-      (src_url.type() == kFileSystemTypeTemporary ||
-       src_url.type() == kFileSystemTypePersistent)) {
-    callback.Run(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
-    return;
-  }
-
   GetUsageAndQuotaThenRunTask(
       dest_url,
       base::Bind(&LocalFileSystemOperation::DoMove,
@@ -455,6 +456,7 @@ LocalFileSystemOperation::LocalFileSystemOperation(
     : operation_context_(operation_context.Pass()),
       src_util_(NULL),
       dest_util_(NULL),
+      is_cross_operation_(false),
       peer_handle_(base::kNullProcessHandle),
       pending_operation_(kOperationNone),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
@@ -769,8 +771,14 @@ base::PlatformFileError LocalFileSystemOperation::SetUp(
     return base::PLATFORM_FILE_ERROR_SECURITY;
 
   if (mode == SETUP_FOR_READ) {
-    operation_context_->access_observers()->Notify(
-        &FileAccessObserver::OnAccess, MakeTuple(url));
+    // TODO(kinuko): This doesn't work well for cross-filesystem operation
+    // in the current architecture since the operation context (thus the
+    // observers) is configured for the destination URL while this method
+    // could be called for both src and dest URL.
+    if (!is_cross_operation_) {
+      operation_context_->access_observers()->Notify(
+          &FileAccessObserver::OnAccess, MakeTuple(url));
+    }
     return base::PLATFORM_FILE_OK;
   }