[Media Galleries] Switch Mac MTP delegate to async interface.

chrome/browser/media_gallery/mac/mtp_device_delegate_impl_mac.h

Index: chrome/browser/media_gallery/mac/mtp_device_delegate_impl_mac.h
diff --git a/chrome/browser/media_gallery/mac/mtp_device_delegate_impl_mac.h b/chrome/browser/media_gallery/mac/mtp_device_delegate_impl_mac.h
index c782255f666668d8a0a795f1be807971fb5bafc6..60e22db64e915fa7e2e2e99d0f25b7025c451167 100644
--- a/chrome/browser/media_gallery/mac/mtp_device_delegate_impl_mac.h
+++ b/chrome/browser/media_gallery/mac/mtp_device_delegate_impl_mac.h
@@ -15,7 +15,7 @@
 #include "base/synchronization/lock.h"

[kmadhusu, 2013/02/26 17:59:15]

Remove this include statement.

[Greg Billock, 2013/02/26 22:02:09]

Done.

 #include "base/synchronization/waitable_event.h"

[kmadhusu, 2013/02/26 17:59:15]

Remove this include statement.

[Greg Billock, 2013/02/26 22:02:09]

Done.

 #include "webkit/fileapi/file_system_file_util.h"

[kmadhusu, 2013/02/26 17:59:15]

Can you include this header file in .cc file?

[Greg Billock, 2013/02/26 22:02:09]

Dropped.

-#include "webkit/fileapi/media/mtp_device_delegate.h"
+#include "webkit/fileapi/media/mtp_device_async_delegate.h"
 
 namespace base {
 class SequencedTaskRunner;

[kmadhusu, 2013/02/26 17:59:15]

No longer used.

[Greg Billock, 2013/02/26 22:02:09]

Done.

@@ -28,22 +28,25 @@ namespace chrome {
 // and names of all files notified through the ItemAdded call will be
 // all appear as children of that directory. (ItemAdded calls with directories
 // will be ignored.)
-class MTPDeviceDelegateImplMac : public fileapi::MTPDeviceDelegate {
+class MTPDeviceDelegateImplMac : public fileapi::MTPDeviceAsyncDelegate {
  public:
   MTPDeviceDelegateImplMac(const std::string& device_id,

[kmadhusu, 2013/02/26 17:59:15]

This can be in private section.

[Greg Billock, 2013/02/26 22:02:09]

Create method needs to call it. It'd require more friend statements. I think
this is OK.

-                           const base::FilePath::StringType& synthetic_path,
-                           base::SequencedTaskRunner* media_task_runner);
+                           const base::FilePath::StringType& synthetic_path);
 
-  // MTPDeviceDelegate:
-  virtual base::PlatformFileError GetFileInfo(
+  // MTPDeviceAsyncDelegate:

[kmadhusu, 2013/02/26 17:59:15]

You can declare these virtual functions in the private section.

[Greg Billock, 2013/02/26 22:02:09]

Technically, yes, but since the interface makes them public, they are
accessible, and the unit tests call them.

+  virtual void GetFileInfo(
       const base::FilePath& file_path,
-      base::PlatformFileInfo* file_info) OVERRIDE;
-  virtual scoped_ptr<fileapi::FileSystemFileUtil::AbstractFileEnumerator>
-      CreateFileEnumerator(const base::FilePath& root, bool recursive) OVERRIDE;
-  virtual base::PlatformFileError CreateSnapshotFile(
+      const GetFileInfoSuccessCallback& success_callback,
+      const ErrorCallback& error_callback) OVERRIDE;
+  virtual void ReadDirectory(
+      const base::FilePath& root,
+      const ReadDirectorySuccessCallback& success_callback,
+      const ErrorCallback& error_callback) OVERRIDE;
+  virtual void CreateSnapshotFile(
       const base::FilePath& device_file_path,
       const base::FilePath& local_path,
-      base::PlatformFileInfo* file_info) OVERRIDE;
+      const CreateSnapshotFileSuccessCallback& success_callback,
+      const ErrorCallback& error_callback) OVERRIDE;
   virtual void CancelPendingTasksAndDeleteDelegate() OVERRIDE;
 
   // Forward delegates for ImageCaptureDeviceListener
@@ -53,33 +56,9 @@ class MTPDeviceDelegateImplMac : public fileapi::MTPDeviceDelegate {
   virtual void DownloadedFile(const std::string& name,
                               base::PlatformFileError error);
 
-  // Implementation returned by |CreateFileEnumerator()|. Must be deleted
-  // before CancelPendingTasksAndDeleteDelegate is called.
-  class Enumerator :
-      public fileapi::FileSystemFileUtil::AbstractFileEnumerator {
-   public:
-    explicit Enumerator(MTPDeviceDelegateImplMac* delegate);
-    virtual ~Enumerator();
-
-    virtual base::FilePath Next() OVERRIDE;
-    virtual int64 Size() OVERRIDE;
-    virtual base::Time LastModifiedTime() OVERRIDE;
-    virtual bool IsDirectory() OVERRIDE;
-
-    // Called by the delegate to signal any waiters that the received items
-    // list has changed state.
-    void ItemsChanged();
-
-   private:
-    MTPDeviceDelegateImplMac* delegate_;
-    size_t position_;
-    base::WaitableEvent wait_for_items_;
-  };
-
-  // GetFile and HasAllFiles called by enumerators.
-  base::FilePath GetFile(size_t index);
-  bool ReceivedAllFiles();
-  void RemoveEnumerator(Enumerator* enumerator);
+  // Public for closures; should not be called except by
+  // CancelTasksAndDeleteDelegate.
+  void CancelAndDelete();
 
  private:
   friend class base::DeleteHelper<MTPDeviceDelegateImplMac>;

[kmadhusu, 2013/02/26 17:59:15]

This is no longer required.

[Greg Billock, 2013/02/26 22:02:09]

Done.

@@ -88,41 +67,46 @@ class MTPDeviceDelegateImplMac : public fileapi::MTPDeviceDelegate {
 
   virtual ~MTPDeviceDelegateImplMac();
 
+  void GetFileInfoImpl(const base::FilePath& file_path,
+                       base::PlatformFileInfo* file_info,
+                       base::PlatformFileError* error);
+
+  void ReadDirectoryImpl(
+      const base::FilePath& root,
+      const ReadDirectorySuccessCallback& success_callback,
+      const ErrorCallback& error_callback);
+
+  void DownloadFile(

[kmadhusu, 2013/02/26 17:59:15]

Please document the new member functions.

[Greg Billock, 2013/02/26 22:02:09]

Done.

+      const base::FilePath& device_file_path,
+      const base::FilePath& local_path,
+      const CreateSnapshotFileSuccessCallback& success_callback,
+      const ErrorCallback& error_callback);
+
+  void CancelDownloads();
+
+  void NotifyReadDir();
+
   std::string device_id_;
   base::FilePath root_path_;
 
-  // Stores a reference to worker pool thread. All requests and response of file
-  // operations are posted on |media_task_runner_|. All callbacks from the
-  // camera will come through this task runner as well.
-  scoped_refptr<base::SequencedTaskRunner> media_task_runner_;
-
   // Interface object for the camera underlying this MTP session.
   scoped_ptr<DeviceListener> camera_interface_;
 
-  // This lock protects all subsequent state. While calling into the delegate
-  // from the FileSystem API happens in sequence, these calls may be
-  // interleaved with calls on other threads in the sequenced task runner
-  // forwarded from the device.
-  base::Lock mutex_;
-
-  // Weak pointer to the enumerator which may be listening for files to come in.
-  Enumerator* enumerator_;
-
   // Stores a map from filename to file metadata received from the camera.
   base::hash_map<base::FilePath::StringType, base::PlatformFileInfo> file_info_;
 
-  // Notification for pending download.
-  base::WaitableEvent* file_download_event_;
-
-  // Resulting error code for pending download.
-  base::PlatformFileError file_download_error_;
-
   // List of files received from the camera.
   std::vector<base::FilePath> file_paths_;
 
   // Set to true when all file metadata has been received from the camera.
   bool received_all_files_;
 
+  ReadDirectorySuccessCallback read_directory_success_callback_;
+  ErrorCallback read_directory_error_callback_;
+
+  CreateSnapshotFileSuccessCallback read_file_success_callback_;
+  ErrorCallback read_file_error_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(MTPDeviceDelegateImplMac);
 };