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

webkit/browser/fileapi/copy_or_move_operation_delegate.cc

Index: webkit/browser/fileapi/copy_or_move_operation_delegate.cc
diff --git a/webkit/browser/fileapi/copy_or_move_operation_delegate.cc b/webkit/browser/fileapi/copy_or_move_operation_delegate.cc
index 8b706902b5c84396c675b884a01422d75f638b4b..49ef3a02d6349547bcd247a51f84210343f1e49c 100644
--- a/webkit/browser/fileapi/copy_or_move_operation_delegate.cc
+++ b/webkit/browser/fileapi/copy_or_move_operation_delegate.cc
@@ -111,7 +111,7 @@ void CopyOrMoveOperationDelegate::DidTryRemoveDestRoot(
   // http://crbug.com/172187
   StartRecursiveOperation(
       src_root_, base::Bind(&CopyOrMoveOperationDelegate::DidFinishCopy,
-                            AsWeakPtr(), src_root_, callback_));
+                            AsWeakPtr(), src_root_, dest_root_, callback_));
 }
 
 void CopyOrMoveOperationDelegate::CopyOrMoveFile(const URLPair& url_pair,
@@ -131,7 +131,7 @@ void CopyOrMoveOperationDelegate::CopyOrMoveFile(const URLPair& url_pair,
   // 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.src, url_pair.dest, callback);
   operation_runner()->CreateSnapshotFile(
       url_pair.src,
       base::Bind(&CopyOrMoveOperationDelegate::DidCreateSnapshot, AsWeakPtr(),
@@ -190,16 +190,76 @@ void CopyOrMoveOperationDelegate::DidValidateFile(
 
 void CopyOrMoveOperationDelegate::DidFinishCopy(
     const FileSystemURL& src,
+    const FileSystemURL& dest,
     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 (operation_type_ == OPERATION_COPY) {

[vandebo (ex-Chrome), 2013/07/02 18:43:05]

Code is basically the same in both cases, combine.

[Greg Billock, 2013/07/02 19:17:39]

I see what you mean below. Yeah, that's a good plan. I'll check operation_type_
further down.

+    if (validator_.get()) {
+      PostWriteValidate(
+          dest, base::Bind(&CopyOrMoveOperationDelegate::DidPostCopyValidation,
+                           AsWeakPtr(), callback));
+    } else {
+      DidPostCopyValidation(callback, base::PLATFORM_FILE_OK);
+    }
+    return;
+  }
+
   DCHECK_EQ(OPERATION_MOVE, operation_type_);
 
+  if (validator_.get()) {
+    PostWriteValidate(
+        dest, base::Bind(&CopyOrMoveOperationDelegate::DidPostMoveValidation,
+                         AsWeakPtr(), src, callback));
+  } else {
+    DidPostMoveValidation(src, callback, base::PLATFORM_FILE_OK);
+  }
+}
+
+void CopyOrMoveOperationDelegate::PostWriteValidate(
+    const FileSystemURL& dest,
+    const StatusCallback& callback) {
+  operation_runner()->CreateSnapshotFile(
+      dest, base::Bind(&CopyOrMoveOperationDelegate::DoPostWriteValidation,
+                       AsWeakPtr(), callback));
+}
+
+void CopyOrMoveOperationDelegate::DoPostWriteValidation(
+    const StatusCallback& callback,
+    base::PlatformFileError error,
+    const base::PlatformFileInfo& file_info,
+    const base::FilePath& platform_path,
+    const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref) {

[vandebo (ex-Chrome), 2013/07/02 18:43:05]

You probably need to pass file_ref down so that the snapshot file isn't deleted.
 Though we think it won't actually be a snapshot file so maybe it's ok for the
ShareableFileReference to go out of scope and check later on that the platform
file is there.

[Greg Billock, 2013/07/02 19:17:39]

So the file_ref is what keeps the snapshot alive? So should that be an arg to
the post-write validation method? Or should we just skip if we have a file_ref?
I think the premise is that we'll have an empty one in cases where we want to
run this validation step.

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

We don't want to validate files that need snapshoting, but that's specific to
our use case.  The file_ref should probably be passed on toe the validation
step.

+  // TODO(gbillock): need to check here that file_ref is null or anything?
+  if (error != base::PLATFORM_FILE_OK) {
+    callback.Run(error);
+    return;
+  }
+
+  DCHECK(validator_.get());
+  validator_->StartPostWriteValidation(platform_path, callback);

[vandebo (ex-Chrome), 2013/07/02 18:43:05]

Instead of passing |callback| down to this point, generate it here.  You have
access to operation_type_.  Better yet, just use operation_type_ in
DidPostCopy/MoveValidation and combine those two methods into one.

[Greg Billock, 2013/07/02 19:17:39]

Done.

+}
+
+void CopyOrMoveOperationDelegate::DidPostCopyValidation(
+    const StatusCallback& callback,
+    base::PlatformFileError error) {
+  callback.Run(error);
+}
+
+void CopyOrMoveOperationDelegate::DidPostMoveValidation(
+    const FileSystemURL& src,
+    const StatusCallback& callback,
+    base::PlatformFileError error) {
+  if (error != base::PLATFORM_FILE_OK) {
+    // TODO(gbillock): Need to remove the destination file here?

[vandebo (ex-Chrome), 2013/07/02 18:43:05]

Yes

[Greg Billock, 2013/07/02 19:17:39]

Added the call, but I'm not sure what to do with errors. I think we want to ship
back the validation error. Currently I just best-effort it.

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

Yea, send back the validation error.

+    callback.Run(error);
+    return;
+  }
+
   // Remove the source for finalizing move operation.
   operation_runner()->Remove(
       src, true /* recursive */,