Add a DiskMountManager method to refresh mount entries in addition to devices.

Lack of the info caused Chrome OS Files.app to fail finding external storages
after browser-only restart (like that from about:flags or from browser crash.)

BUG=356583

Review URL: https://codereview.chromium.org/379743004

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

4 files changed, 97 insertions, 30 deletions

diff --git a/chromeos/disks/disk_mount_manager.cc b/chromeos/disks/disk_mount_manager.cc
index c25cf15..a1f6db8 100644
--- a/chromeos/disks/disk_mount_manager.cc
+++ b/chromeos/disks/disk_mount_manager.cc
@@ -26,7 +26,9 @@ DiskMountManager* g_disk_mount_manager = NULL;
 // The DiskMountManager implementation.
 class DiskMountManagerImpl : public DiskMountManager {
  public:
-  DiskMountManagerImpl() : weak_ptr_factory_(this) {
+  DiskMountManagerImpl() :
+    already_refreshed_(false),
+    weak_ptr_factory_(this) {
     DBusThreadManager* dbus_thread_manager = DBusThreadManager::Get();
     DCHECK(dbus_thread_manager);
     cros_disks_client_ = dbus_thread_manager->GetCrosDisksClient();
@@ -155,7 +157,7 @@ class DiskMountManagerImpl : public DiskMountManager {
     }
 
     // We will send the same callback data object to all Unmount calls and use
-    // it to syncronize callbacks.
+    // it to synchronize callbacks.
     // Note: this implementation has a potential memory leak issue. For
     // example if this instance is destructed before all the callbacks for
     // Unmount are invoked, the memory pointed by |cb_data| will be leaked.
@@ -189,11 +191,22 @@ class DiskMountManagerImpl : public DiskMountManager {
   }
 
   // DiskMountManager override.
-  virtual void RequestMountInfoRefresh() OVERRIDE {
-    cros_disks_client_->EnumerateAutoMountableDevices(
-        base::Bind(&DiskMountManagerImpl::OnRequestMountInfo,
-                   weak_ptr_factory_.GetWeakPtr()),
-        base::Bind(&base::DoNothing));
+  virtual void EnsureMountInfoRefreshed(
+      const EnsureMountInfoRefreshedCallback& callback) OVERRIDE {
+    if (already_refreshed_) {
+      callback.Run(true);
+      return;
+    }
+
+    refresh_callbacks_.push_back(callback);
+    if (refresh_callbacks_.size() == 1) {
+      // If there's no in-flight refreshing task, start it.
+      cros_disks_client_->EnumerateAutoMountableDevices(
+          base::Bind(&DiskMountManagerImpl::RefreshAfterEnumerateDevices,
+                     weak_ptr_factory_.GetWeakPtr()),
+          base::Bind(&DiskMountManagerImpl::RefreshCompleted,
+                     weak_ptr_factory_.GetWeakPtr(), false));
+    }
   }
 
   // DiskMountManager override.
@@ -414,7 +427,7 @@ class DiskMountManagerImpl : public DiskMountManager {
     NotifyFormatStatusUpdate(FORMAT_COMPLETED, error_code, device_path);
   }
 
-  // Callbcak for GetDeviceProperties.
+  // Callback for GetDeviceProperties.
   void OnGetDeviceProperties(const DiskInfo& disk_info) {
     // TODO(zelidrag): Find a better way to filter these out before we
     // fetch the properties:
@@ -454,32 +467,64 @@ class DiskMountManagerImpl : public DiskMountManager {
     NotifyDiskStatusUpdate(is_new ? DISK_ADDED : DISK_CHANGED, disk);
   }
 
-  // Callbcak for RequestMountInfo.
-  void OnRequestMountInfo(const std::vector<std::string>& devices) {
-    std::set<std::string> current_device_set;
-    if (!devices.empty()) {
-      // Initiate properties fetch for all removable disks,
-      for (size_t i = 0; i < devices.size(); i++) {
-        current_device_set.insert(devices[i]);
-        // Initiate disk property retrieval for each relevant device path.
-        cros_disks_client_->GetDeviceProperties(
-            devices[i],
-            base::Bind(&DiskMountManagerImpl::OnGetDeviceProperties,
-                       weak_ptr_factory_.GetWeakPtr()),
-            base::Bind(&base::DoNothing));
-      }
-    }
-    // Search and remove disks that are no longer present.
+  // Part of EnsureMountInfoRefreshed(). Called after the list of devices are
+  // enumerated.
+  void RefreshAfterEnumerateDevices(const std::vector<std::string>& devices) {
+    std::set<std::string> current_device_set(devices.begin(), devices.end());
     for (DiskMap::iterator iter = disks_.begin(); iter != disks_.end(); ) {
       if (current_device_set.find(iter->first) == current_device_set.end()) {
-        Disk* disk = iter->second;
-        NotifyDiskStatusUpdate(DISK_REMOVED, disk);
         delete iter->second;
         disks_.erase(iter++);
       } else {
         ++iter;
       }
     }
+    RefreshDeviceAtIndex(devices, 0);
+  }
+
+  // Part of EnsureMountInfoRefreshed(). Called for each device to refresh info.
+  void RefreshDeviceAtIndex(const std::vector<std::string>& devices,
+                            size_t index) {
+    if (index == devices.size()) {
+      // All devices info retrieved. Proceed to enumerate mount point info.
+      cros_disks_client_->EnumerateMountEntries(
+          base::Bind(&DiskMountManagerImpl::RefreshAfterEnumerateMountEntries,
+                     weak_ptr_factory_.GetWeakPtr()),
+          base::Bind(&DiskMountManagerImpl::RefreshCompleted,
+                     weak_ptr_factory_.GetWeakPtr(), false));
+      return;
+    }
+
+    cros_disks_client_->GetDeviceProperties(
+        devices[index],
+        base::Bind(&DiskMountManagerImpl::RefreshAfterGetDeviceProperties,
+                   weak_ptr_factory_.GetWeakPtr(), devices, index + 1),
+        base::Bind(&DiskMountManagerImpl::RefreshCompleted,
+                   weak_ptr_factory_.GetWeakPtr(), false));
+  }
+
+  // Part of EnsureMountInfoRefreshed().
+  void RefreshAfterGetDeviceProperties(const std::vector<std::string>& devices,
+                                       size_t next_index,
+                                       const DiskInfo& disk_info) {
+    OnGetDeviceProperties(disk_info);
+    RefreshDeviceAtIndex(devices, next_index);
+  }
+
+  // Part of EnsureMountInfoRefreshed(). Called after mount entries are listed.
+  void RefreshAfterEnumerateMountEntries(
+      const std::vector<MountEntry>& entries) {
+    for (size_t i = 0; i < entries.size(); ++i)
+      OnMountCompleted(entries[i]);
+    RefreshCompleted(true);
+  }
+
+  // Part of EnsureMountInfoRefreshed(). Called when the refreshing is done.
+  void RefreshCompleted(bool success) {
+    already_refreshed_ = true;
+    for (size_t i = 0; i < refresh_callbacks_.size(); ++i)
+      refresh_callbacks_[i].Run(success);
+    refresh_callbacks_.clear();
   }
 
   // Callback to handle mount event signals.
@@ -580,6 +625,9 @@ class DiskMountManagerImpl : public DiskMountManager {
   typedef std::set<std::string> SystemPathPrefixSet;
   SystemPathPrefixSet system_path_prefixes_;
 
+  bool already_refreshed_;
+  std::vector<EnsureMountInfoRefreshedCallback> refresh_callbacks_;
+
   base::WeakPtrFactory<DiskMountManagerImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DiskMountManagerImpl);
diff --git a/chromeos/disks/disk_mount_manager.h b/chromeos/disks/disk_mount_manager.h
index 4edc475..4f8862c 100644
--- a/chromeos/disks/disk_mount_manager.h
+++ b/chromeos/disks/disk_mount_manager.h
@@ -198,6 +198,9 @@ class CHROMEOS_EXPORT DiskMountManager {
   // A callback type for UnmountPath method.
   typedef base::Callback<void(MountError error_code)> UnmountPathCallback;
 
+  // A callback type for EnsureMountInfoRefreshed method.
+  typedef base::Callback<void(bool success)> EnsureMountInfoRefreshedCallback;
+
   // Implement this interface to be notified about disk/mount related events.
   class Observer {
    public:
@@ -236,8 +239,11 @@ class CHROMEOS_EXPORT DiskMountManager {
   // Gets the list of mount points.
   virtual const MountPointMap& mount_points() const = 0;
 
-  // Requests refreshing all the information about mounted disks.
-  virtual void RequestMountInfoRefresh() = 0;
+  // Refreshes all the information about mounting if it is not yet done and
+  // invokes |callback| when finished. If the information is already refreshed
+  // It just runs |callback| immediately.
+  virtual void EnsureMountInfoRefreshed(
+      const EnsureMountInfoRefreshedCallback& callback) = 0;
 
   // Mounts a device.
   // Note that the mount operation may fail. To find out the result, one should
diff --git a/chromeos/disks/mock_disk_mount_manager.cc b/chromeos/disks/mock_disk_mount_manager.cc
index c1665b7..a023b1a 100644
--- a/chromeos/disks/mock_disk_mount_manager.cc
+++ b/chromeos/disks/mock_disk_mount_manager.cc
@@ -58,6 +58,9 @@ MockDiskMountManager::MockDiskMountManager() {
   ON_CALL(*this, FindDiskBySourcePath(_))
       .WillByDefault(Invoke(
           this, &MockDiskMountManager::FindDiskBySourcePathInternal));
+  ON_CALL(*this, EnsureMountInfoRefreshed(_))
+      .WillByDefault(Invoke(
+          this, &MockDiskMountManager::EnsureMountInfoRefreshedInternal));
 }
 
 MockDiskMountManager::~MockDiskMountManager() {
@@ -162,7 +165,7 @@ void MockDiskMountManager::SetupDefaultReplies() {
       .WillRepeatedly(ReturnRef(mount_points_));
   EXPECT_CALL(*this, FindDiskBySourcePath(_))
       .Times(AnyNumber());
-  EXPECT_CALL(*this, RequestMountInfoRefresh())
+  EXPECT_CALL(*this, EnsureMountInfoRefreshed(_))
       .Times(AnyNumber());
   EXPECT_CALL(*this, MountPath(_, _, _, _))
       .Times(AnyNumber());
@@ -234,6 +237,11 @@ MockDiskMountManager::FindDiskBySourcePathInternal(
   return disk_it == disks_.end() ? NULL : disk_it->second;
 }
 
+void MockDiskMountManager::EnsureMountInfoRefreshedInternal(
+    const EnsureMountInfoRefreshedCallback& callback) {
+  callback.Run(true);
+}
+
 void MockDiskMountManager::NotifyDiskChanged(
     DiskEvent event,
     const DiskMountManager::Disk* disk) {
diff --git a/chromeos/disks/mock_disk_mount_manager.h b/chromeos/disks/mock_disk_mount_manager.h
index f7baa8e..e00278ac 100644
--- a/chromeos/disks/mock_disk_mount_manager.h
+++ b/chromeos/disks/mock_disk_mount_manager.h
@@ -32,7 +32,8 @@ class MockDiskMountManager : public DiskMountManager {
                      const DiskMountManager::Disk*(const std::string&));
   MOCK_CONST_METHOD0(mount_points,
                      const DiskMountManager::MountPointMap&(void));
-  MOCK_METHOD0(RequestMountInfoRefresh, void(void));
+  MOCK_METHOD1(EnsureMountInfoRefreshed,
+               void(const EnsureMountInfoRefreshedCallback&));
   MOCK_METHOD4(MountPath, void(const std::string&, const std::string&,
                                const std::string&, MountType));
   MOCK_METHOD3(UnmountPath, void(const std::string&,
@@ -88,6 +89,10 @@ class MockDiskMountManager : public DiskMountManager {
   const DiskMountManager::Disk* FindDiskBySourcePathInternal(
       const std::string& source_path) const;
 
+  // Is used to implement EnsureMountInfoRefreshed.
+  void EnsureMountInfoRefreshedInternal(
+      const EnsureMountInfoRefreshedCallback& callback);
+
   // Notifies observers about device status update.
   void NotifyDeviceChanged(DeviceEvent event,
                            const std::string& path);