lacros: Support multiple simultaneous select file dialogs

Previously, select file dialogs opened from lacros-chrome had an empty
RoutingID. This caused console errors, and also meant only one dialog
could be open at a time.

Give each dialog instance a unique ID. For now, this is just an
monotonically increasing integer. Longer-term it will be a window ID
provided from the lacros-chrome process. This will also allow proper
window stacking -- that doesn't work yet.

The method SelectFileWithFileManagerParams() already has 9 arguments.
Rather than adding more, consolidate all "owner window" information
into a struct.

This CL requires deps from //c/b/chromeos to //c/b/ui/views for
SelectFileDialogExtension, similar to //c/b/chromeos/arc/fileapi.
This allows us to avoid polluting the cross-platform SelectFileDialog
API with Chrome OS specific parameters.

Feature design doc at go/lacros-select-file

Bug: 1090587
Change-Id: I52d87c7216fc8a6c6b9eb1ea9c41c23f54f34e75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293164Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787762}

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

@@ -389,11 +389,14 @@ bool SelectFileDialogHolder::SelectFile(
     return false;
   }
 
+  SelectFileDialogExtension::Owner owner;
+  owner.window = owner_window;
+  owner.android_task_id = task_id;
   select_file_dialog_->SelectFileWithFileManagerParams(
       type,
       /*title=*/base::string16(), default_path, file_types,
-      /*file_type_index=*/0, owner_window,
-      /*params=*/nullptr, task_id, show_android_picker_apps);
+      /*file_type_index=*/0,
+      /*params=*/nullptr, owner, show_android_picker_apps);
   return true;
 }
 

chrome/browser/chromeos/crosapi/DEPS

@@ -6,5 +6,7 @@ specific_include_rules = {
   "select_file_crosapi\.cc": [
     # For window parenting.
     "+ash/shell.h",
+    # For Chrome OS-specific file manager parameters.
+    "+chrome/browser/ui/views/select_file_dialog_extension.h",
   ],
 }

chrome/browser/chromeos/crosapi/select_file_crosapi.cc

@@ -12,6 +12,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/numerics/ranges.h"
+#include "chrome/browser/ui/views/select_file_dialog_extension.h"
 #include "chromeos/crosapi/mojom/select_file.mojom.h"
 #include "ui/shell_dialogs/select_file_dialog.h"
 #include "ui/shell_dialogs/select_file_policy.h"
@@ -20,6 +21,9 @@
 
 namespace {
 
+// TODO(https://crbug.com/1090587): Replace with window ID from Wayland client.
+int g_next_window_id = 0;
+
 ui::SelectFileDialog::Type GetUiType(
     crosapi::mojom::SelectFileDialogType type) {
   switch (type) {
@@ -60,13 +64,16 @@ class SelectFileDialogHolder : public ui::SelectFileDialog::Listener {
     // Policy is null because showing the file-dialog-blocked infobar is handled
     // client-side in lacros-chrome.
     select_file_dialog_ =
-        ui::SelectFileDialog::Create(this, /*policy=*/nullptr);
+        SelectFileDialogExtension::Create(this, /*policy=*/nullptr);
 
+    SelectFileDialogExtension::Owner owner;
     // TODO(https://crbug.com/1090587): Parent to the ShellSurface that spawned
     // the dialog. For now, just put it on the default desktop.
-    aura::Window* owning_window = ash::Shell::GetContainer(
+    owner.window = ash::Shell::GetContainer(
         ash::Shell::GetRootWindowForNewWindows(),
         ash::kShellWindowId_DefaultContainerDeprecated);
+    // TODO(https://crbug.com/1090587): Replace with ID from Wayland client.
+    owner.lacros_window_id = g_next_window_id++;
 
     int file_type_index = 0;
     if (options->file_types) {
@@ -88,11 +95,11 @@ class SelectFileDialogHolder : public ui::SelectFileDialog::Listener {
           GetUiAllowedPaths(options->file_types->allowed_paths);
     }
     // |default_extension| is unused on Chrome OS.
-    select_file_dialog_->SelectFile(
+    select_file_dialog_->SelectFileWithFileManagerParams(
         GetUiType(options->type), options->title, options->default_path,
-        file_types_.get(), file_type_index, /*default_extension=*/std::string(),
-        owning_window,
-        /*params=*/nullptr);
+        file_types_.get(), file_type_index,
+        /*params=*/nullptr, owner,
+        /*show_android_picker_apps=*/false);
   }
 
   SelectFileDialogHolder(const SelectFileDialogHolder&) = delete;
@@ -154,7 +161,7 @@ class SelectFileDialogHolder : public ui::SelectFileDialog::Listener {
   crosapi::mojom::SelectFile::SelectCallback select_callback_;
 
   // The file select dialog.
-  scoped_refptr<ui::SelectFileDialog> select_file_dialog_;
+  scoped_refptr<SelectFileDialogExtension> select_file_dialog_;
 
   // Optional file type extension filters.
   std::unique_ptr<ui::SelectFileDialog::FileTypeInfo> file_types_;

chrome/browser/ui/views/select_file_dialog_extension.cc

@@ -165,10 +165,14 @@ void FindRuntimeContext(gfx::NativeWindow owner_window,
 
 SelectFileDialogExtension::RoutingID GetRoutingID(
     content::WebContents* web_contents,
-    int android_task_id) {
-  if (android_task_id != SelectFileDialogExtension::kAndroidTaskIdNone) {
-    return base::StringPrintf("android.%d", android_task_id);
-  } else if (web_contents) {
+    const SelectFileDialogExtension::Owner& owner) {
+  if (owner.android_task_id.has_value())
+    return base::StringPrintf("android.%d", *owner.android_task_id);
+
+  if (owner.lacros_window_id.has_value())
+    return base::StringPrintf("lacros.%d", *owner.lacros_window_id);
+
+  if (web_contents) {
     return base::StringPrintf(
         "web.%d", web_contents->GetMainFrame()->GetFrameTreeNodeId());
   }
@@ -180,6 +184,9 @@ SelectFileDialogExtension::RoutingID GetRoutingID(
 
 /////////////////////////////////////////////////////////////////////////////
 
+SelectFileDialogExtension::Owner::Owner() = default;
+SelectFileDialogExtension::Owner::~Owner() = default;
+
 // static
 SelectFileDialogExtension* SelectFileDialogExtension::Create(
     Listener* listener,
@@ -298,9 +305,8 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
     const base::FilePath& default_path,
     const FileTypeInfo* file_types,
     int file_type_index,
-    gfx::NativeWindow owner_window,
     void* params,
-    int owner_android_task_id,
+    const Owner& owner,
     bool show_android_picker_apps) {
   if (owner_window_) {
     LOG(ERROR) << "File dialog already in use!";
@@ -314,8 +320,8 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
   content::WebContents* web_contents = nullptr;
 
   // Obtain BaseWindow and WebContents if the owner window is browser.
-  if (owner_android_task_id == kAndroidTaskIdNone)
-    FindRuntimeContext(owner_window, &base_window, &web_contents);
+  if (!owner.android_task_id.has_value() && !owner.lacros_window_id.has_value())
+    FindRuntimeContext(owner.window, &base_window, &web_contents);
 
   if (web_contents)
     profile_ = Profile::FromBrowserContext(web_contents->GetBrowserContext());
@@ -329,7 +335,7 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
 
   // Check if we have another dialog opened for the contents. It's unlikely, but
   // possible. In such situation, discard this request.
-  RoutingID routing_id = GetRoutingID(web_contents, owner_android_task_id);
+  RoutingID routing_id = GetRoutingID(web_contents, owner);
   if (PendingExists(routing_id))
     return;
 
@@ -391,7 +397,7 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
 
   ExtensionDialog::InitParams dialog_params(
       {kFileManagerWidth, kFileManagerHeight});
-  dialog_params.is_modal = (owner_window != nullptr);
+  dialog_params.is_modal = (owner.window != nullptr);
   dialog_params.min_size = {kFileManagerMinimumWidth,
                             kFileManagerMinimumHeight};
   dialog_params.title = file_manager::util::GetSelectFileDialogTitle(type);
@@ -402,7 +408,7 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
 
   ExtensionDialog* dialog = ExtensionDialog::Show(
       file_manager_url,
-      base_window ? base_window->GetNativeWindow() : owner_window, profile_,
+      base_window ? base_window->GetNativeWindow() : owner.window, profile_,
       web_contents, this /* ExtensionDialog::Observer */, dialog_params);
   if (!dialog) {
     LOG(ERROR) << "Unable to create extension dialog";
@@ -418,7 +424,7 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
   extension_dialog_ = dialog;
   params_ = params;
   routing_id_ = routing_id;
-  owner_window_ = owner_window;
+  owner_window_ = owner.window;
 }
 
 void SelectFileDialogExtension::SelectFileImpl(
@@ -431,9 +437,11 @@ void SelectFileDialogExtension::SelectFileImpl(
     gfx::NativeWindow owner_window,
     void* params) {
   // |default_extension| is ignored.
-  SelectFileWithFileManagerParams(
-      type, title, default_path, file_types, file_type_index, owner_window,
-      params, kAndroidTaskIdNone, /*show_android_picker_apps=*/false);
+  Owner owner;
+  owner.window = owner_window;
+  SelectFileWithFileManagerParams(type, title, default_path, file_types,
+                                  file_type_index, params, owner,
+                                  /*show_android_picker_apps=*/false);
 }
 
 bool SelectFileDialogExtension::HasMultipleFileTypeChoicesImpl() {

chrome/browser/ui/views/select_file_dialog_extension.h

@@ -11,6 +11,7 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/optional.h"
 #include "chrome/browser/ui/views/extensions/extension_dialog_observer.h"
 #include "ui/gfx/native_widget_types.h"  // gfx::NativeWindow
 #include "ui/shell_dialogs/select_file_dialog.h"
@@ -42,8 +43,6 @@ class SelectFileDialogExtension
   // every WebContents or every Android task ID.
   typedef std::string RoutingID;
 
-  static const int kAndroidTaskIdNone = -1;
-
   static SelectFileDialogExtension* Create(
       ui::SelectFileDialog::Listener* listener,
       std::unique_ptr<ui::SelectFilePolicy> policy);
@@ -70,20 +69,27 @@ class SelectFileDialogExtension
   content::RenderViewHost* GetRenderViewHost();
 
   // Call SelectFile with params specific to Chrome OS file manager.
-  // |owner_android_task_id| is the Android task ID of the owner window if the
-  //     owner is Android, or kAndroidTaskIdNone if the owner is browser.
+  // |owner| specifies the window and app type that opened the dialog.
   // |show_android_picker_apps| determines whether to show Android picker apps
   //     in the select file dialog.
-  void SelectFileWithFileManagerParams(
-      Type type,
-      const base::string16& title,
-      const base::FilePath& default_path,
-      const FileTypeInfo* file_types,
-      int file_type_index,
-      aura::Window* owning_window,
-      void* params,
-      int owner_android_task_id,
-      bool show_android_picker_apps);
+  struct Owner {
+    Owner();
+    ~Owner();
+    // The native window that opened the dialog.
+    aura::Window* window = nullptr;
+    // Android task ID if the owner window is an Android app.
+    base::Optional<int> android_task_id;
+    // Lacros window ID if the owner window is a Lacros browser.
+    base::Optional<int> lacros_window_id;
+  };
+  void SelectFileWithFileManagerParams(Type type,
+                                       const base::string16& title,
+                                       const base::FilePath& default_path,
+                                       const FileTypeInfo* file_types,
+                                       int file_type_index,
+                                       void* params,
+                                       const Owner& owner,
+                                       bool show_android_picker_apps);
 
  protected:
   // SelectFileDialog implementation.