fileapi: Pass virtual path to copy validator.

webkit/fileapi/cross_operation_delegate.h

 // 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.
 
 #ifndef WEBKIT_FILEAPI_CROSS_OPERATION_DELEGATE_H_
 #define WEBKIT_FILEAPI_CROSS_OPERATION_DELEGATE_H_
 
 #include <stack>
 
 #include "base/memory/ref_counted.h"
@@ skipping 32 matching lines @@
   virtual void Run() OVERRIDE;
   virtual void RunRecursively() OVERRIDE;
   virtual void ProcessFile(const FileSystemURL& url,
                            const StatusCallback& callback) OVERRIDE;
   virtual void ProcessDirectory(const FileSystemURL& url,
                                 const StatusCallback& callback) OVERRIDE;
 
   using base::SupportsWeakPtr<CrossOperationDelegate>::AsWeakPtr;
 
  private:
+  struct Request {
+    Request(const FileSystemURL& src, const FileSystemURL& dest,

[kinuko, 2013/04/18 03:32:00]

nit: can we break the line after 'src,'

[vandebo (ex-Chrome), 2013/04/19 01:07:18]

With just src and dest, they fit on one line.  Do you still want to break here?

+            const StatusCallback& callback)
+        : src(src),
+          dest(dest),
+          callback(callback) {
+    }
+    FileSystemURL src;
+    FileSystemURL dest;
+    StatusCallback callback;
+  };
+
   void DidTryCopyOrMoveFile(base::PlatformFileError error);
   void DidTryRemoveDestRoot(base::PlatformFileError error);
-  void CopyOrMoveFile(
-      const FileSystemURL& src,
-      const FileSystemURL& dest,
-      const StatusCallback& callback);
+  void CopyOrMoveFile(const Request& request);
   void DidCreateSnapshot(
-      const FileSystemURL& dest,
-      const StatusCallback& callback,
+      const Request& request,

[vandebo (ex-Chrome), 2013/04/17 23:50:48]

In order to pass virtual_path into the copy validator, we need FileSystemURL
src.  Adding that parameter pushes this method over the base::Bind argument
limit.

src, dest, callback are used in most of these methods, so I pulled it out into a
struct and used it in these private methods.  Happy to do a different solution
if there's one that you prefer.

[kinuko, 2013/04/18 03:32:00]

This looks fine, the name Request sounds a bit unclear to me though. ...I can't
come up with a clean/great name either, but how about naming it FilePair or
CurrentFilePair (indicates current working src/dest parameters) and let it have
only src and dest? (If saving one param works for this patch)

[vandebo (ex-Chrome), 2013/04/19 01:07:18]

Done.

Named URLPair.

Since it's just the two values now, I also only used it where we need both
values.

       base::PlatformFileError error,
       const base::PlatformFileInfo& file_info,
       const base::FilePath& platform_path,
       const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref);
   void DidValidateFile(
-      const FileSystemURL& dest,
-      const StatusCallback& callback,
+      const Request& request,
       const base::PlatformFileInfo& file_info,
       const base::FilePath& platform_path,
       base::PlatformFileError error);
   void DidFinishCopy(
-      const FileSystemURL& src,
-      const StatusCallback& callback,
+      const Request& request,
       base::PlatformFileError error);
   void DidRemoveSourceForMove(
-      const StatusCallback& callback,
+      const Request& request,
       base::PlatformFileError error);
 
   FileSystemURL CreateDestURL(const FileSystemURL& src_url) const;
 
   // Create nested operations for recursive task.
   // When the creation fails it fires callback_ with the
   // error code and returns NULL.
   //
   // - NewDestOperation is basically a thin wrapper of
   //   RecursiveOperationDelegate::NewOperation().
@@ skipping 18 matching lines @@
   scoped_refptr<webkit_blob::ShareableFileReference> current_file_ref_;
 
   scoped_ptr<CopyOrMoveFileValidator> validator_;
 
   DISALLOW_COPY_AND_ASSIGN(CrossOperationDelegate);
 };
 
 }  // namespace fileapi
 
 #endif  // WEBKIT_FILEAPI_CROSS_OPERATION_DELEGATE_H_