Allows any USB device with an ADB interface to connect to a VM.
Some Samsung phones have multiple USB interfaces, the presence of which formerly
suppressed our notification on Add. The notifications are no longer suppressed,
nor are ADB-supporting devices blocked from sharing with a VM.
Bug: 943429
Change-Id: I203d80d38529010bb8ddb1e7427db3dd0620537e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1530215
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641821}diff --git a/chrome/browser/chromeos/usb/cros_usb_detector.cc b/chrome/browser/chromeos/usb/cros_usb_detector.cc
index 06e928b..ab564ab 100644
--- a/chrome/browser/chromeos/usb/cros_usb_detector.cc
+++ b/chrome/browser/chromeos/usb/cros_usb_detector.cc
@@ -217,6 +217,15 @@
UsbFilterByClassCode(USB_CLASS_VIDEO));
guest_os_classes_without_notif_.emplace_back(
UsbFilterByClassCode(USB_CLASS_PERSONAL_HEALTHCARE));
+
+ // If a device has an adb interface, we always allow it.
+ const int kAdbSubclass = 0x42;
+ const int kAdbProtocol = 0x1;
+ adb_device_filter_ = UsbFilterByClassCode(USB_CLASS_VENDOR_SPEC);
+ adb_device_filter_->has_subclass_code = true;
+ adb_device_filter_->subclass_code = kAdbSubclass;
+ adb_device_filter_->has_protocol_code = true;
+ adb_device_filter_->protocol_code = kAdbProtocol;
}
CrosUsbDetector::~CrosUsbDetector() {
@@ -274,6 +283,24 @@
weak_ptr_factory_.GetWeakPtr()));
}
+bool CrosUsbDetector::ShouldShowNotification(
+ const device::mojom::UsbDeviceInfo& device_info) {
+ if (device::UsbDeviceFilterMatches(*adb_device_filter_, device_info)) {
+ return true;
+ }
+ return !device::UsbDeviceFilterMatchesAny(guest_os_classes_without_notif_,
+ device_info);
+}
+
+bool CrosUsbDetector::IsDeviceSharable(
+ const device::mojom::UsbDeviceInfo& device_info) {
+ if (device::UsbDeviceFilterMatches(*adb_device_filter_, device_info)) {
+ return true;
+ }
+ return !device::UsbDeviceFilterMatchesAny(guest_os_classes_blocked_,
+ device_info);
+}
+
void CrosUsbDetector::OnDeviceChecked(
device::mojom::UsbDeviceInfoPtr device_info,
bool hide_notification,
@@ -297,8 +324,7 @@
SignalSharedUsbDeviceObservers();
// Some devices should not trigger the notification.
- if (hide_notification || device::UsbDeviceFilterMatchesAny(
- guest_os_classes_without_notif_, *device_info)) {
+ if (hide_notification || !ShouldShowNotification(*device_info)) {
return;
}
ShowNotificationForDevice(std::move(device_info));
@@ -310,8 +336,7 @@
void CrosUsbDetector::OnDeviceAdded(device::mojom::UsbDeviceInfoPtr device_info,
bool hide_notification) {
- if (device::UsbDeviceFilterMatchesAny(guest_os_classes_blocked_,
- *device_info)) {
+ if (!IsDeviceSharable(*device_info)) {
return; // Guest OS does not handle this kind of device.
}
device_manager_->CheckAccess(
diff --git a/chrome/browser/chromeos/usb/cros_usb_detector.h b/chrome/browser/chromeos/usb/cros_usb_detector.h
index f4651df..426678f 100644
--- a/chrome/browser/chromeos/usb/cros_usb_detector.h
+++ b/chrome/browser/chromeos/usb/cros_usb_detector.h
@@ -140,6 +140,11 @@
uint8_t guest_port,
crostini::CrostiniResult result);
+ // Returns true when a device should show a notification when attached.
+ bool ShouldShowNotification(const device::mojom::UsbDeviceInfo& device_info);
+ // Returns true when a device can be shared.
+ bool IsDeviceSharable(const device::mojom::UsbDeviceInfo& device_info);
+
device::mojom::UsbDeviceManagerPtr device_manager_;
mojo::AssociatedBinding<device::mojom::UsbDeviceManagerClient>
client_binding_;
@@ -147,6 +152,7 @@
std::vector<device::mojom::UsbDeviceFilterPtr> guest_os_classes_blocked_;
std::vector<device::mojom::UsbDeviceFilterPtr>
guest_os_classes_without_notif_;
+ device::mojom::UsbDeviceFilterPtr adb_device_filter_;
// A mapping from GUID -> UsbDeviceInfo for each attached USB device
std::map<std::string, device::mojom::UsbDeviceInfoPtr> available_device_info_;
diff --git a/chrome/browser/chromeos/usb/cros_usb_detector_unittest.cc b/chrome/browser/chromeos/usb/cros_usb_detector_unittest.cc
index 9db1527..5ab7f4f 100644
--- a/chrome/browser/chromeos/usb/cros_usb_detector_unittest.cc
+++ b/chrome/browser/chromeos/usb/cros_usb_detector_unittest.cc
@@ -45,26 +45,40 @@
const int kUsbConfigWithInterfaces = 1;
-scoped_refptr<device::FakeUsbDeviceInfo> CreateTestDeviceOfClass(
- uint8_t device_class) {
+struct InterfaceCodes {
+ InterfaceCodes(uint8_t device_class,
+ uint8_t subclass_code,
+ uint8_t protocol_code)
+ : device_class(device_class),
+ subclass_code(subclass_code),
+ protocol_code(protocol_code) {}
+ uint8_t device_class;
+ uint8_t subclass_code;
+ uint8_t protocol_code;
+};
+
+scoped_refptr<device::FakeUsbDeviceInfo> CreateTestDeviceFromCodes(
+ uint8_t device_class,
+ const std::vector<InterfaceCodes>& interface_codes) {
+ auto config = device::mojom::UsbConfigurationInfo::New();
+ config->configuration_value = kUsbConfigWithInterfaces;
// The usb_utils do not filter by device class, only by configurations, and
// the FakeUsbDeviceInfo does not set up configurations for a fake device's
// class code. This helper sets up a configuration to match a devices class
// code so that USB devices can be filtered out.
- auto alternate = device::mojom::UsbAlternateInterfaceInfo::New();
- alternate->alternate_setting = 0;
- alternate->class_code = device_class;
- alternate->subclass_code = 0xff;
- alternate->protocol_code = 0xff;
+ for (size_t i = 0; i < interface_codes.size(); ++i) {
+ auto alternate = device::mojom::UsbAlternateInterfaceInfo::New();
+ alternate->alternate_setting = 0;
+ alternate->class_code = interface_codes[i].device_class;
+ alternate->subclass_code = interface_codes[i].subclass_code;
+ alternate->protocol_code = interface_codes[i].protocol_code;
- auto interface = device::mojom::UsbInterfaceInfo::New();
- interface->interface_number = 0;
- interface->alternates.push_back(std::move(alternate));
+ auto interface = device::mojom::UsbInterfaceInfo::New();
+ interface->interface_number = i;
+ interface->alternates.push_back(std::move(alternate));
- auto config = device::mojom::UsbConfigurationInfo::New();
- config->configuration_value = kUsbConfigWithInterfaces;
-
- config->interfaces.push_back(std::move(interface));
+ config->interfaces.push_back(std::move(interface));
+ }
std::vector<device::mojom::UsbConfigurationInfoPtr> configs;
configs.push_back(std::move(config));
@@ -76,6 +90,12 @@
return device;
}
+scoped_refptr<device::FakeUsbDeviceInfo> CreateTestDeviceOfClass(
+ uint8_t device_class) {
+ return CreateTestDeviceFromCodes(device_class,
+ {InterfaceCodes(device_class, 0xff, 0xff)});
+}
+
} // namespace
class CrosUsbDetectorTest : public BrowserWithTestWindowTest {
@@ -201,6 +221,27 @@
// TODO(jopra): Check that the device is not available for sharing.
}
+TEST_F(CrosUsbDetectorTest, UsbDeviceClassAdbAdded) {
+ ConnectToDeviceManager();
+ base::RunLoop().RunUntilIdle();
+
+ const int kAdbClass = 0xff;
+ const int kAdbSubclass = 0x42;
+ const int kAdbProtocol = 0x1;
+ // Adb interface as well as a forbidden interface
+ scoped_refptr<device::FakeUsbDeviceInfo> device = CreateTestDeviceFromCodes(
+ /* USB_CLASS_HID */ 0x03,
+ {InterfaceCodes(kAdbClass, kAdbSubclass, kAdbProtocol),
+ InterfaceCodes(0x03, 0xff, 0xff)});
+
+ device_manager_.AddDevice(device);
+ base::RunLoop().RunUntilIdle();
+
+ std::string notification_id =
+ chromeos::CrosUsbDetector::MakeNotificationId(device->guid());
+ ASSERT_TRUE(display_service_->GetNotification(notification_id));
+}
+
TEST_F(CrosUsbDetectorTest, UsbDeviceClassWithoutNotificationAdded) {
ConnectToDeviceManager();
base::RunLoop().RunUntilIdle();
