Refactor MountedDiskMonitior and adds its test.

As a preparation of VolumeManager refactoring, this CL allows injecting
DiskMountManager to MounteddiskMonitor. Along with the change, this CL also
adds unittest for it.

BUG=279276
TEST=Ran unit_tests

Review URL: https://chromiumcodereview.appspot.com/23676008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222072 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/extensions/file_manager/event_router.cc

@@ -33,6 +33,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/file_browser_private.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_state_handler.h"
@@ -329,7 +330,9 @@ void EventRouter::ObserveFileSystemEvents() {
                                                                 FROM_HERE);
   }
 
-  mounted_disk_monitor_.reset(new MountedDiskMonitor());
+  mounted_disk_monitor_.reset(new MountedDiskMonitor(
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient(),
+      disk_mount_manager));
 
   pref_change_registrar_->Init(profile_->GetPrefs());
 

chrome/browser/chromeos/file_manager/mounted_disk_monitor.cc

@@ -5,11 +5,9 @@
 #include "chrome/browser/chromeos/file_manager/mounted_disk_monitor.h"
 
 #include "base/bind.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "content/public/browser/browser_thread.h"
 
-using chromeos::DBusThreadManager;
 using chromeos::disks::DiskMountManager;
 
 namespace file_manager {
@@ -22,22 +20,24 @@ const base::TimeDelta kResumingTimeSpan = base::TimeDelta::FromSeconds(5);
 
 }  // namespace
 
-MountedDiskMonitor::MountedDiskMonitor()
-    : is_resuming_(false),
+MountedDiskMonitor::MountedDiskMonitor(
+    chromeos::PowerManagerClient* power_manager_client,
+    chromeos::disks::DiskMountManager* disk_mount_manager)
+    : power_manager_client_(power_manager_client),
+      disk_mount_manager_(disk_mount_manager),
+      is_resuming_(false),
+      resuming_time_span_(kResumingTimeSpan),
       weak_factory_(this) {
-  DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(this);
-  DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
-  if (disk_mount_manager) {
-    disk_mount_manager->AddObserver(this);
-    disk_mount_manager->RequestMountInfoRefresh();
-  }
+  DCHECK(power_manager_client_);
+  DCHECK(disk_mount_manager_);
+  power_manager_client_->AddObserver(this);
+  disk_mount_manager_->AddObserver(this);
+  disk_mount_manager_->RequestMountInfoRefresh();
 }
 
 MountedDiskMonitor::~MountedDiskMonitor() {
-  DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(this);
-  DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
-  if (disk_mount_manager)
-    disk_mount_manager->RemoveObserver(this);
+  disk_mount_manager_->RemoveObserver(this);
+  power_manager_client_->RemoveObserver(this);
 }
 
 void MountedDiskMonitor::SuspendImminent() {
@@ -54,7 +54,7 @@ void MountedDiskMonitor::SystemResumed(
       FROM_HERE,
       base::Bind(&MountedDiskMonitor::Reset,
                  weak_factory_.GetWeakPtr()),
-      kResumingTimeSpan);
+      resuming_time_span_);
 }
 
 bool MountedDiskMonitor::DiskIsRemounting(
@@ -71,9 +71,8 @@ void MountedDiskMonitor::OnMountEvent(
 
   switch (event) {
     case DiskMountManager::MOUNTING: {
-      DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
       const DiskMountManager::Disk* disk =
-          disk_mount_manager->FindDiskBySourcePath(mount_info.source_path);
+          disk_mount_manager_->FindDiskBySourcePath(mount_info.source_path);
       if (!disk || error_code != chromeos::MOUNT_ERROR_NONE)
         return;
       mounted_disks_[mount_info.source_path] = disk->fs_uuid();

chrome/browser/chromeos/file_manager/mounted_disk_monitor.h

@@ -25,7 +25,9 @@ class MountedDiskMonitor
     : public chromeos::PowerManagerClient::Observer,
       public chromeos::disks::DiskMountManager::Observer {
  public:
-  MountedDiskMonitor();
+  MountedDiskMonitor(
+      chromeos::PowerManagerClient* power_manager_client,
+      chromeos::disks::DiskMountManager* disk_mount_manager);
   virtual ~MountedDiskMonitor();
 
   // PowerManagerClient::Observer overrides:
@@ -53,6 +55,14 @@ class MountedDiskMonitor
   // been unmounted during the resuming time span.
   bool DiskIsRemounting(
       const chromeos::disks::DiskMountManager::Disk& disk) const;
+
+  // In order to avoid consuming time a lot for testing, this allows to set the
+  // resuming time span.
+  void set_resuming_time_span_for_testing(
+      const base::TimeDelta& resuming_time_span) {
+    resuming_time_span_ = resuming_time_span;
+  }
+
  private:
   // Maps source paths with corresponding uuids.
   typedef std::map<std::string, std::string> DiskMap;
@@ -62,9 +72,13 @@ class MountedDiskMonitor
 
   void Reset();
 
+  chromeos::PowerManagerClient* power_manager_client_;
+  chromeos::disks::DiskMountManager* disk_mount_manager_;
+
   bool is_resuming_;
   DiskMap mounted_disks_;
   DiskSet unmounted_while_resuming_;
+  base::TimeDelta resuming_time_span_;
   base::WeakPtrFactory<MountedDiskMonitor> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MountedDiskMonitor);

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/fake_power_manager_client.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace file_manager {
+namespace {
+
+// 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);
+};
+
+// Creates a fake disk with |device_path| and |fs_uuid|.
+scoped_ptr<chromeos::disks::DiskMountManager::Disk> CreateDisk(
+    const std::string& device_path,
+    const std::string& fs_uuid) {
+  return make_scoped_ptr(
+      new chromeos::disks::DiskMountManager::Disk(
+          device_path, "", "", "", "", "", "", "", "", "", fs_uuid, "",
+          chromeos::DEVICE_TYPE_USB, 0, false, false, false, false, false));
+}
+
+}  // namespace
+
+class MountedDiskMonitorTest : public testing::Test {
+ protected:
+  MountedDiskMonitorTest() {
+  }
+
+  virtual ~MountedDiskMonitorTest() {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    power_manager_client_.reset(new chromeos::FakePowerManagerClient);
+    disk_mount_manager_.reset(new FakeDiskMountManager);
+    mounted_disk_monitor_.reset(new MountedDiskMonitor(
+        power_manager_client_.get(),
+        disk_mount_manager_.get()));
+    mounted_disk_monitor_->set_resuming_time_span_for_testing(
+        base::TimeDelta::FromSeconds(0));
+  }
+
+  base::MessageLoop message_loop_;
+  scoped_ptr<chromeos::FakePowerManagerClient> power_manager_client_;
+  scoped_ptr<FakeDiskMountManager> disk_mount_manager_;
+  scoped_ptr<MountedDiskMonitor> mounted_disk_monitor_;
+};
+
+// Makes sure that just mounting and unmounting repeatedly doesn't affect to
+// "remounting" state.
+TEST_F(MountedDiskMonitorTest, WithoutSuspend) {
+  scoped_ptr<chromeos::disks::DiskMountManager::Disk> disk(
+      CreateDisk("removable_device1", "uuid1"));
+
+  chromeos::disks::DiskMountManager::Disk* disk_ptr = disk.get();
+
+  const chromeos::disks::DiskMountManager::MountPointInfo kMountPoint(
+      "removable_device1", "/tmp/removable_device1",
+      chromeos::MOUNT_TYPE_DEVICE, chromeos::disks::MOUNT_CONDITION_NONE);
+
+  disk_mount_manager_->AddDiskForTest(disk.release());
+
+  // First, the disk is not remounting.
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk_ptr));
+
+  // Simple mounting and unmounting doesn't affect remounting state.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk_ptr));
+
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::UNMOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk_ptr));
+
+  // Mounting again also should not affect remounting state.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk_ptr));
+}
+
+// Makes sure that the unmounting after system resuming triggers the
+// "remounting" state, then after some period, the state is reset.
+TEST_F(MountedDiskMonitorTest, SuspendAndResume) {
+  scoped_ptr<chromeos::disks::DiskMountManager::Disk> disk1(
+      CreateDisk("removable_device1", "uuid1"));
+  scoped_ptr<chromeos::disks::DiskMountManager::Disk> disk2(
+      CreateDisk("removable_device2", "uuid2"));
+
+  chromeos::disks::DiskMountManager::Disk* disk1_ptr = disk1.get();
+  chromeos::disks::DiskMountManager::Disk* disk2_ptr = disk2.get();
+
+  const chromeos::disks::DiskMountManager::MountPointInfo kMountPoint1(
+      "removable_device1", "/tmp/removable_device1",
+      chromeos::MOUNT_TYPE_DEVICE, chromeos::disks::MOUNT_CONDITION_NONE);
+  const chromeos::disks::DiskMountManager::MountPointInfo kMountPoint2(
+      "removable_device2", "/tmp/removable_device2",
+      chromeos::MOUNT_TYPE_DEVICE, chromeos::disks::MOUNT_CONDITION_NONE);
+
+  disk_mount_manager_->AddDiskForTest(disk1.release());
+  disk_mount_manager_->AddDiskForTest(disk2.release());
+
+  // Mount |disk1|.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint1);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1_ptr));
+
+  // 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.
+  // During the phase, we expect the disk is remounting.
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::UNMOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint1);
+  EXPECT_TRUE(mounted_disk_monitor_->DiskIsRemounting(*disk1_ptr));
+
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint1);
+  EXPECT_TRUE(mounted_disk_monitor_->DiskIsRemounting(*disk1_ptr));
+
+  // New disk should not be "remounting."
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2_ptr));
+  mounted_disk_monitor_->OnMountEvent(
+      chromeos::disks::DiskMountManager::MOUNTING,
+      chromeos::MOUNT_ERROR_NONE,
+      kMountPoint2);
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2_ptr));
+
+  // After certain period, remounting state should be cleared.
+  base::RunLoop().RunUntilIdle();  // Emulate time passage.
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk1_ptr));
+  EXPECT_FALSE(mounted_disk_monitor_->DiskIsRemounting(*disk2_ptr));
+}
+
+}  // namespace file_manager

chrome/chrome_tests_unit.gypi

@@ -656,6 +656,7 @@
         'browser/chromeos/file_manager/file_tasks_unittest.cc',
         'browser/chromeos/file_manager/file_watcher_unittest.cc',
         'browser/chromeos/file_manager/mime_util_unittest.cc',
+        'browser/chromeos/file_manager/mounted_disk_monitor_unittest.cc',
         'browser/chromeos/file_manager/url_util_unittest.cc',
         'browser/chromeos/file_manager/volume_manager_unittest.cc',
         'browser/chromeos/extensions/wallpaper_private_api_unittest.cc',