Move chromeos_mount.cc from libcros to Chrome tree

chrome/browser/chromeos/dbus/cros_disks_client.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/dbus/cros_disks_client.h"
+
+#include <set>
+#include <vector>
+
+#include <sys/statvfs.h>
+
+#include "base/bind.h"
+#include "base/memory/scoped_vector.h"
+#include "base/string_util.h"
+#include "chrome/browser/chromeos/system/runtime_environment.h"
+#include "content/public/browser/browser_thread.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_proxy.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+using content::BrowserThread;
+
+namespace chromeos {
+
+namespace {
+
+const char* kDeviceNotFound = "Device could not be found";
+
+const char* kDefaultMountOptions[] = {
+  "rw",
+  "nodev",
+  "noexec",
+  "nosuid",
+  "sync"
+};
+const char* kDefaultUnmountOptions[] = {
+  "force"
+};
+
+enum MountEventType {
+  DISK_ADDED,
+  DISK_REMOVED,
+  DISK_CHANGED,
+  DEVICE_ADDED,
+  DEVICE_REMOVED,
+  DEVICE_SCANNED,
+  FORMATTING_FINISHED,
+};
+
+struct DiskInfo {
+ public:
+  DiskInfo(const std::string& path, dbus::Response* response)
+      : path_(path),
+        is_drive_(false),
+        has_media_(false),
+        on_boot_device_(false),
+        device_type_(UNDEFINED),
+        total_size_(0),
+        is_read_only_(false),
+        is_hidden_(true) {
+    InitializeFromResponse(response);
+  }
+
+  // DBus service path.
+  std::string path() const { return path_; }

[satorux1, 2011/11/07 23:21:55]

dbus_service_path() would be clearer. 

[hashimoto, 2011/11/08 07:31:05]

Looking at cros-disks code, device_path() is more suitable name for this
property.  I cannot imagine why the comment says "DBus service path".
http://git.chromium.org/gitweb/?p=chromiumos/platform/cros-disks.git;a=blob;f...

+  // Disk mount path.
+  std::string mount_path() const { return mount_path_; }

[satorux1, 2011/11/07 23:21:55]

Let's add a blank line between accessors.

[hashimoto, 2011/11/08 07:31:05]

Done.

+  // Disk system path.
+  std::string system_path() const { return system_path_; }

[satorux1, 2011/11/07 23:21:55]

Would be nice to add some example for the three paths (dbus service path, mount
path, and system path), as these are somewhat confusing.

[hashimoto, 2011/11/08 07:31:05]

Done.

+  // Is disk into a drive (i.e. /dev/sdb vs, /dev/sdb1).
+  bool is_drive() const { return is_drive_; }
+  // Does the disk have media content.
+  bool has_media() const { return has_media_; }
+  // Is the disk on deveice we booted the machien from.
+  bool on_boot_device() const { return on_boot_device_; }
+  // Disk file path (e.g /dev/sdb).
+  std::string file_path() const { return file_path_; }
+  // Disk label.
+  std::string label() const { return label_; }
+  // Disk model
+  std::string drive_label() const { return drive_model_; }
+  // Partition table path of the device, if device is partition.
+  std::string partition_slave() const { return partition_slave_; }
+  // Device type. Not working well, yet.
+  DeviceType device_type() const { return device_type_; }
+  // Total size of the disk.
+  uint64 size() const { return total_size_; }
+  // Is the device read-only.
+  bool is_read_only() const { return is_read_only_; }
+  bool is_hidden() const { return is_hidden_; }
+
+ private:
+  DeviceType GetDeviceType(bool is_optical, bool is_rotational) {
+    if (is_optical)
+      return OPTICAL;
+    if (is_rotational)
+      return HDD;
+    return FLASH;
+  }
+
+  bool MaybePopValue(dbus::MessageReader* reader, bool* value) {

[satorux1, 2011/11/07 23:21:55]

Function comment is missing. Please fix other places too.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

Every function declaration should have comments immediately preceding it that
describe what the function does and how to use it. 

[hashimoto, 2011/11/08 07:31:05]

Done.

+    if (!reader)
+      return false;
+    reader->PopBool(value);
+    return true;

[satorux1, 2011/11/07 23:21:55]

PopBool() may fail, but this function always returns true. Is this intended?
Otherwise, please change this to

  return reader->PopBool(value);

[hashimoto, 2011/11/08 07:31:05]

Done.

+  }
+
+  bool MaybePopValue(dbus::MessageReader* reader, std::string* value) {

[satorux1, 2011/11/07 23:21:55]

Function overriding like this is prohibited by the style guide. Please give them
unique names, like MaybePopBoolValue, and MaybePopStringValue().

[hashimoto, 2011/11/08 07:31:05]

Done.

+    if (!reader)
+      return false;
+    reader->PopString(value);
+    return true;
+  }
+
+  bool MaybePopValue(dbus::MessageReader* reader, uint64* value) {
+    if (!reader)
+      return false;
+    reader->PopUint64(value);
+    return true;
+  }
+
+  void InitializeFromResponse(dbus::Response* response) {
+    dbus::MessageReader reader(response);
+    dbus::MessageReader array_reader(response);
+    if (!reader.PopArray(&array_reader)) {
+      LOG(ERROR) << "Invalid response: " << response->ToString();
+      return;
+    }
+    ScopedVector<dbus::MessageReader> values_owner;

[satorux1, 2011/11/07 23:21:55]

readers_owner? see below

[hashimoto, 2011/11/08 07:31:05]

Done.

+    std::map<std::string, dbus::MessageReader*> properties;
+    while (array_reader.HasMoreData()) {
+      // |values_owner| is responsible to delete |value|
+      dbus::MessageReader* value = new dbus::MessageReader(response);

[satorux1, 2011/11/07 23:21:55]

value -> reader? This is a MessageReader rather than a value.

[hashimoto, 2011/11/08 07:31:05]

Done.

+      values_owner.push_back(value);
+      dbus::MessageReader dict_entry_reader(response);
+      std::string key;
+      if (!array_reader.PopDictEntry(&dict_entry_reader) ||
+          !dict_entry_reader.PopString(&key) ||
+          !dict_entry_reader.PopVariant(value)) {
+        LOG(ERROR) << "Invalid response: " << response->ToString();
+        return;
+      }
+      properties[key] = value;
+    }
+    MaybePopValue(properties[cros_disks::kDeviceIsDrive], &is_drive_);
+    MaybePopValue(properties[cros_disks::kDeviceIsReadOnly], &is_read_only_);
+    MaybePopValue(properties[cros_disks::kDevicePresentationHide], &is_hidden_);
+    MaybePopValue(properties[cros_disks::kDeviceIsMediaAvailable], &has_media_);
+    MaybePopValue(properties[cros_disks::kDeviceIsOnBootDevice],
+                  &on_boot_device_);
+    MaybePopValue(properties[cros_disks::kNativePath], &system_path_);
+    MaybePopValue(properties[cros_disks::kDeviceFile], &file_path_);
+    MaybePopValue(properties[cros_disks::kDriveModel], &drive_model_);
+    MaybePopValue(properties[cros_disks::kIdLabel], &label_);
+    MaybePopValue(properties[cros_disks::kDeviceSize], &total_size_);

[satorux1, 2011/11/07 23:21:55]

As you know, parsing string-to-varint dictionary sucks. We should probably
rework this using Protocol Buffers. Could you add a TODO comment here?

TODO(satorux): Rework this code using Protocol Buffers.

[hashimoto, 2011/11/08 07:31:05]

Done.

+
+    if (properties[cros_disks::kDeviceMountPaths]) {
+      std::vector<std::string> mount_paths;
+      if (!properties[cros_disks::kDeviceMountPaths]->PopArrayOfStrings(

[satorux1, 2011/11/07 23:21:55]

The map is looked up twice here. You can save one lookup using find(), like:

std::map<std::string, dbus::MessageReader*>::iterator iter =
    properties.find(cros_disks::kDeviceMountPaths]);
if (iter != properties.end()) {
  std::vector<std::string> mount_paths;
  if (iter->second->PopArrayOfStrings(...))

[hashimoto, 2011/11/08 07:31:05]

Done.

+              &mount_paths))
+      if (!mount_paths.empty())
+        mount_path_ = mount_paths[0];
+    }
+    bool is_rotational, is_optical;
+    if (MaybePopValue(properties[cros_disks::kDriveIsRotational],
+                     &is_rotational) &&
+        MaybePopValue(properties[cros_disks::kDeviceIsOpticalDisc],
+                     &is_optical))
+      device_type_ = GetDeviceType(is_optical, is_rotational);
+  }

[satorux1, 2011/11/07 23:21:55]

BTW, structs are supposed be just data. Having non-trivial functions in structs
are generally not 

[hashimoto, 2011/11/08 07:31:05]

Changed it to a class.

+
+  std::string path_;
+  std::string mount_path_;
+  std::string system_path_;
+  bool is_drive_;
+  bool has_media_;
+  bool on_boot_device_;
+
+  std::string file_path_;
+  std::string label_;
+  std::string drive_model_;
+  std::string partition_slave_;
+  DeviceType device_type_;
+  uint64 total_size_;
+  bool is_read_only_;
+  bool is_hidden_;
+};
+
+class CrosDisksDBusProxy {

[satorux1, 2011/11/07 23:21:55]

Please add some class comment.

[hashimoto, 2011/11/08 07:31:05]

Done.

+ public:
+  typedef base::Callback<void()> ErrorCallback;
+  typedef base::Callback<void()> MountCallback;
+  typedef base::Callback<void(const std::string&)> UnmountCallback;
+  typedef base::Callback<void(const std::vector<std::string>)

[satorux1, 2011/11/07 23:21:55]

Can you add '&'?

(const std::vector<std::string>&)

[hashimoto, 2011/11/08 07:31:05]

Done.

+                         > EnumerateAutoMountableDevicesCallback;
+  typedef base::Callback<void(const std::string&, bool)> FormatDeviceCallback;
+  typedef base::Callback<void(const DiskInfo&)> GetDevicePropertiesCallback;
+  typedef base::Callback<void(MountError, const std::string&, MountType,
+                              const std::string&)> MountCompletedHandler;
+  typedef base::Callback<void(MountEventType, const std::string&)
+                         > MountEventHandler;
+
+  explicit CrosDisksDBusProxy(dbus::Bus* bus)
+      : proxy_(bus->GetObjectProxy(cros_disks::kCrosDisksServiceName,
+                                   cros_disks::kCrosDisksServicePath)),
+        weak_ptr_factory_(this) {
+  }
+
+  void Mount(const std::string& source_path, MountType type,
+             const MountPathOptions& options, MountCallback callback,
+             ErrorCallback error_callback) {
+    dbus::MethodCall method_call(cros_disks::kCrosDisksInterface,
+                                 cros_disks::kMount);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(source_path);
+    writer.AppendString("");  // auto detect filesystem.
+    std::vector<std::string> mount_options(kDefaultMountOptions,
+                                           kDefaultMountOptions +
+                                           arraysize(kDefaultMountOptions));
+    writer.AppendArrayOfStrings(mount_options);
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&CrosDisksDBusProxy::OnMount,
+                                  weak_ptr_factory_.GetWeakPtr(),
+                                  callback,
+                                  error_callback));
+  }
+
+  void Unmount(const std::string& device_path, UnmountCallback callback,
+               ErrorCallback error_callback) {
+    dbus::MethodCall method_call(cros_disks::kCrosDisksInterface,
+                                 cros_disks::kUnmount);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(device_path);
+    std::vector<std::string> unmount_options(kDefaultUnmountOptions,
+                                             kDefaultUnmountOptions +
+                                             arraysize(kDefaultUnmountOptions));
+    writer.AppendArrayOfStrings(unmount_options);
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&CrosDisksDBusProxy::OnUnmount,
+                                  weak_ptr_factory_.GetWeakPtr(),
+                                  device_path,
+                                  callback,
+                                  error_callback));
+  }
+
+  void EnumerateAutoMountableDevices(
+      EnumerateAutoMountableDevicesCallback callback,
+      ErrorCallback error_callback) {
+    dbus::MethodCall method_call(cros_disks::kCrosDisksInterface,
+                                 cros_disks::kEnumerateAutoMountableDevices);
+    proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&CrosDisksDBusProxy::OnEnumerateAutoMountableDevices,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   callback,
+                   error_callback));
+  }
+
+  void FormatDevice(const std::string& device_path,
+                    const std::string& filesystem,
+                    FormatDeviceCallback callback,
+                    ErrorCallback error_callback) {
+    dbus::MethodCall method_call(cros_disks::kCrosDisksInterface,
+                                 cros_disks::kFormatDevice);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(device_path);
+    writer.AppendString(filesystem);
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&CrosDisksDBusProxy::OnFormatDevice,
+                                  weak_ptr_factory_.GetWeakPtr(),
+                                  device_path,
+                                  callback,
+                                  error_callback));
+  }
+
+  void GetDeviceProperties(const std::string& device_path,
+                           GetDevicePropertiesCallback callback,
+                           ErrorCallback error_callback) {
+    dbus::MethodCall method_call(cros_disks::kCrosDisksInterface,
+                                 cros_disks::kGetDeviceProperties);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(device_path);
+    proxy_->CallMethod(&method_call,
+                       dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&CrosDisksDBusProxy::OnGetDeviceProperties,
+                                  weak_ptr_factory_.GetWeakPtr(),
+                                  device_path,
+                                  callback,
+                                  error_callback));
+  }
+
+  void SetUpConnections(MountEventHandler mount_event_handler,
+                        MountCompletedHandler mount_completed_handler) {
+    static const SignalEventTuple kSignalEventTuples[] = {
+      { "DeviceAdded", DEVICE_ADDED },
+      { "DeviceScanned", DEVICE_SCANNED },
+      { "DeviceRemoved", DEVICE_REMOVED },
+      { "DiskAdded", DISK_ADDED },
+      { "DiskChanged", DISK_CHANGED },
+      { "DiskRemoved", DISK_REMOVED },
+      { "FormattingFinished", FORMATTING_FINISHED },
+    };
+    const size_t kNumSignalEventTuples = arraysize(kSignalEventTuples);
+
+    for (size_t i = 0; i < kNumSignalEventTuples; ++i) {
+      proxy_->ConnectToSignal(
+          cros_disks::kCrosDisksInterface,
+          kSignalEventTuples[i].signal_name,
+          base::Bind(&CrosDisksDBusProxy::OnMountEvent,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     kSignalEventTuples[i].event_type,
+                     mount_event_handler),
+          base::Bind(&CrosDisksDBusProxy::OnSignalConnected,
+                     weak_ptr_factory_.GetWeakPtr()));
+    }
+    proxy_->ConnectToSignal(
+        cros_disks::kCrosDisksInterface,
+        "MountCompleted",
+        base::Bind(&CrosDisksDBusProxy::OnMountCompleted,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   mount_completed_handler ),
+        base::Bind(&CrosDisksDBusProxy::OnSignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+ private:
+  struct SignalEventTuple{

[satorux1, 2011/11/07 23:21:55]

space before {

[hashimoto, 2011/11/08 07:31:05]

Done.

+    const char *signal_name;
+    MountEventType event_type;
+  };
+
+  void OnMount(MountCallback callback, ErrorCallback error_callback,

[satorux1, 2011/11/07 23:21:55]

Move  ErrorCallback error_callback to the next line?

More like a matter of taste, but we often list parameters vertically, when they
cannot fit on one line.

[hashimoto, 2011/11/08 07:31:05]

Done.

+               dbus::Response* response) {
+    if (!response) {
+      error_callback.Run();
+      return;
+    }
+    callback.Run();
+  }
+
+  void OnUnmount(const std::string& device_path, UnmountCallback callback,
+                 ErrorCallback error_callback, dbus::Response* response) {
+    if (!response) {
+      error_callback.Run();
+      return;
+    }
+    callback.Run(device_path);
+  }
+
+  void OnEnumerateAutoMountableDevices(
+      EnumerateAutoMountableDevicesCallback callback,
+      ErrorCallback error_callback,
+      dbus::Response* response) {
+    if (!response) {
+      error_callback.Run();
+      return;
+    }
+    dbus::MessageReader reader(response);
+    std::vector<std::string> device_paths;
+    if (!reader.PopArrayOfStrings(&device_paths)) {

[satorux1, 2011/11/07 23:21:55]

Add something like

LOG(ERROR) << "Invalid response: " << response->ToString()

[hashimoto, 2011/11/08 07:31:05]

Done.

+      error_callback.Run();
+      return;
+    }
+    callback.Run(device_paths);
+  }
+
+  void OnFormatDevice(const std::string& device_path,
+                      FormatDeviceCallback callback,
+                      ErrorCallback error_callback,
+                      dbus::Response* response) {
+    if (!response) {
+      error_callback.Run();
+      return;
+    }
+    dbus::MessageReader reader(response);
+    bool success = false;
+    if (!reader.PopBool(&success)) {

[satorux1, 2011/11/07 23:21:55]

ditto.

[hashimoto, 2011/11/08 07:31:05]

Done.

+      error_callback.Run();
+      return;
+    }
+    callback.Run(device_path, success);
+  }
+
+  void OnGetDeviceProperties(const std::string& device_path,
+                             GetDevicePropertiesCallback callback,
+                             ErrorCallback error_callback,
+                             dbus::Response* response) {
+    if (!response) {
+      error_callback.Run();
+      return;
+    }
+    DiskInfo disk(device_path, response);
+    callback.Run(disk);
+  }
+
+  void OnMountEvent(MountEventType event_type, MountEventHandler handler,
+                    dbus::Signal* signal) {
+    dbus::MessageReader reader(signal);
+    std::string device;
+    if (!reader.PopString(&device)) {
+      LOG(ERROR) << "Invalid signal: " << signal->ToString();
+      return;
+    }
+    handler.Run(event_type, device);
+  }
+
+  void OnMountCompleted(MountCompletedHandler handler, dbus::Signal* signal) {
+    dbus::MessageReader reader(signal);
+    unsigned int error_code = 0;
+    std::string source_path;
+    unsigned int mount_type = 0;
+    std::string mount_path;
+    if (!reader.PopUint32(&error_code) ||
+        !reader.PopString(&source_path) ||
+        !reader.PopUint32(&mount_type) ||
+        !reader.PopString(&mount_path)) {
+      LOG(ERROR) << "Invalid signal: " << signal->ToString();
+      return;
+    }
+    handler.Run(static_cast<MountError>(error_code), source_path,
+                static_cast<MountType>(mount_type), mount_path);
+  }
+
+  void OnSignalConnected(const std::string& interface,
+                         const std::string& signal, bool successed) {
+    LOG_IF(ERROR, !successed) << "Connect to " << interface << " " <<
+        signal << " failed.";
+  }
+
+  dbus::ObjectProxy* proxy_;
+  base::WeakPtrFactory<CrosDisksDBusProxy> weak_ptr_factory_;
+};
+
+class CrosDisksClientImpl : public CrosDisksClient {

[satorux1, 2011/11/07 23:21:55]

Please add some class comment.

[hashimoto, 2011/11/08 07:31:05]

Done.

+ public:
+  explicit CrosDisksClientImpl(dbus::Bus* bus)
+      : dbus_proxy_(new CrosDisksDBusProxy(bus)),
+        weak_ptr_factory_(this) {
+    dbus_proxy_->SetUpConnections(
+        base::Bind(&CrosDisksClientImpl::OnMountEvent,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&CrosDisksClientImpl::OnMountCompleted,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  ~CrosDisksClientImpl() {
+  }
+
+  virtual void AddObserver(Observer* observer) OVERRIDE {
+    observers_.AddObserver(observer);
+  }
+
+  virtual void RemoveObserver(Observer* observer) OVERRIDE {
+    observers_.RemoveObserver(observer);
+  }
+
+  virtual void MountPath(const std::string& source_path, MountType type,
+                         const MountPathOptions& options) OVERRIDE {
+    // Hidden and non-existent devices should not be mounted.
+    if (type == MOUNT_TYPE_DEVICE) {
+      DiskMap::const_iterator it = disks_.find(source_path);
+      if (it == disks_.end() || it->second->is_hidden()) {
+        OnMountCompleted(MOUNT_ERROR_INTERNAL, source_path, type, "");
+        return;
+      }
+    }
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    dbus_proxy_->Mount(source_path, type, options,
+                       // When succeeds, OnMountCompleted will be called by
+                       // "MountCompleted" signal instead.
+                       base::Bind(&DoNothing),

[satorux1, 2011/11/07 23:21:55]

Instead of this, I think you can pass base::Closure()

[hashimoto, 2011/11/08 07:31:05]

Passing base::Closure() results in calling NULL function pointer and crash.
I know having a function like DoNothing is an ugly way.
But I prefer it rather than checking callback.is_null() every time before
function call.

+                       base::Bind(&CrosDisksClientImpl::OnMountCompleted,
+                                  weak_ptr_factory_.GetWeakPtr(),
+                                  MOUNT_ERROR_INTERNAL,
+                                  source_path,
+                                  type,
+                                  ""));
+  }
+
+  virtual void UnmountPath(const std::string& mount_path) OVERRIDE {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    dbus_proxy_->Unmount(mount_path,
+                         base::Bind(&CrosDisksClientImpl::OnUnmountPath,
+                                    weak_ptr_factory_.GetWeakPtr()),
+                         base::Bind(&DoNothing));
+  }
+
+  virtual void GetSizeStatsOnFileThread(const std::string& mount_path,
+                                        size_t* total_size_kb,
+                                        size_t* remaining_size_kb) OVERRIDE {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+    uint64_t total_size_uint64 = 0;
+    uint64_t remaining_size_uint64 = 0;
+
+    struct statvfs stat;

[satorux1, 2011/11/07 23:21:55]

Maybe zero-clear the stat?

  struct statvfs stat = {};  // Zero-clear

[hashimoto, 2011/11/08 07:31:05]

Done.

+    if (statvfs(mount_path.c_str(), &stat) == 0) {
+      total_size_uint64 =
+          static_cast<uint64_t>(stat.f_blocks) * stat.f_frsize;
+      remaining_size_uint64 =
+          static_cast<uint64_t>(stat.f_bfree) * stat.f_frsize;
+    }
+    *total_size_kb = static_cast<size_t>(total_size_uint64 / 1024);
+    *remaining_size_kb = static_cast<size_t>(remaining_size_uint64 / 1024);
+  }
+
+  virtual void FormatUnmountedDevice(const std::string& file_path) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    for (CrosDisksClient::DiskMap::iterator it = disks_.begin();
+         it != disks_.end(); ++it) {
+      if (it->second->file_path() == file_path &&
+          !it->second->mount_path().empty()) {
+        LOG(ERROR) << "Device is still mounted: " << file_path;
+        OnFormatDevice(file_path, false);
+        return;
+      }
+    }
+    dbus_proxy_->FormatDevice(file_path, "vfat",

[satorux1, 2011/11/07 23:21:55]

"vfat" -> make it a constant like const char kFormatVFAT[] = "vfat"

[hashimoto, 2011/11/08 07:31:05]

Done.

+                              base::Bind(&CrosDisksClientImpl::OnFormatDevice,
+                                         weak_ptr_factory_.GetWeakPtr()),
+                              base::Bind(&CrosDisksClientImpl::OnFormatDevice,
+                                         weak_ptr_factory_.GetWeakPtr(),
+                                         file_path,
+                                         false));
+  }
+
+  virtual void FormatMountedDevice(const std::string& mount_path) OVERRIDE {
+    Disk* disk = NULL;
+    for (CrosDisksClient::DiskMap::iterator it = disks_.begin();
+         it != disks_.end(); ++it) {
+      if (it->second->mount_path() == mount_path) {
+        disk = it->second;
+        break;
+      }
+    }
+    if (!disk) {
+      LOG(ERROR) << "Device with this mount path not found: " << mount_path;
+      OnFormatDevice(mount_path, false);
+      return;
+    }
+    if (formatting_pending_.find(disk->device_path()) !=
+        formatting_pending_.end()) {
+      LOG(ERROR) << "Formatting is already pending: " << mount_path;
+      OnFormatDevice(mount_path, false);
+      return;
+    }
+    // Formatting process continues, after unmounting.
+    formatting_pending_[disk->device_path()] = disk->file_path();
+    UnmountPath(disk->mount_path());
+  }
+
+  virtual void UnmountDeviceRecursive(
+      const std::string& device_path,
+      UnmountDeviceRecursiveCallbackType callback,
+      void* user_data) OVERRIDE {
+    bool success = true;
+    std::string error_message;
+    std::vector<std::string> devices_to_unmount;
+
+    // Get list of all devices to unmount.
+    int device_path_len = device_path.length();
+    for (DiskMap::iterator it = disks_.begin(); it != disks_.end(); ++it) {
+      if (!it->second->mount_path().empty() &&
+          strncmp(device_path.c_str(), it->second->device_path().c_str(),
+                  device_path_len) == 0) {
+        devices_to_unmount.push_back(it->second->mount_path());
+      }
+    }
+    // We should detect at least original device.
+    if (devices_to_unmount.size() == 0) {

[satorux1, 2011/11/07 23:21:55]

devices_to_unmount.empty() ?

[hashimoto, 2011/11/08 07:31:05]

Done.

+      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(user_data, callback,
+                                                 devices_to_unmount.size());
+      for (std::vector<std::string>::iterator it = devices_to_unmount.begin();
+           it != devices_to_unmount.end();
+           ++it) {
+        dbus_proxy_->Unmount(
+            *it,
+            base::Bind(&CrosDisksClientImpl::OnUnmountDeviceRecursive,
+                       weak_ptr_factory_.GetWeakPtr(), cb_data, true),
+            base::Bind(&CrosDisksClientImpl::OnUnmountDeviceRecursive,
+                       weak_ptr_factory_.GetWeakPtr(), cb_data, false, *it));
+      }
+    } else {
+      LOG(WARNING) << "Unmount recursive request failed for device "
+                   << device_path << ", with error: " << error_message;
+      callback(user_data, false);
+    }
+  }
+
+  virtual void RequestMountInfoRefresh() OVERRIDE {
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    dbus_proxy_->EnumerateAutoMountableDevices(
+        base::Bind(&CrosDisksClientImpl::OnRequestMountInfo,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&DoNothing));
+  }
+
+  const DiskMap& disks() const OVERRIDE { return disks_; }
+  const MountPointMap& mount_points() const OVERRIDE { return mount_points_; }
+
+ private:
+  struct UnmountDeviceRecursiveCallbackData {
+    void* user_data;
+    UnmountDeviceRecursiveCallbackType callback;
+    size_t pending_callbacks_count;
+
+    UnmountDeviceRecursiveCallbackData(void* ud,
+                                       UnmountDeviceRecursiveCallbackType cb,
+                                       int count)
+        : user_data(ud),
+          callback(cb),
+          pending_callbacks_count(count) {
+    }
+  };
+
+  // Callback for UnmountDeviceRecursive.
+  void OnUnmountDeviceRecursive(UnmountDeviceRecursiveCallbackData* cb_data,
+                                bool success, const std::string& mount_path) {
+    if (success) {
+      // Do standard processing for Unmount event.
+      OnUnmountPath(mount_path);
+      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, success);
+      delete cb_data;
+    }
+  }
+
+  void OnMountCompleted(MountError error_code, const std::string& source_path,
+                        MountType type, const std::string& mount_path) {
+    MountCondition mount_condition = MOUNT_CONDITION_NONE;
+    if (type == MOUNT_TYPE_DEVICE) {
+      if (error_code == MOUNT_ERROR_UNKNOWN_FILESYSTEM)
+        mount_condition = MOUNT_CONDITION_UNKNOWN_FILESYSTEM;
+      if (error_code == MOUNT_ERROR_UNSUPORTED_FILESYSTEM)
+        mount_condition = MOUNT_CONDITION_UNSUPPORTED_FILESYSTEM;
+    }
+    const MountPointInfo mount_info(source_path, mount_path, type,
+                                    mount_condition);
+
+    FireMountCompleted(MOUNTING, error_code, mount_info);
+
+    // If the device is corrupted but it's still possible to format it, it will
+    // be fake mounted.
+    if ((error_code == MOUNT_ERROR_NONE || mount_info.mount_condition) &&
+        mount_points_.find(mount_info.mount_path) == mount_points_.end()) {
+      mount_points_.insert(MountPointMap::value_type(mount_info.mount_path,
+                                                     mount_info));
+    }
+    if ((error_code == MOUNT_ERROR_NONE || mount_info.mount_condition) &&
+        mount_info.mount_type == MOUNT_TYPE_DEVICE &&
+        !mount_info.source_path.empty() &&
+        !mount_info.mount_path.empty()) {
+      DiskMap::iterator iter = disks_.find(mount_info.source_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);
+      FireDiskStatusUpdate(MOUNT_DISK_MOUNTED, disk);
+    }
+  }
+
+  void OnUnmountPath(const std::string& 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,
+                                      mount_points_it->second.mount_path,
+                                      mount_points_it->second.mount_type,
+                                      mount_points_it->second.mount_condition));
+    std::string path(mount_points_it->second.source_path);
+    mount_points_.erase(mount_points_it);
+    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();
+    // Check if there is a formatting scheduled
+    PathMap::iterator it = formatting_pending_.find(disk->device_path());
+    if (it != formatting_pending_.end()) {
+      const std::string file_path = it->second;
+      formatting_pending_.erase(it);
+      FormatUnmountedDevice(file_path);
+    }
+  }
+
+  void OnFormatDevice(const std::string& device_path, bool success) {
+    if (success) {
+      FireDeviceStatusUpdate(MOUNT_FORMATTING_STARTED, device_path);
+    } else {
+      FireDeviceStatusUpdate(MOUNT_FORMATTING_STARTED,
+                             std::string("!") + device_path);
+      LOG(WARNING) << "Format request failed for device " << device_path;
+    }
+  }
+
+  void OnGetDeviceProperties(const DiskInfo& disk) {

[satorux1, 2011/11/07 23:21:55]

disk -> disk_info ?

[hashimoto, 2011/11/08 07:31:05]

Done.

+    // TODO(zelidrag): Find a better way to filter these out before we
+    // fetch the properties:
+    // Ignore disks coming from the device we booted the system from.
+    if (disk.on_boot_device())
+      return;
+
+    LOG(WARNING) << "Found disk " << disk.path();
+    // Delete previous disk info for this path:
+    bool is_new = true;
+    DiskMap::iterator iter = disks_.find(disk.path());
+    if (iter != disks_.end()) {
+      delete iter->second;
+      disks_.erase(iter);
+      is_new = false;
+    }
+    Disk* new_disk = new Disk(disk.path(),
+                              disk.mount_path(),
+                              disk.system_path(),
+                              disk.file_path(),
+                              disk.label(),
+                              disk.drive_label(),
+                              disk.partition_slave(),
+                              FindSystemPathPrefix(disk.system_path()),
+                              disk.device_type(),
+                              disk.size(),
+                              disk.is_drive(),
+                              disk.is_read_only(),
+                              disk.has_media(),
+                              disk.on_boot_device(),
+                              disk.is_hidden());
+    disks_.insert(std::pair<std::string, Disk*>(disk.path(), new_disk));
+    FireDiskStatusUpdate(is_new ? MOUNT_DISK_ADDED : MOUNT_DISK_CHANGED,
+                         new_disk);
+  }
+
+  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.
+        dbus_proxy_->GetDeviceProperties(
+            devices[i],
+            base::Bind(&CrosDisksClientImpl::OnGetDeviceProperties,
+                       weak_ptr_factory_.GetWeakPtr()),
+            base::Bind(&DoNothing));
+      }
+    }
+    // Search and remove disks that are no longer present.
+    for (DiskMap::iterator iter = disks_.begin(); iter != disks_.end(); ) {
+      if (current_device_set.find(iter->first) == current_device_set.end()) {
+        Disk* disk = iter->second;
+        FireDiskStatusUpdate(MOUNT_DISK_REMOVED, disk);
+        delete iter->second;
+        disks_.erase(iter++);
+      } else {
+        ++iter;
+      }
+    }
+  }
+
+  void OnMountEvent(MountEventType evt, std::string device_path) {
+    CrosDisksClientEventType type = MOUNT_DEVICE_ADDED;
+    switch (evt) {
+      case DISK_ADDED: {
+        dbus_proxy_->GetDeviceProperties(
+            device_path,
+            base::Bind(&CrosDisksClientImpl::OnGetDeviceProperties,
+                       weak_ptr_factory_.GetWeakPtr()),
+            base::Bind(&DoNothing));
+        return;
+      }
+      case DISK_REMOVED: {
+        // Search and remove disks that are no longer present.
+        CrosDisksClient::DiskMap::iterator iter = disks_.find(device_path);
+        if (iter != disks_.end()) {
+          Disk* disk = iter->second;
+          FireDiskStatusUpdate(MOUNT_DISK_REMOVED, disk);
+          delete iter->second;
+          disks_.erase(iter);
+        }
+        return;
+      }
+      case DEVICE_ADDED: {
+        type = MOUNT_DEVICE_ADDED;
+        system_path_prefixes_.insert(device_path);
+        break;
+      }
+      case DEVICE_REMOVED: {
+        type = MOUNT_DEVICE_REMOVED;
+        system_path_prefixes_.erase(device_path);
+        break;
+      }
+      case DEVICE_SCANNED: {
+        type = MOUNT_DEVICE_SCANNED;
+        break;
+      }
+      case FORMATTING_FINISHED: {
+        // FORMATTING_FINISHED actually returns file path instead of device
+        // path.
+        device_path = FilePathToDevicePath(device_path);
+        if (device_path.empty()) {
+          LOG(ERROR) << "Error while handling disks metadata. Cannot find "
+                     << "device that is being formatted.";
+          return;
+        }
+        type = MOUNT_FORMATTING_FINISHED;
+        break;
+      }
+      default: {
+        return;
+      }
+    }
+    FireDeviceStatusUpdate(type, device_path);
+  }
+
+  void FireDiskStatusUpdate(CrosDisksClientEventType evt, const Disk* disk) {
+    // Make sure we run on UI thread.
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    FOR_EACH_OBSERVER(Observer, observers_, DiskChanged(evt, disk));
+  }
+
+  void FireDeviceStatusUpdate(CrosDisksClientEventType evt,
+                              const std::string& device_path) {
+    // Make sure we run on UI thread.
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    FOR_EACH_OBSERVER(Observer, observers_, DeviceChanged(evt, device_path));
+  }
+
+  void FireMountCompleted(MountEvent event_type, MountError error_code,
+                          const MountPointInfo& mount_info) {
+    // Make sure we run on UI thread.
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    FOR_EACH_OBSERVER(Observer, observers_,
+                      MountCompleted(event_type, error_code, mount_info));
+  }
+
+  std::string FilePathToDevicePath(const std::string& file_path) {
+    int failed = (file_path[0] == '!') ? 1 : 0;
+    for (CrosDisksClient::DiskMap::iterator it = disks_.begin();
+         it != disks_.end(); ++it) {
+      if (it->second->file_path().compare(file_path.c_str() + failed) == 0) {
+        if (failed)
+          return std::string("!") + it->second->device_path();
+        else
+          return it->second->device_path();
+      }
+    }
+    return EmptyString();
+  }
+
+  const std::string& FindSystemPathPrefix(const std::string& system_path) {
+    if (system_path.empty())
+      return EmptyString();
+    for (SystemPathPrefixSet::const_iterator it = system_path_prefixes_.begin();
+         it != system_path_prefixes_.end();
+         ++it) {
+      if (system_path.find(*it, 0) == 0)
+        return *it;
+    }
+    return EmptyString();
+  }
+
+  // A function to be used as an empty callback
+  static void DoNothing() {

[satorux1, 2011/11/07 23:21:55]

I think you don't need this. See my comment above.

+  }
+
+  // Mount event change observers.
+  ObserverList<Observer> observers_;
+
+  scoped_ptr<CrosDisksDBusProxy> dbus_proxy_;
+
+  // The list of disks found.
+  CrosDisksClient::DiskMap disks_;
+
+  CrosDisksClient::MountPointMap mount_points_;
+
+  typedef std::set<std::string> SystemPathPrefixSet;
+  SystemPathPrefixSet system_path_prefixes_;
+
+  // Set of devices that are supposed to be formated, but are currently waiting
+  // to be unmounted. When device is in this map, the formatting process HAVEN'T
+  // started yet.
+  PathMap formatting_pending_;
+
+  base::WeakPtrFactory<CrosDisksClientImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CrosDisksClientImpl);
+};
+
+class CrosDisksClientStubImpl : public CrosDisksClient {
+ public:
+  CrosDisksClientStubImpl() {}
+  virtual ~CrosDisksClientStubImpl() {}
+
+  // CrosDisksClient 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 std::string& source_path, MountType type,
+                         const MountPathOptions& options) OVERRIDE {}
+  virtual void UnmountPath(const std::string& mount_path) OVERRIDE {}
+  virtual void GetSizeStatsOnFileThread(const std::string& mount_path,
+                                        size_t* total_size_kb,
+                                        size_t* remaining_size_kb) OVERRIDE {}
+  virtual void FormatUnmountedDevice(const std::string& device_path) OVERRIDE {}
+  virtual void FormatMountedDevice(const std::string& mount_path) OVERRIDE {}
+  virtual void UnmountDeviceRecursive(
+      const std::string& device_path,
+      UnmountDeviceRecursiveCallbackType callback,
+      void* user_data) OVERRIDE {}
+ private:
+  DiskMap disks_; // The list of disks found.
+  MountPointMap mount_points_;
+
+  DISALLOW_COPY_AND_ASSIGN(CrosDisksClientStubImpl);
+};
+
+} // namespace
+
+// static
+std::string CrosDisksClient::MountTypeToString(MountType type) {
+  switch (type) {
+    case MOUNT_TYPE_DEVICE:
+      return "device";
+    case MOUNT_TYPE_ARCHIVE:
+      return "file";
+    case MOUNT_TYPE_NETWORK_STORAGE:
+      return "network";
+    case MOUNT_TYPE_INVALID:
+      return "invalid";
+    default:
+      NOTREACHED();
+  }
+  return EmptyString();
+}
+
+// static
+std::string CrosDisksClient::MountConditionToString(MountCondition condition) {
+  switch (condition) {
+    case MOUNT_CONDITION_NONE:
+      return EmptyString();
+    case MOUNT_CONDITION_UNKNOWN_FILESYSTEM:
+      return "unknown_filesystem";
+    case MOUNT_CONDITION_UNSUPPORTED_FILESYSTEM:
+      return "unsupported_filesystem";
+    default:
+      NOTREACHED();
+  }
+  return EmptyString();
+}
+
+// static
+MountType CrosDisksClient::MountTypeFromString(const std::string& type_str) {
+  if (type_str == "device")
+    return MOUNT_TYPE_DEVICE;
+  else if (type_str == "network")
+    return MOUNT_TYPE_NETWORK_STORAGE;
+  else if (type_str == "file")
+    return MOUNT_TYPE_ARCHIVE;
+  else
+    return MOUNT_TYPE_INVALID;
+}
+
+CrosDisksClient::Disk::Disk(const std::string& device_path,
+                            const std::string& mount_path,
+                            const std::string& system_path,
+                            const std::string& file_path,
+                            const std::string& device_label,
+                            const std::string& drive_label,
+                            const std::string& parent_path,
+                            const std::string& system_path_prefix,
+                            DeviceType device_type,
+                            uint64 total_size,
+                            bool is_parent,
+                            bool is_read_only,
+                            bool has_media,
+                            bool on_boot_device,
+                            bool is_hidden)
+    : device_path_(device_path),
+      mount_path_(mount_path),
+      system_path_(system_path),
+      file_path_(file_path),
+      device_label_(device_label),
+      drive_label_(drive_label),
+      parent_path_(parent_path),
+      system_path_prefix_(system_path_prefix),
+      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),
+      is_hidden_(is_hidden) {
+}
+
+CrosDisksClient::Disk::~Disk() {}
+
+// static
+CrosDisksClient* CrosDisksClient::Create(dbus::Bus* bus) {
+  CrosDisksClient* impl;
+  if (system::runtime_environment::IsRunningOnChromeOS())
+    impl = new CrosDisksClientImpl(bus);
+  else
+    impl = new CrosDisksClientStubImpl();
+  return impl;
+}
+
+}  // namespace chromeos