[FileSystem] Add another copy-or-move validation hook for post-write.

webkit/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 "webkit/browser/fileapi/copy_or_move_operation_delegate.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "webkit/browser/fileapi/copy_or_move_file_validator.h"
 #include "webkit/browser/fileapi/file_system_context.h"
@@ skipping 93 matching lines @@
     callback_.Run(error);
     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_, base::Bind(&CopyOrMoveOperationDelegate::DidFinishCopy,
-                            AsWeakPtr(), src_root_, callback_));
+                            AsWeakPtr(), URLPair(src_root_, dest_root_),
+                            callback_));
 }
 
 void CopyOrMoveOperationDelegate::CopyOrMoveFile(const URLPair& url_pair,
                                             const StatusCallback& callback) {
   // Same filesystem case.
   if (same_file_system_) {
     if (operation_type_ == OPERATION_MOVE) {
       operation_runner()->MoveFileLocal(url_pair.src, url_pair.dest, callback);
     } else {
       operation_runner()->CopyFileLocal(url_pair.src, url_pair.dest, callback);
     }
     return;
   }
 
   // Cross filesystem case.
   // Perform CreateSnapshotFile, CopyInForeignFile and then calls
   // copy_callback which removes the source file if operation_type == MOVE.
   StatusCallback copy_callback =
       base::Bind(&CopyOrMoveOperationDelegate::DidFinishCopy, AsWeakPtr(),
-                 url_pair.src, callback);
+                 url_pair, callback);
   operation_runner()->CreateSnapshotFile(
       url_pair.src,
       base::Bind(&CopyOrMoveOperationDelegate::DidCreateSnapshot, AsWeakPtr(),
                  url_pair, copy_callback));
 }
 
 void CopyOrMoveOperationDelegate::DidCreateSnapshot(
     const URLPair& url_pair,
     const StatusCallback& callback,
     base::PlatformFileError error,
@@ skipping 37 matching lines @@
     base::PlatformFileError error) {
   if (error != base::PLATFORM_FILE_OK) {
     callback.Run(error);
     return;
   }
 
   operation_runner()->CopyInForeignFile(platform_path, dest, callback);
 }
 
 void CopyOrMoveOperationDelegate::DidFinishCopy(
-    const FileSystemURL& src,
+    const URLPair& url_pair,
     const StatusCallback& callback,
     base::PlatformFileError error) {
-  if (error != base::PLATFORM_FILE_OK ||
-      operation_type_ == OPERATION_COPY) {
+  if (error != base::PLATFORM_FILE_OK) {
     callback.Run(error);
     return;
   }
+
+  if (validator_.get()) {
+    operation_runner()->CreateSnapshotFile(
+        url_pair.dest,
+        base::Bind(&CopyOrMoveOperationDelegate::DoPostWriteValidation,
+                   AsWeakPtr(), url_pair, callback));
+  } else {
+    DidPostWriteValidation(url_pair, callback, base::PLATFORM_FILE_OK);
+  }
+}
+
+void CopyOrMoveOperationDelegate::DoPostWriteValidation(
+    const URLPair& url_pair,
+    const StatusCallback& callback,
+    base::PlatformFileError error,
+    const base::PlatformFileInfo& file_info,
+    const base::FilePath& platform_path,
+    const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref) {
+  // TODO(gbillock): need to check here that file_ref is null or anything?
+  if (error != base::PLATFORM_FILE_OK) {
+    operation_runner()->Remove(
+        url_pair.dest, true,
+        base::Bind(&CopyOrMoveOperationDelegate::DidRemoveDestForError,

[vandebo (ex-Chrome), 2013/07/02 20:15:02]

Shouldn't this take the callback and invoke it after the remove is done?

[Greg Billock, 2013/07/03 16:08:27]

I want to return the validation error, not the remove-file error. I just left
this best effort at this point. We'd need to change around a lot of code to
return both, and I'm not sure it's worth it, since there's not much the client
can do in that case.

+                   AsWeakPtr()));
+    callback.Run(error);
+    return;
+  }
+
+  DCHECK(validator_.get());
+  validator_->StartPostWriteValidation(
+      platform_path,
+      base::Bind(&CopyOrMoveOperationDelegate::DidPostWriteValidation,
+                 AsWeakPtr(), url_pair, callback));
+}
+
+void CopyOrMoveOperationDelegate::DidPostWriteValidation(
+    const URLPair& url_pair,
+    const StatusCallback& callback,
+    base::PlatformFileError error) {
+  if (error != base::PLATFORM_FILE_OK) {
+    operation_runner()->Remove(
+        url_pair.dest, true,
+        base::Bind(&CopyOrMoveOperationDelegate::DidRemoveDestForError,
+                   AsWeakPtr()));
+    callback.Run(error);

[vandebo (ex-Chrome), 2013/07/02 20:15:02]

Same here

[Greg Billock, 2013/07/03 16:08:27]

ditto

+    return;
+  }
+
+  if (operation_type_ == OPERATION_COPY) {
+    callback.Run(error);
+    return;
+  }
 
   DCHECK_EQ(OPERATION_MOVE, operation_type_);
 
   // Remove the source for finalizing move operation.
   operation_runner()->Remove(
-      src, true /* recursive */,
+      url_pair.src, true /* recursive */,
       base::Bind(&CopyOrMoveOperationDelegate::DidRemoveSourceForMove,
                  AsWeakPtr(), callback));
 }
 
 void CopyOrMoveOperationDelegate::DidRemoveSourceForMove(
     const StatusCallback& callback,
     base::PlatformFileError error) {
   if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND)
     error = base::PLATFORM_FILE_OK;
   callback.Run(error);
 }
 
 FileSystemURL CopyOrMoveOperationDelegate::CreateDestURL(
     const FileSystemURL& src_url) const {
   DCHECK_EQ(src_root_.type(), src_url.type());
   DCHECK_EQ(src_root_.origin(), src_url.origin());
 
   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);
 }
 
+void CopyOrMoveOperationDelegate::DidRemoveDestForError(
+    base::PlatformFileError error) {
+  // TODO(gbillock): What to do here? Currently nothing....

[vandebo (ex-Chrome), 2013/07/02 20:15:02]

Remove it if you don't need it... but you do.  You should save the error that
put you on the path to here and run the callback with that, ignoring the error
from Remove.

[Greg Billock, 2013/07/03 16:08:27]

That's basically what's happening now. I should wait to return the error until
post-delete-attempt, though, which is probably better -- that way the client
doesn't potentially see the being-deleted file.

+}
+
 }  // namespace fileapi