FileSelectHelper: Simplify directory-enumeration code.

A FileSelectHelper instance handles at most one directory enumeration
during its lifetime.
- FileSelectHelper doesn't need to have |std::map<int,
  ActiveDirectoryEnumeration*>|. One |std::unique_ptr<
  ActiveDirectoryEnumeration>| and one |int request_id_| are enough.
- We don't need DirectoryListerDispatchDelegate. FileSelectHelper
  implements net::DirectoryLister::DirectoryListerDelegate.

This CL has no behavior changes.
This CL is a preparation to mojoify FileChooser IPC.

Bug: 869257
Change-Id: I19ecfbb49b54c2430f7f0358ca787e831c493e29
Reviewed-on: https://chromium-review.googlesource.com/1177082
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583870}

chrome/browser/file_select_helper.cc

@@ -126,7 +126,6 @@ struct FileSelectHelper::ActiveDirectoryEnumeration {
   explicit ActiveDirectoryEnumeration(const base::FilePath& path)
       : rvh_(NULL), path_(path) {}
 
-  std::unique_ptr<DirectoryListerDispatchDelegate> delegate_;
   std::unique_ptr<net::DirectoryLister> lister_;
   RenderViewHost* rvh_;
   const base::FilePath path_;
@@ -148,25 +147,6 @@ FileSelectHelper::~FileSelectHelper() {
   // away so they don't try and call back to us.
   if (select_file_dialog_.get())
     select_file_dialog_->ListenerDestroyed();
-
-  // Stop any pending directory enumeration, prevent a callback, and free
-  // allocated memory.
-  std::map<int, ActiveDirectoryEnumeration*>::iterator iter;
-  for (iter = directory_enumerations_.begin();
-       iter != directory_enumerations_.end();
-       ++iter) {
-    iter->second->lister_.reset();
-    delete iter->second;
-  }
-}
-
-void FileSelectHelper::DirectoryListerDispatchDelegate::OnListFile(
-    const net::DirectoryLister::DirectoryListerData& data) {
-  parent_->OnListFile(id_, data);
-}
-
-void FileSelectHelper::DirectoryListerDispatchDelegate::OnListDone(int error) {
-  parent_->OnListDone(id_, error);
 }
 
 void FileSelectHelper::FileSelected(const base::FilePath& path,
@@ -245,25 +225,22 @@ void FileSelectHelper::FileSelectionCanceled(void* params) {
 void FileSelectHelper::StartNewEnumeration(const base::FilePath& path,
                                            int request_id,
                                            RenderViewHost* render_view_host) {
+  request_id_ = request_id;
   auto entry = std::make_unique<ActiveDirectoryEnumeration>(path);
   entry->rvh_ = render_view_host;
-  entry->delegate_.reset(new DirectoryListerDispatchDelegate(this, request_id));
   entry->lister_.reset(new net::DirectoryLister(
-      path, net::DirectoryLister::NO_SORT_RECURSIVE, entry->delegate_.get()));
+      path, net::DirectoryLister::NO_SORT_RECURSIVE, this));
   entry->lister_->Start();
-  directory_enumerations_[request_id] = entry.release();
+  directory_enumeration_ = std::move(entry);
 }
 
 void FileSelectHelper::OnListFile(
-    int id,
     const net::DirectoryLister::DirectoryListerData& data) {
-  ActiveDirectoryEnumeration* entry = directory_enumerations_[id];
-
   // Directory upload only cares about files.
   if (data.info.IsDirectory())
     return;
 
-  entry->results_.push_back(data.path);
+  directory_enumeration_->results_.push_back(data.path);
 }
 
 void FileSelectHelper::LaunchConfirmationDialog(
@@ -275,11 +252,10 @@ void FileSelectHelper::LaunchConfirmationDialog(
       std::move(selected_files), web_contents_);
 }
 
-void FileSelectHelper::OnListDone(int id, int error) {
+void FileSelectHelper::OnListDone(int error) {
   // This entry needs to be cleaned up when this function is done.
-  std::unique_ptr<ActiveDirectoryEnumeration> entry(
-      directory_enumerations_[id]);
-  directory_enumerations_.erase(id);
+  std::unique_ptr<ActiveDirectoryEnumeration> entry =
+      std::move(directory_enumeration_);
   if (!entry->rvh_)
     return;
   if (error) {
@@ -290,10 +266,10 @@ void FileSelectHelper::OnListDone(int id, int error) {
   std::vector<ui::SelectedFileInfo> selected_files =
       FilePathListToSelectedFileInfoList(entry->results_);
 
-  if (id == kFileSelectEnumerationId) {
+  if (request_id_ == kFileSelectEnumerationId) {
     LaunchConfirmationDialog(entry->path_, std::move(selected_files));
   } else {
-    entry->rvh_->DirectoryEnumerationFinished(id, entry->results_);
+    entry->rvh_->DirectoryEnumerationFinished(request_id_, entry->results_);
     EnumerateDirectoryEnd();
   }
 }

chrome/browser/file_select_helper.h

@@ -45,7 +45,8 @@ class FileSelectHelper : public base::RefCountedThreadSafe<
                              content::BrowserThread::DeleteOnUIThread>,
                          public ui::SelectFileDialog::Listener,
                          public content::WebContentsObserver,
-                         public content::RenderWidgetHostObserver {
+                         public content::RenderWidgetHostObserver,
+                         private net::DirectoryLister::DirectoryListerDelegate {
  public:
   // Show the file chooser dialog.
   static void RunFileChooser(content::RenderFrameHost* render_frame_host,
@@ -69,27 +70,6 @@ class FileSelectHelper : public base::RefCountedThreadSafe<
   explicit FileSelectHelper(Profile* profile);
   ~FileSelectHelper() override;
 
-  // Utility class which can listen for directory lister events and relay
-  // them to the main object with the correct tracking id.
-  class DirectoryListerDispatchDelegate
-      : public net::DirectoryLister::DirectoryListerDelegate {
-   public:
-    DirectoryListerDispatchDelegate(FileSelectHelper* parent, int id)
-        : parent_(parent),
-          id_(id) {}
-    ~DirectoryListerDispatchDelegate() override {}
-    void OnListFile(
-        const net::DirectoryLister::DirectoryListerData& data) override;
-    void OnListDone(int error) override;
-
-   private:
-    // This FileSelectHelper owns this object.
-    FileSelectHelper* parent_;
-    int id_;
-
-    DISALLOW_COPY_AND_ASSIGN(DirectoryListerDispatchDelegate);
-  };
-
   void RunFileChooser(content::RenderFrameHost* render_frame_host,
                       std::unique_ptr<content::FileChooserParams> params);
   void GetFileTypesInThreadPool(
@@ -146,11 +126,10 @@ class FileSelectHelper : public base::RefCountedThreadSafe<
                            int request_id,
                            content::RenderViewHost* render_view_host);
 
-  // Callbacks from directory enumeration.
-  virtual void OnListFile(
-      int id,
-      const net::DirectoryLister::DirectoryListerData& data);
-  virtual void OnListDone(int id, int error);
+  // net::DirectoryLister::DirectoryListerDelegate overrides.
+  void OnListFile(
+      const net::DirectoryLister::DirectoryListerData& data) override;
+  void OnListDone(int error) override;
 
   void LaunchConfirmationDialog(
       const base::FilePath& path,
@@ -244,11 +223,13 @@ class FileSelectHelper : public base::RefCountedThreadSafe<
   // The mode of file dialog last shown.
   content::FileChooserParams::Mode dialog_mode_;
 
-  // Maintain a list of active directory enumerations.  These could come from
-  // the file select dialog or from drag-and-drop of directories, so there could
-  // be more than one going on at a time.
+  // Maintain an active directory enumeration.  These could come from the file
+  // select dialog or from drag-and-drop of directories.  There could not be
+  // more than one going on at a time.
   struct ActiveDirectoryEnumeration;
-  std::map<int, ActiveDirectoryEnumeration*> directory_enumerations_;
+  std::unique_ptr<ActiveDirectoryEnumeration> directory_enumeration_;
+  // Keep |request_id| argument of EnumerateDirectory() to reply to RVH.
+  int request_id_;
 
   ScopedObserver<content::RenderWidgetHost, content::RenderWidgetHostObserver>
       observer_;