Add fileManagerPrivate.onCopyError event.

storage/browser/fileapi/copy_or_move_operation_delegate.cc

 // Copyright (c) 2013 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 "storage/browser/fileapi/copy_or_move_operation_delegate.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ skipping 712 matching lines @@
   else
     Read(callback);
 }
 
 CopyOrMoveOperationDelegate::CopyOrMoveOperationDelegate(
     FileSystemContext* file_system_context,
     const FileSystemURL& src_root,
     const FileSystemURL& dest_root,
     OperationType operation_type,
     CopyOrMoveOption option,
+    const bool continue_with_error,

[mtomasz, 2015/06/20 02:42:56]

Bools are hard to read. Can we made an enum, like enum ErrorBehavior {
  ERROR_BEHAVIOR_TERMINATE,
  ERROR_BEHAVIOR_CONTINUE
}?

+    const CopyOrMoveErrorCallback& error_callback,
     const CopyProgressCallback& progress_callback,
     const StatusCallback& callback)
     : RecursiveOperationDelegate(file_system_context),
       src_root_(src_root),
       dest_root_(dest_root),
       operation_type_(operation_type),
       option_(option),
+      continue_with_error_(continue_with_error),
+      error_callback_(error_callback),
       progress_callback_(progress_callback),
       callback_(callback),
       weak_factory_(this) {
   same_file_system_ = src_root_.IsInSameFileSystem(dest_root_);
 }
 
 CopyOrMoveOperationDelegate::~CopyOrMoveOperationDelegate() {
   STLDeleteElements(&running_copy_set_);
 }
 
@@ skipping 16 matching lines @@
     // wants to return success and we have a code path that returns error in
     // Blink for JS (http://crbug.com/329517).
     callback_.Run(base::File::FILE_OK);
     return;
   }
 
   // Start to process the source directory recursively.
   // TODO(kinuko): This could be too expensive for same_file_system_==true
   // and operation==MOVE case, probably we can just rename the root directory.
   // http://crbug.com/172187
-  StartRecursiveOperation(src_root_, callback_);
+  if (continue_with_error_)
+    StartRecursiveOperationWithIgnoringError(src_root_, ErrorCallback(),

[mtomasz, 2015/06/20 02:42:56]

I'm thinking whether we can simplify this logic. Instead of having two different
code paths, we could make the progress callback have a return value. The
returned value would determine whether to terminate copying or continue. Would
it work?

[yawano, 2015/06/22 02:28:27]

I don't like the idea that the progress callback returns a value. It seems to
make the behavior of recursive operation delegate more complex, i.e. by passing
the option at the start call, we can see how it works at a glance.

Because it takes a new error callback, I added
StartRecursiveOperationWithIgnoringError method. But since the new ErrorCallback
is not used for now, we can remove it and integrate these two methods into one
method.

StartRecursiveOperation(const FileSystemURL, const StatusCallback, const
ErrorBehavior).

@mtomasz, @tzik: WDYT?

[mtomasz, 2015/06/22 02:31:41]

SGTM

+                                             callback_);
+  else
+    StartRecursiveOperation(src_root_, callback_);
 }
 
 void CopyOrMoveOperationDelegate::ProcessFile(
     const FileSystemURL& src_url,
     const StatusCallback& callback) {
   if (!progress_callback_.is_null()) {
     progress_callback_.Run(
         FileSystemOperation::BEGIN_COPY_ENTRY, src_url, FileSystemURL(), 0);
   }
 
   FileSystemURL dest_url = CreateDestURL(src_url);
   CopyOrMoveImpl* impl = NULL;
   if (same_file_system_ &&
       (file_system_context()
            ->GetFileSystemBackend(src_url.type())
            ->HasInplaceCopyImplementation(src_url.type()) ||
        operation_type_ == OPERATION_MOVE)) {
     impl = new CopyOrMoveOnSameFileSystemImpl(
         operation_runner(), operation_type_, src_url, dest_url, option_,
         base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
                    weak_factory_.GetWeakPtr(), src_url));
   } else {
     // Cross filesystem case.
     base::File::Error error = base::File::FILE_ERROR_FAILED;
     CopyOrMoveFileValidatorFactory* validator_factory =
         file_system_context()->GetCopyOrMoveFileValidatorFactory(
             dest_root_.type(), &error);
     if (error != base::File::FILE_OK) {
+      if (!error_callback_.is_null())
+        error_callback_.Run(error, src_url, dest_url);
+
       callback.Run(error);
       return;
     }
 
     if (!validator_factory) {
       scoped_ptr<storage::FileStreamReader> reader =
           file_system_context()->CreateFileStreamReader(
               src_url, 0 /* offset */, storage::kMaximumLength, base::Time());
       scoped_ptr<FileStreamWriter> writer =
           file_system_context()->CreateFileStreamWriter(dest_url, 0);
@@ skipping 78 matching lines @@
 
 void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
     const FileSystemURL& src_url,
     const FileSystemURL& dest_url,
     const StatusCallback& callback,
     CopyOrMoveImpl* impl,
     base::File::Error error) {
   running_copy_set_.erase(impl);
   delete impl;
 
+  if (!error_callback_.is_null() && error != base::File::FILE_OK &&
+      error != base::File::FILE_ERROR_NOT_A_FILE)
+    error_callback_.Run(error, src_url, dest_url);
+
   if (!progress_callback_.is_null() && error == base::File::FILE_OK) {
     progress_callback_.Run(
         FileSystemOperation::END_COPY_ENTRY, src_url, dest_url, 0);
   }
 
   callback.Run(error);
 }
 
 void CopyOrMoveOperationDelegate::DidTryRemoveDestRoot(
     const StatusCallback& callback,
@@ skipping 103 matching lines @@
   base::FilePath relative = dest_root_.virtual_path();
   src_root_.virtual_path().AppendRelativePath(src_url.virtual_path(),
                                               &relative);
   return file_system_context()->CreateCrackedFileSystemURL(
       dest_root_.origin(),
       dest_root_.mount_type(),
       relative);
 }
 
 }  // namespace storage