diff options
4 files changed, 68 insertions, 22 deletions
diff --git a/chrome/browser/system_monitor/removable_device_notifications_chromeos.cc b/chrome/browser/system_monitor/removable_device_notifications_chromeos.cc index 1b17b92..4a9a49f 100644 --- a/chrome/browser/system_monitor/removable_device_notifications_chromeos.cc +++ b/chrome/browser/system_monitor/removable_device_notifications_chromeos.cc @@ -11,15 +11,55 @@ #include "base/metrics/histogram.h" #include "base/stl_util.h" #include "base/string_number_conversions.h" +#include "base/stringprintf.h" #include "base/utf_string_conversions.h" #include "chrome/browser/system_monitor/media_device_notifications_utils.h" #include "chrome/browser/system_monitor/media_storage_util.h" +#include "chrome/browser/system_monitor/removable_device_constants.h" #include "content/public/browser/browser_thread.h" namespace chromeos { namespace { +// Construct a device name using label or manufacturer (vendor and product) name +// details. +string16 GetDeviceName(const disks::DiskMountManager::Disk& disk) { + std::string device_name = disk.device_label(); + if (device_name.empty()) { + device_name = disk.vendor_name(); + const std::string& product_name = disk.product_name(); + if (!product_name.empty()) { + if (!device_name.empty()) + device_name += chrome::kSpaceDelim; + device_name += product_name; + } + } + return UTF8ToUTF16(device_name); +} + +// Construct a device id using uuid or manufacturer (vendor and product) id +// details. +std::string MakeDeviceUniqueId(const disks::DiskMountManager::Disk& disk) { + std::string uuid = disk.fs_uuid(); + if (!uuid.empty()) + return chrome::kFSUniqueIdPrefix + uuid; + + // If one of the vendor or product information is missing, its value in the + // string is empty. + // Format: VendorModelSerial:VendorInfo:ModelInfo:SerialInfo + // TODO(kmadhusu) Extract serial information for the disks and append it to + // the device unique id. + const std::string& vendor = disk.vendor_id(); + const std::string& product = disk.product_id(); + if (vendor.empty() && product.empty()) + return std::string(); + return base::StringPrintf("%s%s%s%s%s", + chrome::kVendorModelSerialPrefix, + vendor.c_str(), chrome::kNonSpaceDelim, + product.c_str(), chrome::kNonSpaceDelim); +} + // Returns true if the requested device is valid, else false. On success, fills // in |unique_id| and |device_label| bool GetDeviceInfo(const std::string& source_path, std::string* unique_id, @@ -30,13 +70,11 @@ bool GetDeviceInfo(const std::string& source_path, std::string* unique_id, if (!disk || disk->device_type() == DEVICE_TYPE_UNKNOWN) return false; - *unique_id = disk->fs_uuid(); + if (unique_id) + *unique_id = MakeDeviceUniqueId(*disk); - // TODO(kmadhusu): If device label is empty, extract vendor and model details - // and use them as device_label. - *device_label = UTF8ToUTF16(disk->device_label().empty() ? - FilePath(source_path).BaseName().value() : - disk->device_label()); + if (device_label) + *device_label = GetDeviceName(*disk); return true; } @@ -151,7 +189,9 @@ void RemovableDeviceNotificationsCros::AddMountedPathOnUIThread( // Keep track of device uuid, to see how often we receive empty uuid values. UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.DeviceUUIDAvailable", !unique_id.empty()); - if (unique_id.empty()) + UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.DeviceNameAvailable", + !device_label.empty()); + if (unique_id.empty() || device_label.empty()) return; chrome::MediaStorageUtil::Type type = has_dcim ? diff --git a/chrome/browser/system_monitor/removable_device_notifications_chromeos_unittest.cc b/chrome/browser/system_monitor/removable_device_notifications_chromeos_unittest.cc index 7664bbb..753854c 100644 --- a/chrome/browser/system_monitor/removable_device_notifications_chromeos_unittest.cc +++ b/chrome/browser/system_monitor/removable_device_notifications_chromeos_unittest.cc @@ -15,6 +15,7 @@ #include "base/test/mock_devices_changed_observer.h" #include "base/utf_string_conversions.h" #include "chrome/browser/system_monitor/media_storage_util.h" +#include "chrome/browser/system_monitor/removable_device_constants.h" #include "chromeos/disks/mock_disk_mount_manager.h" #include "content/public/test/test_browser_thread.h" #include "testing/gtest/include/gtest/gtest.h" @@ -36,7 +37,8 @@ const char kMountPointB[] = "mnt_b"; std::string GetDCIMDeviceId(const std::string& unique_id) { return chrome::MediaStorageUtil::MakeDeviceId( - chrome::MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM, unique_id); + chrome::MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM, + chrome::kFSUniqueIdPrefix + unique_id); } class RemovableDeviceNotificationsCrosTest : public testing::Test { @@ -80,10 +82,11 @@ class RemovableDeviceNotificationsCrosTest : public testing::Test { void MountDevice(MountError error_code, const DiskMountManager::MountPointInfo& mount_info, - const std::string& unique_id) { + const std::string& unique_id, + const std::string& device_label) { if (error_code == MOUNT_ERROR_NONE) { disk_mount_manager_mock_->CreateDiskEntryForMountDevice( - mount_info, unique_id); + mount_info, unique_id, device_label); } notifications_->MountCompleted(disks::DiskMountManager::MOUNTING, error_code, @@ -167,7 +170,7 @@ TEST_F(RemovableDeviceNotificationsCrosTest, BasicAttachDetach) { ASCIIToUTF16(kDevice1Name), mount_path1.value())) .InSequence(mock_sequence); - MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId0); + MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId0, kDevice1Name); EXPECT_CALL(observer(), OnRemovableStorageDetached(GetDCIMDeviceId(kUniqueId0))) @@ -187,7 +190,7 @@ TEST_F(RemovableDeviceNotificationsCrosTest, BasicAttachDetach) { ASCIIToUTF16(kDevice2Name), mount_path2.value())) .InSequence(mock_sequence); - MountDevice(MOUNT_ERROR_NONE, mount_info2, kUniqueId1); + MountDevice(MOUNT_ERROR_NONE, mount_info2, kUniqueId1, kDevice2Name); EXPECT_CALL(observer(), OnRemovableStorageDetached(GetDCIMDeviceId(kUniqueId1))) @@ -206,11 +209,12 @@ TEST_F(RemovableDeviceNotificationsCrosTest, NoDCIM) { MOUNT_TYPE_DEVICE, disks::MOUNT_CONDITION_NONE); const std::string device_id = chrome::MediaStorageUtil::MakeDeviceId( - chrome::MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM, kUniqueId); + chrome::MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM, + chrome::kFSUniqueIdPrefix + kUniqueId); EXPECT_CALL(observer(), OnRemovableStorageAttached(device_id, ASCIIToUTF16(kDevice1Name), mount_path.value())).Times(1); - MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId); + MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId, kDevice1Name); } // Non device mounts and mount errors are ignored. @@ -226,18 +230,18 @@ TEST_F(RemovableDeviceNotificationsCrosTest, Ignore) { MOUNT_TYPE_DEVICE, disks::MOUNT_CONDITION_NONE); EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0); - MountDevice(MOUNT_ERROR_UNKNOWN, mount_info, kUniqueId); + MountDevice(MOUNT_ERROR_UNKNOWN, mount_info, kUniqueId, kDevice1Name); // Not a device mount_info.mount_type = MOUNT_TYPE_ARCHIVE; EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0); - MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId); + MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId, kDevice1Name); // Unsupported file system. mount_info.mount_type = MOUNT_TYPE_DEVICE; mount_info.mount_condition = disks::MOUNT_CONDITION_UNSUPPORTED_FILESYSTEM; EXPECT_CALL(observer(), OnRemovableStorageAttached(_, _, _)).Times(0); - MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId); + MountDevice(MOUNT_ERROR_NONE, mount_info, kUniqueId, kDevice1Name); } } // namespace diff --git a/chromeos/disks/mock_disk_mount_manager.cc b/chromeos/disks/mock_disk_mount_manager.cc index 610efbb..9af1651 100644 --- a/chromeos/disks/mock_disk_mount_manager.cc +++ b/chromeos/disks/mock_disk_mount_manager.cc @@ -182,12 +182,13 @@ void MockDiskMountManager::SetupDefaultReplies() { void MockDiskMountManager::CreateDiskEntryForMountDevice( const DiskMountManager::MountPointInfo& mount_info, - const std::string& device_id) { + const std::string& device_id, + const std::string& device_label) { Disk* disk = new DiskMountManager::Disk(std::string(mount_info.source_path), std::string(mount_info.mount_path), std::string(), // system_path std::string(), // file_path - std::string(), // device_label + device_label, // device_label std::string(), // drive_label std::string(), // vendor_id std::string(), // vendor_name diff --git a/chromeos/disks/mock_disk_mount_manager.h b/chromeos/disks/mock_disk_mount_manager.h index 169f33d..abe2459 100644 --- a/chromeos/disks/mock_disk_mount_manager.h +++ b/chromeos/disks/mock_disk_mount_manager.h @@ -49,13 +49,14 @@ class MockDiskMountManager : public DiskMountManager { void SetupDefaultReplies(); // Creates a fake disk entry for the mounted device. This function is - // primarily for MediaDeviceNotificationsTest. + // primarily for RemovableDeviceNotificationsTest. void CreateDiskEntryForMountDevice( const DiskMountManager::MountPointInfo& mount_info, - const std::string& device_id); + const std::string& device_id, + const std::string& device_label); // Removes the fake disk entry associated with the mounted device. This - // function is primarily for MediaDeviceNotificationsTest. + // function is primarily for RemovableDeviceNotificationsTest. void RemoveDiskEntryForMountDevice( const DiskMountManager::MountPointInfo& mount_info); |