Some filebrowser bugs and retiring DiskChanged event in filebrowser

chrome/browser/chromeos/cros/mount_library.cc

 // Copyright (c) 2011 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/cros/mount_library.h"
 
 #include <set>
 
 #include "base/message_loop.h"
 #include "base/string_util.h"
@@ skipping 55 matching lines @@
       file_path_(file_path),
       device_label_(device_label),
       drive_label_(drive_label),
       parent_path_(parent_path),
       device_type_(device_type),
       total_size_(total_size),
       is_parent_(is_parent),
       is_read_only_(is_read_only),
       has_media_(has_media),
       on_boot_device_(on_boot_device) {
-  // Add trailing slash to mount path.
-  if (mount_path_.length() && mount_path_.at(mount_path_.length() -1) != '/')
-    mount_path_ = mount_path_.append("/");
 }
 
 MountLibrary::Disk::~Disk() {}
 
 class MountLibraryImpl : public MountLibrary {
 
   struct UnmountDeviceRecursiveCallbackData {
     MountLibraryImpl* const object;
     void* user_data;
     UnmountDeviceRecursiveCallbackType callback;
@@ skipping 37 matching lines @@
                          const MountPathOptions& options) OVERRIDE {
     CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (!CrosLibrary::Get()->EnsureLoaded()) {
       OnMountCompleted(MOUNT_ERROR_LIBRARY_NOT_LOADED,
                        MountPointInfo(source_path, NULL, type));
       return;
     }
     MountSourcePath(source_path, type, options, &MountCompletedHandler, this);
   }
 
-  virtual void UnmountPath(const char* path) OVERRIDE {
+  virtual void UnmountPath(const char* mount_path) OVERRIDE {
     CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (!CrosLibrary::Get()->EnsureLoaded()) {
-      OnUnmountPath(path,
+      OnUnmountPath(mount_path,
                     MOUNT_METHOD_ERROR_LOCAL,
                     kLibraryNotLoaded);
       return;
     }
-    UnmountMountPoint(path, &MountLibraryImpl::UnmountMountPointCallback, this);
+
+    UnmountMountPoint(mount_path, &MountLibraryImpl::UnmountMountPointCallback,
+                      this);
   }
 
   virtual void UnmountDeviceRecursive(const char* device_path,
       UnmountDeviceRecursiveCallbackType callback, void* user_data)
       OVERRIDE {
     bool success = true;
     const char* error_message = NULL;
     std::vector<const char*> devices_to_unmount;
 
     if (!CrosLibrary::Get()->EnsureLoaded()) {
       success = false;
       error_message = kLibraryNotLoaded;
     } else {
       // Get list of all devices to unmount.
       int device_path_len = strlen(device_path);
       for (DiskMap::iterator it = disks_.begin(); it != disks_.end(); ++it) {
-        if (strncmp(device_path, it->second->device_path().c_str(),
-            device_path_len) == 0) {
-          devices_to_unmount.push_back(it->second->device_path().c_str());
+        if (!it->second->mount_path().empty() &&
+            strncmp(device_path, it->second->device_path().c_str(),
+                    device_path_len) == 0) {
+          devices_to_unmount.push_back(it->second->mount_path().c_str());
         }
       }
 
       // We should detect at least original device.
       if (devices_to_unmount.size() == 0) {
-        success = false;
-        error_message = kDeviceNotFound;
+        if (disks_.find(device_path) == disks_.end()) {
+          success = false;
+          error_message = kDeviceNotFound;
+        } else {
+          // Nothing to unmount.
+          callback(user_data, true);
+          return;
+        }
       }
     }
 
     if (success) {
       // We will send the same callback data object to all Unmount calls and use
       // it to syncronize callbacks.
       UnmountDeviceRecursiveCallbackData*
           cb_data = new UnmountDeviceRecursiveCallbackData(this, user_data,
           callback, devices_to_unmount.size());
       for (std::vector<const char*>::iterator it = devices_to_unmount.begin();
@@ skipping 35 matching lines @@
                                     MountType type,
                                     const char* mount_path) {
     DCHECK(object);
     MountLibraryImpl* self = static_cast<MountLibraryImpl*>(object);
     self->OnMountCompleted(static_cast<MountError>(error_code),
                            MountPointInfo(source_path, mount_path, type));
   }
 
   // Callback for UnmountRemovableDevice method.
   static void UnmountMountPointCallback(void* object,
-                                        const char* device_path,
+                                        const char* mount_path,
                                         MountMethodErrorType error,
                                         const char* error_message) {
     DCHECK(object);
     MountLibraryImpl* self = static_cast<MountLibraryImpl*>(object);
-    self->OnUnmountPath(device_path, error, error_message);
+    self->OnUnmountPath(mount_path, error, error_message);
   }
 
   // Callback for UnmountDeviceRecursive.
   static void UnmountDeviceRecursiveCallback(void* object,
-                                             const char* device_path,
+                                             const char* mount_path,
                                              MountMethodErrorType error,
                                              const char* error_message) {
     DCHECK(object);
     UnmountDeviceRecursiveCallbackData* cb_data =
         static_cast<UnmountDeviceRecursiveCallbackData*>(object);
 
     // Do standard processing for Unmount event.
-    cb_data->object->OnUnmountPath(device_path,
+    cb_data->object->OnUnmountPath(mount_path,
                                    error,
                                    error_message);
     if (error == MOUNT_METHOD_ERROR_LOCAL) {
       cb_data->success = false;
     } else if (error == MOUNT_METHOD_ERROR_NONE) {
-      LOG(WARNING) << device_path <<  " unmounted.";
+      LOG(INFO) << mount_path <<  " unmounted.";
    }
 
     // This is safe as long as all callbacks are called on the same thread as
     // UnmountDeviceRecursive.
     cb_data->pending_callbacks_count--;
 
     if (cb_data->pending_callbacks_count == 0) {
       cb_data->callback(cb_data->user_data, cb_data->success);
       delete cb_data;
     }
@@ skipping 34 matching lines @@
     DCHECK(object);
     MountLibraryImpl* self = static_cast<MountLibraryImpl*>(object);
     self->OnMountEvent(evt, device_path);
   }
 
 
   void OnMountCompleted(MountError error_code,
                         const MountPointInfo& mount_info) {
     DCHECK(!mount_info.source_path.empty());
 
-    FireMountCompleted(MOUNTING,
-                       error_code,
-                       mount_info);
+    FireMountCompleted(MOUNTING, error_code, mount_info);
 
     if (error_code == MOUNT_ERROR_NONE &&
-        mount_points_.find(mount_info.source_path) == mount_points_.end()) {
+        mount_points_.find(mount_info.mount_path) == mount_points_.end()) {
       mount_points_.insert(MountPointMap::value_type(
-                               mount_info.source_path.c_str(),
+                               mount_info.mount_path.c_str(),
                                mount_info));
     }
 
     if (error_code == MOUNT_ERROR_NONE &&
         mount_info.mount_type == MOUNT_TYPE_DEVICE &&
         !mount_info.source_path.empty() &&
         !mount_info.mount_path.empty()) {
       std::string path(mount_info.source_path);
       DiskMap::iterator iter = disks_.find(path);
       if (iter == disks_.end()) {
         // disk might have been removed by now?
         return;
       }
       Disk* disk = iter->second;
       DCHECK(disk);
       disk->set_mount_path(mount_info.mount_path.c_str());
       FireDiskStatusUpdate(MOUNT_DISK_MOUNTED, disk);
     }
   }
 
-  void OnUnmountPath(const char* source_path,

[zel, 2011/08/09 00:09:45]

is this param now really mount_path? or is it mount_source_path?

[tbarzic, 2011/08/09 00:35:07]

Its really mount_path.. calling with source path does unmount device, but
GetMountPoints does not work fine with it..

+  void OnUnmountPath(const char* mount_path,
                      MountMethodErrorType error,
                      const char* error_message) {
-    DCHECK(source_path);
-
-    if (error == MOUNT_METHOD_ERROR_NONE && source_path) {
-      MountPointMap::iterator mount_points_it = mount_points_.find(source_path);
+    DCHECK(mount_path);
+    if (error == MOUNT_METHOD_ERROR_NONE && mount_path) {
+      MountPointMap::iterator mount_points_it = mount_points_.find(mount_path);
       if (mount_points_it == mount_points_.end())
         return;
       // TODO(tbarzic): Add separate, PathUnmounted event to Observer.
       FireMountCompleted(
           UNMOUNTING,
           MOUNT_ERROR_NONE,
           MountPointInfo(mount_points_it->second.source_path.c_str(),
                          mount_points_it->second.mount_path.c_str(),
                          mount_points_it->second.mount_type));
+      std::string path(mount_points_it->second.source_path);
       mount_points_.erase(mount_points_it);
 
-      std::string path(source_path);
       DiskMap::iterator iter = disks_.find(path);
       if (iter == disks_.end()) {
         // disk might have been removed by now?
         return;
       }
       Disk* disk = iter->second;
       DCHECK(disk);
       disk->clear_mount_path();
       FireDiskStatusUpdate(MOUNT_DISK_UNMOUNTED, disk);
     } else {
       LOG(WARNING) << "Unmount request failed for device "
-                   << source_path << ", with error: "
+                   << mount_path << ", with error: "
                    << (error_message ? error_message : "Unknown");
     }
   }
 
   void OnGetDiskProperties(const char* device_path,
                            const DiskInfo* disk1,
                            MountMethodErrorType error,
                            const char* error_message) {
     DCHECK(device_path);
     if (error == MOUNT_METHOD_ERROR_NONE && device_path) {
@@ skipping 209 matching lines @@
   // MountLibrary overrides.
   virtual void AddObserver(Observer* observer) OVERRIDE {}
   virtual void RemoveObserver(Observer* observer) OVERRIDE {}
   virtual const DiskMap& disks() const OVERRIDE { return disks_; }
   virtual const MountPointMap& mount_points() const OVERRIDE {
     return mount_points_;
   }
   virtual void RequestMountInfoRefresh() OVERRIDE {}
   virtual void MountPath(const char* source_path, MountType type,
                          const MountPathOptions& options) OVERRIDE {}
-  virtual void UnmountPath(const char* path) OVERRIDE {}
+  virtual void UnmountPath(const char* mount_path) OVERRIDE {}
   virtual void UnmountDeviceRecursive(const char* device_path,
       UnmountDeviceRecursiveCallbackType callback, void* user_data)
       OVERRIDE {}
 
  private:
   // The list of disks found.
   DiskMap disks_;
   MountPointMap mount_points_;
 
   DISALLOW_COPY_AND_ASSIGN(MountLibraryStubImpl);
 };
 
 // static
 MountLibrary* MountLibrary::GetImpl(bool stub) {
   if (stub)
     return new MountLibraryStubImpl();
   else
     return new MountLibraryImpl();
 }
 
 }  // namespace chromeos
 
 // Allows InvokeLater without adding refcounting. This class is a Singleton and
 // won't be deleted until it's last InvokeLater is run.
 DISABLE_RUNNABLE_METHOD_REFCOUNT(chromeos::MountLibraryImpl);