chromeos: Select file dialog implementation for lacros, part 2

Fill out the ash-chrome side of the implementation. Add support for
all dialog types (open, open folder, save, etc.).

For now, place the file dialog in the default ash window container.
Future CLs will add plumbing to use the Lacros window's exo
ShellSurface as the parent. Fix the ExtensionDialog code to allow a
parent aura::Window that isn't associated with a views::Widget.

1-page design doc at go/lacros-file-picker
Lacros project overview at go/lacros

Bug: 1090587
Test: manual, can open/save files with Ctrl-O/Ctrl-S
Change-Id: I54f461e92b3e5d75808bcf04ce903b003b12490b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2265191
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782846}

chrome/browser/chromeos/lacros/DEPS

+specific_include_rules = {
+  "select_file_impl\.cc": [
+    # For window parenting.
+    "+ash/shell.h",
+  ],
+}

chrome/browser/chromeos/lacros/ash_chrome_service_impl.h

@@ -30,7 +30,6 @@ class AshChromeServiceImpl : public lacros::mojom::AshChromeService {
  private:
   mojo::Receiver<lacros::mojom::AshChromeService> receiver_;
 
-  // TODO(jamescook): Support more than one select dialog at a time.
   std::unique_ptr<SelectFileImpl> select_file_impl_;
 };
 

chrome/browser/chromeos/lacros/select_file_impl.cc

@@ -7,12 +7,108 @@
 #include <utility>
 #include <vector>
 
+#include "ash/public/cpp/shell_window_ids.h"
+#include "ash/shell.h"
 #include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
 #include "chromeos/lacros/mojom/select_file.mojom.h"
+#include "ui/shell_dialogs/select_file_dialog.h"
+#include "ui/shell_dialogs/select_file_policy.h"
+#include "ui/shell_dialogs/selected_file_info.h"
 
 namespace chromeos {
+namespace {
 
-// TODO(jamescook): Connection error handling.
+ui::SelectFileDialog::Type GetUiType(lacros::mojom::SelectFileDialogType type) {
+  switch (type) {
+    case lacros::mojom::SelectFileDialogType::kFolder:
+      return ui::SelectFileDialog::Type::SELECT_FOLDER;
+    case lacros::mojom::SelectFileDialogType::kUploadFolder:
+      return ui::SelectFileDialog::Type::SELECT_UPLOAD_FOLDER;
+    case lacros::mojom::SelectFileDialogType::kExistingFolder:
+      return ui::SelectFileDialog::Type::SELECT_EXISTING_FOLDER;
+    case lacros::mojom::SelectFileDialogType::kOpenFile:
+      return ui::SelectFileDialog::Type::SELECT_OPEN_FILE;
+    case lacros::mojom::SelectFileDialogType::kOpenMultiFile:
+      return ui::SelectFileDialog::Type::SELECT_OPEN_MULTI_FILE;
+    case lacros::mojom::SelectFileDialogType::kSaveAsFile:
+      return ui::SelectFileDialog::Type::SELECT_SAVEAS_FILE;
+  }
+}
+
+// Manages a single open/save dialog. There may be multiple dialogs showing at
+// the same time. Deletes itself when the dialog is closed.
+class SelectFileDialogHolder : public ui::SelectFileDialog::Listener {
+ public:
+  SelectFileDialogHolder(lacros::mojom::SelectFileOptionsPtr options,
+                         lacros::mojom::SelectFile::SelectCallback callback)
+      : select_callback_(std::move(callback)) {
+    // 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);
+
+    // 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(
+        ash::Shell::GetRootWindowForNewWindows(),
+        ash::kShellWindowId_DefaultContainerDeprecated);
+
+    // TODO(https://crbug.com/1090587): File type filter support.
+    select_file_dialog_->SelectFile(
+        GetUiType(options->type), options->title, options->default_path,
+        /*file_types=*/nullptr,
+        /*file_type_index=*/0,
+        /*default_extension=*/std::string(), owning_window,
+        /*params=*/nullptr);
+  }
+
+  SelectFileDialogHolder(const SelectFileDialogHolder&) = delete;
+  SelectFileDialogHolder& operator=(const SelectFileDialogHolder&) = delete;
+  ~SelectFileDialogHolder() override = default;
+
+ private:
+  // ui::SelectFileDialog::Listener:
+  void FileSelected(const base::FilePath& path,
+                    int index,
+                    void* params) override {
+    OnSelected({path});
+  }
+
+  void MultiFilesSelected(const std::vector<base::FilePath>& files,
+                          void* params) override {
+    OnSelected(files);
+  }
+
+  void FileSelectionCanceled(void* params) override {
+    // Cancel is the same as selecting nothing.
+    OnSelected({});
+  }
+
+  // Invokes |select_callback_| with the list of files and deletes this object.
+  void OnSelected(const std::vector<base::FilePath>& paths) {
+    std::vector<lacros::mojom::SelectedFileInfoPtr> files;
+    for (const auto& path : paths) {
+      lacros::mojom::SelectedFileInfoPtr file =
+          lacros::mojom::SelectedFileInfo::New();
+      file->file_path = path;
+      files.push_back(std::move(file));
+    }
+    std::move(select_callback_)
+        .Run(lacros::mojom::SelectFileResult::kSuccess, std::move(files));
+    delete this;
+  }
+
+  // Callback run after files are selected or the dialog is canceled.
+  lacros::mojom::SelectFile::SelectCallback select_callback_;
+
+  // The file select dialog.
+  scoped_refptr<ui::SelectFileDialog> select_file_dialog_;
+};
+
+}  // namespace
+
+// TODO(https://crbug.com/1090587): Connection error handling.
 SelectFileImpl::SelectFileImpl(
     mojo::PendingReceiver<lacros::mojom::SelectFile> receiver)
     : receiver_(this, std::move(receiver)) {}
@@ -21,15 +117,8 @@ SelectFileImpl::~SelectFileImpl() = default;
 
 void SelectFileImpl::Select(lacros::mojom::SelectFileOptionsPtr options,
                             SelectCallback callback) {
-  // TODO(jamescook): Open a real select file dialog. For now, just pretend we
-  // selected a well-known existing file.
-  lacros::mojom::SelectedFileInfoPtr file =
-      lacros::mojom::SelectedFileInfo::New();
-  file->file_path = base::FilePath("/etc/lsb-release");
-  std::vector<lacros::mojom::SelectedFileInfoPtr> files;
-  files.push_back(std::move(file));
-  std::move(callback).Run(lacros::mojom::SelectFileResult::kSuccess,
-                          std::move(files));
+  // Deletes itself when the dialog closes.
+  new SelectFileDialogHolder(std::move(options), std::move(callback));
 }
 
 }  // namespace chromeos

chrome/browser/ui/views/extensions/extension_dialog.cc

@@ -202,10 +202,13 @@ ExtensionDialog::ExtensionDialog(
   gfx::Rect screen_rect = display::Screen::GetScreen()
                               ->GetDisplayNearestWindow(parent_window)
                               .work_area();
-  gfx::Rect bounds =
-      parent_window ? views::Widget::GetWidgetForNativeWindow(parent_window)
-                          ->GetWindowBoundsInScreen()
-                    : screen_rect;
+  gfx::Rect bounds = screen_rect;
+  if (parent_window) {
+    views::Widget* parent_widget =
+        views::Widget::GetWidgetForNativeWindow(parent_window);
+    if (parent_widget)
+      bounds = parent_widget->GetWindowBoundsInScreen();
+  }
   bounds.ClampToCenteredSize(init_params.size);
 
   // Make sure bounds is larger than {min_size}.

chromeos/lacros/browser/lacros_chrome_service_impl.cc

@@ -23,6 +23,10 @@ LacrosChromeServiceImpl* LacrosChromeServiceImpl::Get() {
 LacrosChromeServiceImpl::LacrosChromeServiceImpl()
     : pending_ash_chrome_service_receiver_(
           ash_chrome_service_.BindNewPipeAndPassReceiver()) {
+  // Bind remote interfaces in ash-chrome.
+  ash_chrome_service_->BindSelectFile(
+      select_file_remote_.BindNewPipeAndPassReceiver());
+
   DCHECK(!g_instance);
   g_instance = this;
 }

chromeos/lacros/browser/lacros_chrome_service_impl.h

@@ -7,6 +7,7 @@
 
 #include "base/component_export.h"
 #include "chromeos/lacros/mojom/lacros.mojom.h"
+#include "chromeos/lacros/mojom/select_file.mojom.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
 namespace chromeos {
@@ -21,8 +22,8 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl
   LacrosChromeServiceImpl();
   ~LacrosChromeServiceImpl() override;
 
-  mojo::Remote<lacros::mojom::AshChromeService>& ash_chrome_service() {
-    return ash_chrome_service_;
+  mojo::Remote<lacros::mojom::SelectFile>& select_file_remote() {
+    return select_file_remote_;
   }
 
   // lacros::mojom::LacrosChromeService:
@@ -42,6 +43,9 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl
   // should be available.
   mojo::PendingReceiver<lacros::mojom::AshChromeService>
       pending_ash_chrome_service_receiver_;
+
+  // Proxy to SelectFile interface in ash-chrome.
+  mojo::Remote<lacros::mojom::SelectFile> select_file_remote_;
 };
 
 }  // namespace chromeos

ui/shell_dialogs/select_file_dialog_lacros.cc

@@ -7,12 +7,35 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/notreached.h"
 #include "chromeos/lacros/browser/lacros_chrome_service_impl.h"
-#include "chromeos/lacros/mojom/select_file.mojom-shared.h"
 #include "chromeos/lacros/mojom/select_file.mojom.h"
 #include "ui/shell_dialogs/select_file_policy.h"
 
 namespace ui {
+namespace {
+
+lacros::mojom::SelectFileDialogType GetMojoType(SelectFileDialog::Type type) {
+  switch (type) {
+    case SelectFileDialog::Type::SELECT_FOLDER:
+      return lacros::mojom::SelectFileDialogType::kFolder;
+    case SelectFileDialog::Type::SELECT_UPLOAD_FOLDER:
+      return lacros::mojom::SelectFileDialogType::kUploadFolder;
+    case SelectFileDialog::Type::SELECT_EXISTING_FOLDER:
+      return lacros::mojom::SelectFileDialogType::kExistingFolder;
+    case SelectFileDialog::Type::SELECT_OPEN_FILE:
+      return lacros::mojom::SelectFileDialogType::kOpenFile;
+    case SelectFileDialog::Type::SELECT_OPEN_MULTI_FILE:
+      return lacros::mojom::SelectFileDialogType::kOpenMultiFile;
+    case SelectFileDialog::Type::SELECT_SAVEAS_FILE:
+      return lacros::mojom::SelectFileDialogType::kSaveAsFile;
+    case SelectFileDialog::Type::SELECT_NONE:
+      NOTREACHED();
+      return lacros::mojom::SelectFileDialogType::kOpenFile;
+  }
+}
+
+}  // namespace
 
 SelectFileDialogLacros::Factory::Factory() = default;
 SelectFileDialogLacros::Factory::~Factory() = default;
@@ -26,20 +49,7 @@ ui::SelectFileDialog* SelectFileDialogLacros::Factory::Create(
 SelectFileDialogLacros::SelectFileDialogLacros(
     Listener* listener,
     std::unique_ptr<ui::SelectFilePolicy> policy)
-    : ui::SelectFileDialog(listener, std::move(policy)) {
-  auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
-  // TODO(jamescook): Move LacrosChromeServiceImpl construction earlier and
-  // remove these checks. This function is racy with lacros-chrome startup and
-  // the initial mojo connection. In practice, however, the remote is bound
-  // long before the user can trigger a select dialog.
-  if (!lacros_chrome_service ||
-      !lacros_chrome_service->ash_chrome_service().is_bound()) {
-    LOG(ERROR) << "Not connected to ash-chrome.";
-    return;
-  }
-  lacros_chrome_service->ash_chrome_service()->BindSelectFile(
-      select_file_remote_.BindNewPipeAndPassReceiver());
-}
+    : ui::SelectFileDialog(listener, std::move(policy)) {}
 
 SelectFileDialogLacros::~SelectFileDialogLacros() = default;
 
@@ -62,15 +72,25 @@ void SelectFileDialogLacros::SelectFileImpl(
     void* params) {
   params_ = params;
 
+  auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
+  // TODO(https://crbug.com/1090587): Move LacrosChromeServiceImpl construction
+  // earlier and remove these checks. This function is racy with lacros-chrome
+  // startup. In practice, however, the remote is bound long before the user
+  // can trigger a select dialog.
+  if (!lacros_chrome_service ||
+      !lacros_chrome_service->select_file_remote().is_bound()) {
+    LOG(ERROR) << "Not connected to ash-chrome.";
+    return;
+  }
+
   lacros::mojom::SelectFileOptionsPtr options =
       lacros::mojom::SelectFileOptions::New();
-  // TODO(jamescook): Correct type.
-  options->type = lacros::mojom::SelectFileDialogType::kOpenFile;
+  options->type = GetMojoType(type);
   options->title = title;
   options->default_path = default_path;
 
   // Send request to ash-chrome.
-  select_file_remote_->Select(
+  lacros_chrome_service->select_file_remote()->Select(
       std::move(options),
       base::BindOnce(&SelectFileDialogLacros::OnSelected, this));
 }

ui/shell_dialogs/select_file_dialog_lacros.h

@@ -7,8 +7,7 @@
 
 #include <vector>
 
-#include "chromeos/lacros/mojom/select_file.mojom.h"
-#include "mojo/public/cpp/bindings/remote.h"
+#include "chromeos/lacros/mojom/select_file.mojom-forward.h"
 #include "ui/shell_dialogs/select_file_dialog.h"
 #include "ui/shell_dialogs/select_file_dialog_factory.h"
 #include "ui/shell_dialogs/shell_dialogs_export.h"
@@ -61,9 +60,6 @@ class SHELL_DIALOGS_EXPORT SelectFileDialogLacros : public SelectFileDialog {
 
   // Cached parameters from the call to SelectFileImpl.
   void* params_ = nullptr;
-
-  // Remote in the ash-chrome process.
-  mojo::Remote<lacros::mojom::SelectFile> select_file_remote_;
 };
 
 }  // namespace ui