summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chromeos/dbus/cros_disks_client.cc59
-rw-r--r--chromeos/dbus/cros_disks_client.h42
-rw-r--r--chromeos/dbus/fake_cros_disks_client.cc23
-rw-r--r--chromeos/dbus/fake_cros_disks_client.h13
-rw-r--r--chromeos/disks/disk_mount_manager.cc76
-rw-r--r--chromeos/disks/disk_mount_manager_unittest.cc28
6 files changed, 154 insertions, 87 deletions
diff --git a/chromeos/dbus/cros_disks_client.cc b/chromeos/dbus/cros_disks_client.cc
index fae607d..39bdb6f 100644
--- a/chromeos/dbus/cros_disks_client.cc
+++ b/chromeos/dbus/cros_disks_client.cc
@@ -183,9 +183,8 @@ class CrosDisksClientImpl : public CrosDisksClient {
}
// CrosDisksClient override.
- virtual void SetUpConnections(
- const MountEventHandler& mount_event_handler,
- const MountCompletedHandler& mount_completed_handler) OVERRIDE {
+ virtual void SetMountEventHandler(
+ const MountEventHandler& mount_event_handler) OVERRIDE {
static const SignalEventTuple kSignalEventTuples[] = {
{ cros_disks::kDeviceAdded, CROS_DISKS_DEVICE_ADDED },
{ cros_disks::kDeviceScanned, CROS_DISKS_DEVICE_SCANNED },
@@ -193,7 +192,6 @@ class CrosDisksClientImpl : public CrosDisksClient {
{ cros_disks::kDiskAdded, CROS_DISKS_DISK_ADDED },
{ cros_disks::kDiskChanged, CROS_DISKS_DISK_CHANGED },
{ cros_disks::kDiskRemoved, CROS_DISKS_DISK_REMOVED },
- { cros_disks::kFormattingFinished, CROS_DISKS_FORMATTING_FINISHED },
};
const size_t kNumSignalEventTuples = arraysize(kSignalEventTuples);
@@ -208,6 +206,11 @@ class CrosDisksClientImpl : public CrosDisksClient {
base::Bind(&CrosDisksClientImpl::OnSignalConnected,
weak_ptr_factory_.GetWeakPtr()));
}
+ }
+
+ // CrosDisksClient override.
+ virtual void SetMountCompletedHandler(
+ const MountCompletedHandler& mount_completed_handler) OVERRIDE {
proxy_->ConnectToSignal(
cros_disks::kCrosDisksInterface,
cros_disks::kMountCompleted,
@@ -218,6 +221,19 @@ class CrosDisksClientImpl : public CrosDisksClient {
weak_ptr_factory_.GetWeakPtr()));
}
+ // CrosDisksClient override.
+ virtual void SetFormatCompletedHandler(
+ const FormatCompletedHandler& format_completed_handler) OVERRIDE {
+ proxy_->ConnectToSignal(
+ cros_disks::kCrosDisksInterface,
+ cros_disks::kFormatCompleted,
+ base::Bind(&CrosDisksClientImpl::OnFormatCompleted,
+ weak_ptr_factory_.GetWeakPtr(),
+ format_completed_handler),
+ base::Bind(&CrosDisksClientImpl::OnSignalConnected,
+ weak_ptr_factory_.GetWeakPtr()));
+ }
+
protected:
virtual void Init(dbus::Bus* bus) OVERRIDE {
proxy_ = bus->GetObjectProxy(
@@ -227,7 +243,7 @@ class CrosDisksClientImpl : public CrosDisksClient {
private:
// A struct to contain a pair of signal name and mount event type.
- // Used by SetUpConnections.
+ // Used by SetMountEventHandler.
struct SignalEventTuple {
const char *signal_name;
MountEventType event_type;
@@ -262,7 +278,7 @@ class CrosDisksClientImpl : public CrosDisksClient {
// make this fail if reader is not able to read the error code value from
// the response.
dbus::MessageReader reader(response);
- unsigned int error_code;
+ uint32 error_code = 0;
if (reader.PopUint32(&error_code) &&
static_cast<MountError>(error_code) != MOUNT_ERROR_NONE) {
error_callback.Run();
@@ -341,9 +357,9 @@ class CrosDisksClientImpl : public CrosDisksClient {
// Handles MountCompleted signal and calls |handler|.
void OnMountCompleted(MountCompletedHandler handler, dbus::Signal* signal) {
dbus::MessageReader reader(signal);
- unsigned int error_code = 0;
+ uint32 error_code = 0;
std::string source_path;
- unsigned int mount_type = 0;
+ uint32 mount_type = 0;
std::string mount_path;
if (!reader.PopUint32(&error_code) ||
!reader.PopString(&source_path) ||
@@ -356,6 +372,18 @@ class CrosDisksClientImpl : public CrosDisksClient {
static_cast<MountType>(mount_type), mount_path);
}
+ // Handles FormatCompleted signal and calls |handler|.
+ void OnFormatCompleted(FormatCompletedHandler handler, dbus::Signal* signal) {
+ dbus::MessageReader reader(signal);
+ uint32 error_code = 0;
+ std::string device_path;
+ if (!reader.PopUint32(&error_code) || !reader.PopString(&device_path)) {
+ LOG(ERROR) << "Invalid signal: " << signal->ToString();
+ return;
+ }
+ handler.Run(static_cast<FormatError>(error_code), device_path);
+ }
+
// Handles the result of signal connection setup.
void OnSignalConnected(const std::string& interface,
const std::string& signal,
@@ -458,13 +486,21 @@ class CrosDisksClientStubImpl : public CrosDisksClient {
base::MessageLoopProxy::current()->PostTask(FROM_HERE, error_callback);
}
- virtual void SetUpConnections(
- const MountEventHandler& mount_event_handler,
- const MountCompletedHandler& mount_completed_handler) OVERRIDE {
+ virtual void SetMountEventHandler(
+ const MountEventHandler& mount_event_handler) OVERRIDE {
mount_event_handler_ = mount_event_handler;
+ }
+
+ virtual void SetMountCompletedHandler(
+ const MountCompletedHandler& mount_completed_handler) OVERRIDE {
mount_completed_handler_ = mount_completed_handler;
}
+ virtual void SetFormatCompletedHandler(
+ const FormatCompletedHandler& format_completed_handler) OVERRIDE {
+ format_completed_handler_ = format_completed_handler;
+ }
+
private:
// Performs file actions for Mount().
static MountError PerformFakeMount(const std::string& source_path,
@@ -532,6 +568,7 @@ class CrosDisksClientStubImpl : public CrosDisksClient {
MountEventHandler mount_event_handler_;
MountCompletedHandler mount_completed_handler_;
+ FormatCompletedHandler format_completed_handler_;
base::WeakPtrFactory<CrosDisksClientStubImpl> weak_ptr_factory_;
diff --git a/chromeos/dbus/cros_disks_client.h b/chromeos/dbus/cros_disks_client.h
index 92871da82..3cd9ee9 100644
--- a/chromeos/dbus/cros_disks_client.h
+++ b/chromeos/dbus/cros_disks_client.h
@@ -91,7 +91,6 @@ enum MountEventType {
CROS_DISKS_DEVICE_ADDED,
CROS_DISKS_DEVICE_REMOVED,
CROS_DISKS_DEVICE_SCANNED,
- CROS_DISKS_FORMATTING_FINISHED,
};
// Additional unmount flags to be added to unmount request.
@@ -192,8 +191,8 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
public:
// A callback to handle the result of EnumerateAutoMountableDevices.
// The argument is the enumerated device paths.
- typedef base::Callback<void(const std::vector<std::string>& device_paths)
- > EnumerateAutoMountableDevicesCallback;
+ typedef base::Callback<void(const std::vector<std::string>& device_paths)>
+ EnumerateAutoMountableDevicesCallback;
// A callback to handle the result of FormatDevice.
// The argument is true when formatting succeeded.
@@ -201,8 +200,8 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
// A callback to handle the result of GetDeviceProperties.
// The argument is the information about the specified device.
- typedef base::Callback<void(const DiskInfo& disk_info)
- > GetDevicePropertiesCallback;
+ typedef base::Callback<void(const DiskInfo& disk_info)>
+ GetDevicePropertiesCallback;
// A callback to handle MountCompleted signal.
// The first argument is the error code.
@@ -212,15 +211,22 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
typedef base::Callback<void(MountError error_code,
const std::string& source_path,
MountType mount_type,
- const std::string& mount_path)
- > MountCompletedHandler;
+ const std::string& mount_path)>
+ MountCompletedHandler;
+
+ // A callback to handle FormatCompleted signal.
+ // The first argument is the error code.
+ // The second argument is the device path.
+ typedef base::Callback<void(FormatError error_code,
+ const std::string& device_path)>
+ FormatCompletedHandler;
// A callback to handle mount events.
// The first argument is the event type.
// The second argument is the device path.
typedef base::Callback<void(MountEventType event_type,
- const std::string& device_path)
- > MountEventHandler;
+ const std::string& device_path)>
+ MountEventHandler;
virtual ~CrosDisksClient();
@@ -265,13 +271,21 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
const GetDevicePropertiesCallback& callback,
const base::Closure& error_callback) = 0;
- // Registers given callback for events.
- // |mount_event_handler| is called when mount event signal is received.
- // |mount_completed_handler| is called when MountCompleted signal is received.
- virtual void SetUpConnections(
- const MountEventHandler& mount_event_handler,
+ // Registers |mount_event_handler| as a callback to be invoked when a mount
+ // event signal is received.
+ virtual void SetMountEventHandler(
+ const MountEventHandler& mount_event_handler) = 0;
+
+ // Registers |mount_completed_handler| as a callback to be invoked when a
+ // MountCompleted signal is received.
+ virtual void SetMountCompletedHandler(
const MountCompletedHandler& mount_completed_handler) = 0;
+ // Registers |format_completed_handler| as a callback to be invoked when a
+ // FormatCompleted signal is received.
+ virtual void SetFormatCompletedHandler(
+ const FormatCompletedHandler& format_completed_handler) = 0;
+
// Factory function, creates a new instance and returns ownership.
// For normal usage, access the singleton via DBusThreadManager::Get().
static CrosDisksClient* Create(DBusClientImplementationType type);
diff --git a/chromeos/dbus/fake_cros_disks_client.cc b/chromeos/dbus/fake_cros_disks_client.cc
index 2578077..8e96bf2 100644
--- a/chromeos/dbus/fake_cros_disks_client.cc
+++ b/chromeos/dbus/fake_cros_disks_client.cc
@@ -75,13 +75,21 @@ void FakeCrosDisksClient::GetDeviceProperties(
const base::Closure& error_callback) {
}
-void FakeCrosDisksClient::SetUpConnections(
- const MountEventHandler& mount_event_handler,
- const MountCompletedHandler& mount_completed_handler) {
+void FakeCrosDisksClient::SetMountEventHandler(
+ const MountEventHandler& mount_event_handler) {
mount_event_handler_ = mount_event_handler;
+}
+
+void FakeCrosDisksClient::SetMountCompletedHandler(
+ const MountCompletedHandler& mount_completed_handler) {
mount_completed_handler_ = mount_completed_handler;
}
+void FakeCrosDisksClient::SetFormatCompletedHandler(
+ const FormatCompletedHandler& format_completed_handler) {
+ format_completed_handler_ = format_completed_handler;
+}
+
bool FakeCrosDisksClient::SendMountEvent(MountEventType event,
const std::string& path) {
if (mount_event_handler_.is_null())
@@ -101,4 +109,13 @@ bool FakeCrosDisksClient::SendMountCompletedEvent(
return true;
}
+bool FakeCrosDisksClient::SendFormatCompletedEvent(
+ FormatError error_code,
+ const std::string& device_path) {
+ if (format_completed_handler_.is_null())
+ return false;
+ format_completed_handler_.Run(error_code, device_path);
+ return true;
+}
+
} // namespace chromeos
diff --git a/chromeos/dbus/fake_cros_disks_client.h b/chromeos/dbus/fake_cros_disks_client.h
index 855a3c1..f51a4be 100644
--- a/chromeos/dbus/fake_cros_disks_client.h
+++ b/chromeos/dbus/fake_cros_disks_client.h
@@ -41,17 +41,23 @@ class FakeCrosDisksClient : public CrosDisksClient {
const std::string& device_path,
const GetDevicePropertiesCallback& callback,
const base::Closure& error_callback) OVERRIDE;
- virtual void SetUpConnections(
- const MountEventHandler& mount_event_handler,
+ virtual void SetMountEventHandler(
+ const MountEventHandler& mount_event_handler) OVERRIDE;
+ virtual void SetMountCompletedHandler(
const MountCompletedHandler& mount_completed_handler) OVERRIDE;
+ virtual void SetFormatCompletedHandler(
+ const FormatCompletedHandler& format_completed_handler) OVERRIDE;
// Used in tests to simulate signals sent by cros disks layer.
- // Invokes handlers set in |SetUpConnections|.
+ // Invokes handlers set in |SetMountEventHandler|, |SetMountCompletedHandler|,
+ // and |SetFormatCompletedHandler|.
bool SendMountEvent(MountEventType event, const std::string& path);
bool SendMountCompletedEvent(MountError error_code,
const std::string& source_path,
MountType mount_type,
const std::string& mount_path);
+ bool SendFormatCompletedEvent(FormatError error_code,
+ const std::string& device_path);
// Returns how many times Unmount() was called.
int unmount_call_count() const {
@@ -105,6 +111,7 @@ class FakeCrosDisksClient : public CrosDisksClient {
private:
MountEventHandler mount_event_handler_;
MountCompletedHandler mount_completed_handler_;
+ FormatCompletedHandler format_completed_handler_;
int unmount_call_count_;
std::string last_unmount_device_path_;
diff --git a/chromeos/disks/disk_mount_manager.cc b/chromeos/disks/disk_mount_manager.cc
index a781107..75ab642 100644
--- a/chromeos/disks/disk_mount_manager.cc
+++ b/chromeos/disks/disk_mount_manager.cc
@@ -31,11 +31,15 @@ class DiskMountManagerImpl : public DiskMountManager {
DCHECK(dbus_thread_manager);
cros_disks_client_ = dbus_thread_manager->GetCrosDisksClient();
DCHECK(cros_disks_client_);
- cros_disks_client_->SetUpConnections(
+ cros_disks_client_->SetMountEventHandler(
base::Bind(&DiskMountManagerImpl::OnMountEvent,
- weak_ptr_factory_.GetWeakPtr()),
+ weak_ptr_factory_.GetWeakPtr()));
+ cros_disks_client_->SetMountCompletedHandler(
base::Bind(&DiskMountManagerImpl::OnMountCompleted,
weak_ptr_factory_.GetWeakPtr()));
+ cros_disks_client_->SetFormatCompletedHandler(
+ base::Bind(&DiskMountManagerImpl::OnFormatCompleted,
+ weak_ptr_factory_.GetWeakPtr()));
}
virtual ~DiskMountManagerImpl() {
@@ -280,7 +284,7 @@ class DiskMountManagerImpl : public DiskMountManager {
if (success) {
// Do standard processing for Unmount event.
OnUnmountPath(UnmountPathCallback(), true, mount_path);
- LOG(INFO) << mount_path << " unmounted.";
+ VLOG(1) << mount_path << " unmounted.";
}
// This is safe as long as all callbacks are called on the same thread as
// UnmountDeviceRecursively.
@@ -407,6 +411,33 @@ class DiskMountManagerImpl : public DiskMountManager {
NotifyFormatStatusUpdate(FORMAT_STARTED, error_code, device_path);
}
+ // Callback to handle FormatCompleted signal and Format method call failure.
+ void OnFormatCompleted(FormatError error_code,
+ const std::string& device_path) {
+ std::string actual_device_path;
+ // Depending on cros-disks implementation the event may return either file
+ // path or device path. We want to use device path.
+ // TODO(benchan): This shouldn't be necessary. Find out the issue and ensure
+ // that cros-disks simply returns the path format that Chrome expects, then
+ // we should remove the following code.
+ for (DiskMountManager::DiskMap::const_iterator it = disks_.begin();
+ it != disks_.end(); ++it) {
+ if (it->second->file_path() == device_path ||
+ it->second->device_path() == device_path) {
+ actual_device_path = it->second->device_path();
+ break;
+ }
+ }
+
+ if (actual_device_path.empty()) {
+ LOG(ERROR) << "Error while handling disks metadata. Cannot find "
+ << "device that is being formatted.";
+ return;
+ }
+
+ NotifyFormatStatusUpdate(FORMAT_COMPLETED, error_code, actual_device_path);
+ }
+
// Callbcak for GetDeviceProperties.
void OnGetDeviceProperties(const DiskInfo& disk_info) {
// TODO(zelidrag): Find a better way to filter these out before we
@@ -513,20 +544,6 @@ class DiskMountManagerImpl : public DiskMountManager {
NotifyDeviceStatusUpdate(DEVICE_SCANNED, device_path);
break;
}
- case CROS_DISKS_FORMATTING_FINISHED: {
- std::string path;
- FormatError error_code;
- ParseFormatFinishedPath(device_path, &path, &error_code);
-
- if (!path.empty()) {
- NotifyFormatStatusUpdate(FORMAT_COMPLETED, error_code, path);
- break;
- }
-
- LOG(ERROR) << "Error while handling disks metadata. Cannot find "
- << "device that is being formatted.";
- break;
- }
default: {
LOG(ERROR) << "Unknown event: " << event;
}
@@ -560,31 +577,6 @@ class DiskMountManagerImpl : public DiskMountManager {
OnFormatEvent(event, error_code, device_path));
}
- // Converts file path to device path.
- void ParseFormatFinishedPath(const std::string& received_path,
- std::string* device_path,
- FormatError* error_code) {
- // TODO(tbarzic): Refactor error handling code like here.
- // Appending "!" is not the best way to indicate error. This kind of trick
- // also makes it difficult to simplify the code paths.
- bool success = !StartsWithASCII(received_path, "!", true);
- *error_code = success ? FORMAT_ERROR_NONE : FORMAT_ERROR_UNKNOWN;
-
- std::string path = received_path.substr(success ? 0 : 1);
-
- // Depending on cros disks implementation the event may return either file
- // path or device path. We want to use device path.
- for (DiskMountManager::DiskMap::iterator it = disks_.begin();
- it != disks_.end(); ++it) {
- // Skip the leading '!' on the failure case.
- if (it->second->file_path() == path ||
- it->second->device_path() == path) {
- *device_path = it->second->device_path();
- return;
- }
- }
- }
-
// Finds system path prefix from |system_path|.
const std::string& FindSystemPathPrefix(const std::string& system_path) {
if (system_path.empty())
diff --git a/chromeos/disks/disk_mount_manager_unittest.cc b/chromeos/disks/disk_mount_manager_unittest.cc
index 36e67ad..3786dcd3 100644
--- a/chromeos/disks/disk_mount_manager_unittest.cc
+++ b/chromeos/disks/disk_mount_manager_unittest.cc
@@ -386,7 +386,7 @@ TEST_F(DiskMountManagerTest, Format_FormatFails) {
// The observer should get notified that the device was unmounted and that
// formatting has started.
// After the formatting starts, the test will simulate failing
- // FORMATTING_FINISHED signal, so the observer should also be notified the
+ // FORMAT_COMPLETED signal, so the observer should also be notified the
// formatting has failed (FORMAT_COMPLETED event).
{
InSequence s;
@@ -429,14 +429,14 @@ TEST_F(DiskMountManagerTest, Format_FormatFails) {
// The device should be unmounted by now.
EXPECT_FALSE(HasMountPoint("/device/mount_path"));
- // Send failing FORMATTING_FINISHED signal.
+ // Send failing FORMAT_COMPLETED signal.
// The failure is marked by ! in fromt of the path (but this should change
// soon).
- fake_cros_disks_client_->SendMountEvent(
- chromeos::CROS_DISKS_FORMATTING_FINISHED, "!/device/source_path");
+ fake_cros_disks_client_->SendFormatCompletedEvent(
+ chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path");
}
-// Tests the same case as Format_FormatFails, but the FORMATTING_FINISHED event
+// Tests the same case as Format_FormatFails, but the FORMAT_COMPLETED event
// is sent with file_path of the formatted device (instead of its device path).
TEST_F(DiskMountManagerTest, Format_FormatFailsAndReturnFilePath) {
// Set up expectations for observer mock.
@@ -481,9 +481,9 @@ TEST_F(DiskMountManagerTest, Format_FormatFailsAndReturnFilePath) {
// The device should be unmounted by now.
EXPECT_FALSE(HasMountPoint("/device/mount_path"));
- // Send failing FORMATTING_FINISHED signal with the device's file path.
- fake_cros_disks_client_->SendMountEvent(
- chromeos::CROS_DISKS_FORMATTING_FINISHED, "!/device/file_path");
+ // Send failing FORMAT_COMPLETED signal with the device's file path.
+ fake_cros_disks_client_->SendFormatCompletedEvent(
+ chromeos::FORMAT_ERROR_UNKNOWN, "/device/file_path");
}
// Tests the case when formatting completes successfully.
@@ -536,8 +536,8 @@ TEST_F(DiskMountManagerTest, Format_FormatSuccess) {
EXPECT_FALSE(HasMountPoint("/device/mount_path"));
// Simulate cros_disks reporting success.
- fake_cros_disks_client_->SendMountEvent(
- chromeos::CROS_DISKS_FORMATTING_FINISHED, "/device/source_path");
+ fake_cros_disks_client_->SendFormatCompletedEvent(
+ chromeos::FORMAT_ERROR_NONE, "/device/source_path");
}
// Tests that it's possible to format the device twice in a row (this may not be
@@ -597,8 +597,8 @@ TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
EXPECT_FALSE(HasMountPoint("/device/mount_path"));
// Simulate cros_disks reporting success.
- fake_cros_disks_client_->SendMountEvent(
- chromeos::CROS_DISKS_FORMATTING_FINISHED, "/device/source_path");
+ fake_cros_disks_client_->SendFormatCompletedEvent(
+ chromeos::FORMAT_ERROR_NONE, "/device/source_path");
// Simulate the device remounting.
fake_cros_disks_client_->SendMountCompletedEvent(
@@ -627,8 +627,8 @@ TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
fake_cros_disks_client_->last_format_device_filesystem());
// Simulate cros_disks reporting success.
- fake_cros_disks_client_->SendMountEvent(
- chromeos::CROS_DISKS_FORMATTING_FINISHED, "/device/source_path");
+ fake_cros_disks_client_->SendFormatCompletedEvent(
+ chromeos::FORMAT_ERROR_NONE, "/device/source_path");
}
} // namespace