chromeos: Select file dialog implementation for lacros, part 1

Add the skeleton for a mojo API for open/save file dialogs, roughly
following the existing SelectFileDialog C++ interface.

Add SelectFileDialogLacros, which lives in lacros-chrome and binds
to a remote SelectFile interface in ash-chrome. The class lives in
//ui/shell_dialogs to be close to other per-platform select dialog
wrappers, as well as to demonstrate how a lacros mojo API can be
called from a component outside src/chrome.

Add a stub SelectFileImpl in ash-chrome. For now this just returns
a single file immediately. Follow-up CLs will actually open a dialog,
using the existing file manager-backed dialog used by ash.

1-page design doc at go/lacros-file-picker

Bug: 1090587
Test: manual, hit Ctrl-O and /etc/lsb-release is loaded in a tab
Change-Id: I9ebd9ff25f8aa32deac5eabb85f40a37e4b72856
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261405
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782801}

chrome/browser/chromeos/BUILD.gn

@@ -1359,6 +1359,8 @@ source_set("chromeos") {
     "lacros/lacros_manager.h",
     "lacros/lacros_util.cc",
     "lacros/lacros_util.h",
+    "lacros/select_file_impl.cc",
+    "lacros/select_file_impl.h",
     "language_preferences.cc",
     "language_preferences.h",
     "launcher_search_provider/error_reporter.cc",

chrome/browser/chromeos/lacros/ash_chrome_service_impl.cc

@@ -7,6 +7,8 @@
 #include <utility>
 
 #include "base/logging.h"
+#include "chrome/browser/chromeos/lacros/select_file_impl.h"
+#include "chromeos/lacros/mojom/select_file.mojom.h"
 
 namespace chromeos {
 
@@ -20,4 +22,9 @@ AshChromeServiceImpl::AshChromeServiceImpl(
 
 AshChromeServiceImpl::~AshChromeServiceImpl() = default;
 
+void AshChromeServiceImpl::BindSelectFile(
+    mojo::PendingReceiver<lacros::mojom::SelectFile> receiver) {
+  select_file_impl_ = std::make_unique<SelectFileImpl>(std::move(receiver));
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/lacros/ash_chrome_service_impl.h

@@ -5,12 +5,16 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LACROS_ASH_CHROME_SERVICE_IMPL_H_
 #define CHROME_BROWSER_CHROMEOS_LACROS_ASH_CHROME_SERVICE_IMPL_H_
 
+#include <memory>
+
 #include "chromeos/lacros/mojom/lacros.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 
 namespace chromeos {
 
+class SelectFileImpl;
+
 // Implementation of AshChromeService. It provides a set of APIs that
 // lacros-chrome can call into.
 class AshChromeServiceImpl : public lacros::mojom::AshChromeService {
@@ -19,8 +23,15 @@ class AshChromeServiceImpl : public lacros::mojom::AshChromeService {
       mojo::PendingReceiver<lacros::mojom::AshChromeService> pending_receiver);
   ~AshChromeServiceImpl() override;
 
+  // lacros::mojom::AshChromeService:
+  void BindSelectFile(
+      mojo::PendingReceiver<lacros::mojom::SelectFile> receiver) override;
+
  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_;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/lacros/select_file_impl.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/lacros/select_file_impl.h"
+
+#include <utility>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "chromeos/lacros/mojom/select_file.mojom.h"
+
+namespace chromeos {
+
+// TODO(jamescook): Connection error handling.
+SelectFileImpl::SelectFileImpl(
+    mojo::PendingReceiver<lacros::mojom::SelectFile> receiver)
+    : receiver_(this, std::move(receiver)) {}
+
+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));
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/lacros/select_file_impl.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LACROS_SELECT_FILE_IMPL_H_
+#define CHROME_BROWSER_CHROMEOS_LACROS_SELECT_FILE_IMPL_H_
+
+#include "chromeos/lacros/mojom/select_file.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+
+namespace chromeos {
+
+// Implements the SelectFile mojo interface for open/save dialogs. Wraps the
+// underlying Chrome OS SelectFileExtension implementation, which uses the WebUI
+// file manager to provide the dialogs. Lives on the UI thread.
+class SelectFileImpl : public lacros::mojom::SelectFile {
+ public:
+  explicit SelectFileImpl(
+      mojo::PendingReceiver<lacros::mojom::SelectFile> receiver);
+  SelectFileImpl(const SelectFileImpl&) = delete;
+  SelectFileImpl& operator=(const SelectFileImpl&) = delete;
+  ~SelectFileImpl() override;
+
+  // lacros::mojom::SelectFile:
+  void Select(lacros::mojom::SelectFileOptionsPtr options,
+              SelectCallback callback) override;
+
+ private:
+  mojo::Receiver<lacros::mojom::SelectFile> receiver_;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LACROS_SELECT_FILE_IMPL_H_

chromeos/lacros/browser/BUILD.gn

@@ -2,6 +2,11 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/chromeos/ui_mode.gni")
+
+# Code lives in the lacros-chrome browser only, not ash-chrome.
+assert(chromeos_is_browser_only)
+
 # Reset sources_assignment_filter for the BUILD.gn file to prevent
 # regression during the migration of Chromium away from the feature.
 # See docs/no_sources_assignment_filter.md for more information.
@@ -13,6 +18,7 @@ component("browser") {
   deps = [
     "//base",
     "//chromeos/lacros/mojom",
+    "//mojo/public/cpp/bindings",
   ]
   sources = [
     "lacros_chrome_service_impl.cc",

chromeos/lacros/browser/lacros_chrome_service_impl.cc

@@ -9,12 +9,28 @@
 #include "base/logging.h"
 
 namespace chromeos {
+namespace {
+
+LacrosChromeServiceImpl* g_instance = nullptr;
+
+}  // namespace
+
+// static
+LacrosChromeServiceImpl* LacrosChromeServiceImpl::Get() {
+  return g_instance;
+}
 
 LacrosChromeServiceImpl::LacrosChromeServiceImpl()
     : pending_ash_chrome_service_receiver_(
-          ash_chrome_service_.BindNewPipeAndPassReceiver()) {}
+          ash_chrome_service_.BindNewPipeAndPassReceiver()) {
+  DCHECK(!g_instance);
+  g_instance = this;
+}
 
-LacrosChromeServiceImpl::~LacrosChromeServiceImpl() = default;
+LacrosChromeServiceImpl::~LacrosChromeServiceImpl() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+}
 
 void LacrosChromeServiceImpl::RequestAshChromeServiceReceiver(
     RequestAshChromeServiceReceiverCallback callback) {

chromeos/lacros/browser/lacros_chrome_service_impl.h

@@ -11,23 +11,26 @@
 
 namespace chromeos {
 
-// Implements LacrosChromeService.
+// Implements LacrosChromeService, which owns the mojo remote connection to
+// ash-chrome.
 class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl
     : public lacros::mojom::LacrosChromeService {
  public:
-  // TODO(hidehiko): Add static getter of the instance.
-  // The instance of this class should be globally unique.
+  static LacrosChromeServiceImpl* Get();
 
   LacrosChromeServiceImpl();
   ~LacrosChromeServiceImpl() override;
 
+  mojo::Remote<lacros::mojom::AshChromeService>& ash_chrome_service() {
+    return ash_chrome_service_;
+  }
+
   // lacros::mojom::LacrosChromeService:
   void RequestAshChromeServiceReceiver(
       RequestAshChromeServiceReceiverCallback callback) override;
 
  private:
   // Proxy to AshChromeService in ash-chrome.
-  // TODO(hidehiko): Add getter for Remote<AshChromeService>.
   mojo::Remote<lacros::mojom::AshChromeService> ash_chrome_service_;
 
   // Pending receiver of AshChromeService.

chromeos/lacros/mojom/BUILD.gn

@@ -5,5 +5,10 @@
 import("//mojo/public/tools/bindings/mojom.gni")
 
 mojom("mojom") {
-  sources = [ "lacros.mojom" ]
+  sources = [
+    "lacros.mojom",
+    "select_file.mojom",
+  ]
+
+  public_deps = [ "//mojo/public/mojom/base" ]
 }

chromeos/lacros/mojom/lacros.mojom

@@ -4,9 +4,13 @@
 
 module lacros.mojom;
 
+import "chromeos/lacros/mojom/select_file.mojom";
+
 // AshChromeService defines the APIs that live in ash-chrome and are
 // accessed from lacros-chrome.
 interface AshChromeService {
+  // Binds the SelectFile interface for open/save dialogs.
+  BindSelectFile@0(pending_receiver<SelectFile> receiver);
 };
 
 // LacrosChromeService defines the APIs that live in lacros-chrome and

chromeos/lacros/mojom/select_file.mojom

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module lacros.mojom;
+
+import "mojo/public/mojom/base/file_path.mojom";
+import "mojo/public/mojom/base/string16.mojom";
+
+// The type of dialog to open. The integer values intentionally do not match
+// ui::SelectFileDialog::Type because there is no "none" value and kOpenFile
+// is the common case.
+[Extensible]
+enum SelectFileDialogType {
+  // For opening a file.
+  kOpenFile = 0,
+
+  // Like kOpenFile, but allowing multiple files to open.
+  kOpenMultiFile = 1,
+
+  // For opening a folder.
+  kFolder = 2,
+
+  // Like kFolder, but the dialog UI shows it's specifically for "upload".
+  kUploadFolder = 3,
+
+  // Like kFolder, but folder creation is disabled.
+  kExistingFolder = 4,
+
+  // For saving into a file, allowing a nonexistent file to be selected.
+  kSaveAsFile = 5,
+};
+
+// Options for the select file dialog.
+struct SelectFileOptions {
+  // Dialog type.
+  SelectFileDialogType type;
+
+  // Window title.
+  mojo_base.mojom.String16 title;
+
+  // Default path to open.
+  mojo_base.mojom.FilePath default_path;
+
+  // TODO(jamescook): Many more.
+};
+
+// Result of the select file dialog.
+[Extensible]
+enum SelectFileResult {
+  // Everything worked.
+  kSuccess = 0,
+
+  // TODO(jamescook): Error handling.
+};
+
+// Information about the selected file or files.
+struct SelectedFileInfo {
+  // Path to the file.
+  mojo_base.mojom.FilePath file_path;
+
+  // TODO(jamescook): Many more.
+};
+
+// Select file dialog interface. Wrapper around the C++ SelectFileDialog
+// class. Called from lacros-chrome. Implemented in ash-chrome using the
+// file manager dialog.
+interface SelectFile {
+  // Selects one or more files. If the dialog is closed or canceled without
+  // selecting a file, or if there is an error, |files| will be empty.
+  Select@0(SelectFileOptions options) =>
+      (SelectFileResult result, array<SelectedFileInfo> files);
+};

ui/shell_dialogs/BUILD.gn

@@ -114,6 +114,11 @@ jumbo_component("shell_dialogs") {
       "select_file_dialog_lacros.cc",
       "select_file_dialog_lacros.h",
     ]
+    deps += [
+      "//chromeos/lacros/browser",
+      "//chromeos/lacros/mojom",
+      "//mojo/public/cpp/bindings",
+    ]
   }
 }
 

ui/shell_dialogs/DEPS

 include_rules = [
+  "+chromeos/lacros",
   "+components/remote_cocoa",
   "+mojo/core/embedder",
   "+mojo/public/cpp/bindings",

ui/shell_dialogs/OWNERS

 per-file *android*=miguelg@chromium.org
 per-file *android*=qinmin@chromium.org
 
+per-file *lacros*=file://chromeos/LACROS_OWNERS
+
 per-file *win*=robliao@chromium.org
 # COMPONENT: Internals>PlatformIntegration

ui/shell_dialogs/select_file_dialog_lacros.cc

@@ -4,9 +4,12 @@
 
 #include "ui/shell_dialogs/select_file_dialog_lacros.h"
 
+#include <utility>
+
 #include "base/bind.h"
-#include "base/notreached.h"
-#include "base/task/thread_pool.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 {
@@ -23,7 +26,20 @@ ui::SelectFileDialog* SelectFileDialogLacros::Factory::Create(
 SelectFileDialogLacros::SelectFileDialogLacros(
     Listener* listener,
     std::unique_ptr<ui::SelectFilePolicy> policy)
-    : ui::SelectFileDialog(listener, std::move(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());
+}
 
 SelectFileDialogLacros::~SelectFileDialogLacros() = default;
 
@@ -44,17 +60,41 @@ void SelectFileDialogLacros::SelectFileImpl(
     const base::FilePath::StringType& default_extension,
     gfx::NativeWindow owning_window,
     void* params) {
-  // TODO(https://crbug.com/1090587): Proxy the request over IPC to ash-chrome.
-  NOTIMPLEMENTED();
-  // Until we have an implementation, pretend the user cancelled the dialog.
-  // Post a task to avoid reentrancy issues. |this| is ref-counted.
-  base::ThreadPool::PostTask(
-      FROM_HERE, base::BindOnce(&SelectFileDialogLacros::Cancel, this, params));
+  params_ = params;
+
+  lacros::mojom::SelectFileOptionsPtr options =
+      lacros::mojom::SelectFileOptions::New();
+  // TODO(jamescook): Correct type.
+  options->type = lacros::mojom::SelectFileDialogType::kOpenFile;
+  options->title = title;
+  options->default_path = default_path;
+
+  // Send request to ash-chrome.
+  select_file_remote_->Select(
+      std::move(options),
+      base::BindOnce(&SelectFileDialogLacros::OnSelected, this));
 }
 
-void SelectFileDialogLacros::Cancel(void* params) {
-  if (listener_)
-    listener_->FileSelectionCanceled(params);
+void SelectFileDialogLacros::OnSelected(
+    lacros::mojom::SelectFileResult result,
+    std::vector<lacros::mojom::SelectedFileInfoPtr> files) {
+  if (!listener_)
+    return;
+  if (files.empty()) {
+    listener_->FileSelectionCanceled(params_);
+    return;
+  }
+  if (files.size() == 1) {
+    // TODO(jamescook): Support correct file filter |index|.
+    // TODO(jamescook): Use FileSelectedWithExtraInfo instead.
+    listener_->FileSelected(files[0]->file_path, /*index=*/0, params_);
+    return;
+  }
+  std::vector<base::FilePath> paths;
+  for (auto& file : files)
+    paths.push_back(std::move(file->file_path));
+  // TODO(jamescook): Use MultiFilesSelectedWithExtraInfo instead.
+  listener_->MultiFilesSelected(paths, params_);
 }
 
 }  // namespace ui

ui/shell_dialogs/select_file_dialog_lacros.h

@@ -5,6 +5,10 @@
 #ifndef UI_SHELL_DIALOGS_SELECT_FILE_DIALOG_LACROS_H_
 #define UI_SHELL_DIALOGS_SELECT_FILE_DIALOG_LACROS_H_
 
+#include <vector>
+
+#include "chromeos/lacros/mojom/select_file.mojom.h"
+#include "mojo/public/cpp/bindings/remote.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"
@@ -14,7 +18,7 @@ namespace ui {
 // SelectFileDialogLacros implements file open and save dialogs for the
 // lacros-chrome binary. The dialog itself is handled by the file manager in
 // ash-chrome.
-class SelectFileDialogLacros : public SelectFileDialog {
+class SHELL_DIALOGS_EXPORT SelectFileDialogLacros : public SelectFileDialog {
  public:
   class SHELL_DIALOGS_EXPORT Factory : public SelectFileDialogFactory {
    public:
@@ -51,8 +55,15 @@ class SelectFileDialogLacros : public SelectFileDialog {
   // Private because SelectFileDialog is ref-counted.
   ~SelectFileDialogLacros() override;
 
-  // Calls the listener's cancel method with |params|.
-  void Cancel(void* params);
+  // Callback for file selection.
+  void OnSelected(lacros::mojom::SelectFileResult result,
+                  std::vector<lacros::mojom::SelectedFileInfoPtr> files);
+
+  // 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