Recursive operation delegate continues operations with ignoring errors.

Previously recursive operation delegate aborted its operation while an error has happened.
This CL adds a method to run a recursive operation with ignoring errors.

BUG=499642
TEST=content_unittests:RecursiveOperationDelegateTest.*

Review URL: https://codereview.chromium.org/1184343002

Cr-Commit-Position: refs/heads/master@{#334783}

4 files changed, 194 insertions, 17 deletions

diff --git a/content/browser/fileapi/recursive_operation_delegate_unittest.cc b/content/browser/fileapi/recursive_operation_delegate_unittest.cc
index 54b9626..ba712bb 100644
--- a/content/browser/fileapi/recursive_operation_delegate_unittest.cc
+++ b/content/browser/fileapi/recursive_operation_delegate_unittest.cc
@@ -55,9 +55,19 @@ class LoggingRecursiveOperation : public storage::RecursiveOperationDelegate {
 
   void RunRecursively() override { StartRecursiveOperation(root_, callback_); }
 
+  void RunRecursivelyWithIgnoringError(const ErrorCallback& error_callback) {
+    StartRecursiveOperationWithIgnoringError(root_, error_callback, callback_);
+  }
+
   void ProcessFile(const FileSystemURL& url,
                    const StatusCallback& callback) override {
     RecordLogEntry(LogEntry::PROCESS_FILE, url);
+
+    if (error_url_.is_valid() && error_url_ == url) {
+      callback.Run(base::File::FILE_ERROR_FAILED);
+      return;
+    }
+
     operation_runner()->GetMetadata(
         url,
         base::Bind(&LoggingRecursiveOperation::DidGetMetadata,
@@ -76,6 +86,8 @@ class LoggingRecursiveOperation : public storage::RecursiveOperationDelegate {
     callback.Run(base::File::FILE_OK);
   }
 
+  void SetEntryToFail(const FileSystemURL& url) { error_url_ = url; }
+
  private:
   void RecordLogEntry(LogEntry::Type type, const FileSystemURL& url) {
     LogEntry entry;
@@ -100,6 +112,7 @@ class LoggingRecursiveOperation : public storage::RecursiveOperationDelegate {
   FileSystemURL root_;
   StatusCallback callback_;
   std::vector<LogEntry> log_entries_;
+  FileSystemURL error_url_;
 
   base::WeakPtrFactory<LoggingRecursiveOperation> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(LoggingRecursiveOperation);
@@ -111,6 +124,15 @@ void ReportStatus(base::File::Error* out_error,
   *out_error = error;
 }
 
+typedef std::pair<FileSystemURL, base::File::Error> ErrorEntry;
+
+void ReportError(std::vector<ErrorEntry>* out_errors,
+                 const FileSystemURL& url,
+                 base::File::Error error) {
+  DCHECK(out_errors);
+  out_errors->push_back(std::make_pair(url, error));
+}
+
 // To test the Cancel() during operation, calls Cancel() of |operation|
 // after |counter| times message posting.
 void CallCancelLater(storage::RecursiveOperationDelegate* operation,
@@ -276,4 +298,103 @@ TEST_F(RecursiveOperationDelegateTest, Cancel) {
   ASSERT_EQ(base::File::FILE_ERROR_ABORT, error);
 }
 
+TEST_F(RecursiveOperationDelegateTest, AbortWithError) {
+  FileSystemURL src_root(CreateDirectory("src"));
+  FileSystemURL src_dir1(CreateDirectory("src/dir1"));
+  FileSystemURL src_file1(CreateFile("src/file1"));
+  FileSystemURL src_file2(CreateFile("src/dir1/file2"));
+  FileSystemURL src_file3(CreateFile("src/dir1/file3"));
+
+  base::File::Error error = base::File::FILE_ERROR_FAILED;
+  scoped_ptr<FileSystemOperationContext> context = NewContext();
+  scoped_ptr<LoggingRecursiveOperation> operation(
+      new LoggingRecursiveOperation(context->file_system_context(), src_root,
+                                    base::Bind(&ReportStatus, &error)));
+  operation->SetEntryToFail(src_file1);
+  operation->RunRecursively();
+  base::RunLoop().RunUntilIdle();
+
+  ASSERT_EQ(base::File::FILE_ERROR_FAILED, error);
+
+  // Confirm that operation has been aborted in the middle.
+  const std::vector<LoggingRecursiveOperation::LogEntry>& log_entries =
+      operation->log_entries();
+  ASSERT_EQ(3U, log_entries.size());
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_FILE,
+            log_entries[0].type);
+  EXPECT_EQ(src_root, log_entries[0].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_DIRECTORY,
+            log_entries[1].type);
+  EXPECT_EQ(src_root, log_entries[1].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_FILE,
+            log_entries[2].type);
+  EXPECT_EQ(src_file1, log_entries[2].url);
+}
+
+TEST_F(RecursiveOperationDelegateTest, ContinueWithError) {
+  FileSystemURL src_root(CreateDirectory("src"));
+  FileSystemURL src_dir1(CreateDirectory("src/dir1"));
+  FileSystemURL src_file1(CreateFile("src/file1"));
+  FileSystemURL src_file2(CreateFile("src/dir1/file2"));
+  FileSystemURL src_file3(CreateFile("src/dir1/file3"));
+
+  base::File::Error error = base::File::FILE_ERROR_FAILED;
+  std::vector<ErrorEntry> errors;
+  scoped_ptr<FileSystemOperationContext> context = NewContext();
+  scoped_ptr<LoggingRecursiveOperation> operation(
+      new LoggingRecursiveOperation(context->file_system_context(), src_root,
+                                    base::Bind(&ReportStatus, &error)));
+  operation->SetEntryToFail(src_file1);
+  operation->RunRecursivelyWithIgnoringError(base::Bind(&ReportError, &errors));
+  base::RunLoop().RunUntilIdle();
+
+  // Error code should be base::File::FILE_ERROR_FAILED.
+  ASSERT_EQ(base::File::FILE_ERROR_FAILED, error);
+
+  // Error callback should be called.
+  ASSERT_EQ(1U, errors.size());
+  ASSERT_EQ(src_file1, errors[0].first);
+  ASSERT_EQ(base::File::FILE_ERROR_FAILED, errors[0].second);
+
+  // Confirm that operation continues after the error.
+  const std::vector<LoggingRecursiveOperation::LogEntry>& log_entries =
+      operation->log_entries();
+  ASSERT_EQ(8U, log_entries.size());
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_FILE,
+            log_entries[0].type);
+  EXPECT_EQ(src_root, log_entries[0].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_DIRECTORY,
+            log_entries[1].type);
+  EXPECT_EQ(src_root, log_entries[1].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_FILE,
+            log_entries[2].type);
+  EXPECT_EQ(src_file1, log_entries[2].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_DIRECTORY,
+            log_entries[3].type);
+  EXPECT_EQ(src_dir1, log_entries[3].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_FILE,
+            log_entries[4].type);
+  EXPECT_EQ(src_file3, log_entries[4].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::PROCESS_FILE,
+            log_entries[5].type);
+  EXPECT_EQ(src_file2, log_entries[5].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::POST_PROCESS_DIRECTORY,
+            log_entries[6].type);
+  EXPECT_EQ(src_dir1, log_entries[6].url);
+
+  EXPECT_EQ(LoggingRecursiveOperation::LogEntry::POST_PROCESS_DIRECTORY,
+            log_entries[7].type);
+  EXPECT_EQ(src_root, log_entries[7].url);
+}
+
 }  // namespace content
diff --git a/storage/browser/fileapi/file_system_operation.h b/storage/browser/fileapi/file_system_operation.h
index 4b38da0..bc5bc87 100644
--- a/storage/browser/fileapi/file_system_operation.h
+++ b/storage/browser/fileapi/file_system_operation.h
@@ -68,6 +68,12 @@ class FileSystemOperation {
   // Used for CreateFile(), etc. |result| is the return code of the operation.
   typedef base::Callback<void(base::File::Error result)> StatusCallback;
 
+  // Called when an error had happened during operation.
+  // |url| is the url of processed entry.
+  // |error| is the error code of the failed operation.
+  typedef base::Callback<void(const FileSystemURL& url,
+                              base::File::Error error)> ErrorCallback;
+
   // Used for GetMetadata(). |result| is the return code of the operation,
   // |file_info| is the obtained file info.
   typedef base::Callback<
diff --git a/storage/browser/fileapi/recursive_operation_delegate.cc b/storage/browser/fileapi/recursive_operation_delegate.cc
index 609df31..a3cb1a86 100644
--- a/storage/browser/fileapi/recursive_operation_delegate.cc
+++ b/storage/browser/fileapi/recursive_operation_delegate.cc
@@ -21,7 +21,9 @@ RecursiveOperationDelegate::RecursiveOperationDelegate(
     FileSystemContext* file_system_context)
     : file_system_context_(file_system_context),
       inflight_operations_(0),
-      canceled_(false) {
+      canceled_(false),
+      ignore_error_(false),
+      failed_some_operations_(false) {
 }
 
 RecursiveOperationDelegate::~RecursiveOperationDelegate() {
@@ -40,11 +42,29 @@ void RecursiveOperationDelegate::StartRecursiveOperation(
   DCHECK_EQ(0, inflight_operations_);
 
   callback_ = callback;
+
+  TryProcessFile(root);
+}
+
+void RecursiveOperationDelegate::StartRecursiveOperationWithIgnoringError(
+    const FileSystemURL& root,
+    const ErrorCallback& error_callback,
+    const StatusCallback& status_callback) {
+  DCHECK(pending_directory_stack_.empty());
+  DCHECK(pending_files_.empty());
+  DCHECK_EQ(0, inflight_operations_);
+
+  error_callback_ = error_callback;
+  callback_ = status_callback;
+  ignore_error_ = true;
+
+  TryProcessFile(root);
+}
+
+void RecursiveOperationDelegate::TryProcessFile(const FileSystemURL& root) {
   ++inflight_operations_;
-  ProcessFile(
-      root,
-      base::Bind(&RecursiveOperationDelegate::DidTryProcessFile,
-                 AsWeakPtr(), root));
+  ProcessFile(root, base::Bind(&RecursiveOperationDelegate::DidTryProcessFile,
+                               AsWeakPtr(), root));
 }
 
 FileSystemOperationRunner* RecursiveOperationDelegate::operation_runner() {
@@ -159,23 +179,31 @@ void RecursiveOperationDelegate::ProcessPendingFiles() {
     ++inflight_operations_;
     current_task_runner->PostTask(
         FROM_HERE,
-        base::Bind(&RecursiveOperationDelegate::ProcessFile,
-                   AsWeakPtr(), pending_files_.front(),
+        base::Bind(&RecursiveOperationDelegate::ProcessFile, AsWeakPtr(),
+                   pending_files_.front(),
                    base::Bind(&RecursiveOperationDelegate::DidProcessFile,
-                              AsWeakPtr())));
+                              AsWeakPtr(), pending_files_.front())));
     pending_files_.pop();
   }
 }
 
-void RecursiveOperationDelegate::DidProcessFile(
-    base::File::Error error) {
+void RecursiveOperationDelegate::DidProcessFile(const FileSystemURL& url,
+                                                base::File::Error error) {
   --inflight_operations_;
+
   if (error != base::File::FILE_OK) {
-    // If an error occurs, invoke Done immediately (even if there remain
-    // running operations). It is because in the callback, this instance is
-    // deleted.
-    Done(error);
-    return;
+    if (!ignore_error_) {
+      // If an error occurs, invoke Done immediately (even if there remain
+      // running operations). It is because in the callback, this instance is
+      // deleted.
+      Done(error);
+      return;
+    } else {
+      failed_some_operations_ = true;
+
+      if (!error_callback_.is_null())
+        error_callback_.Run(url, error);
+    }
   }
 
   ProcessPendingFiles();
@@ -234,7 +262,10 @@ void RecursiveOperationDelegate::Done(base::File::Error error) {
   if (canceled_ && error == base::File::FILE_OK) {
     callback_.Run(base::File::FILE_ERROR_ABORT);
   } else {
-    callback_.Run(error);
+    if (ignore_error_ && failed_some_operations_)
+      callback_.Run(base::File::FILE_ERROR_FAILED);
+    else
+      callback_.Run(error);
   }
 }
 
diff --git a/storage/browser/fileapi/recursive_operation_delegate.h b/storage/browser/fileapi/recursive_operation_delegate.h
index f7f07c2..d120c33 100644
--- a/storage/browser/fileapi/recursive_operation_delegate.h
+++ b/storage/browser/fileapi/recursive_operation_delegate.h
@@ -28,6 +28,7 @@ class STORAGE_EXPORT RecursiveOperationDelegate
     : public base::SupportsWeakPtr<RecursiveOperationDelegate> {
  public:
   typedef FileSystemOperation::StatusCallback StatusCallback;
+  typedef FileSystemOperation::ErrorCallback ErrorCallback;
   typedef FileSystemOperation::FileEntryList FileEntryList;
 
   virtual ~RecursiveOperationDelegate();
@@ -108,6 +109,20 @@ class STORAGE_EXPORT RecursiveOperationDelegate
   void StartRecursiveOperation(const FileSystemURL& root,
                                const StatusCallback& callback);
 
+  // Starts to process files/directories recursively from the given |root|.
+  // Compared with StartRecursiveOperation, this continues operation with
+  // ignoring erros of ProcessFile.
+  //
+  // |error_callback| is fired when a ProcessFile has failed in the middle of
+  // operations. If some errors had happened, |status_callback| is fired with
+  // base::File::FILE_ERROR_FAILED at the end.
+  //
+  // TODO(yawano): Handle errors of ProcessDirectory as well.
+  void StartRecursiveOperationWithIgnoringError(
+      const FileSystemURL& root,
+      const ErrorCallback& error_callback,
+      const StatusCallback& status_callback);
+
   FileSystemContext* file_system_context() { return file_system_context_; }
   const FileSystemContext* file_system_context() const {
     return file_system_context_;
@@ -120,6 +135,7 @@ class STORAGE_EXPORT RecursiveOperationDelegate
   virtual void OnCancel();
 
  private:
+  void TryProcessFile(const FileSystemURL& root);
   void DidTryProcessFile(const FileSystemURL& root,
                          base::File::Error error);
   void ProcessNextDirectory();
@@ -129,7 +145,7 @@ class STORAGE_EXPORT RecursiveOperationDelegate
                         const FileEntryList& entries,
                         bool has_more);
   void ProcessPendingFiles();
-  void DidProcessFile(base::File::Error error);
+  void DidProcessFile(const FileSystemURL& url, base::File::Error error);
   void ProcessSubDirectory();
   void DidPostProcessDirectory(base::File::Error error);
 
@@ -138,11 +154,14 @@ class STORAGE_EXPORT RecursiveOperationDelegate
 
   FileSystemContext* file_system_context_;
   StatusCallback callback_;
+  ErrorCallback error_callback_;
   std::stack<FileSystemURL> pending_directories_;
   std::stack<std::queue<FileSystemURL> > pending_directory_stack_;
   std::queue<FileSystemURL> pending_files_;
   int inflight_operations_;
   bool canceled_;
+  bool ignore_error_;
+  bool failed_some_operations_;
 
   DISALLOW_COPY_AND_ASSIGN(RecursiveOperationDelegate);
 };