Settings Split: Check bluetooth device type to make sure it matches
Bug: b/241965700
Change-Id: I7386773f17cd881957b9c9dbee7a747e36f52f06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4544064
Commit-Queue: David Padlipsky <dpad@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1147529}
diff --git a/ash/bluetooth_devices_observer.cc b/ash/bluetooth_devices_observer.cc
index 198f006..c6f28d3a 100644
--- a/ash/bluetooth_devices_observer.cc
+++ b/ash/bluetooth_devices_observer.cc
@@ -51,25 +51,31 @@
bool BluetoothDevicesObserver::IsConnectedBluetoothDevice(
const ui::InputDevice& input_device) const {
+ return GetConnectedBluetoothDevice(input_device) != nullptr;
+}
+
+device::BluetoothDevice* BluetoothDevicesObserver::GetConnectedBluetoothDevice(
+ const ui::InputDevice& input_device) const {
if (!bluetooth_adapter_ || !bluetooth_adapter_->IsPresent() ||
!bluetooth_adapter_->IsInitialized() ||
!bluetooth_adapter_->IsPowered()) {
- return false;
+ return nullptr;
}
// Since there is no map from an InputDevice to a BluetoothDevice. We just
// comparing their vendor id and product id to guess a match.
for (auto* device : bluetooth_adapter_->GetDevices()) {
- if (!device->IsConnected())
+ if (!device->IsConnected()) {
continue;
+ }
if (device->GetVendorID() == input_device.vendor_id &&
device->GetProductID() == input_device.product_id) {
- return true;
+ return device;
}
}
- return false;
+ return nullptr;
}
} // namespace ash
diff --git a/ash/bluetooth_devices_observer.h b/ash/bluetooth_devices_observer.h
index 3a2332d0..cbbdad4 100644
--- a/ash/bluetooth_devices_observer.h
+++ b/ash/bluetooth_devices_observer.h
@@ -9,6 +9,7 @@
#include "base/functional/callback.h"
#include "base/memory/weak_ptr.h"
#include "device/bluetooth/bluetooth_adapter.h"
+#include "device/bluetooth/bluetooth_device.h"
#include "ui/events/devices/input_device.h"
namespace ash {
@@ -45,14 +46,27 @@
// Returns true if |input_device| is a connected bluetooth device. Note this
// function may not work well if there are more than one identical Bluetooth
// devices: the function might return true even if it should return false.
- // E.g., Connect two idential bluetooth devices (Input device A & input device
- // B, thus the same vendor id and same product id) to Chrome OS, and then
- // disconnect device A, calling IsConnectedBluetoothDevice(Input device A)
- // still returns true although it should return false as Input device B is
+ // E.g., Connect two identical bluetooth devices (Input device A & input
+ // device B, thus the same vendor id and same product id) to Chrome OS, and
+ // then disconnect device A, calling IsConnectedBluetoothDevice(Input device
+ // A) still returns true although it should return false as Input device B is
// still connected. Unfortunately there is no good map from an InputDevice to
// a BluetoothDevice, thus we can only guess a match.
bool IsConnectedBluetoothDevice(const ui::InputDevice& input_device) const;
+ // Returns the bluetooth device if |input_device| is a connected bluetooth
+ // device. Note this function may not work well if there are more than one
+ // identical Bluetooth devices: the function might return true even if it
+ // should return false. E.g., Connect two identical bluetooth devices (Input
+ // device A & input device B, thus the same vendor id and same product id) to
+ // Chrome OS, and then disconnect device A, calling
+ // IsConnectedBluetoothDevice(Input device A) still returns true although it
+ // should return false as Input device B is still connected. Unfortunately
+ // there is no good map from an InputDevice to a BluetoothDevice, thus we can
+ // only guess a match.
+ device::BluetoothDevice* GetConnectedBluetoothDevice(
+ const ui::InputDevice& input_device) const;
+
private:
void InitializeOnAdapterReady(
scoped_refptr<device::BluetoothAdapter> adapter);
diff --git a/ash/system/input_device_settings/input_device_notifier.cc b/ash/system/input_device_settings/input_device_notifier.cc
index b97ee47..4bc3fa2 100644
--- a/ash/system/input_device_settings/input_device_notifier.cc
+++ b/ash/system/input_device_settings/input_device_notifier.cc
@@ -4,6 +4,7 @@
#include "ash/system/input_device_settings/input_device_notifier.h"
+#include "ash/bluetooth_devices_observer.h"
#include "ash/public/cpp/input_device_settings_controller.h"
#include "ash/public/mojom/input_device_settings.mojom-forward.h"
#include "ash/public/mojom/input_device_settings.mojom.h"
@@ -12,6 +13,8 @@
#include "base/containers/fixed_flat_set.h"
#include "base/containers/flat_map.h"
#include "base/ranges/algorithm.h"
+#include "device/bluetooth/bluetooth_common.h"
+#include "device/bluetooth/bluetooth_device.h"
#include "ui/events/devices/device_data_manager.h"
#include "ui/events/devices/input_device.h"
#include "ui/events/devices/keyboard_device.h"
@@ -28,12 +31,14 @@
}
template <class DeviceMojomPtr>
-bool IsDeviceASuspectedImposter(const ui::InputDevice& device) {
+bool IsDeviceASuspectedImposter(BluetoothDevicesObserver* bluetooth_observer,
+ const ui::InputDevice& device) {
return device.suspected_imposter;
}
template <>
bool IsDeviceASuspectedImposter<mojom::KeyboardPtr>(
+ BluetoothDevicesObserver* bluetooth_observer,
const ui::InputDevice& device) {
// If the device is a keyboard that is known to pretend to have mouse-like
// functionality, do not use the `suspected_imposter` field.
@@ -41,11 +46,31 @@
return false;
}
- return device.type != ui::INPUT_DEVICE_BLUETOOTH && device.suspected_imposter;
+ // If the device is bluetooth, check the bluetooth device to see if it is a
+ // keyboard or keyboard/mouse combo.
+ if (device.type == ui::INPUT_DEVICE_BLUETOOTH) {
+ const auto* bluetooth_device =
+ bluetooth_observer->GetConnectedBluetoothDevice(device);
+ if (!bluetooth_device) {
+ return false;
+ }
+
+ if (bluetooth_device->GetDeviceType() ==
+ device::BluetoothDeviceType::KEYBOARD ||
+ bluetooth_device->GetDeviceType() ==
+ device::BluetoothDeviceType::KEYBOARD_MOUSE_COMBO) {
+ return false;
+ }
+
+ return true;
+ }
+
+ return device.suspected_imposter;
}
template <>
bool IsDeviceASuspectedImposter<mojom::MousePtr>(
+ BluetoothDevicesObserver* bluetooth_observer,
const ui::InputDevice& device) {
// If the device is a keyboard that is known to pretend to have mouse-like
// functionality, the device should always be considered an imposter.
@@ -53,7 +78,26 @@
return true;
}
- return device.type != ui::INPUT_DEVICE_BLUETOOTH && device.suspected_imposter;
+ // If the device is bluetooth, check the bluetooth device to see if it is a
+ // mouse or mouse/keyboard combo.
+ if (device.type == ui::INPUT_DEVICE_BLUETOOTH) {
+ const auto* bluetooth_device =
+ bluetooth_observer->GetConnectedBluetoothDevice(device);
+ if (!bluetooth_device) {
+ return false;
+ }
+
+ if (bluetooth_device->GetDeviceType() ==
+ device::BluetoothDeviceType::MOUSE ||
+ bluetooth_device->GetDeviceType() ==
+ device::BluetoothDeviceType::KEYBOARD_MOUSE_COMBO) {
+ return false;
+ }
+
+ return true;
+ }
+
+ return device.suspected_imposter;
}
template <typename T>
@@ -67,6 +111,7 @@
// `devices_to_remove` will be cleared before being filled with the result.
template <class DeviceMojomPtr, typename InputDeviceType>
void GetAddedAndRemovedDevices(
+ BluetoothDevicesObserver* bluetooth_observer,
std::vector<InputDeviceType> updated_device_list,
const base::flat_map<DeviceId, DeviceMojomPtr>& connected_devices,
std::vector<InputDeviceType>* devices_to_add,
@@ -80,8 +125,10 @@
// Remove any devices marked as imposters as well.
base::ranges::sort(updated_device_list, base::ranges::less(),
ExtractDeviceIdFromInputDevice);
- base::EraseIf(updated_device_list,
- IsDeviceASuspectedImposter<DeviceMojomPtr>);
+ base::EraseIf(updated_device_list, [&](const ui::InputDevice& device) {
+ return IsDeviceASuspectedImposter<DeviceMojomPtr>(bluetooth_observer,
+ device);
+ });
// Generate a vector with only the device ids from the connected_devices
// map. Guaranteed to be sorted as flat_map is always in sorted order by
@@ -112,6 +159,7 @@
/*Proj1=*/base::identity(),
/*Proj2=*/ExtractDeviceIdFromInputDevice);
}
+
} // namespace
template <typename MojomDevicePtr, typename InputDeviceType>
@@ -122,6 +170,11 @@
device_lists_updated_callback_(callback) {
DCHECK(connected_devices_);
ui::DeviceDataManager::GetInstance()->AddObserver(this);
+ bluetooth_devices_observer_ =
+ std::make_unique<BluetoothDevicesObserver>(base::BindRepeating(
+ &InputDeviceNotifier<MojomDevicePtr, InputDeviceType>::
+ OnBluetoothAdapterOrDeviceChanged,
+ base::Unretained(this)));
RefreshDevices();
}
@@ -135,7 +188,8 @@
std::vector<InputDeviceType> devices_to_add;
std::vector<DeviceId> device_ids_to_remove;
- GetAddedAndRemovedDevices(GetUpdatedDeviceList(), *connected_devices_,
+ GetAddedAndRemovedDevices(bluetooth_devices_observer_.get(),
+ GetUpdatedDeviceList(), *connected_devices_,
&devices_to_add, &device_ids_to_remove);
device_lists_updated_callback_.Run(std::move(devices_to_add),
@@ -154,6 +208,12 @@
RefreshDevices();
}
+template <typename MojomDevicePtr, typename InputDeviceType>
+void InputDeviceNotifier<MojomDevicePtr, InputDeviceType>::
+ OnBluetoothAdapterOrDeviceChanged(device::BluetoothDevice* device) {
+ RefreshDevices();
+}
+
// Template specialization for retrieving the updated device lists for each
// device type.
template <>
diff --git a/ash/system/input_device_settings/input_device_notifier.h b/ash/system/input_device_settings/input_device_notifier.h
index 7b7b7131..f1d0afd 100644
--- a/ash/system/input_device_settings/input_device_notifier.h
+++ b/ash/system/input_device_settings/input_device_notifier.h
@@ -8,6 +8,7 @@
#include <vector>
#include "ash/ash_export.h"
+#include "ash/bluetooth_devices_observer.h"
#include "ash/public/cpp/input_device_settings_controller.h"
#include "ash/public/mojom/input_device_settings.mojom-forward.h"
#include "base/containers/flat_map.h"
@@ -21,6 +22,8 @@
namespace ash {
+class BluetoothDevicesObserver;
+
// Calls the given callback every time a device is connected or removed with
// lists of which were added or removed.
template <typename MojomDevicePtr, typename InputDeviceType>
@@ -44,6 +47,10 @@
void OnInputDeviceConfigurationChanged(uint8_t input_device_type) override;
void OnDeviceListsComplete() override;
+ // Used as callback for `bluetooth_devices_observer_` whenever a bluetooth
+ // device state changes.
+ void OnBluetoothAdapterOrDeviceChanged(device::BluetoothDevice* device);
+
private:
void RefreshDevices();
@@ -54,6 +61,8 @@
// will always outlive `InputDeviceNotifier`.
raw_ptr<base::flat_map<DeviceId, MojomDevicePtr>> connected_devices_;
InputDeviceListsUpdatedCallback device_lists_updated_callback_;
+
+ std::unique_ptr<BluetoothDevicesObserver> bluetooth_devices_observer_;
};
// Below explicit template instantiations needed for all supported types.
diff --git a/ash/system/input_device_settings/input_device_notifier_unittest.cc b/ash/system/input_device_settings/input_device_notifier_unittest.cc
index 62214aef..af13ac9 100644
--- a/ash/system/input_device_settings/input_device_notifier_unittest.cc
+++ b/ash/system/input_device_settings/input_device_notifier_unittest.cc
@@ -13,6 +13,9 @@
#include "base/functional/bind.h"
#include "base/ranges/algorithm.h"
#include "base/ranges/functional.h"
+#include "device/bluetooth/bluetooth_adapter_factory.h"
+#include "device/bluetooth/bluetooth_common.h"
+#include "device/bluetooth/test/mock_bluetooth_adapter.h"
#include "ui/events/devices/device_data_manager_test_api.h"
#include "ui/events/devices/input_device.h"
#include "ui/events/devices/keyboard_device.h"
@@ -22,6 +25,10 @@
using DeviceId = InputDeviceSettingsController::DeviceId;
namespace {
+
+const char kBluetoothDeviceName[] = "Bluetooth Device";
+const char kBluetoothDevicePublicAddress[] = "01:23:45:67:89:AB";
+
const ui::KeyboardDevice kSampleKeyboardInternal = {
5, ui::INPUT_DEVICE_INTERNAL, "kSampleKeyboardInternal"};
const ui::KeyboardDevice kSampleKeyboardBluetooth = {
@@ -92,6 +99,14 @@
// testing::Test:
void SetUp() override {
+ bluetooth_adapter_ =
+ base::MakeRefCounted<testing::NiceMock<device::MockBluetoothAdapter>>();
+ device::BluetoothAdapterFactory::SetAdapterForTesting(bluetooth_adapter_);
+ ON_CALL(*bluetooth_adapter_, IsPowered)
+ .WillByDefault(testing::Return(true));
+ ON_CALL(*bluetooth_adapter_, IsPresent)
+ .WillByDefault(testing::Return(true));
+
AshTestBase::SetUp();
notifier_ = std::make_unique<
@@ -117,6 +132,8 @@
std::unique_ptr<InputDeviceNotifier<mojom::KeyboardPtr, ui::KeyboardDevice>>
notifier_;
base::flat_map<DeviceId, mojom::KeyboardPtr> keyboards_;
+ scoped_refptr<testing::NiceMock<device::MockBluetoothAdapter>>
+ bluetooth_adapter_;
std::vector<ui::KeyboardDevice> devices_to_add_;
std::vector<DeviceId> device_ids_to_remove_;
@@ -168,6 +185,71 @@
EXPECT_EQ(imposter_keyboard.id, devices_to_add_[1].id);
}
+TEST_F(InputDeviceStateNotifierTest, BluetoothKeyboardTest) {
+ uint32_t test_vendor_id = 0x1111;
+ uint32_t test_product_id = 0x1112;
+
+ std::unique_ptr<device::MockBluetoothDevice> mock_device =
+ std::make_unique<testing::NiceMock<device::MockBluetoothDevice>>(
+ bluetooth_adapter_.get(), /*bluetooth_class=*/0, kBluetoothDeviceName,
+ kBluetoothDevicePublicAddress,
+ /*initially_paired=*/true, /*connected=*/true);
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::KEYBOARD));
+ ON_CALL(*mock_device, GetVendorID)
+ .WillByDefault(testing::Return(test_vendor_id));
+ ON_CALL(*mock_device, GetProductID)
+ .WillByDefault(testing::Return(test_product_id));
+
+ std::vector<const device::BluetoothDevice*> devices;
+ devices.push_back(mock_device.get());
+ ON_CALL(*bluetooth_adapter_, GetDevices)
+ .WillByDefault(testing::Return(devices));
+
+ ui::KeyboardDevice bluetooth_keyboard = kSampleKeyboardBluetooth;
+ bluetooth_keyboard.product_id = test_product_id;
+ bluetooth_keyboard.vendor_id = test_vendor_id;
+ ui::DeviceDataManagerTestApi().SetKeyboardDevices(
+ {bluetooth_keyboard, kSampleKeyboardInternal});
+ ASSERT_EQ(2u, devices_to_add_.size());
+
+ // Keyboard + Mouse combo devices are included in the keyboard list.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(
+ testing::Return(device::BluetoothDeviceType::KEYBOARD_MOUSE_COMBO));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversKeyboardDeviceConfigurationChanged();
+ ASSERT_EQ(2u, devices_to_add_.size());
+
+ // Mice should not be included with keyboards.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::MOUSE));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversKeyboardDeviceConfigurationChanged();
+ ASSERT_EQ(1u, devices_to_add_.size());
+
+ // Gamepads should not be included with keyboards.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::GAMEPAD));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversKeyboardDeviceConfigurationChanged();
+ ASSERT_EQ(1u, devices_to_add_.size());
+
+ // Once it is a keyboard again, it should be added back to the list.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::KEYBOARD));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversKeyboardDeviceConfigurationChanged();
+ ASSERT_EQ(2u, devices_to_add_.size());
+
+ // If there are no bluetooth devices according to the bluetooth_adapter_, the
+ // notifier should just trust the device data manager.
+ devices.clear();
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversKeyboardDeviceConfigurationChanged();
+ ASSERT_EQ(2u, devices_to_add_.size());
+}
+
class InputDeviceNotifierParamaterizedTest
: public InputDeviceStateNotifierTest,
public testing::WithParamInterface<
@@ -333,6 +415,14 @@
// testing::Test:
void SetUp() override {
+ bluetooth_adapter_ =
+ base::MakeRefCounted<testing::NiceMock<device::MockBluetoothAdapter>>();
+ device::BluetoothAdapterFactory::SetAdapterForTesting(bluetooth_adapter_);
+ ON_CALL(*bluetooth_adapter_, IsPowered)
+ .WillByDefault(testing::Return(true));
+ ON_CALL(*bluetooth_adapter_, IsPresent)
+ .WillByDefault(testing::Return(true));
+
AshTestBase::SetUp();
notifier_ =
@@ -345,6 +435,7 @@
void TearDown() override {
devices_to_add_.clear();
device_ids_to_remove_.clear();
+ mice_.clear();
AshTestBase::TearDown();
}
@@ -359,6 +450,8 @@
std::unique_ptr<InputDeviceNotifier<mojom::MousePtr, ui::InputDevice>>
notifier_;
base::flat_map<DeviceId, mojom::MousePtr> mice_;
+ scoped_refptr<testing::NiceMock<device::MockBluetoothAdapter>>
+ bluetooth_adapter_;
std::vector<ui::InputDevice> devices_to_add_;
std::vector<DeviceId> device_ids_to_remove_;
@@ -377,4 +470,69 @@
EXPECT_EQ(kSampleMouseBluetooth.id, devices_to_add_[1].id);
}
+TEST_F(InputDeviceMouseNotifierTest, BluetoothMouseTest) {
+ uint32_t test_vendor_id = 0x1111;
+ uint32_t test_product_id = 0x1112;
+
+ std::unique_ptr<device::MockBluetoothDevice> mock_device =
+ std::make_unique<testing::NiceMock<device::MockBluetoothDevice>>(
+ bluetooth_adapter_.get(), /*bluetooth_class=*/0, kBluetoothDeviceName,
+ kBluetoothDevicePublicAddress,
+ /*initially_paired=*/true, /*connected=*/true);
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::MOUSE));
+ ON_CALL(*mock_device, GetVendorID)
+ .WillByDefault(testing::Return(test_vendor_id));
+ ON_CALL(*mock_device, GetProductID)
+ .WillByDefault(testing::Return(test_product_id));
+
+ std::vector<const device::BluetoothDevice*> devices;
+ devices.push_back(mock_device.get());
+ ON_CALL(*bluetooth_adapter_, GetDevices)
+ .WillByDefault(testing::Return(devices));
+
+ ui::InputDevice bluetooth_mouse = kSampleMouseBluetooth;
+ bluetooth_mouse.product_id = test_product_id;
+ bluetooth_mouse.vendor_id = test_vendor_id;
+ ui::DeviceDataManagerTestApi().SetMouseDevices(
+ {bluetooth_mouse, kSampleMouseUsb});
+ ASSERT_EQ(2u, devices_to_add_.size());
+
+ // Keyboard + Mouse combo devices are included in the mouse list.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(
+ testing::Return(device::BluetoothDeviceType::KEYBOARD_MOUSE_COMBO));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversMouseDeviceConfigurationChanged();
+ ASSERT_EQ(2u, devices_to_add_.size());
+
+ // Keyboards should not be included with mice.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::KEYBOARD));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversMouseDeviceConfigurationChanged();
+ ASSERT_EQ(1u, devices_to_add_.size());
+
+ // Gamepads should not be included with mice.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::GAMEPAD));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversMouseDeviceConfigurationChanged();
+ ASSERT_EQ(1u, devices_to_add_.size());
+
+ // Once it is a mouse again, it should be added back to the list.
+ ON_CALL(*mock_device, GetDeviceType)
+ .WillByDefault(testing::Return(device::BluetoothDeviceType::MOUSE));
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversMouseDeviceConfigurationChanged();
+ ASSERT_EQ(2u, devices_to_add_.size());
+
+ // If there are no bluetooth devices according to the bluetooth_adapter_, the
+ // notifier should just trust the device data manager.
+ devices.clear();
+ ui::DeviceDataManagerTestApi()
+ .NotifyObserversMouseDeviceConfigurationChanged();
+ ASSERT_EQ(2u, devices_to_add_.size());
+}
+
} // namespace ash