Skip to content

Commit

Permalink
[CrOS Bluetooth] Do not filter out Poly devices from pairing UI
Browse files Browse the repository at this point in the history
This fixes an issue where Poly speakers are not shown in the pairing UI.

(cherry picked from commit 419ea9a)

Bug: b/228118615
Test: device_unittests, plus on-device verification with Poly device
Change-Id: Ic2ec9ad62ab57648d7dcb59caa5eeae767071565
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3612430
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Gordon Seto <gordonseto@google.com>
Auto-Submit: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1002931}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3652543
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#57}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Kyle Horimoto authored and Chromium LUCI CQ committed May 17, 2022
1 parent 5390ada commit cd283d5
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 2 deletions.
3 changes: 3 additions & 0 deletions device/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ test("device_unittests") {
"bluetooth/chromeos/bluetooth_utils_unittest.cc",
]
deps += [ "//chromeos/constants" ]
if (is_chromeos_ash) {
deps += [ "//ash/constants" ]
}
if (is_chromeos_lacros) {
deps += [ "//chromeos/lacros:test_support" ]
}
Expand Down
7 changes: 6 additions & 1 deletion device/bluetooth/bluetooth_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ bool BluetoothDevice::IsPairable() const {
BluetoothDeviceType type = GetDeviceType();

// Get the vendor part of the address: "00:11:22" for "00:11:22:33:44:55"
std::string vendor = GetAddress().substr(0, 8);
std::string vendor = GetOuiPortionOfBluetoothAddress();

// Verbatim "Bluetooth Mouse", model 96674
if (type == BluetoothDeviceType::MOUSE && vendor == "00:12:A1")
Expand Down Expand Up @@ -459,6 +459,11 @@ std::string BluetoothDevice::GetIdentifier() const {
return GetAddress();
}

std::string BluetoothDevice::GetOuiPortionOfBluetoothAddress() const {
// Get the vendor part of the address: "00:11:22" for "00:11:22:33:44:55".
return GetAddress().substr(0, 8);
}

void BluetoothDevice::UpdateAdvertisementData(
int8_t rssi,
absl::optional<uint8_t> flags,
Expand Down
4 changes: 4 additions & 0 deletions device/bluetooth/bluetooth_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDevice {
// a unique key to identify the device and copied where needed.
virtual std::string GetAddress() const = 0;

// Returns the OUI portion of the Bluetooth address, which refers to the
// device's vendor.
std::string GetOuiPortionOfBluetoothAddress() const;

// Returns the Bluetooth address type of the device. Currently available on
// Linux and Chrome OS.
virtual AddressType GetAddressType() const = 0;
Expand Down
26 changes: 25 additions & 1 deletion device/bluetooth/chromeos/bluetooth_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include "device/bluetooth/chromeos/bluetooth_utils.h"

#include "ash/constants/ash_features.h"
#include "base/containers/contains.h"
#include "base/containers/fixed_flat_set.h"
#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_functions.h"
Expand All @@ -21,6 +21,7 @@
#include "third_party/abseil-cpp/absl/types/optional.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_switches.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

Expand Down Expand Up @@ -150,6 +151,23 @@ std::string GetTransportName(BluetoothTransport transport) {
}
}

bool IsPolyDevicePairingAllowed() {
#if BUILDFLAG(IS_CHROMEOS_ASH)
return ash::features::IsPolyDevicePairingAllowed();
#else
return false;
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
}

bool IsPolyDevice(const device::BluetoothDevice* device) {
// OUI portions of Bluetooth addresses for devices manufactured by Poly. See
// https://standards-oui.ieee.org/.
constexpr auto kPolyOuis = base::MakeFixedFlatSet<base::StringPiece>(
{"64:16:7F", "48:25:67", "00:04:F2"});

return base::Contains(kPolyOuis, device->GetOuiPortionOfBluetoothAddress());
}

} // namespace

device::BluetoothAdapter::DeviceList FilterBluetoothDeviceList(
Expand Down Expand Up @@ -182,6 +200,12 @@ bool IsUnsupportedDevice(const device::BluetoothDevice* device) {
}
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)

// Never filter out Poly devices; this requires a special case since these
// devices often identify themselves as phones, which are disallowed below.
// See b/228118615.
if (IsPolyDevicePairingAllowed() && IsPolyDevice(device))
return false;

// Always filter out laptops, etc. There is no intended use case or
// Bluetooth profile in this context.
if (device->GetDeviceType() == BluetoothDeviceType::COMPUTER)
Expand Down
49 changes: 49 additions & 0 deletions device/bluetooth/chromeos/bluetooth_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/command_line.h"
#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "build/chromeos_buildflags.h"
#include "device/bluetooth/bluetooth_adapter_factory.h"
Expand All @@ -17,6 +18,10 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/constants/ash_features.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chromeos/lacros/lacros_test_helper.h"
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
Expand All @@ -30,6 +35,14 @@ constexpr char kTestBluetoothDeviceAddress[] = "01:02:03:04:05:06";
constexpr char kHIDServiceUUID[] = "1812";
constexpr char kSecurityKeyServiceUUID[] = "FFFD";
constexpr char kUnexpectedServiceUUID[] = "1234";

#if BUILDFLAG(IS_CHROMEOS_ASH)
// Note: The first 3 hex bytes represent the OUI portion of the address, which
// indicates the device vendor. In this case, "64:16:7F:**:**:**" represents a
// device manufactured by Poly.
constexpr char kFakePolyDeviceAddress[] = "64:16:7F:12:34:56";
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

const size_t kMaxDevicesForFilter = 5;

} // namespace
Expand Down Expand Up @@ -63,6 +76,19 @@ class BluetoothUtilsTest : public testing::Test {
return mock_bluetooth_device_ptr;
}

#if BUILDFLAG(IS_CHROMEOS_ASH)
void AddMockPolyDeviceToAdapter() {
MockBluetoothDevice* mock_bluetooth_device =
AddMockBluetoothDeviceToAdapter(BLUETOOTH_TRANSPORT_CLASSIC);
ON_CALL(*mock_bluetooth_device, GetName)
.WillByDefault(testing::Return(absl::nullopt));
ON_CALL(*mock_bluetooth_device, GetDeviceType)
.WillByDefault(testing::Return(BluetoothDeviceType::UNKNOWN));
ON_CALL(*mock_bluetooth_device, GetAddress)
.WillByDefault(testing::Return(kFakePolyDeviceAddress));
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

MockBluetoothAdapter* adapter() { return adapter_.get(); }

MockBluetoothDevice* GetMockBluetoothDevice(size_t index) {
Expand Down Expand Up @@ -147,6 +173,29 @@ TEST_F(BluetoothUtilsTest,
1u /* num_expected_remaining_devices */);
}

#if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(BluetoothUtilsTest, ShowPolyDevice_PolyFlagDisabled) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
ash::features::kAllowPolyDevicePairing);

AddMockPolyDeviceToAdapter();
VerifyFilterBluetoothDeviceList(BluetoothFilterType::KNOWN,
0u /* num_expected_remaining_devices */);
}

// Regression test for b/228118615.
TEST_F(BluetoothUtilsTest, ShowPolyDevice_PolyFlagEnabled) {
base::test::ScopedFeatureList scoped_feature_list{
ash::features::kAllowPolyDevicePairing};

// Poly devices should not be filtered out, regardless of device type.
AddMockPolyDeviceToAdapter();
VerifyFilterBluetoothDeviceList(BluetoothFilterType::KNOWN,
1u /* num_expected_remaining_devices */);
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

TEST_F(
BluetoothUtilsTest,
TestFilterBluetoothDeviceList_FilterKnown_RemoveClassicDevicesWithoutNames) {
Expand Down

0 comments on commit cd283d5

Please sign in to comment.