On some Chrome OS devices there are drives that appear removable to the user...

On some Chrome OS devices there are drives that appear removable to the user but are listed in the system as fixed.  This is the case with SD cards on parrot, where they are exposed as a fixed mmcblk device.

This CL creates a new Chrome OS implementation for listing removable drives that uses the chromeos::disks::DiskMountManager, similar to how the Chrome OS ImageBurner is set up.

BUG=352430

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

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

chrome/browser/extensions/api/image_writer_private/removable_storage_provider_chromeos.cc

+// Copyright 2014 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/extensions/api/image_writer_private/removable_storage_provider.h"
+#include "chromeos/disks/disk_mount_manager.h"
+
+namespace extensions {
+
+const char kUnknownSDDiskModel[] = "SD Card";
+const char kUnknownUSBDiskModel[] = "USB Drive";
+
+using chromeos::disks::DiskMountManager;
+
+// The Chrome OS implementation takes advantage of the Chrome OS
+// DiskMountManager.  This does not expose whether the device is a removable or
+// fixed disk.  In fact, some SD cards will present themselves as fixed disks
+// (see http://crbug.com/340761).  Thus we just expose all USB and SD drives.
+// static
+void RemovableStorageProvider::GetAllDevices(DeviceListReadyCallback callback) {
+  scoped_refptr<StorageDeviceList> device_list(new StorageDeviceList());
+
+  DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
+  const DiskMountManager::DiskMap& disks = disk_mount_manager->disks();
+
+  for (DiskMountManager::DiskMap::const_iterator iter = disks.begin();
+       iter != disks.end();
+       ++iter) {
+    const DiskMountManager::Disk& disk = *iter->second;
+    if (disk.is_parent() && !disk.on_boot_device() && disk.has_media() &&
+        (disk.device_type() == chromeos::DEVICE_TYPE_USB ||
+         disk.device_type() == chromeos::DEVICE_TYPE_SD)) {
+      linked_ptr<api::image_writer_private::RemovableStorageDevice> device(
+          new api::image_writer_private::RemovableStorageDevice());
+      device->storage_unit_id = disk.file_path();
+      device->capacity = disk.total_size_in_bytes();
+      device->vendor = disk.vendor_name();
+      device->model = disk.product_name();
+
+      if (device->model.empty() && device->vendor.empty()) {
+        if (disk.device_type() == chromeos::DEVICE_TYPE_USB) {
+          device->model = kUnknownUSBDiskModel;
+        } else {
+          device->model = kUnknownSDDiskModel;
+        }
+      }
+
+      device_list->data.push_back(device);
+    }
+  }
+
+  callback.Run(device_list, true);
+}
+
+}  // namespace extensions

chrome/browser/extensions/api/image_writer_private/removable_storage_provider_chromeos_unittest.cc

+// Copyright 2014 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 "base/bind.h"
+#include "chrome/browser/extensions/api/image_writer_private/removable_storage_provider.h"
+#include "chromeos/disks/mock_disk_mount_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+namespace {
+
+using namespace chromeos::disks;
+using namespace api::image_writer_private;
+
+const char kDevicePathUSB[] = "/dev/test-usb";
+const char kDevicePathSD[] = "/dev/test-sd";
+const char kMountPath[] = "/test-mount";
+const char kDeviceId[] = "FFFF-FFFF";
+const char kDeviceName[] = "Test Device Name";
+const char kVendorName[] = "Test Vendor";
+const char kProductName[] = "Test Product";
+const uint64 kDeviceSize = 1024 * 1024 * 1024;
+
+const char kUnknownSDDiskModel[] = "SD Card";
+const char kUnknownUSBDiskModel[] = "USB Drive";
+
+class RemovableStorageProviderChromeOsUnitTest : public testing::Test {
+ public:
+  virtual void SetUp() OVERRIDE {
+    disk_mount_manager_mock_ = new MockDiskMountManager();
+    DiskMountManager::InitializeForTesting(disk_mount_manager_mock_);
+    disk_mount_manager_mock_->SetupDefaultReplies();
+  }
+
+  virtual void TearDown() OVERRIDE { DiskMountManager::Shutdown(); }
+
+  void DevicesCallback(scoped_refptr<StorageDeviceList> devices, bool success) {
+    devices_ = devices;
+  }
+
+  void CreateDisk(const std::string& device_path,
+                  chromeos::DeviceType device_type,
+                  bool is_parent,
+                  bool has_media,
+                  bool on_boot_device) {
+    return CreateDisk(device_path,
+                      kVendorName,
+                      kProductName,
+                      device_type,
+                      is_parent,
+                      has_media,
+                      on_boot_device);
+  }
+
+  void CreateDisk(const std::string& device_path,
+                  const std::string& vendor_name,
+                  const std::string& product_name,
+                  chromeos::DeviceType device_type,
+                  bool is_parent,
+                  bool has_media,
+                  bool on_boot_device) {
+    DiskMountManager::MountPointInfo mount_info(
+        device_path,
+        kMountPath,
+        chromeos::MOUNT_TYPE_DEVICE,
+        chromeos::disks::MOUNT_CONDITION_NONE);
+    disk_mount_manager_mock_->CreateDiskEntryForMountDevice(mount_info,
+                                                            kDeviceId,
+                                                            kDeviceName,
+                                                            vendor_name,
+                                                            product_name,
+                                                            device_type,
+                                                            kDeviceSize,
+                                                            is_parent,
+                                                            has_media,
+                                                            on_boot_device);
+  }
+
+  // Checks if the DeviceList has a specific entry.
+  RemovableStorageDevice* FindDevice(StorageDeviceList* list,
+                                     const std::string& file_path) {
+    for (std::vector<linked_ptr<RemovableStorageDevice> >::const_iterator iter =
+             list->data.begin();
+         iter != list->data.end();
+         ++iter) {
+      if ((*iter)->storage_unit_id == file_path) {
+        return (*iter).get();
+      }
+    }
+    return NULL;
+  }
+
+  void ExpectDevice(StorageDeviceList* list,
+                    const std::string& device_path,
+                    const std::string& vendor,
+                    const std::string& model,
+                    uint64 capacity) {
+    RemovableStorageDevice* device = FindDevice(devices_, device_path);
+
+    ASSERT_TRUE(device != NULL);
+
+    EXPECT_EQ(device_path, device->storage_unit_id);
+    EXPECT_EQ(vendor, device->vendor);
+    EXPECT_EQ(model, device->model);
+    EXPECT_EQ(capacity, device->capacity);
+  }
+
+  MockDiskMountManager* disk_mount_manager_mock_;
+  scoped_refptr<StorageDeviceList> devices_;
+};
+
+}  // namespace
+
+// Tests that GetAllDevices works as expected, only exposing USB and SD cards
+// that are parents, have media and are not boot devices.  Other flags are
+// uninteresting or should not occur for these device types.
+TEST_F(RemovableStorageProviderChromeOsUnitTest, GetAllDevices) {
+  CreateDisk(kDevicePathUSB, chromeos::DEVICE_TYPE_USB, true, true, false);
+  CreateDisk(kDevicePathSD, chromeos::DEVICE_TYPE_SD, true, true, false);
+  CreateDisk("/dev/NotParent", chromeos::DEVICE_TYPE_USB, false, true, false);
+  CreateDisk("/dev/NoMedia", chromeos::DEVICE_TYPE_USB, true, false, false);
+  CreateDisk("/dev/OnBootDevice", chromeos::DEVICE_TYPE_USB, true, true, true);
+
+  RemovableStorageProvider::GetAllDevices(
+      base::Bind(&RemovableStorageProviderChromeOsUnitTest::DevicesCallback,
+                 base::Unretained(this)));
+
+  ASSERT_EQ(2U, devices_->data.size());
+
+  ExpectDevice(
+      devices_, kDevicePathUSB, kVendorName, kProductName, kDeviceSize);
+  ExpectDevice(devices_, kDevicePathSD, kVendorName, kProductName, kDeviceSize);
+}
+
+// Tests that a USB drive with an empty vendor and product gets a generic name.
+TEST_F(RemovableStorageProviderChromeOsUnitTest, EmptyProductAndModel) {
+  CreateDisk(
+      kDevicePathUSB, "", "", chromeos::DEVICE_TYPE_USB, true, true, false);
+  CreateDisk(
+      kDevicePathSD, "", "", chromeos::DEVICE_TYPE_SD, true, true, false);
+
+  RemovableStorageProvider::GetAllDevices(
+      base::Bind(&RemovableStorageProviderChromeOsUnitTest::DevicesCallback,
+                 base::Unretained(this)));
+
+  ASSERT_EQ(2U, devices_->data.size());
+
+  ExpectDevice(devices_, kDevicePathUSB, "", kUnknownUSBDiskModel, kDeviceSize);
+  ExpectDevice(devices_, kDevicePathSD, "", kUnknownSDDiskModel, kDeviceSize);
+}
+
+}  // namespace extensions

chrome/chrome_browser_extensions.gypi

@@ -334,6 +334,7 @@
         'browser/extensions/api/image_writer_private/image_writer_private_api.h',
         'browser/extensions/api/image_writer_private/removable_storage_provider.h',
         'browser/extensions/api/image_writer_private/removable_storage_provider_linux.cc',
+        'browser/extensions/api/image_writer_private/removable_storage_provider_chromeos.cc',
         'browser/extensions/api/image_writer_private/removable_storage_provider_mac.cc',
         'browser/extensions/api/image_writer_private/removable_storage_provider_win.cc',
         'browser/extensions/api/image_writer_private/write_from_file_operation.cc',
@@ -892,6 +893,7 @@
             'browser/extensions/api/audio/audio_service.cc',
             'browser/extensions/api/feedback_private/feedback_service_nonchromeos.cc',
             'browser/extensions/api/image_writer_private/operation_nonchromeos.cc',
+            'browser/extensions/api/image_writer_private/removable_storage_provider_linux.cc',
             'browser/extensions/api/system_display/display_info_provider_aura.cc',
             'browser/extensions/default_apps.cc',
             'browser/extensions/default_apps.h',

chrome/chrome_tests_unit.gypi

@@ -883,6 +883,7 @@
         'browser/extensions/api/image_writer_private/destroy_partitions_operation_unittest.cc',
         'browser/extensions/api/image_writer_private/operation_manager_unittest.cc',
         'browser/extensions/api/image_writer_private/operation_unittest.cc',
+        'browser/extensions/api/image_writer_private/removable_storage_provider_chromeos_unittest.cc',
         'browser/extensions/api/image_writer_private/test_utils.cc',
         'browser/extensions/api/image_writer_private/write_from_file_operation_unittest.cc',
         'browser/extensions/api/image_writer_private/write_from_url_operation_unittest.cc',

chromeos/disks/mock_disk_mount_manager.cc

@@ -181,25 +181,28 @@ void MockDiskMountManager::CreateDiskEntryForMountDevice(
     const std::string& vendor_name,
     const std::string& product_name,
     DeviceType device_type,
-    uint64 total_size_in_bytes) {
-  Disk* disk = new DiskMountManager::Disk(std::string(mount_info.source_path),
-                                          std::string(mount_info.mount_path),
+    uint64 total_size_in_bytes,
+    bool is_parent,
+    bool has_media,
+    bool on_boot_device) {
+  Disk* disk = new DiskMountManager::Disk(mount_info.source_path,
+                                          mount_info.mount_path,
                                           std::string(),  // system_path
-                                          std::string(),  // file_path
-                                          device_label,  // device_label
+                                          mount_info.source_path,
+                                          device_label,
                                           std::string(),  // drive_label
                                           std::string(),  // vendor_id
                                           vendor_name,
                                           std::string(),  // product_id
                                           product_name,
-                                          device_id,  // fs_uuid
+                                          device_id,      // fs_uuid
                                           std::string(),  // system_path_prefix
                                           device_type,
                                           total_size_in_bytes,
-                                          false,  // is_parent
+                                          is_parent,
                                           false,  // is_read_only
-                                          true,  // has_media
-                                          false,  // on_boot_device
+                                          has_media,
+                                          on_boot_device,
                                           false);  // is_hidden
   DiskMountManager::DiskMap::iterator it = disks_.find(mount_info.source_path);
   if (it == disks_.end()) {

chromeos/disks/mock_disk_mount_manager.h

@@ -16,6 +16,8 @@
 namespace chromeos {
 namespace disks {
 
+// TODO(tbarzic): Replace this mock with a fake implementation
+// (http://crbug.com/355757)
 class MockDiskMountManager : public DiskMountManager {
  public:
   MockDiskMountManager();
@@ -60,7 +62,10 @@ class MockDiskMountManager : public DiskMountManager {
       const std::string& vendor_name,
       const std::string& product_name,
       DeviceType device_type,
-      uint64 total_size_in_bytes);
+      uint64 total_size_in_bytes,
+      bool is_parent,
+      bool has_media,
+      bool on_boot_device);
 
   // Removes the fake disk entry associated with the mounted device. This
   // function is primarily for StorageMonitorTest.

components/storage_monitor/storage_monitor_chromeos_unittest.cc

@@ -196,8 +196,16 @@ void StorageMonitorCrosTest::MountDevice(
     uint64 device_size_in_bytes) {
   if (error_code == chromeos::MOUNT_ERROR_NONE) {
     disk_mount_manager_mock_->CreateDiskEntryForMountDevice(
-        mount_info, unique_id, device_label, vendor_name, product_name,
-        device_type, device_size_in_bytes);
+        mount_info,
+        unique_id,
+        device_label,
+        vendor_name,
+        product_name,
+        device_type,
+        device_size_in_bytes,
+        false /* is_parent */,
+        true /* has_media */,
+        false /* on_boot_device */);
   }
   monitor_->OnMountEvent(DiskMountManager::MOUNTING, error_code, mount_info);
   WaitForFileThread();