ArcSelectFilesHandler : Make it possible to show multiple dialogs at a time.

Before:
ArcFileSystemBridge owns a single instance of ArcSelectFilesHandler.
ARC apps can't request to open multiple dialogs at a time.

After:
ArcFileSystemBridge owns an instance of ArcSelectFilesHandlersManager,
which manages multiple ArcSelectFilesHandler instances.
ARC apps can request to open multiple dialogs at a time,
with the limitation of 1 dialog per 1 task ID.

Bug: 970262
Change-Id: I0227aa8ef2329699e79ba0a9d2c3f244d42e6085
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1669249
Commit-Queue: Satoshi Niwa <niwa@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672009}

chrome/browser/chromeos/arc/fileapi/arc_file_system_bridge.cc

@@ -175,7 +175,8 @@ ArcFileSystemBridge::ArcFileSystemBridge(content::BrowserContext* context,
                                          ArcBridgeService* bridge_service)
     : profile_(Profile::FromBrowserContext(context)),
       bridge_service_(bridge_service),
-      select_files_handler_(std::make_unique<ArcSelectFilesHandler>(context)),
+      select_files_handlers_manager_(
+          std::make_unique<ArcSelectFilesHandlersManager>(context)),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   bridge_service_->file_system()->SetHost(this);
@@ -325,7 +326,8 @@ void ArcFileSystemBridge::OpenFileToRead(const std::string& url,
 
 void ArcFileSystemBridge::SelectFiles(mojom::SelectFilesRequestPtr request,
                                       SelectFilesCallback callback) {
-  select_files_handler_->SelectFiles(std::move(request), std::move(callback));
+  select_files_handlers_manager_->SelectFiles(std::move(request),
+                                              std::move(callback));
 }
 
 void ArcFileSystemBridge::OnFileSelectorEvent(
@@ -337,8 +339,8 @@ void ArcFileSystemBridge::OnFileSelectorEvent(
     std::move(callback).Run();
     return;
   }
-  select_files_handler_->OnFileSelectorEvent(std::move(event),
-                                             std::move(callback));
+  select_files_handlers_manager_->OnFileSelectorEvent(std::move(event),
+                                                      std::move(callback));
 }
 
 void ArcFileSystemBridge::GetFileSelectorElements(
@@ -350,8 +352,8 @@ void ArcFileSystemBridge::GetFileSelectorElements(
     std::move(callback).Run(mojom::FileSelectorElements::New());
     return;
   }
-  select_files_handler_->GetFileSelectorElements(std::move(request),
-                                                 std::move(callback));
+  select_files_handlers_manager_->GetFileSelectorElements(std::move(request),
+                                                          std::move(callback));
 }
 
 void ArcFileSystemBridge::OpenFileToReadAfterGetFileSize(

chrome/browser/chromeos/arc/fileapi/arc_file_system_bridge.h

@@ -121,7 +121,7 @@ class ArcFileSystemBridge : public KeyedService, public mojom::FileSystemHost {
 
   std::list<FileStreamForwarderPtr> file_stream_forwarders_;
 
-  std::unique_ptr<ArcSelectFilesHandler> select_files_handler_;
+  std::unique_ptr<ArcSelectFilesHandlersManager> select_files_handlers_manager_;
 
   base::WeakPtrFactory<ArcFileSystemBridge> weak_ptr_factory_;
 

chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.cc

@@ -157,6 +157,69 @@ void BuildFileTypeInfo(const mojom::SelectFilesRequestPtr& request,
 
 }  // namespace
 
+ArcSelectFilesHandlersManager::ArcSelectFilesHandlersManager(
+    content::BrowserContext* context)
+    : context_(context), weak_ptr_factory_(this) {}
+
+ArcSelectFilesHandlersManager::~ArcSelectFilesHandlersManager() = default;
+
+void ArcSelectFilesHandlersManager::SelectFiles(
+    const mojom::SelectFilesRequestPtr& request,
+    mojom::FileSystemHost::SelectFilesCallback callback) {
+  int task_id = request->task_id;
+  if (handlers_by_task_id_.find(task_id) != handlers_by_task_id_.end()) {
+    LOG(ERROR) << "SelectFileDialog is already shown for task ID : " << task_id;
+    std::move(callback).Run(mojom::SelectFilesResult::New());
+    return;
+  }
+
+  auto handler = std::make_unique<ArcSelectFilesHandler>(context_);
+  auto* handler_ptr = handler.get();
+  handlers_by_task_id_.emplace(task_id, std::move(handler));
+
+  // Make sure that the handler is erased when the SelectFileDialog is closed.
+  handler_ptr->SelectFiles(
+      std::move(request),
+      base::BindOnce(&ArcSelectFilesHandlersManager::EraseHandlerAndRunCallback,
+                     weak_ptr_factory_.GetWeakPtr(), task_id,
+                     std::move(callback)));
+}
+
+void ArcSelectFilesHandlersManager::OnFileSelectorEvent(
+    mojom::FileSelectorEventPtr event,
+    mojom::FileSystemHost::OnFileSelectorEventCallback callback) {
+  int task_id = event->creator_task_id;
+  auto iter = handlers_by_task_id_.find(task_id);
+  if (iter == handlers_by_task_id_.end()) {
+    LOG(ERROR) << "Can't find a SelectFileDialog for task ID : " << task_id;
+    std::move(callback).Run();
+    return;
+  }
+  iter->second->OnFileSelectorEvent(std::move(event), std::move(callback));
+}
+
+void ArcSelectFilesHandlersManager::GetFileSelectorElements(
+    mojom::GetFileSelectorElementsRequestPtr request,
+    mojom::FileSystemHost::GetFileSelectorElementsCallback callback) {
+  int task_id = request->creator_task_id;
+  auto iter = handlers_by_task_id_.find(task_id);
+  if (iter == handlers_by_task_id_.end()) {
+    LOG(ERROR) << "Can't find a SelectFileDialog for task ID : " << task_id;
+    std::move(callback).Run(mojom::FileSelectorElements::New());
+    return;
+  }
+  iter->second->GetFileSelectorElements(std::move(request),
+                                        std::move(callback));
+}
+
+void ArcSelectFilesHandlersManager::EraseHandlerAndRunCallback(
+    int task_id,
+    mojom::FileSystemHost::SelectFilesCallback callback,
+    mojom::SelectFilesResultPtr result) {
+  handlers_by_task_id_.erase(task_id);
+  std::move(callback).Run(std::move(result));
+}
+
 ArcSelectFilesHandler::ArcSelectFilesHandler(content::BrowserContext* context)
     : profile_(Profile::FromBrowserContext(context)) {
   dialog_holder_ = std::make_unique<SelectFileDialogHolder>(this);
@@ -169,13 +232,6 @@ void ArcSelectFilesHandler::SelectFiles(
     mojom::FileSystemHost::SelectFilesCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (!callback_.is_null()) {
-    LOG(ERROR)
-        << "There is already a ui::SelectFileDialog being shown currently. "
-        << "We can't open multiple ui::SelectFileDialogs at one time.";
-    std::move(callback).Run(mojom::SelectFilesResult::New());
-    return;
-  }
   callback_ = std::move(callback);
 
   // TODO(niwa): Convert all request options.

chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h

@@ -10,6 +10,7 @@
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/ui/views/select_file_dialog_extension.h"
 #include "components/arc/common/file_system.mojom.h"
 #include "content/public/browser/render_frame_host.h"
@@ -23,6 +24,7 @@ class BrowserContext;
 
 namespace arc {
 
+class ArcSelectFilesHandler;
 class SelectFileDialogHolder;
 
 // Exposed for testing.
@@ -32,7 +34,47 @@ extern const char kScriptClickDirectory[];
 extern const char kScriptClickFile[];
 extern const char kScriptGetElements[];
 
-// Handler for FileSystemHost.SelectFiles.
+// Manages multiple ArcSelectFilesHandler instances.
+class ArcSelectFilesHandlersManager {
+ public:
+  explicit ArcSelectFilesHandlersManager(content::BrowserContext* context);
+  ~ArcSelectFilesHandlersManager();
+
+  // Handler for FileSystemHost.SelectFiles.
+  // Creates a new ArcSelectFilesHandler instance.
+  void SelectFiles(const mojom::SelectFilesRequestPtr& request,
+                   mojom::FileSystemHost::SelectFilesCallback callback);
+
+  // Handler for FileSystemHost.OnFileSelectorEvent.
+  // Routes the request to the right ArcSelectFilesHandler instance.
+  void OnFileSelectorEvent(
+      mojom::FileSelectorEventPtr event,
+      mojom::FileSystemHost::OnFileSelectorEventCallback callback);
+
+  // Handler for FileSystemHost.GetFileSelectorElements.
+  // Routes the request to the right ArcSelectFilesHandler instance.
+  void GetFileSelectorElements(
+      mojom::GetFileSelectorElementsRequestPtr request,
+      mojom::FileSystemHost::GetFileSelectorElementsCallback callback);
+
+ private:
+  // Helper function for SelectFiles.
+  void EraseHandlerAndRunCallback(
+      int task_id,
+      mojom::FileSystemHost::SelectFilesCallback callback,
+      mojom::SelectFilesResultPtr result);
+
+  content::BrowserContext* const context_;
+
+  // Map of Task ID -> ArcSelectFilesHandler.
+  std::map<int, std::unique_ptr<ArcSelectFilesHandler>> handlers_by_task_id_;
+
+  base::WeakPtrFactory<ArcSelectFilesHandlersManager> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcSelectFilesHandlersManager);
+};
+
+// Manages a single SelectFileDialog instance.
 class ArcSelectFilesHandler : public ui::SelectFileDialog::Listener {
  public:
   explicit ArcSelectFilesHandler(content::BrowserContext* context);