lacros: Support "Open" in chrome://downloads and download shelf

Clicking on an item in the download shelf or in chrome://downloads
attempts to open it in the OS-native application for that file type.
For example, it might open a video player or image viewer.

This needs to be handled by ash, so wire it through the
crosapi::mojom::FileManager API.

In the future we will need to show error messages for files that fail
to open. The new methods return a result enum. We also need this for
the existing ShowItemInFolder method, so deprecate the old one and
introduce an updated version.

This implementation isn't perfect. In some cases the file should be
routed back to the browser for display in a tab. Right now some files
that can't be opened in an app end up opening in an ash-chrome tab.
In the future we will need to wire up the ash file_manager code to
make "open in browser" redirect back to lacros-chrome.

Bug: 1141616
Test: added to lacros_browser_tests
Change-Id: I8cd4166bc594bf83a3ce9458d7c1d767bbb8c041
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511589
Auto-Submit: James Cook <jamescook@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823381}

chrome/browser/chromeos/crosapi/file_manager_ash.cc

@@ -4,13 +4,66 @@
 
 #include "chrome/browser/chromeos/crosapi/file_manager_ash.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/files/file_path.h"
+#include "chrome/browser/chromeos/file_manager/open_util.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chromeos/crosapi/cpp/crosapi_constants.h"
+#include "chromeos/crosapi/mojom/file_manager.mojom.h"
 
 namespace crosapi {
+namespace {
+
+// Lacros does not support multi-signin. Lacros uses /home/chronos/user as the
+// base for all system-level directories but the file manager expects the raw
+// profile path with the /home/chronos/u-{hash} prefix. Clean up the path.
+base::FilePath ExpandPath(Profile* primary_profile,
+                          const base::FilePath& path) {
+  return file_manager::util::ReplacePathPrefix(
+      path, base::FilePath(crosapi::kHomeChronosUserPath),
+      primary_profile->GetPath());
+}
+
+// Adapts a platform_util::OpenOperationResult to a crosapi::mojom::OpenResult
+// when running a |callback|.
+void RunWithOpenResult(base::OnceCallback<void(mojom::OpenResult)> callback,
+                       platform_util::OpenOperationResult result) {
+  mojom::OpenResult mojo_result;
+  switch (result) {
+    case platform_util::OPEN_SUCCEEDED:
+      mojo_result = mojom::OpenResult::kSucceeded;
+      break;
+    case platform_util::OPEN_FAILED_PATH_NOT_FOUND:
+      mojo_result = mojom::OpenResult::kFailedPathNotFound;
+      break;
+    case platform_util::OPEN_FAILED_INVALID_TYPE:
+      mojo_result = mojom::OpenResult::kFailedInvalidType;
+      break;
+    case platform_util::OPEN_FAILED_NO_HANLDER_FOR_FILE_TYPE:
+      mojo_result = mojom::OpenResult::kFailedNoHandlerForFileType;
+      break;
+    case platform_util::OPEN_FAILED_FILE_ERROR:
+      mojo_result = mojom::OpenResult::kFailedFileError;
+      break;
+  }
+  std::move(callback).Run(mojo_result);
+}
+
+// Opens an item of |type| at |path| and runs |callback| with the result.
+void OpenItem(const base::FilePath& path,
+              platform_util::OpenItemType item_type,
+              base::OnceCallback<void(mojom::OpenResult)> callback) {
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
+  base::FilePath full_path = ExpandPath(primary_profile, path);
+  file_manager::util::OpenItem(
+      primary_profile, full_path, item_type,
+      base::BindOnce(&RunWithOpenResult, std::move(callback)));
+}
+
+}  // namespace
 
 FileManagerAsh::FileManagerAsh(
     mojo::PendingReceiver<mojom::FileManager> receiver)
@@ -18,18 +71,33 @@ FileManagerAsh::FileManagerAsh(
 
 FileManagerAsh::~FileManagerAsh() = default;
 
-void FileManagerAsh::ShowItemInFolder(const base::FilePath& path) {
+void FileManagerAsh::DeprecatedShowItemInFolder(const base::FilePath& path) {
   Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
-  // Lacros does not support multi-signin. Lacros uses /home/chronos/user as the
-  // base for all system-level directories but the file manager expects the raw
-  // profile path with the /home/chronos/u-{hash} prefix. Clean up the path.
-  base::FilePath final_path = file_manager::util::ReplacePathPrefix(
-      path, base::FilePath(crosapi::kHomeChronosUserPath),
-      primary_profile->GetPath());
-  // Use platform_util instead of calling directly into file manager code
-  // because platform_util_ash.cc handles showing error dialogs for files and
-  // paths that cannot be opened.
+  base::FilePath final_path = ExpandPath(primary_profile, path);
+  // NOTE: This will show error dialogs for files and paths that cannot be
+  // opened, but the dialogs are modal to an ash-chrome window, not the
+  // lacros-chrome window that opened the file. That's why this version is
+  // deprecated.
   platform_util::ShowItemInFolder(primary_profile, final_path);
 }
 
+void FileManagerAsh::ShowItemInFolder(const base::FilePath& path,
+                                      ShowItemInFolderCallback callback) {
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
+  base::FilePath full_path = ExpandPath(primary_profile, path);
+  file_manager::util::ShowItemInFolder(
+      primary_profile, full_path,
+      base::BindOnce(&RunWithOpenResult, std::move(callback)));
+}
+
+void FileManagerAsh::OpenFolder(const base::FilePath& path,
+                                OpenFolderCallback callback) {
+  OpenItem(path, platform_util::OPEN_FOLDER, std::move(callback));
+}
+
+void FileManagerAsh::OpenFile(const base::FilePath& path,
+                              OpenFileCallback callback) {
+  OpenItem(path, platform_util::OPEN_FILE, std::move(callback));
+}
+
 }  // namespace crosapi

chrome/browser/chromeos/crosapi/file_manager_ash.h

@@ -22,7 +22,12 @@ class FileManagerAsh : public mojom::FileManager {
   ~FileManagerAsh() override;
 
   // crosapi::mojom::FileManager:
-  void ShowItemInFolder(const base::FilePath& path) override;
+  void DeprecatedShowItemInFolder(const base::FilePath& path) override;
+  void ShowItemInFolder(const base::FilePath& path,
+                        ShowItemInFolderCallback callback) override;
+  void OpenFolder(const base::FilePath& path,
+                  OpenFolderCallback callback) override;
+  void OpenFile(const base::FilePath& path, OpenFileCallback callback) override;
 
  private:
   mojo::Receiver<mojom::FileManager> receiver_;

chrome/browser/lacros/file_manager_lacros_browsertest.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 "base/files/file_path.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/crosapi/mojom/file_manager.mojom-test-utils.h"
+#include "chromeos/crosapi/mojom/file_manager.mojom.h"
+#include "chromeos/lacros/lacros_chrome_service_impl.h"
+#include "content/public/test/browser_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using FileManagerLacrosBrowserTest = InProcessBrowserTest;
+
+IN_PROC_BROWSER_TEST_F(FileManagerLacrosBrowserTest, Basics) {
+  crosapi::mojom::FileManagerAsyncWaiter waiter(
+      chromeos::LacrosChromeServiceImpl::Get()->file_manager_remote().get());
+  crosapi::mojom::OpenResult result;
+
+  // The file manager requires a large amount of setup to get it to run in
+  // tests. See FileManagerBrowserTestBase. For example, by default it won't
+  // show files at arbitrary paths, like the sorts of temporary files you can
+  // safely create in a test. Since we don't have a test mojo API to put the
+  // file manager into a suitable state, exercise our API via error cases.
+  base::FilePath bad_path("/does/not/exist");
+  waiter.ShowItemInFolder(bad_path, &result);
+  EXPECT_EQ(crosapi::mojom::OpenResult::kFailedPathNotFound, result);
+
+  waiter.OpenFolder(bad_path, &result);
+  EXPECT_EQ(crosapi::mojom::OpenResult::kFailedPathNotFound, result);
+
+  waiter.OpenFile(bad_path, &result);
+  EXPECT_EQ(crosapi::mojom::OpenResult::kFailedPathNotFound, result);
+
+  base::FilePath malformed_path("!@#$%");
+  waiter.ShowItemInFolder(malformed_path, &result);
+  EXPECT_EQ(crosapi::mojom::OpenResult::kFailedPathNotFound, result);
+}

chrome/browser/platform_util_lacros.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/platform_util.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/notreached.h"
 #include "chrome/browser/platform_util_internal.h"
@@ -11,21 +12,56 @@
 #include "chromeos/lacros/lacros_chrome_service_impl.h"
 
 namespace platform_util {
+namespace {
+
+void OnOpenResult(const base::FilePath& path,
+                  crosapi::mojom::OpenResult result) {
+  if (result == crosapi::mojom::OpenResult::kSucceeded)
+    return;
+  // TODO(https://crbug.com/1144316): Show error messages. This will require
+  // refactoring the existing file manager string files, or introducing new
+  // lacros strings.
+  LOG(ERROR) << "Unable to open " << path.AsUTF8Unsafe() << " " << result;
+}
+
+}  // namespace
+
 namespace internal {
 
 void PlatformOpenVerifiedItem(const base::FilePath& path, OpenItemType type) {
-  NOTIMPLEMENTED();
+  auto* service = chromeos::LacrosChromeServiceImpl::Get();
+  if (service->GetInterfaceVersion(crosapi::mojom::FileManager::Uuid_) < 1) {
+    LOG(ERROR) << "Unsupported ash version.";
+    return;
+  }
+  switch (type) {
+    case OPEN_FILE:
+      service->file_manager_remote()->OpenFile(
+          path, base::BindOnce(&OnOpenResult, path));
+      break;
+    case OPEN_FOLDER:
+      service->file_manager_remote()->OpenFolder(
+          path, base::BindOnce(&OnOpenResult, path));
+      break;
+  }
 }
 
 }  // namespace internal
 
 void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) {
   auto* service = chromeos::LacrosChromeServiceImpl::Get();
-  if (!service->IsFileManagerAvailable()) {
+  int interface_version =
+      service->GetInterfaceVersion(crosapi::mojom::FileManager::Uuid_);
+  if (interface_version < 0) {
     DLOG(ERROR) << "Unsupported ash version.";
     return;
   }
-  service->file_manager_remote()->ShowItemInFolder(full_path);
+  if (interface_version < 1) {
+    service->file_manager_remote()->DeprecatedShowItemInFolder(full_path);
+    return;
+  }
+  service->file_manager_remote()->ShowItemInFolder(
+      full_path, base::BindOnce(&OnOpenResult, full_path));
 }
 
 void OpenExternal(Profile* profile, const GURL& url) {

chrome/test/BUILD.gn

@@ -3171,6 +3171,7 @@ if (chromeos_is_browser_only) {
     ]
 
     sources = [
+      "../browser/lacros/file_manager_lacros_browsertest.cc",
       "../browser/lacros/keystore_service_lacros_browsertest.cc",
       "../browser/lacros/message_center_lacros_browsertest.cc",
       "../browser/lacros/screen_manager_lacros_browsertest.cc",

chromeos/crosapi/mojom/file_manager.mojom

@@ -6,12 +6,43 @@ module crosapi.mojom;
 
 import "mojo/public/mojom/base/file_path.mojom";
 
+[Stable, Extensible]
+enum OpenResult {
+  kUnknown = 0,
+  kSucceeded = 1,
+  // Specified path does not exist.
+  kFailedPathNotFound = 2,
+  // Type of object found at path does not match what was expected. For example,
+  // OpenFile was called on a folder.
+  kFailedInvalidType = 3,
+  // No file handler found for the file. For example, user tried to open a macOS
+  // .dmg file.
+  kFailedNoHandlerForFileType = 4,
+  // Open failed due to some other error.
+  kFailedFileError = 5,
+};
+
 // Interacts with the file manager (the Chrome OS equivalent of macOS Finder or
 // Windows Explorer). Implemented in ash-chrome.
 [Stable, Uuid="61e61690-fb13-40c8-a167-d7fc18a8fe6b"]
 interface FileManager {
-  // Opens the folder containing the item specified by |full_path| and selects
-  // the item. If the path is invalid or the file doesn't exist, an error dialog
-  // is shown.
-  ShowItemInFolder@0(mojo_base.mojom.FilePath path);
+  // Deprecated. Prefer ShowItemInFolder() below.
+  DeprecatedShowItemInFolder@0(mojo_base.mojom.FilePath path);
+
+  // Opens the folder containing the item specified by |path| and selects the
+  // item. If the path is invalid or the file doesn't exist, an error dialog is
+  // shown. Added in M88.
+  [MinVersion=1]
+  ShowItemInFolder@1(mojo_base.mojom.FilePath path) => (OpenResult result);
+
+  // As above, but opens the folder at |path| and does not select an item.
+  // Added in M88.
+  [MinVersion=1]
+  OpenFolder@2(mojo_base.mojom.FilePath path) => (OpenResult result);
+
+  // Opens the file specified by |path| in the desktop's default manner. For
+  // example, an image file might be opened by the image gallery app. Added in
+  // M88.
+  [MinVersion=1]
+  OpenFile@3(mojo_base.mojom.FilePath path) => (OpenResult result);
 };