Refactor MountedDiskMonitior and adds its test.

chrome/browser/chromeos/file_manager/mounted_disk_monitor_unittest.cc

+// Copyright 2013 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/file_manager/mounted_disk_monitor.h"
+
+#include "base/basictypes.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/stl_util.h"
+#include "chromeos/dbus/power_manager_client.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace file_manager {
+namespace {
+
+// Fake implementation of PowerManagerClient. Does nothing.
+class FakePowerManagerClient : public chromeos::PowerManagerClient {

[hashimoto, 2013/09/09 12:15:17]

I think you can use the one declared in
chromeos/dbus/fake_power_manager_client.h

[hidehiko, 2013/09/09 12:44:11]

Great to know. I checked once, but I seemed to overlook it...
Done.

+ public:
+  FakePowerManagerClient() {}
+  virtual ~FakePowerManagerClient() {}
+
+  // DBusClient overrides.
+  virtual void Init(dbus::Bus* bus) OVERRIDE {}
+
+  // PowerManagerClient overrides.
+  virtual void AddObserver(Observer* observer) OVERRIDE {}
+  virtual void RemoveObserver(Observer* observer) OVERRIDE {}
+  virtual bool HasObserver(Observer* observer) OVERRIDE { return false; }
+  virtual void DecreaseScreenBrightness(bool allow_off) OVERRIDE {}
+  virtual void IncreaseScreenBrightness() OVERRIDE {}
+  virtual void SetScreenBrightnessPercent(
+      double percent, bool gradual) OVERRIDE {}
+  virtual void GetScreenBrightnessPercent(
+      const chromeos::GetScreenBrightnessPercentCallback& callback) OVERRIDE {}
+  virtual void DecreaseKeyboardBrightness() OVERRIDE {}
+  virtual void IncreaseKeyboardBrightness() OVERRIDE {}
+  virtual void RequestStatusUpdate() OVERRIDE {}
+  virtual void RequestRestart() OVERRIDE {}
+  virtual void RequestShutdown() OVERRIDE {}
+  virtual void RequestIdleNotification(int64 threshold_secs) OVERRIDE {}
+  virtual void NotifyUserActivity(
+      power_manager::UserActivityType type) OVERRIDE {}
+  virtual void NotifyVideoActivity(bool is_fullscreen) OVERRIDE {}
+  virtual void SetPolicy(
+      const power_manager::PowerManagementPolicy& policy) OVERRIDE {}
+  virtual void SetIsProjecting(bool is_projecting) OVERRIDE {}
+  virtual base::Closure GetSuspendReadinessCallback() OVERRIDE {
+    return base::Closure();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FakePowerManagerClient);
+};
+
+// Fake implementation of DiskMountManager. Does nothing but returns some
+// disk information.
+class FakeDiskMountManager : public chromeos::disks::DiskMountManager {
+ public:
+  FakeDiskMountManager() {}
+  virtual ~FakeDiskMountManager() {
+    STLDeleteValues(&disks_);
+  }
+
+  // DiskMountManager overrides.
+  virtual void AddObserver(Observer* observer) OVERRIDE {}
+  virtual void RemoveObserver(Observer* observer) OVERRIDE {}
+  virtual const DiskMap& disks() const OVERRIDE { return disks_; }
+
+  virtual const Disk* FindDiskBySourcePath(
+      const std::string& source_path) const OVERRIDE {
+    DiskMap::const_iterator iter = disks_.find(source_path);
+    if (iter == disks_.end())
+      return NULL;
+    return iter->second;
+  };
+
+  virtual const MountPointMap& mount_points() const OVERRIDE {
+    return mount_points_;
+  }
+  virtual void RequestMountInfoRefresh() OVERRIDE {}
+  virtual void MountPath(const std::string& source_path,
+                         const std::string& source_format,
+                         const std::string& mount_label,
+                         chromeos::MountType type) OVERRIDE {}
+  virtual void UnmountPath(const std::string& mount_path,
+                           chromeos::UnmountOptions options,
+                           const UnmountPathCallback& callback) OVERRIDE {}
+  virtual void FormatMountedDevice(const std::string& mount_path) OVERRIDE {}
+  virtual void UnmountDeviceRecursively(
+      const std::string& device_path,
+      const UnmountDeviceRecursivelyCallbackType& callback) OVERRIDE {}
+
+  virtual bool AddDiskForTest(Disk* disk) OVERRIDE {
+    DCHECK(disk);
+    DCHECK(disks_.find(disk->device_path()) == disks_.end());
+    disks_[disk->device_path()] = disk;
+    return true;
+  }
+  virtual bool AddMountPointForTest(
+      const MountPointInfo& mount_point) OVERRIDE { return false; }
+
+ private:
+  DiskMap disks_;
+  MountPointMap mount_points_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeDiskMountManager);
+};
+
+}  // namespace
+
+class MountedDiskMonitorTest : public testing::Test {
+ protected:
+  MountedDiskMonitorTest() {
+  }
+
+  virtual ~MountedDiskMonitorTest() {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    power_manager_client_.reset(new FakePowerManagerClient);
+    disk_mount_manager_.reset(new FakeDiskMountManager);
+    mounted_disk_monitor_.reset(new MountedDiskMonitor(
+        power_manager_client_.get(),
+        disk_mount_manager_.get()));
+    mounted_disk_monitor_->SetResumingTimeSpanForTesting(
+        base::TimeDelta::FromSeconds(0));
+  }
+
+  base::MessageLoop message_loop_;
+  scoped_ptr<FakePowerManagerClient> power_manager_client_;
+  scoped_ptr<FakeDiskMountManager> disk_mount_manager_;
+  scoped_ptr<MountedDiskMonitor> mounted_disk_monitor_;
+};
+

[hashimoto, 2013/09/09 12:15:17]

nit: Could you briefly describe what kind of scenario is tested here?

[hidehiko, 2013/09/09 12:44:11]

Done.

+TEST_F(MountedDiskMonitorTest, ScenarioTest) {

[hashimoto, 2013/09/09 12:15:17]

nit: ScenarioTest sounds too general. What if we want to add new tests later?
How about renaming this test to something like BasicScenarioTest or
SuspendAndResume?

[hidehiko, 2013/09/09 12:44:11]

SuspendAndResume sounds better.

+  chromeos::disks::DiskMountManager::Disk* disk1 =
+      new chromeos::disks::DiskMountManager::Disk(

[hashimoto, 2013/09/09 12:15:17]

nit: Could you make it easier to understand what do these parameters mean?

[hidehiko, 2013/09/09 12:44:11]

Extracted into a utility function.

+          "removable_device1", "", "", "", "", "", "", "", "", "", "uuid1", "",
+          chromeos::DEVICE_TYPE_USB, 0, false, false, false, false, false);
+  chromeos::disks::DiskMountManager::Disk* disk2 =
+      new chromeos::disks::DiskMountManager::Disk(

[hashimoto, 2013/09/09 12:15:17]

ditto.

[hidehiko, 2013/09/09 12:44:11]

Done.

+          "removable_device2", "", "", "", "", "", "", "", "", "", "uuid2", "",
+          chromeos::DEVICE_TYPE_USB, 0, false, false, false, false, false);
+
+  const chromeos::disks::DiskMountManager::MountPointInfo kMount1(
+      "removable_device1", "/tmp/removable_device1",
+      chromeos::MOUNT_TYPE_DEVICE, chromeos::disks::MOUNT_CONDITION_NONE);
+  const chromeos::disks::DiskMountManager::MountPointInfo kMount2(
+      "removable_device2", "/tmp/removable_device2",
+      chromeos::MOUNT_TYPE_DEVICE, chromeos::disks::MOUNT_CONDITION_NONE);
+
+  // The ownership of Disks are taken by DiskMountManager.
+  disk_mount_manager_->AddDiskForTest(disk1);
+  disk_mount_manager_->AddDiskForTest(disk2);
+
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1));
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2));
+
+  // Mount |disk1|.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMount1);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1));
+
+  // Unmount |disk1|.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::UNMOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMount1);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1));

[hashimoto, 2013/09/09 12:15:17]

Could you briefly describe what do you want to test with this first pair of
mount and unmount? Or, how about separating this part to another test?

[hidehiko, 2013/09/09 12:44:11]

Done.

+
+  // Again mount |disk1|.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMount1);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1));
+
+  // Pseudo system suspend and resume.
+  mounted_disk_monitor_->SuspendImminent();
+  mounted_disk_monitor_->SystemResumed(base::TimeDelta::FromSeconds(0));
+
+  // On system resume, we expect unmount and then mount immediately.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::UNMOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMount1);
+  EXPECT_TRUE(mounted_disk_monitor_->DiskIsRemounting(*disk1));
+
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMount1);
+  EXPECT_TRUE(mounted_disk_monitor_->DiskIsRemounting(*disk1));
+
+  // New disk should not be "remounting."
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2));
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMount2);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2));
+
+  // After certain period, remounting state should be cleared.
+  base::RunLoop().RunUntilIdle();  // Emulate time passage.
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1));
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2));
+}
+
+}  // namespace file_manager