FileChooser: Mojoify FileChooser IPC, part 4

Introduce content::FileSelectListener, which receives the result of FileSelectHelper.
An implementation class of FileSelectListener will implement blink.mojom.FileChooser
mojo interface in a following CL.

a) Add content::FileSelectListener
 - content/public/browser/BUILD.gn
 - content/public/browser/file_select_listener.h

b) Receive FileSelectHelper arguments, and call its methods to notify results
  instead of RFH::FilesSelectedInChooser() and RVH::
  DirectoryEnumerationFinished().
 - chrome/browser/file_select_helper.cc
 - chrome/browser/file_select_helper.h

c) Add FileChooserImpl class implementing content::FileSelectListener. A
  FileChooserImpl instance is passed to RenderFrameHostDelgate::
  RunFileChooser().  RFH::FilesSelectedInChooser() is removed because
  FileChooserImpl receives a file list, and it sends the list via IPC.
 - content/public/browser/render_frame_host.h
 - content/browser/frame_host/render_frame_host_impl.cc
 - content/browser/frame_host/render_frame_host_impl.h

d) Add ViewFileChooser class implementing content::FileSelectListener. A
  ViewFileChooser instance is passed to WebContentsDelegate::
  EnumerateDirectory(). RVH::DirectoryEnumerationFinished() is called by
  ViewFileChooser instead of FileSelectHelper.
 - content/browser/web_contents/web_contents_impl.cc
 - content/browser/web_contents/web_contents_impl.h

e) Add FileSelectListener argument, and call FileSelectListener::FileSelected()
  instead of RFH::FilesSelectedInChooser().  Maintain a FileSelectListener instance
  in AwWebContentsDelegate class.
 - android_webview/browser/aw_web_contents_delegate.cc
 - android_webview/browser/aw_web_contents_delegate.h


f) Add FileSelectListener argument, and forward it to another function or call
  FileSelectListener::FileSelectionCanceled()
 - chrome/browser/android/tab_web_contents_delegate_android.cc
 - chrome/browser/android/tab_web_contents_delegate_android.h
 - chrome/browser/devtools/devtools_window.cc
 - chrome/browser/devtools/devtools_window.h
 - chrome/browser/extensions/extension_view_host.cc
 - chrome/browser/extensions/extension_view_host.h
 - chrome/browser/ui/apps/chrome_app_delegate.cc
 - chrome/browser/ui/apps/chrome_app_delegate.h
 - chrome/browser/ui/browser.cc
 - chrome/browser/ui/browser.h
 - components/guest_view/browser/guest_view_base.cc
 - components/guest_view/browser/guest_view_base.h
 - content/browser/frame_host/render_frame_host_delegate.cc
 - content/browser/frame_host/render_frame_host_delegate.h
 - content/public/browser/web_contents_delegate.cc
 - content/public/browser/web_contents_delegate.h
 - extensions/browser/app_window/app_delegate.h
 - extensions/browser/app_window/app_window.cc
 - extensions/browser/app_window/app_window.h
 - extensions/shell/browser/shell_app_delegate.cc
 - extensions/shell/browser/shell_app_delegate.h

g) Add FileSelectListener argument, and call FileSelectListener::FileSelected()
  instead of RFH::FilesSelectedInChooser()
 - chrome/browser/ssl/security_state_tab_helper_browsertest.cc
 - content/test/content_browser_test_utils_internal.cc
 - content/test/content_browser_test_utils_internal.h

h) Add MockFileLister and pass it to RunFileChooser() to avoid null dereference.
 - content/browser/web_contents/web_contents_impl_browsertest.cc


FYI: All-in-one CL: https://chromium-review.googlesource.com/1170454

Bug: 869257
Change-Id: I190159bd5819d228c703028584b9929aa2ad80c2
Reviewed-on: https://chromium-review.googlesource.com/c/1251182
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596451}

android_webview/browser/aw_web_contents_delegate.cc

@@ -21,6 +21,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/navigation_interception/intercept_navigation_delegate.h"
+#include "content/public/browser/file_select_listener.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -89,11 +90,14 @@ void AwWebContentsDelegate::CanDownload(
 
 void AwWebContentsDelegate::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
+    std::unique_ptr<content::FileSelectListener> listener,
     const FileChooserParams& params) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> java_delegate = GetJavaDelegate(env);
-  if (!java_delegate.obj())
+  if (!java_delegate.obj()) {
+    listener->FileSelectionCanceled();
     return;
+  }
 
   int mode_flags = 0;
   if (params.mode == FileChooserParams::Mode::kOpenMultiple) {
@@ -103,12 +107,14 @@ void AwWebContentsDelegate::RunFileChooser(
     mode_flags |= kFileChooserModeOpenMultiple | kFileChooserModeOpenFolder;
   } else if (params.mode == FileChooserParams::Mode::kSave) {
     // Save not supported, so cancel it.
-    render_frame_host->FilesSelectedInChooser(
-        std::vector<FileChooserFileInfoPtr>(), params.mode);
+    listener->FileSelectionCanceled();
     return;
   } else {
     DCHECK_EQ(FileChooserParams::Mode::kOpen, params.mode);
   }
+  DCHECK(!file_select_listener_)
+      << "Multiple concurrent FileChooser requests are not supported.";
+  file_select_listener_ = std::move(listener);
   Java_AwWebContentsDelegate_runFileChooser(
       env, java_delegate, render_frame_host->GetProcess()->GetID(),
       render_frame_host->GetRoutingID(), mode_flags,
@@ -276,6 +282,11 @@ void AwWebContentsDelegate::UpdateUserGestureCarryoverInfo(
     intercept_navigation_delegate->UpdateLastUserGestureCarryoverTimestamp();
 }
 
+std::unique_ptr<content::FileSelectListener>
+AwWebContentsDelegate::TakeFileSelectListener() {
+  return std::move(file_select_listener_);
+}
+
 static void JNI_AwWebContentsDelegate_FilesSelectedInChooser(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,
@@ -286,8 +297,20 @@ static void JNI_AwWebContentsDelegate_FilesSelectedInChooser(
     const JavaParamRef<jobjectArray>& display_names) {
   content::RenderFrameHost* rfh =
       content::RenderFrameHost::FromID(process_id, render_id);
-  if (!rfh)
+  auto* web_contents = WebContents::FromRenderFrameHost(rfh);
+  if (!web_contents)
+    return;
+  auto* delegate =
+      static_cast<AwWebContentsDelegate*>(web_contents->GetDelegate());
+  if (!delegate)
     return;
+  std::unique_ptr<content::FileSelectListener> listener =
+      delegate->TakeFileSelectListener();
+
+  if (!file_paths.obj()) {
+    listener->FileSelectionCanceled();
+    return;
+  }
 
   std::vector<std::string> file_path_str;
   std::vector<base::string16> display_name_str;
@@ -325,7 +348,7 @@ static void JNI_AwWebContentsDelegate_FilesSelectedInChooser(
   }
   DVLOG(0) << "File Chooser result: mode = " << mode
            << ", file paths = " << base::JoinString(file_path_str, ":");
-  rfh->FilesSelectedInChooser(files, mode);
+  listener->FileSelected(std::move(files), mode);
 }
 
 }  // namespace android_webview

android_webview/browser/aw_web_contents_delegate.h

@@ -29,6 +29,7 @@ class AwWebContentsDelegate
                    const std::string& request_method,
                    const base::Callback<void(bool)>& callback) override;
   void RunFileChooser(content::RenderFrameHost* render_frame_host,
+                      std::unique_ptr<content::FileSelectListener> listener,
                       const blink::mojom::FileChooserParams& params) override;
   void AddNewContents(content::WebContents* source,
                       std::unique_ptr<content::WebContents> new_contents,
@@ -65,8 +66,14 @@ class AwWebContentsDelegate
   void UpdateUserGestureCarryoverInfo(
       content::WebContents* web_contents) override;
 
+  std::unique_ptr<content::FileSelectListener> TakeFileSelectListener();
+
  private:
   bool is_fullscreen_;
+
+  // Maintain a FileSelectListener instance passed to RunFileChooser() until
+  // a callback is called.
+  std::unique_ptr<content::FileSelectListener> file_select_listener_;
 };
 
 }  // namespace android_webview

chrome/browser/android/tab_web_contents_delegate_android.cc

@@ -44,6 +44,7 @@
 #include "components/infobars/core/infobar.h"
 #include "components/navigation_interception/intercept_navigation_delegate.h"
 #include "components/security_state/content/content_utils.h"
+#include "content/public/browser/file_select_listener.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
@@ -124,13 +125,16 @@ TabWebContentsDelegateAndroid::~TabWebContentsDelegateAndroid() {
 
 void TabWebContentsDelegateAndroid::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
+    std::unique_ptr<content::FileSelectListener> listener,
     const FileChooserParams& params) {
   if (vr::VrTabHelper::IsUiSuppressedInVr(
           WebContents::FromRenderFrameHost(render_frame_host),
           vr::UiSuppressedElement::kFileChooser)) {
+    listener->FileSelectionCanceled();
     return;
   }
-  FileSelectHelper::RunFileChooser(render_frame_host, params);
+  FileSelectHelper::RunFileChooser(render_frame_host, std::move(listener),
+                                   params);
 }
 
 std::unique_ptr<BluetoothChooser>

chrome/browser/android/tab_web_contents_delegate_android.h

@@ -36,6 +36,7 @@ class TabWebContentsDelegateAndroid
   ~TabWebContentsDelegateAndroid() override;
 
   void RunFileChooser(content::RenderFrameHost* render_frame_host,
+                      std::unique_ptr<content::FileSelectListener> listener,
                       const blink::mojom::FileChooserParams& params) override;
   std::unique_ptr<content::BluetoothChooser> RunBluetoothChooser(
       content::RenderFrameHost* frame,

chrome/browser/devtools/devtools_window.cc

@@ -45,6 +45,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/devtools_agent_host.h"
+#include "content/public/browser/file_select_listener.h"
 #include "content/public/browser/keyboard_event_processing_result.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "content/public/browser/navigation_controller.h"
@@ -1274,8 +1275,10 @@ content::ColorChooser* DevToolsWindow::OpenColorChooser(
 
 void DevToolsWindow::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
+    std::unique_ptr<content::FileSelectListener> listener,
     const blink::mojom::FileChooserParams& params) {
-  FileSelectHelper::RunFileChooser(render_frame_host, params);
+  FileSelectHelper::RunFileChooser(render_frame_host, std::move(listener),
+                                   params);
 }
 
 bool DevToolsWindow::PreHandleGestureEvent(

chrome/browser/devtools/devtools_window.h

@@ -330,6 +330,7 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
       const std::vector<blink::mojom::ColorSuggestionPtr>& suggestions)
       override;
   void RunFileChooser(content::RenderFrameHost* render_frame_host,
+                      std::unique_ptr<content::FileSelectListener> listener,
                       const blink::mojom::FileChooserParams& params) override;
   bool PreHandleGestureEvent(content::WebContents* source,
                              const blink::WebGestureEvent& event) override;

chrome/browser/extensions/extension_view_host.cc

@@ -19,6 +19,7 @@
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
+#include "content/public/browser/file_select_listener.h"
 #include "content/public/browser/keyboard_event_processing_result.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_view_host.h"
@@ -224,10 +225,12 @@ content::ColorChooser* ExtensionViewHost::OpenColorChooser(
 
 void ExtensionViewHost::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
+    std::unique_ptr<content::FileSelectListener> listener,
     const blink::mojom::FileChooserParams& params) {
   // For security reasons opening a file picker requires a visible <input>
   // element to click on, so this code only exists for extensions with a view.
-  FileSelectHelper::RunFileChooser(render_frame_host, params);
+  FileSelectHelper::RunFileChooser(render_frame_host, std::move(listener),
+                                   params);
 }
 
 

chrome/browser/extensions/extension_view_host.h

@@ -83,6 +83,7 @@ class ExtensionViewHost
       const std::vector<blink::mojom::ColorSuggestionPtr>& suggestions)
       override;
   void RunFileChooser(content::RenderFrameHost* render_frame_host,
+                      std::unique_ptr<content::FileSelectListener> listener,
                       const blink::mojom::FileChooserParams& params) override;
   void ResizeDueToAutoResize(content::WebContents* source,
                              const gfx::Size& new_size) override;

chrome/browser/file_select_helper.cc

@@ -27,6 +27,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/file_select_listener.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -61,11 +62,6 @@ using content::WebContents;
 
 namespace {
 
-// There is only one file-selection happening at any given time,
-// so we allocate an enumeration ID for that purpose.  All IDs from
-// the renderer must start at 0 and increase.
-const int kFileSelectEnumerationId = -1;
-
 // Converts a list of FilePaths to a list of ui::SelectedFileInfo.
 std::vector<ui::SelectedFileInfo> FilePathListToSelectedFileInfoList(
     const std::vector<base::FilePath>& paths) {
@@ -125,10 +121,9 @@ void InterpretSafeBrowsingVerdict(const base::Callback<void(bool)>& recipient,
 
 struct FileSelectHelper::ActiveDirectoryEnumeration {
   explicit ActiveDirectoryEnumeration(const base::FilePath& path)
-      : rvh_(NULL), path_(path) {}
+      : path_(path) {}
 
   std::unique_ptr<net::DirectoryLister> lister_;
-  RenderViewHost* rvh_;
   const base::FilePath path_;
   std::vector<base::FilePath> results_;
 };
@@ -173,8 +168,7 @@ void FileSelectHelper::FileSelectedWithExtraInfo(
 
   const base::FilePath& path = file.local_path;
   if (dialog_type_ == ui::SelectFileDialog::SELECT_UPLOAD_FOLDER) {
-    StartNewEnumeration(path, kFileSelectEnumerationId,
-                        render_frame_host_->GetRenderViewHost());
+    StartNewEnumeration(path);
     return;
   }
 
@@ -220,15 +214,11 @@ void FileSelectHelper::MultiFilesSelectedWithExtraInfo(
 }
 
 void FileSelectHelper::FileSelectionCanceled(void* params) {
-  NotifyRenderFrameHostAndEnd(std::vector<ui::SelectedFileInfo>());
+  RunFileChooserEnd();
 }
 
-void FileSelectHelper::StartNewEnumeration(const base::FilePath& path,
-                                           int request_id,
-                                           RenderViewHost* render_view_host) {
-  request_id_ = request_id;
+void FileSelectHelper::StartNewEnumeration(const base::FilePath& path) {
   auto entry = std::make_unique<ActiveDirectoryEnumeration>(path);
-  entry->rvh_ = render_view_host;
   entry->lister_.reset(new net::DirectoryLister(
       path, net::DirectoryLister::NO_SORT_RECURSIVE, this));
   entry->lister_->Start();
@@ -257,8 +247,6 @@ void FileSelectHelper::OnListDone(int error) {
   // This entry needs to be cleaned up when this function is done.
   std::unique_ptr<ActiveDirectoryEnumeration> entry =
       std::move(directory_enumeration_);
-  if (!entry->rvh_)
-    return;
   if (error) {
     FileSelectionCanceled(NULL);
     return;
@@ -267,10 +255,18 @@ void FileSelectHelper::OnListDone(int error) {
   std::vector<ui::SelectedFileInfo> selected_files =
       FilePathListToSelectedFileInfoList(entry->results_);
 
-  if (request_id_ == kFileSelectEnumerationId) {
+  if (dialog_type_ == ui::SelectFileDialog::SELECT_UPLOAD_FOLDER) {
     LaunchConfirmationDialog(entry->path_, std::move(selected_files));
   } else {
-    entry->rvh_->DirectoryEnumerationFinished(request_id_, entry->results_);
+    std::vector<FileChooserFileInfoPtr> chooser_files;
+    for (const auto& file_path : entry->results_) {
+      chooser_files.push_back(FileChooserFileInfo::NewNativeFile(
+          blink::mojom::NativeFileInfo::New(file_path, base::string16())));
+    }
+
+    listener_->FileSelected(std::move(chooser_files),
+                            FileChooserParams::Mode::kUploadFolder);
+    listener_.reset();
     EnumerateDirectoryEnd();
   }
 }
@@ -317,8 +313,8 @@ void FileSelectHelper::NotifyRenderFrameHostAndEnd(
 
 void FileSelectHelper::NotifyRenderFrameHostAndEndAfterConversion(
     std::vector<FileChooserFileInfoPtr> list) {
-  if (render_frame_host_)
-    render_frame_host_->FilesSelectedInChooser(list, dialog_mode_);
+  listener_->FileSelected(std::move(list), dialog_mode_);
+  listener_.reset();
 
   // No members should be accessed from here on.
   RunFileChooserEnd();
@@ -421,32 +417,37 @@ FileSelectHelper::GetFileTypesFromAcceptType(
 // static
 void FileSelectHelper::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
+    std::unique_ptr<content::FileSelectListener> listener,
     const FileChooserParams& params) {
   Profile* profile = Profile::FromBrowserContext(
       render_frame_host->GetProcess()->GetBrowserContext());
   // FileSelectHelper will keep itself alive until it sends the result message.
   scoped_refptr<FileSelectHelper> file_select_helper(
       new FileSelectHelper(profile));
-  file_select_helper->RunFileChooser(render_frame_host, params.Clone());
+  file_select_helper->RunFileChooser(render_frame_host, std::move(listener),
+                                     params.Clone());
 }
 
 // static
-void FileSelectHelper::EnumerateDirectory(content::WebContents* tab,
-                                          int request_id,
-                                          const base::FilePath& path) {
+void FileSelectHelper::EnumerateDirectory(
+    content::WebContents* tab,
+    std::unique_ptr<content::FileSelectListener> listener,
+    const base::FilePath& path) {
   Profile* profile = Profile::FromBrowserContext(tab->GetBrowserContext());
   // FileSelectHelper will keep itself alive until it sends the result message.
   scoped_refptr<FileSelectHelper> file_select_helper(
       new FileSelectHelper(profile));
-  file_select_helper->EnumerateDirectory(
-      request_id, tab->GetRenderViewHost(), path);
+  file_select_helper->EnumerateDirectory(std::move(listener), path);
 }
 
 void FileSelectHelper::RunFileChooser(
     content::RenderFrameHost* render_frame_host,
+    std::unique_ptr<content::FileSelectListener> listener,
     FileChooserParamsPtr params) {
   DCHECK(!render_frame_host_);
   DCHECK(!web_contents_);
+  DCHECK(listener);
+  DCHECK(!listener_);
   DCHECK(params->default_file_name.empty() ||
          params->mode == FileChooserParams::Mode::kSave)
       << "The default_file_name parameter should only be specified for Save "
@@ -456,6 +457,7 @@ void FileSelectHelper::RunFileChooser(
 
   render_frame_host_ = render_frame_host;
   web_contents_ = WebContents::FromRenderFrameHost(render_frame_host);
+  listener_ = std::move(listener);
   observer_.RemoveAll();
   content::WebContentsObserver::Observe(web_contents_);
   observer_.Add(render_frame_host_->GetRenderViewHost()->GetWidget());
@@ -542,7 +544,7 @@ void FileSelectHelper::ProceedWithSafeBrowsingVerdict(
     FileChooserParamsPtr params,
     bool allowed_by_safe_browsing) {
   if (!allowed_by_safe_browsing) {
-    NotifyRenderFrameHostAndEnd(std::vector<ui::SelectedFileInfo>());
+    RunFileChooserEnd();
     return;
   }
   RunFileChooserOnUIThread(default_file_path, std::move(params));
@@ -616,21 +618,27 @@ void FileSelectHelper::RunFileChooserEnd() {
   if (!temporary_files_.empty())
     return;
 
+  if (listener_)
+    listener_->FileSelectionCanceled();
   render_frame_host_ = nullptr;
   web_contents_ = nullptr;
   Release();
 }
 
-void FileSelectHelper::EnumerateDirectory(int request_id,
-                                          RenderViewHost* render_view_host,
-                                          const base::FilePath& path) {
+void FileSelectHelper::EnumerateDirectory(
+    std::unique_ptr<content::FileSelectListener> listener,
+    const base::FilePath& path) {
+  DCHECK(listener);
+  DCHECK(!listener_);
+  dialog_type_ = ui::SelectFileDialog::SELECT_NONE;
+  listener_ = std::move(listener);
   // Because this class returns notifications to the RenderViewHost, it is
   // difficult for callers to know how long to keep a reference to this
   // instance. We AddRef() here to keep the instance alive after we return
   // to the caller, until the last callback is received from the enumeration
   // code. At that point, we must call EnumerateDirectoryEnd().
   AddRef();
-  StartNewEnumeration(path, request_id, render_view_host);
+  StartNewEnumeration(path);
 }
 
 // This method is called when we receive the last callback from the enumeration