arc: Implement (Create|Destroy)Moniker() in FileSystemHost

ARCVM will call CreateMoniker() instead of GetVirtualFileId() to access
Chrome's virtual file system files provided by ChromeContentProvider.
DestroyMoniker() will be called when the corresponding FD is closed on
the guest side.
This change will allow us to replace VirtualFileProvider with Fusebox,
which provides a more generalized and better virtual file system
integration (incl. write support) with ARCVM.

Bug: b:358171341
Test: unit_tests --gtest_filter="ArcFileSystemBridgeTest.*"
Test: Manual with ag/27824875: Confirm that Linux files and WebShare
      files can still be shared with ARCVM, and write operations are
      additionally supported after fixing b/346906502.

Change-Id: I57f31a478faea6f4513f1fb89c7ce201140103ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5629191
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Nigel Tao <nigeltao@chromium.org>
Commit-Queue: Youkichi Hosoi <youkichihosoi@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350586}

ash/components/arc/mojom/file_system.mojom

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Next MinVersion: 25
+// Next MinVersion: 26
 
 module arc.mojom;
 
@@ -13,6 +13,7 @@ import "ash/components/arc/mojom/intent_common.mojom";
 import "ash/components/arc/mojom/video_common.mojom";
 import "mojo/public/mojom/base/file_path.mojom";
 import "mojo/public/mojom/base/time.mojom";
+import "mojo/public/mojom/base/token.mojom";
 import "url/mojom/url.mojom";
 
 // TODO(b/193097194): Use structured types (e.g., url.mojom.Url for URLs).
@@ -314,7 +315,7 @@ union MediaStoreMetadata {
   MediaStoreDownloadMetadata download;
 };
 
-// Next method ID: 13
+// Next method ID: 15
 interface FileSystemHost {
   // Returns the name of the file specified by the URL.
   // When an error occurs, returns null value.
@@ -371,6 +372,22 @@ interface FileSystemHost {
   // with the given |metadata|.
   [MinVersion=23] OnMediaStoreUriAdded@12(
       url.mojom.Url uri, MediaStoreMetadata metadata);
+
+  // Creates a Fusebox Moniker associated with the given content URI, shares the
+  // Moniker's path with the guest via seneschal, and returns the Moniker. A
+  // null value is returned on failure. On success, the caller is responsible
+  // for releasing the Moniker by calling DestroyMoniker() after they have
+  // finished using it.
+  [MinVersion=25] CreateMoniker@13(url.mojom.Url content_uri, bool read_only) =>
+      (mojo_base.mojom.Token? moniker);
+
+  // Destroys the given Fusebox Moniker and resources accosiated with it.
+  // Returns true when the Moniker and the resources have been successfully
+  // released. Otherwise false is returned, which indicates that the given
+  // Moniker has already been destroyed via DestroyMoniker(), or it is not the
+  // one created by ArcFileSystemBridge.
+  [MinVersion=25] DestroyMoniker@14(mojo_base.mojom.Token moniker) =>
+      (bool success);
 };
 
 // The browser process is using this interface in ArcFileSystemOperationRunner,

chrome/browser/ash/arc/fileapi/BUILD.gn

@@ -67,11 +67,15 @@ static_library("fileapi") {
     "//ash/constants",
     "//base",
     "//chrome/browser/ash/drive",
+    "//chrome/browser/ash/file_manager",
     "//chrome/browser/ash/fileapi",
+    "//chrome/browser/ash/fusebox",
+    "//chrome/browser/ash/guest_os",
     "//chrome/browser/ash/policy/dlp",
     "//chrome/browser/chromeos/policy/dlp",
     "//chrome/browser/profiles:profile",
     "//chrome/common",
+    "//chromeos/ash/components/dbus/concierge:concierge_proto",
     "//chromeos/ash/components/dbus/virtual_file_provider",
     "//content/public/browser",
     "//extensions/browser/api/file_handlers",
@@ -86,7 +90,10 @@ static_library("fileapi") {
     "//chrome/browser/apps/app_preload_service/proto",
   ]
 
-  allow_circular_includes_from = [ "//chrome/browser/ash/fileapi" ]
+  allow_circular_includes_from = [
+    "//chrome/browser/ash/file_manager",
+    "//chrome/browser/ash/fileapi",
+  ]
 }
 
 source_set("unit_tests") {
@@ -118,7 +125,11 @@ source_set("unit_tests") {
     "//chrome/browser/ash/file_system_provider",
     "//chrome/browser/ash/file_system_provider:test_support",
     "//chrome/browser/ash/fileapi",
+    "//chrome/browser/ash/fusebox",
+    "//chrome/browser/ash/guest_os",
+    "//chrome/browser/ash/guest_os/public",
     "//chrome/test:test_support",
+    "//chromeos/ash/components/dbus/seneschal",
     "//chromeos/ash/components/dbus/virtual_file_provider",
     "//components/keyed_service/core",
     "//content/test:test_support",

chrome/browser/ash/arc/fileapi/DEPS

@@ -19,8 +19,11 @@ include_rules = [
   "+chrome/browser/ash/fileapi",
   "+chrome/browser/ash/file_manager",
   "+chrome/browser/ash/file_system_provider",
+  "+chrome/browser/ash/fusebox",
+  "+chrome/browser/ash/guest_os",
   "+chrome/browser/ash/policy/dlp",
   "+chrome/browser/chromeos/policy/dlp",
+  "+chrome/browser/ash/login/users",
   "+chrome/browser/profiles",
   "+chrome/browser/ui/ash/shelf",
   "+chrome/browser/ui/chrome_select_file_policy.h",

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

@@ -13,10 +13,12 @@
 #include <vector>
 
 #include "ash/components/arc/arc_browser_context_keyed_service_factory_base.h"
+#include "ash/components/arc/arc_util.h"
 #include "ash/components/arc/session/arc_bridge_service.h"
 #include "base/functional/bind.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
+#include "base/not_fatal_until.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/escape.h"
 #include "base/system/sys_info.h"
@@ -31,8 +33,13 @@
 #include "chrome/browser/ash/file_manager/fileapi_util.h"
 #include "chrome/browser/ash/file_manager/path_util.h"
 #include "chrome/browser/ash/fileapi/external_file_url_util.h"
+#include "chrome/browser/ash/fusebox/fusebox_moniker.h"
+#include "chrome/browser/ash/fusebox/fusebox_server.h"
+#include "chrome/browser/ash/guest_os/guest_os_session_tracker.h"
+#include "chrome/browser/ash/guest_os/guest_os_share_path.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/ash/components/dbus/virtual_file_provider/virtual_file_provider_client.h"
+#include "chromeos/ash/components/dbus/vm_concierge/concierge_service.pb.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
@@ -53,6 +60,12 @@ namespace arc {
 
 namespace {
 
+// The maximum number of Fusebox Monikers that can be shared concurrently.
+// When this number is reached, CreateMoniker() does not create a new Moniker,
+// and returns a null response. A large number is randomly chosen to not block
+// usual user flows.
+constexpr size_t kMaxNumberOfSharedMonikers = 16384;
+
 // Returns true if it's OK to allow ARC apps to read the given URL.
 bool IsUrlAllowed(const GURL& url) {
   // Currently, only externalfile URLs are allowed.
@@ -423,6 +436,104 @@ void ArcFileSystemBridge::OnMediaStoreUriAdded(
     observer.OnMediaStoreUriAdded(uri, *metadata);
 }
 
+void ArcFileSystemBridge::CreateMoniker(const GURL& content_uri,
+                                        bool read_only,
+                                        CreateMonikerCallback callback) {
+  CHECK_CURRENTLY_ON(content::BrowserThread::UI, base::NotFatalUntil::M132);
+
+  if (shared_monikers_.size() >= kMaxNumberOfSharedMonikers) {
+    // Avoid creating too many Monikers without closing.
+    LOG(WARNING) << "Rejecting to create a Fusebox Moniker for " << content_uri
+                 << " because the maximum number of shared Monikers ("
+                 << kMaxNumberOfSharedMonikers << ") is reached";
+    std::move(callback).Run(std::nullopt);
+    return;
+  }
+
+  const GURL url_decoded = DecodeFromChromeContentProviderUrl(content_uri);
+  if (url_decoded.is_empty() || !IsUrlAllowed(url_decoded)) {
+    LOG(ERROR) << "Invalid ChromeContentProvider URI: " << content_uri;
+    std::move(callback).Run(std::nullopt);
+    return;
+  }
+
+  scoped_refptr<storage::FileSystemContext> context =
+      GetFileSystemContext(profile_);
+  const file_manager::util::FileSystemURLAndHandle fs_url_and_handle =
+      GetFileSystemURL(*context, url_decoded);
+  if (!fs_url_and_handle.url.is_valid()) {
+    LOG(ERROR) << "Failed to get FileSystemURL for URL: " << url_decoded;
+    std::move(callback).Run(std::nullopt);
+    return;
+  }
+
+  const auto& vm_info =
+      guest_os::GuestOsSessionTracker::GetForProfile(profile_)->GetVmInfo(
+          kArcVmName);
+  if (!vm_info) {
+    LOG(ERROR) << "ARCVM is not running";
+    std::move(callback).Run(std::nullopt);
+    return;
+  }
+
+  auto* fusebox_server = fusebox::Server::GetInstance();
+  if (!fusebox_server) {
+    LOG(ERROR) << "Failed to get Fusebox server";
+    std::move(callback).Run(std::nullopt);
+    return;
+  }
+  const fusebox::Moniker moniker =
+      fusebox_server->CreateMoniker(fs_url_and_handle.url, read_only);
+  shared_monikers_.insert(moniker);
+
+  guest_os::GuestOsSharePath::GetForProfile(profile_)->SharePath(
+      kArcVmName, vm_info->seneschal_server_handle(),
+      base::FilePath(fusebox::MonikerMap::GetFilename(moniker)),
+      base::BindOnce(&ArcFileSystemBridge::OnShareMonikerPath,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback),
+                     moniker));
+}
+
+void ArcFileSystemBridge::OnShareMonikerPath(
+    CreateMonikerCallback callback,
+    const fusebox::Moniker& moniker,
+    const base::FilePath& path,
+    bool success,
+    const std::string& failure_reason) {
+  if (!success) {
+    LOG(ERROR) << "Failed to share Fusebox Moniker path with ARCVM: reason: "
+               << failure_reason << ", path: " << path;
+    std::move(callback).Run(std::nullopt);
+    DestroyMoniker(moniker, base::DoNothing());
+    return;
+  }
+  std::move(callback).Run(moniker);
+}
+
+void ArcFileSystemBridge::DestroyMoniker(const fusebox::Moniker& moniker,
+                                         DestroyMonikerCallback callback) {
+  CHECK_CURRENTLY_ON(content::BrowserThread::UI, base::NotFatalUntil::M132);
+
+  if (!shared_monikers_.erase(moniker)) {
+    LOG(ERROR) << "Failed to destroy unknown Fusebox Moniker: "
+               << moniker.ToString();
+    std::move(callback).Run(false);
+    return;
+  }
+
+  auto* fusebox_server = fusebox::Server::GetInstance();
+  if (fusebox_server) {
+    fusebox_server->DestroyMoniker(moniker);
+  } else {
+    LOG(ERROR) << "Failed to get Fusebox server";
+    // Not returning false, since the Moniker and the resources held by the
+    // Fusebox server should have already gone.
+  }
+  // No need to call GuestOsSharePath::UnsharePath(), because the method is
+  // eventually called from GuestOsSharePath::PathDeleted().
+  std::move(callback).Run(true);
+}
+
 void ArcFileSystemBridge::GenerateVirtualFileId(
     const GURL& url_decoded,
     GenerateVirtualFileIdCallback callback,

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

@@ -10,16 +10,19 @@
 #include <list>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 #include "ash/components/arc/mojom/file_system.mojom-forward.h"
 #include "ash/components/arc/session/connection_observer.h"
+#include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "chrome/browser/ash/arc/fileapi/arc_select_files_handler.h"
 #include "chrome/browser/ash/arc/fileapi/file_stream_forwarder.h"
+#include "chrome/browser/ash/fusebox/fusebox_moniker.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "storage/browser/file_system/file_system_operation.h"
 #include "storage/browser/file_system/watcher_manager.h"
@@ -122,6 +125,11 @@ class ArcFileSystemBridge
       GetFileSelectorElementsCallback callback) override;
   void OnMediaStoreUriAdded(const GURL& uri,
                             mojom::MediaStoreMetadataPtr metadata) override;
+  void CreateMoniker(const GURL& content_uri,
+                     bool read_only,
+                     CreateMonikerCallback callback) override;
+  void DestroyMoniker(const fusebox::Moniker& moniker,
+                      DestroyMonikerCallback callback) override;
 
   // ConnectionObserver<mojom::FileSystemInstance> overrides:
   void OnConnectionClosed() override;
@@ -169,6 +177,14 @@ class ArcFileSystemBridge
                       const std::string& id,
                       base::ScopedFD fd);
 
+  // Called from CreateMoniker() after sharing the new Moniker's path with
+  // ARCVM.
+  void OnShareMonikerPath(CreateMonikerCallback callback,
+                          const fusebox::Moniker& moniker,
+                          const base::FilePath& path,
+                          bool success,
+                          const std::string& failure_reason);
+
   // Used to implement OpenFileToRead(), needs to be testable.
   //
   // Decode a percent-encoded externalfile: URL to an absolute path on
@@ -204,6 +220,9 @@ class ArcFileSystemBridge
   // Map from file descriptor IDs to requested URLs.
   std::map<std::string, GURL> id_to_url_;
 
+  // Set of Fusebox Monikers currently shared with ARC.
+  std::set<fusebox::Moniker> shared_monikers_;
+
   std::list<FileStreamForwarderPtr> file_stream_forwarders_;
 
   std::unique_ptr<ArcSelectFilesHandlersManager> select_files_handlers_manager_;

chrome/browser/ash/arc/fileapi/arc_file_system_bridge_unittest.cc

@@ -7,6 +7,7 @@
 #include <utility>
 #include <vector>
 
+#include "ash/components/arc/arc_util.h"
 #include "ash/components/arc/mojom/file_system.mojom.h"
 #include "ash/components/arc/session/arc_bridge_service.h"
 #include "ash/components/arc/test/connection_holder_util.h"
@@ -19,6 +20,7 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
+#include "base/test/test_future.h"
 #include "base/time/time.h"
 #include "chrome/browser/ash/arc/fileapi/chrome_content_provider_url_util.h"
 #include "chrome/browser/ash/file_manager/path_util.h"
@@ -27,11 +29,20 @@
 #include "chrome/browser/ash/file_system_provider/service_factory.h"
 #include "chrome/browser/ash/fileapi/external_file_url_util.h"
 #include "chrome/browser/ash/fileapi/file_system_backend.h"
+#include "chrome/browser/ash/fusebox/fusebox_moniker.h"
+#include "chrome/browser/ash/fusebox/fusebox_server.h"
+#include "chrome/browser/ash/guest_os/guest_id.h"
+#include "chrome/browser/ash/guest_os/guest_os_session_tracker.h"
+#include "chrome/browser/ash/guest_os/guest_os_share_path.h"
+#include "chrome/browser/ash/guest_os/public/types.h"
+#include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "chromeos/ash/components/dbus/concierge/concierge_client.h"
+#include "chromeos/ash/components/dbus/seneschal/seneschal_client.h"
 #include "chromeos/ash/components/dbus/virtual_file_provider/fake_virtual_file_provider_client.h"
 #include "chromeos/ash/components/dbus/virtual_file_provider/virtual_file_provider_client.h"
+#include "components/user_manager/scoped_user_manager.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/test_utils.h"
 #include "storage/browser/file_system/external_mount_points.h"
@@ -62,11 +73,14 @@ class ArcFileSystemBridgeTest : public testing::Test {
   void SetUp() override {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     ash::ConciergeClient::InitializeFake(/*fake_cicerone_client=*/nullptr);
+    ash::SeneschalClient::InitializeFake();
     ash::VirtualFileProviderClient::InitializeFake();
+
     profile_manager_ = std::make_unique<TestingProfileManager>(
         TestingBrowserProcess::GetGlobal());
     ASSERT_TRUE(profile_manager_->SetUp());
     profile_ = profile_manager_->CreateTestingProfile(kTestingProfileName);
+
     auto fake_provider =
         ash::file_system_provider::FakeExtensionProvider::Create(kExtensionId);
     const auto kProviderId = fake_provider->GetId();
@@ -76,6 +90,12 @@ class ArcFileSystemBridgeTest : public testing::Test {
                              ash::file_system_provider::MountOptions(
                                  kFileSystemId, "Test FileSystem"));
 
+    fake_user_manager_.Reset(std::make_unique<ash::FakeChromeUserManager>());
+    const AccountId account_id(
+        AccountId::FromUserEmail(profile_->GetProfileUserName()));
+    fake_user_manager_->AddUser(account_id);
+    fake_user_manager_->LoginUser(account_id);
+
     arc_file_system_bridge_ =
         std::make_unique<ArcFileSystemBridge>(profile_, &arc_bridge_service_);
     arc_bridge_service_.file_system()->SetInstance(&fake_file_system_);
@@ -85,8 +105,10 @@ class ArcFileSystemBridgeTest : public testing::Test {
   void TearDown() override {
     arc_bridge_service_.file_system()->CloseInstance(&fake_file_system_);
     arc_file_system_bridge_.reset();
+    fake_user_manager_.Reset();
     profile_manager_.reset();
     ash::VirtualFileProviderClient::Shutdown();
+    ash::SeneschalClient::Shutdown();
     ash::ConciergeClient::Shutdown();
   }
 
@@ -94,6 +116,8 @@ class ArcFileSystemBridgeTest : public testing::Test {
   base::ScopedTempDir temp_dir_;
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<TestingProfileManager> profile_manager_;
+  user_manager::TypedScopedUserManager<ash::FakeChromeUserManager>
+      fake_user_manager_;
   raw_ptr<Profile, DanglingUntriaged> profile_ = nullptr;
 
   FakeFileSystemInstance fake_file_system_;
@@ -265,6 +289,35 @@ TEST_F(ArcFileSystemBridgeTest, OpenFileToRead) {
   EXPECT_TRUE(arc_file_system_bridge_->HandleIdReleased(kId));
 }
 
+TEST_F(ArcFileSystemBridgeTest, CreateAndDestroyMoniker) {
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+  command_line->InitFromArgv({"", "--enable-arcvm"});
+  EXPECT_TRUE(IsArcVmEnabled());
+
+  const guest_os::GuestId arcvm_id = guest_os::GuestId(
+      guest_os::VmType::ARCVM, kArcVmName, /*container_name=*/"");
+  guest_os::GuestOsSessionTracker::GetForProfile(profile_)->AddGuestForTesting(
+      arcvm_id,
+      guest_os::GuestInfo{arcvm_id, /*cid=*/32, /*username=*/"",
+                          /*homedir=*/base::FilePath(), /*ipv4_address=*/"",
+                          /*sftp_vsock_port=*/0});
+
+  fusebox::Server fusebox_server(/*delegate=*/nullptr);
+  base::test::TestFuture<const std::optional<fusebox::Moniker>&> create_future;
+  arc_file_system_bridge_->CreateMoniker(
+      EncodeToChromeContentProviderUrl(GURL(kTestUrl)),
+      /*read_only=*/true, create_future.GetCallback());
+  EXPECT_TRUE(create_future.Get().has_value());
+  const fusebox::Moniker moniker = create_future.Get().value();
+  EXPECT_TRUE(guest_os::GuestOsSharePath::GetForProfile(profile_)->IsPathShared(
+      kArcVmName, base::FilePath(fusebox::MonikerMap::GetFilename(moniker))));
+
+  base::test::TestFuture<bool> destroy_future;
+  arc_file_system_bridge_->DestroyMoniker(moniker,
+                                          destroy_future.GetCallback());
+  EXPECT_TRUE(destroy_future.Get());
+}
+
 TEST_F(ArcFileSystemBridgeTest, GetLinuxVFSPathFromExternalFileURL) {
   storage::ExternalMountPoints* system_mount_points =
       storage::ExternalMountPoints::GetSystemInstance();

chrome/browser/ash/file_manager/BUILD.gn

@@ -145,7 +145,6 @@ static_library("file_manager") {
     "//chrome/browser/apps/app_service:constants",
     "//chrome/browser/ash/app_list",
     "//chrome/browser/ash/app_list/search",
-    "//chrome/browser/ash/arc/fileapi",
     "//chrome/browser/ash/bruschetta",
     "//chrome/browser/ash/crostini",
     "//chrome/browser/ash/drive",
@@ -205,7 +204,6 @@ static_library("file_manager") {
   allow_circular_includes_from = [
     "//chrome/browser/ash/app_list",
     "//chrome/browser/ash/app_list/search",
-    "//chrome/browser/ash/arc/fileapi",
     "//chrome/browser/ash/crostini",
     "//chrome/browser/ash/drive",
     "//chrome/browser/ash/file_manager/virtual_tasks",