Add fileManagerPrivate.onCopyError event.

storage/browser/fileapi/copy_or_move_operation_delegate.cc

Index: storage/browser/fileapi/copy_or_move_operation_delegate.cc
diff --git a/storage/browser/fileapi/copy_or_move_operation_delegate.cc b/storage/browser/fileapi/copy_or_move_operation_delegate.cc
index 1cb92f5a838d371443aa749598db26de68449003..9c50a44c38942bcafc547c3ebc4752f410e91943 100644
--- a/storage/browser/fileapi/copy_or_move_operation_delegate.cc
+++ b/storage/browser/fileapi/copy_or_move_operation_delegate.cc
@@ -730,6 +730,8 @@ CopyOrMoveOperationDelegate::CopyOrMoveOperationDelegate(
     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),
@@ -737,6 +739,8 @@ CopyOrMoveOperationDelegate::CopyOrMoveOperationDelegate(
       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) {
@@ -773,7 +777,11 @@ void CopyOrMoveOperationDelegate::RunRecursively() {
   // 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(
@@ -802,6 +810,9 @@ void CopyOrMoveOperationDelegate::ProcessFile(
         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;
     }
@@ -900,6 +911,10 @@ void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
   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);