[Linux, Media Gallery] Fix MTPRecursiveDeviceObjectEnumerator to recursively enumerate all the medi…

chrome/browser/media_gallery/linux/mtp_recursive_device_object_enumerator.cc

Index: chrome/browser/media_gallery/linux/mtp_recursive_device_object_enumerator.cc
diff --git a/chrome/browser/media_gallery/linux/mtp_recursive_device_object_enumerator.cc b/chrome/browser/media_gallery/linux/mtp_recursive_device_object_enumerator.cc
index 2b5ceebea0759b0b0c5e0735df5953bbf83c3388..d8ce582080744c6dcfb99b37c65570706a754487 100644
--- a/chrome/browser/media_gallery/linux/mtp_recursive_device_object_enumerator.cc
+++ b/chrome/browser/media_gallery/linux/mtp_recursive_device_object_enumerator.cc
@@ -37,29 +37,16 @@ FilePath MTPRecursiveDeviceObjectEnumerator::Next() {
     return FilePath();
   }
 
-  FilePath path = current_enumerator_->Next();
-  if (!path.empty())
-    return path;
-
-  // We reached the end.
-  if (file_entry_iter_ == file_entries_.end())
+  MaybeUpdateCurrentFileEntryList();

[Lei Zhang, 2013/01/11 06:35:44]

side note: functions named "MaybeSomething" sounds like they are not
deterministic.

Q1: Is it going to do Something?
A: Maybe?

Q2: What conditions govern when it works?
A2: The name doesn't really say.

[kmadhusu, 2013/01/11 22:34:22]

Renamed the function and updated the function definition.

+  if (file_entries_.empty())
     return FilePath();
 
-  // Enumerate subdirectories of the next media file entry.
-  MtpFileEntry next_file_entry = *file_entry_iter_;
-  ++file_entry_iter_;
-
-  // Create a MTPReadDirectoryWorker object to enumerate sub directories.
-  scoped_refptr<MTPReadDirectoryWorker> worker(new MTPReadDirectoryWorker(
-      device_handle_, next_file_entry.item_id(), media_task_runner_,
-      on_task_completed_event_, on_shutdown_event_));
-  worker->Run();
-  if (!worker->get_file_entries().empty()) {
-    current_enumerator_.reset(
-        new MTPDeviceObjectEnumerator(worker->get_file_entries()));

[kmadhusu, 2013/01/11 04:51:39]

This code was listing only the top 2 levels of the directory contents but not
the entire subtree. 

-  } else {
-    current_enumerator_.reset(
-        new fileapi::FileSystemFileUtil::EmptyFileEnumerator());
+  DCHECK(file_entry_iter_ != file_entries_.end());
+  MtpFileEntry curr_file_entry = *(file_entry_iter_++);
+  if (curr_file_entry.file_type() == MtpFileEntry::FILE_TYPE_FOLDER) {
+    // If |curr_file_entry| is a directory, add it to
+    // |unparsed_directory_entry_ids_| to scan after scanning this directory.
+    unparsed_directory_entry_ids_.push(curr_file_entry.item_id());
   }
   return current_enumerator_->Next();
 }
@@ -76,4 +63,33 @@ base::Time MTPRecursiveDeviceObjectEnumerator::LastModifiedTime() {
   return current_enumerator_->LastModifiedTime();
 }
 
+void MTPRecursiveDeviceObjectEnumerator::MaybeUpdateCurrentFileEntryList() {
+  if (file_entry_iter_ != file_entries_.end())
+    return;
+
+  file_entries_.clear();
+  while (file_entries_.empty() &&
+         !unparsed_directory_entry_ids_.empty()) {
+    // Create a MTPReadDirectoryWorker object to enumerate sub directories.
+    DirectoryEntryId dir_entry_id = unparsed_directory_entry_ids_.front();
+    unparsed_directory_entry_ids_.pop();
+    scoped_refptr<MTPReadDirectoryWorker> worker(new MTPReadDirectoryWorker(
+        device_handle_, dir_entry_id, media_task_runner_,
+        on_task_completed_event_, on_shutdown_event_));
+    worker->Run();
+    file_entries_ = worker->get_file_entries();
+    if (!file_entries_.empty()) {
+      current_enumerator_.reset(new MTPDeviceObjectEnumerator(file_entries_));
+      break;
+    }
+  }
+
+  if (file_entries_.empty()) {
+    // Reached the end. Parsed all the sub directories.
+    current_enumerator_.reset(
+        new fileapi::FileSystemFileUtil::EmptyFileEnumerator());
+  }
+  file_entry_iter_ = file_entries_.begin();
+}
+
 }  // namespace chrome