Add a MountPoint class to track mount point lifetime.

Also, create a factory function, MountPoint::Mount(), to help simplify
the process of mounting.

Bug: 939235
Change-Id: Ide29dea654a634ab1dbf57f1fe9ab2255eec7fc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1951265Reviewed-by: Sergei Datsenko <dats@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722380}

chromeos/disks/BUILD.gn

@@ -20,6 +20,8 @@ component("disks") {
     "disk.h",
     "disk_mount_manager.cc",
     "disk_mount_manager.h",
+    "mount_point.cc",
+    "mount_point.h",
     "suspend_unmount_manager.cc",
     "suspend_unmount_manager.h",
   ]
@@ -60,6 +62,7 @@ source_set("unit_tests") {
   sources = [
     "disk_mount_manager_unittest.cc",
     "disk_unittest.cc",
+    "mount_point_unittest.cc",
     "suspend_unmount_manager_unittest.cc",
   ]
 }

chromeos/disks/mount_point.cc

+// Copyright 2019 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 "chromeos/disks/mount_point.h"
+
+#include <string>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/task/post_task.h"
+
+namespace chromeos {
+namespace disks {
+namespace {
+
+class MountWatcher : public DiskMountManager::Observer {
+ public:
+  MountWatcher() = delete;
+  MountWatcher(const MountWatcher&) = delete;
+  MountWatcher& operator=(const MountWatcher&) = delete;
+
+  MountWatcher(DiskMountManager* disk_mount_manager,
+               const std::string& source_path,
+               MountType mount_type,
+               MountPoint::DoneCallback callback)
+      : disk_mount_manager_(disk_mount_manager),
+        source_path_(source_path),
+        mount_type_(mount_type),
+        callback_(std::move(callback)) {
+    DCHECK(callback_);
+    disk_mount_manager_->AddObserver(this);
+  }
+
+  ~MountWatcher() override { disk_mount_manager_->RemoveObserver(this); }
+
+ private:
+  // DiskMountManager::Observer overrides.
+  void OnMountEvent(
+      DiskMountManager::MountEvent event,
+      MountError error_code,
+      const DiskMountManager::MountPointInfo& mount_info) override {
+    if (mount_info.mount_type != mount_type_ ||
+        mount_info.source_path != source_path_ ||
+        event != chromeos::disks::DiskMountManager::MOUNTING) {
+      return;
+    }
+
+    DCHECK(callback_);
+    std::unique_ptr<MountPoint> mount_point;
+    if (error_code == chromeos::MOUNT_ERROR_NONE) {
+      DCHECK(!mount_info.mount_path.empty());
+      mount_point = std::make_unique<MountPoint>(
+          base::FilePath(mount_info.mount_path), disk_mount_manager_);
+    }
+
+    // Post a task to guarantee the callback isn't called inline with the
+    // Mount() call.
+    base::PostTask(FROM_HERE, {base::CurrentThread()},
+                   base::BindOnce(std::move(callback_), error_code,
+                                  std::move(mount_point)));
+
+    delete this;
+  }
+
+  DiskMountManager* const disk_mount_manager_;
+  const std::string source_path_;
+  const MountType mount_type_;
+  MountPoint::DoneCallback callback_;
+};
+
+}  // namespace
+
+// static
+void MountPoint::Mount(DiskMountManager* disk_mount_manager,
+                       const std::string& source_path,
+                       const std::string& source_format,
+                       const std::string& mount_label,
+                       const std::vector<std::string>& mount_options,
+                       MountType mount_type,
+                       MountAccessMode access_mode,
+                       DoneCallback callback) {
+  // MountWatcher needs to be created before mounting because MountPath() may
+  // signal a result inline.
+  // Note: MountWatcher owns itself.
+  new MountWatcher(disk_mount_manager, source_path, mount_type,
+                   std::move(callback));
+
+  disk_mount_manager->MountPath(source_path, source_format, mount_label,
+                                mount_options, mount_type, access_mode);
+}
+
+MountPoint::MountPoint(const base::FilePath& mount_path,
+                       DiskMountManager* disk_mount_manager)
+    : mount_path_(mount_path), disk_mount_manager_(disk_mount_manager) {
+  DCHECK(!mount_path_.empty());
+}
+
+MountPoint::~MountPoint() {
+  if (!mount_path_.empty()) {
+    disk_mount_manager_->UnmountPath(
+        mount_path_.value(), base::BindOnce([](MountError error_code) {
+          LOG_IF(WARNING, error_code != MOUNT_ERROR_NONE)
+              << "Failed to unmount with error code: " << error_code;
+        }));
+  }
+}
+
+void MountPoint::Unmount(MountPoint::UnmountCallback callback) {
+  DCHECK(callback);
+  DCHECK(!mount_path_.empty());
+
+  // Make a copy of the |mount_path_| on the stack and clear it, in case the
+  // callback runs inline and deletes |this|.
+  const std::string mount_path = mount_path_.value();
+  mount_path_.clear();
+  disk_mount_manager_->UnmountPath(mount_path, std::move(callback));
+}
+
+}  // namespace disks
+}  // namespace chromeos

chromeos/disks/mount_point.h

+// Copyright 2019 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 CHROMEOS_DISKS_MOUNT_POINT_H_
+#define CHROMEOS_DISKS_MOUNT_POINT_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/component_export.h"
+#include "base/files/file_path.h"
+#include "chromeos/disks/disk_mount_manager.h"
+
+namespace chromeos {
+namespace disks {
+
+class DiskMountManager;
+
+// MountPoint is a thin wrapper around a mount point that was mounted with
+// DiskMountManager. MountPoint 'owns' the mount point and unmounts it on
+// destruction.
+class COMPONENT_EXPORT(CHROMEOS_DISKS) MountPoint {
+ public:
+  using DoneCallback =
+      base::OnceCallback<void(MountError, std::unique_ptr<MountPoint>)>;
+  using UnmountCallback = DiskMountManager::UnmountPathCallback;
+
+  // Mounts a device, archive, or network filesystem, and runs |callback| when
+  // done. |callback| will never be called inline. |callback| should be bound
+  // with a WeakPtr<> since Mount() can take an indefinite amount of time.
+  // See DiskMountManager::MountPath() for other argument details.
+  static void Mount(DiskMountManager* disk_mount_manager,
+                    const std::string& source_path,
+                    const std::string& source_format,
+                    const std::string& mount_label,
+                    const std::vector<std::string>& mount_options,
+                    MountType mount_type,
+                    MountAccessMode access_mode,
+                    DoneCallback callback);
+
+  MountPoint() = delete;
+  MountPoint(const MountPoint&) = delete;
+  MountPoint& operator=(const MountPoint&) = delete;
+
+  MountPoint(const base::FilePath& mount_path,
+             DiskMountManager* disk_mount_manager);
+  ~MountPoint();
+
+  // Unmounts the mount point, and runs |callback| when done. |callback| must be
+  // non-null.
+  void Unmount(UnmountCallback callback);
+
+  const base::FilePath& mount_path() const { return mount_path_; }
+
+ private:
+  base::FilePath mount_path_;
+  DiskMountManager* const disk_mount_manager_;
+};
+
+}  // namespace disks
+}  // namespace chromeos
+
+#endif  // CHROMEOS_DISKS_MOUNT_POINT_H_

chromeos/disks/mount_point_unittest.cc

+// Copyright 2019 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 "chromeos/disks/mount_point.h"
+
+#include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/gmock_callback_support.h"
+#include "base/test/task_environment.h"
+#include "chromeos/disks/mock_disk_mount_manager.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::WithoutArgs;
+
+namespace chromeos {
+namespace disks {
+namespace {
+
+constexpr char kSourcePath[] = "/source/path";
+constexpr char kMountPath[] = "/mount/path";
+constexpr char kOtherPath[] = "/other/path";
+
+class MountPointTest : public testing::Test {
+ public:
+  MountPointTest() = default;
+
+ protected:
+  base::test::TaskEnvironment task_environment_;
+  MockDiskMountManager disk_mount_manager_;
+};
+
+TEST_F(MountPointTest, Mount) {
+  EXPECT_CALL(disk_mount_manager_,
+              MountPath(kSourcePath, "", "", _, MOUNT_TYPE_DEVICE,
+                        MOUNT_ACCESS_MODE_READ_WRITE))
+      .WillOnce(WithoutArgs([this]() {
+        // Ignore other mount events.
+        disk_mount_manager_.NotifyMountEvent(
+            DiskMountManager::MOUNTING, MOUNT_ERROR_NONE,
+            DiskMountManager::MountPointInfo(kOtherPath, kOtherPath,
+                                             MOUNT_TYPE_DEVICE,
+                                             MOUNT_CONDITION_NONE));
+        disk_mount_manager_.NotifyMountEvent(
+            DiskMountManager::UNMOUNTING, MOUNT_ERROR_NONE,
+            DiskMountManager::MountPointInfo(kSourcePath, kOtherPath,
+                                             MOUNT_TYPE_DEVICE,
+                                             MOUNT_CONDITION_NONE));
+        disk_mount_manager_.NotifyMountEvent(
+            DiskMountManager::MOUNTING, MOUNT_ERROR_NONE,
+            DiskMountManager::MountPointInfo(kSourcePath, kOtherPath,
+                                             MOUNT_TYPE_ARCHIVE,
+                                             MOUNT_CONDITION_NONE));
+
+        // This is the real mount event.
+        disk_mount_manager_.NotifyMountEvent(
+            DiskMountManager::MOUNTING, MOUNT_ERROR_NONE,
+            DiskMountManager::MountPointInfo(kSourcePath, kMountPath,
+                                             MOUNT_TYPE_DEVICE,
+                                             MOUNT_CONDITION_NONE));
+      }));
+  EXPECT_CALL(disk_mount_manager_, UnmountPath(kMountPath, _)).Times(1);
+
+  base::RunLoop run_loop;
+  MountPoint::Mount(&disk_mount_manager_, kSourcePath, "", "", {},
+                    MOUNT_TYPE_DEVICE, MOUNT_ACCESS_MODE_READ_WRITE,
+                    base::BindLambdaForTesting(
+                        [&run_loop](MountError mount_error,
+                                    std::unique_ptr<MountPoint> mount) {
+                          EXPECT_EQ(MOUNT_ERROR_NONE, mount_error);
+                          EXPECT_EQ(kMountPath, mount->mount_path().value());
+                          run_loop.Quit();
+                        }));
+  run_loop.Run();
+}
+
+TEST_F(MountPointTest, MountFailure) {
+  EXPECT_CALL(disk_mount_manager_,
+              MountPath(kSourcePath, "", "", _, MOUNT_TYPE_DEVICE,
+                        MOUNT_ACCESS_MODE_READ_WRITE))
+      .WillOnce(WithoutArgs([this]() {
+        disk_mount_manager_.NotifyMountEvent(
+            DiskMountManager::MOUNTING, MOUNT_ERROR_UNKNOWN,
+            DiskMountManager::MountPointInfo(
+                kSourcePath, kMountPath, MOUNT_TYPE_DEVICE,
+                MOUNT_CONDITION_UNSUPPORTED_FILESYSTEM));
+      }));
+  EXPECT_CALL(disk_mount_manager_, UnmountPath(_, _)).Times(0);
+
+  base::RunLoop run_loop;
+  MountPoint::Mount(&disk_mount_manager_, kSourcePath, "", "", {},
+                    MOUNT_TYPE_DEVICE, MOUNT_ACCESS_MODE_READ_WRITE,
+                    base::BindLambdaForTesting(
+                        [&run_loop](MountError mount_error,
+                                    std::unique_ptr<MountPoint> mount) {
+                          EXPECT_EQ(MOUNT_ERROR_UNKNOWN, mount_error);
+                          EXPECT_FALSE(mount);
+                          run_loop.Quit();
+                        }));
+  run_loop.Run();
+}
+
+TEST_F(MountPointTest, Unmount) {
+  EXPECT_CALL(disk_mount_manager_, UnmountPath(kMountPath, _))
+      .WillOnce(base::test::RunOnceCallback<1>(MOUNT_ERROR_INTERNAL));
+
+  base::RunLoop run_loop;
+  MountPoint mount_point(base::FilePath(kMountPath), &disk_mount_manager_);
+  mount_point.Unmount(base::BindLambdaForTesting([&run_loop](MountError error) {
+    EXPECT_EQ(MOUNT_ERROR_INTERNAL, error);
+    run_loop.Quit();
+  }));
+  run_loop.Run();
+}
+
+TEST_F(MountPointTest, UnmountOnDestruction) {
+  EXPECT_CALL(disk_mount_manager_, UnmountPath(kMountPath, _)).Times(1);
+
+  MountPoint mount_point(base::FilePath(kMountPath), &disk_mount_manager_);
+}
+
+}  // namespace
+}  // namespace disks
+}  // namespace chromeos