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

chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm

Index: chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm
diff --git a/chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm b/chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm
index a998f89e4da06ff2b42857e44aabec2beb986d25..20c4fbde9f536d75c299fd95f95a42aff4af5a90 100644
--- a/chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm
+++ b/chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm
@@ -6,10 +6,12 @@
 #import <ImageCaptureCore/ImageCaptureCore.h>
 
 #include "base/file_util.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/mac/cocoa_protocols.h"
 #include "base/mac/foundation_util.h"
 #include "base/memory/scoped_nsobject.h"
 #include "base/message_loop.h"
+#include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/test/sequenced_worker_pool_owner.h"
@@ -175,6 +177,12 @@ class MTPDeviceDelegateImplMacTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     ui_thread_.reset(new content::TestBrowserThread(
         content::BrowserThread::UI, &message_loop_));
+    file_thread_.reset(new content::TestBrowserThread(
+          content::BrowserThread::FILE, &message_loop_));
+    io_thread_.reset(new content::TestBrowserThread(
+        content::BrowserThread::IO));
+    ASSERT_TRUE(io_thread_->Start());
+    // Need a waitable event on io thread startup?

[Lei Zhang, 2013/04/04 03:46:21]

remove?

[Greg Billock, 2013/04/04 17:00:51]

Done.

 
     manager_.SetNotifications(monitor_.receiver());
 
@@ -182,34 +190,133 @@ class MTPDeviceDelegateImplMacTest : public testing::Test {
     id<ICDeviceBrowserDelegate> delegate = manager_.device_browser();
     [delegate deviceBrowser:nil didAddDevice:camera_ moreComing:NO];
 
-    base::SequencedWorkerPool* pool = content::BrowserThread::GetBlockingPool();
-    task_runner_ = pool->GetSequencedTaskRunner(
-        pool->GetNamedSequenceToken("token-name"));
-    delegate_ = new chrome::MTPDeviceDelegateImplMac(
-        "id", "/ic:id", task_runner_.get());
+    delegate_ = new chrome::MTPDeviceDelegateImplMac("id", "/ic:id");

[Lei Zhang, 2013/04/04 03:46:21]

there's already a const for "id", and constify "/ic:id" ?

[Greg Billock, 2013/04/04 17:00:51]

Done.

+  }
+
+  void OnError(base::WaitableEvent* event, base::PlatformFileError error) {
+    error_ = error;
+    event->Signal();
+  }
+
+  void OverlappedOnError(base::WaitableEvent* event,
+                         base::PlatformFileError error) {
+    overlapped_error_ = error;
+    event->Signal();
+  }

[Lei Zhang, 2013/04/04 03:46:21]

nit: separate with newline

[Greg Billock, 2013/04/04 17:00:51]

Done.

+  void OnFileInfo(base::WaitableEvent* event,
+                  const base::PlatformFileInfo& info) {
+    error_ = base::PLATFORM_FILE_OK;
+    info_ = info;
+    event->Signal();
+  }
+
+  void OnReadDir(base::WaitableEvent* event,
+                 const fileapi::AsyncFileUtil::EntryList& files,
+                 bool has_more) {
+    error_ = base::PLATFORM_FILE_OK;
+    ASSERT_FALSE(has_more);
+    file_list_ = files;
+    event->Signal();
+  }
+
+  void OverlappedOnReadDir(base::WaitableEvent* event,
+                           const fileapi::AsyncFileUtil::EntryList& files,
+                           bool has_more) {
+    overlapped_error_ = base::PLATFORM_FILE_OK;
+    ASSERT_FALSE(has_more);
+    overlapped_file_list_ = files;
+    event->Signal();
+  }
+
+  void OnDownload(base::WaitableEvent* event,
+                  const base::PlatformFileInfo& file_info,
+                  const base::FilePath& local_path) {
+    error_ = base::PLATFORM_FILE_OK;
+    event->Signal();
+  }
+
+  base::PlatformFileError GetFileInfo(const base::FilePath& path,
+                                      base::PlatformFileInfo* info) {
+    base::WaitableEvent wait(true, false);

[Lei Zhang, 2013/04/04 03:46:21]

IWYU for WaitableEvent.

[Greg Billock, 2013/04/04 17:00:51]

It's there.

+    delegate_->GetFileInfo(
+      path,
+      base::Bind(&MTPDeviceDelegateImplMacTest::OnFileInfo,
+                 base::Unretained(this),
+                 &wait),
+      base::Bind(&MTPDeviceDelegateImplMacTest::OnError,
+                 base::Unretained(this),
+                 &wait));
+    base::RunLoop loop;
+    loop.RunUntilIdle();
+    wait.Wait();

[Lei Zhang, 2013/04/04 03:46:21]

Can we just check IsSignaled() instead? If we already ran the message loop, the
signal should have fired.

[Greg Billock, 2013/04/04 17:00:51]

Done.

+    *info = info_;
+    return error_;
+  }
+
+  base::PlatformFileError ReadDir(const base::FilePath& path) {
+    base::WaitableEvent wait(true, false);
+    delegate_->ReadDirectory(
+        path,
+        base::Bind(&MTPDeviceDelegateImplMacTest::OnReadDir,
+                   base::Unretained(this),
+                   &wait),
+        base::Bind(&MTPDeviceDelegateImplMacTest::OnError,
+                   base::Unretained(this),
+                   &wait));
+    base::RunLoop loop;
+    loop.RunUntilIdle();
+    wait.Wait();
+    return error_;
+  }
+
+  base::PlatformFileError DownloadFile(
+      const base::FilePath& path,
+      const base::FilePath& local_path) {
+    base::WaitableEvent wait(true, false);
+    delegate_->CreateSnapshotFile(
+        path, local_path,
+        base::Bind(&MTPDeviceDelegateImplMacTest::OnDownload,
+                   base::Unretained(this),
+                   &wait),
+        base::Bind(&MTPDeviceDelegateImplMacTest::OnError,
+                   base::Unretained(this),
+                   &wait));
+    base::RunLoop loop;
+    loop.RunUntilIdle();
+    wait.Wait();
+    return error_;
   }
 
   virtual void TearDown() OVERRIDE {
     id<ICDeviceBrowserDelegate> delegate = manager_.device_browser();
     [delegate deviceBrowser:nil didRemoveDevice:camera_ moreGoing:NO];
 
-    task_runner_->PostTask(FROM_HERE,
-        base::Bind(&chrome::MTPDeviceDelegateImplMac::
-                        CancelPendingTasksAndDeleteDelegate,
-                   base::Unretained(delegate_)));
+    delegate_->CancelPendingTasksAndDeleteDelegate();
+
+    io_thread_->Stop();
   }
 
  protected:
   MessageLoopForUI message_loop_;
+  // Note: threads must be made in this order: UI > FILE > IO
   scoped_ptr<content::TestBrowserThread> ui_thread_;
+  scoped_ptr<content::TestBrowserThread> file_thread_;
+  scoped_ptr<content::TestBrowserThread> io_thread_;
+  base::ScopedTempDir temp_dir_;
   chrome::test::TestStorageMonitor monitor_;
   chrome::ImageCaptureDeviceManager manager_;
-  ICCameraDevice* camera_;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  MockMTPICCameraDevice* camera_;
 
   // This object needs special deletion inside the above |task_runner_|.
   chrome::MTPDeviceDelegateImplMac* delegate_;
 
+  base::PlatformFileError error_;
+  base::PlatformFileInfo info_;
+  fileapi::AsyncFileUtil::EntryList file_list_;
+
+  base::PlatformFileError overlapped_error_;
+  fileapi::AsyncFileUtil::EntryList overlapped_file_list_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MTPDeviceDelegateImplMacTest);
 };
@@ -219,17 +326,61 @@ TEST_F(MTPDeviceDelegateImplMacTest, TestGetRootFileInfo) {
   // Making a fresh delegate should have a single file entry for the synthetic
   // root directory, with the name equal to the device id string.
   EXPECT_EQ(base::PLATFORM_FILE_OK,
-            delegate_->GetFileInfo(base::FilePath("/ic:id"), &info));
+            GetFileInfo(base::FilePath("/ic:id"), &info));
   EXPECT_TRUE(info.is_directory);
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND,
-            delegate_->GetFileInfo(base::FilePath("/nonexistent"), &info));
+            GetFileInfo(base::FilePath("/nonexistent"), &info));
+
+  // Signal the delegate that no files are coming.
+  delegate_->NoMoreItems();
+
+  EXPECT_EQ(base::PLATFORM_FILE_OK, ReadDir(base::FilePath("/ic:id")));
+  EXPECT_EQ(0U, file_list_.size());
+}
+
+TEST_F(MTPDeviceDelegateImplMacTest, TestOverlappedReadDir) {
+  base::Time time1 = base::Time::Now();
+  base::PlatformFileInfo info1;
+  info1.size = 1;
+  info1.is_directory = false;
+  info1.is_symbolic_link = false;
+  info1.last_modified = time1;
+  info1.last_accessed = time1;
+  info1.creation_time = time1;
+  delegate_->ItemAdded("name1", info1);
+
+  base::WaitableEvent wait(true, false);
+
+  delegate_->ReadDirectory(
+      base::FilePath("/ic:id"),
+      base::Bind(&MTPDeviceDelegateImplMacTest::OnReadDir,
+                 base::Unretained(this),
+                 &wait),
+      base::Bind(&MTPDeviceDelegateImplMacTest::OnError,
+                 base::Unretained(this),
+                 &wait));
+
+  delegate_->ReadDirectory(
+      base::FilePath("/ic:id"),
+      base::Bind(&MTPDeviceDelegateImplMacTest::OverlappedOnReadDir,
+                 base::Unretained(this),
+                 &wait),
+      base::Bind(&MTPDeviceDelegateImplMacTest::OverlappedOnError,
+                 base::Unretained(this),
+                 &wait));
+
 
   // Signal the delegate that no files are coming.
   delegate_->NoMoreItems();
 
-  scoped_ptr<fileapi::FileSystemFileUtil::AbstractFileEnumerator> enumerator =
-      delegate_->CreateFileEnumerator(base::FilePath("/ic:id"), true);
-  EXPECT_TRUE(enumerator->Next().empty());
+  base::RunLoop loop;
+  loop.RunUntilIdle();
+  wait.Wait();
+
+  EXPECT_EQ(base::PLATFORM_FILE_OK, error_);
+  EXPECT_EQ(1U, file_list_.size());
+  EXPECT_EQ(base::PLATFORM_FILE_OK, overlapped_error_);
+  EXPECT_EQ(1U, overlapped_file_list_.size());
 }
 
 TEST_F(MTPDeviceDelegateImplMacTest, TestGetFileInfo) {
@@ -245,7 +396,7 @@ TEST_F(MTPDeviceDelegateImplMacTest, TestGetFileInfo) {
 
   base::PlatformFileInfo info;
   EXPECT_EQ(base::PLATFORM_FILE_OK,
-            delegate_->GetFileInfo(base::FilePath("/ic:id/name1"), &info));
+            GetFileInfo(base::FilePath("/ic:id/name1"), &info));
   EXPECT_EQ(info1.size, info.size);
   EXPECT_EQ(info1.is_directory, info.is_directory);
   EXPECT_EQ(info1.last_modified, info.last_modified);
@@ -257,27 +408,19 @@ TEST_F(MTPDeviceDelegateImplMacTest, TestGetFileInfo) {
   delegate_->NoMoreItems();
 
   EXPECT_EQ(base::PLATFORM_FILE_OK,
-            delegate_->GetFileInfo(base::FilePath("/ic:id/name2"), &info));
+            GetFileInfo(base::FilePath("/ic:id/name2"), &info));
   EXPECT_EQ(info1.size, info.size);
 
-  scoped_ptr<fileapi::FileSystemFileUtil::AbstractFileEnumerator> enumerator =
-      delegate_->CreateFileEnumerator(base::FilePath("/ic:id"), true);
-  base::FilePath next = enumerator->Next();
-  ASSERT_FALSE(next.empty());
-  EXPECT_EQ(1, enumerator->Size());
-  EXPECT_EQ(time1, enumerator->LastModifiedTime());
-  EXPECT_FALSE(enumerator->IsDirectory());
-  EXPECT_EQ("/ic:id/name1", next.value());
-
-  next = enumerator->Next();
-  ASSERT_FALSE(next.empty());
-  EXPECT_EQ(2, enumerator->Size());
-  EXPECT_EQ(time1, enumerator->LastModifiedTime());
-  EXPECT_FALSE(enumerator->IsDirectory());
-  EXPECT_EQ("/ic:id/name2", next.value());
-
-  next = enumerator->Next();
-  EXPECT_TRUE(next.empty());
+  EXPECT_EQ(base::PLATFORM_FILE_OK, ReadDir(base::FilePath("/ic:id")));
+
+  ASSERT_EQ(2U, file_list_.size());
+  EXPECT_EQ(time1, file_list_[0].last_modified_time);
+  EXPECT_FALSE(file_list_[0].is_directory);
+  EXPECT_EQ("/ic:id/name1", file_list_[0].name);
+
+  EXPECT_EQ(time1, file_list_[1].last_modified_time);
+  EXPECT_FALSE(file_list_[1].is_directory);
+  EXPECT_EQ("/ic:id/name2", file_list_[1].name);
 }
 
 TEST_F(MTPDeviceDelegateImplMacTest, TestIgnoreDirectories) {
@@ -299,54 +442,49 @@ TEST_F(MTPDeviceDelegateImplMacTest, TestIgnoreDirectories) {
   delegate_->ItemAdded("name2", info1);
   delegate_->NoMoreItems();
 
-  scoped_ptr<fileapi::FileSystemFileUtil::AbstractFileEnumerator> enumerator =
-      delegate_->CreateFileEnumerator(base::FilePath("/ic:id"), true);
-  base::FilePath next = enumerator->Next();
-  ASSERT_FALSE(next.empty());
-  EXPECT_EQ("/ic:id/name1", next.value());
+  EXPECT_EQ(base::PLATFORM_FILE_OK, ReadDir(base::FilePath("/ic:id")));
 
-  next = enumerator->Next();
-  ASSERT_FALSE(next.empty());
-  EXPECT_EQ("/ic:id/name2", next.value());
+  ASSERT_EQ(2U, file_list_.size());
+  EXPECT_EQ(time1, file_list_[0].last_modified_time);
+  EXPECT_FALSE(file_list_[0].is_directory);
+  EXPECT_EQ("/ic:id/name1", file_list_[0].name);
 
-  next = enumerator->Next();
-  EXPECT_TRUE(next.empty());
+  EXPECT_EQ(time1, file_list_[1].last_modified_time);
+  EXPECT_FALSE(file_list_[1].is_directory);
+  EXPECT_EQ("/ic:id/name2", file_list_[1].name);
 }
 
-TEST_F(MTPDeviceDelegateImplMacTest, EnumeratorWaitsForEntries) {
-  base::Time time1 = base::Time::Now();
-  base::PlatformFileInfo info1;
-  info1.size = 1;
-  info1.is_directory = false;
-  info1.is_symbolic_link = false;
-  info1.last_modified = time1;
-  info1.last_accessed = time1;
-  info1.creation_time = time1;
-  delegate_->ItemAdded("name1", info1);
-
-  scoped_ptr<fileapi::FileSystemFileUtil::AbstractFileEnumerator> enumerator =
-      delegate_->CreateFileEnumerator(base::FilePath("/ic:id"), true);
-  // Event is manually reset, initially unsignaled
-  base::WaitableEvent event(true, false);
-  base::FilePath next;
-  task_runner_->PostTask(FROM_HERE,
-                         base::Bind(&EnumerateAndSignal,
-                         enumerator.get(), &event, &next));
-  message_loop_.RunUntilIdle();
-  ASSERT_TRUE(event.IsSignaled());
-  EXPECT_EQ("/ic:id/name1", next.value());
-
-  event.Reset();
-
-  // This method will block until it is sure there are no more items.
-  task_runner_->PostTask(FROM_HERE,
-                         base::Bind(&EnumerateAndSignal,
-                         enumerator.get(), &event, &next));
-  message_loop_.RunUntilIdle();
-  ASSERT_FALSE(event.IsSignaled());
+TEST_F(MTPDeviceDelegateImplMacTest, TestDownload) {
+  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+  base::Time t1 = base::Time::Now();
+  base::PlatformFileInfo info;
+  info.size = 4;
+  info.is_directory = false;
+  info.is_symbolic_link = false;
+  info.last_modified = t1;
+  info.last_accessed = t1;
+  info.creation_time = t1;
+  std::string kTestFileName("filename");
+  scoped_nsobject<MockMTPICCameraFile> picture1(
+      [[MockMTPICCameraFile alloc]
+          init:base::SysUTF8ToNSString(kTestFileName)]);
+  [camera_ addMediaFile:picture1];
+  delegate_->ItemAdded(kTestFileName, info);
   delegate_->NoMoreItems();
-  event.Wait();
-  ASSERT_TRUE(event.IsSignaled());
-  EXPECT_TRUE(next.empty());
-  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(base::PLATFORM_FILE_OK, ReadDir(base::FilePath("/ic:id")));
+  ASSERT_EQ(1U, file_list_.size());
+  ASSERT_EQ("/ic:id/filename", file_list_[0].name);
+
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND,
+             DownloadFile(base::FilePath("/ic:id/nonexist"),
+                          temp_dir_.path().Append("target")));
+
+  EXPECT_EQ(base::PLATFORM_FILE_OK,
+             DownloadFile(base::FilePath("/ic:id/filename"),
+                          temp_dir_.path().Append("target")));
+  std::string contents;
+  EXPECT_TRUE(file_util::ReadFileToString(temp_dir_.path().Append("target"),
+                                          &contents));
+  EXPECT_EQ(kTestFileContents, contents);
 }