Quota calculation is wrong in cross-filesystem operation

webkit/fileapi/local_file_system_operation.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/local_file_system_operation.h"
 
 #include "base/bind.h"
 #include "base/single_thread_task_runner.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
@@ skipping 18 matching lines @@
 namespace fileapi {
 
 namespace {
 
 bool IsMediaFileSystemType(FileSystemType type) {
   return type == kFileSystemTypeNativeMedia ||
       type == kFileSystemTypeDeviceMedia;
 }
 
 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);
 }
 
 }  // namespace
 
 class LocalFileSystemOperation::ScopedUpdateNotifier {
  public:
   ScopedUpdateNotifier(FileSystemOperationContext* operation_context,
@@ skipping 59 matching lines @@
       url,
       base::Bind(&LocalFileSystemOperation::DoCreateDirectory,
                  base::Unretained(this), url, callback, exclusive, recursive),
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
 }
 
 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)
     result = SetUp(dest_url, &dest_util_, SETUP_FOR_CREATE);
   if (result == base::PLATFORM_FILE_OK) {
     if (!IsCrossOperationAllowed(src_url.type(), dest_url.type()))
       result = base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
   }
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
     delete this;
     return;
   }
 
   GetUsageAndQuotaThenRunTask(
       dest_url,
       base::Bind(&LocalFileSystemOperation::DoCopy,
                  base::Unretained(this), src_url, dest_url, callback),
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
 }
 
 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);
   if (result == base::PLATFORM_FILE_OK) {
     if (!IsCrossOperationAllowed(src_url.type(), dest_url.type()))
       result = base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
   }
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result);
     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,
                  base::Unretained(deleter.release()),
                  src_url, dest_url, callback),
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
 }
 
 void LocalFileSystemOperation::DirectoryExists(const FileSystemURL& url,
                                                const StatusCallback& callback) {
@@ skipping 269 matching lines @@
                  callback),
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED));
 }
 
 LocalFileSystemOperation::LocalFileSystemOperation(
     FileSystemContext* file_system_context,
     scoped_ptr<FileSystemOperationContext> operation_context)
     : 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)) {
   DCHECK(operation_context_.get());
 }
 
 void LocalFileSystemOperation::GetUsageAndQuotaThenRunTask(
     const FileSystemURL& url,
     const base::Closure& task,
     const base::Closure& error_callback) {
@@ skipping 294 matching lines @@
           url.type())->IsAccessAllowed(url))
     return base::PLATFORM_FILE_ERROR_SECURITY;
 
   DCHECK(file_util);
   if (!*file_util)
     *file_util = file_system_context()->GetFileUtil(url.type());
   if (!*file_util)
     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;
   }
 
   DCHECK(mode == SETUP_FOR_WRITE || mode == SETUP_FOR_CREATE);
 
   scoped_update_notifiers_.push_back(new ScopedUpdateNotifier(
       operation_context_.get(), url));
 
   // Any write access is disallowed on the root path.
   if (url.path().value().length() == 0 ||
@@ skipping 13 matching lines @@
 }
 
 bool LocalFileSystemOperation::SetPendingOperationType(OperationType type) {
   if (pending_operation_ != kOperationNone)
     return false;
   pending_operation_ = type;
   return true;
 }
 
 }  // namespace fileapi