bluetooth: Consider filters that cannot be mapped to OS

We used to only consider filters if they had something to filter.
However, this means that if there is a filter with a name prefix we do
not consider it as Android does not have a name prefix filter. Since
the filters are unioned together, missing a filter could cause results
we are looking for to be filtered out. To ensure this does not happen
we are adding an open filter to replace the name prefix filter instead
of disregarding it entirely. Therefore we never filter out a result
which should pass through the name prefix filter.

Bug: 1121697
Change-Id: Ib6f4ff52978c62e0a5f16056524459f1116576a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2389340
Commit-Queue: James Hollyer <jameshollyer@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815902}

content/browser/bluetooth/bluetooth_device_chooser_controller.cc

@@ -157,38 +157,6 @@ bool MatchesFilters(
   return false;
 }
 
-std::unique_ptr<device::BluetoothDiscoveryFilter> ComputeScanFilter(
-    const base::Optional<
-        std::vector<blink::mojom::WebBluetoothLeScanFilterPtr>>& filters) {
-  // There isn't much support for GATT over BR/EDR from neither platforms nor
-  // devices so performing a Dual scan will find devices that the API is not
-  // able to interact with. To avoid wasting power and confusing users with
-  // devices they are not able to interact with, we only perform an LE Scan.
-  auto discovery_filter = std::make_unique<device::BluetoothDiscoveryFilter>(
-      device::BLUETOOTH_TRANSPORT_LE);
-
-  if (filters) {
-    for (const auto& filter : filters.value()) {
-      device::BluetoothDiscoveryFilter::DeviceInfoFilter device_filter;
-      bool useful_filter = false;
-      if (filter->services) {
-        device_filter.uuids =
-            base::flat_set<device::BluetoothUUID>(filter->services.value());
-        useful_filter = true;
-      }
-      if (filter->name) {
-        device_filter.name = filter->name.value();
-        useful_filter = true;
-      }
-      if (useful_filter) {
-        discovery_filter->AddDeviceFilter(device_filter);
-      }
-    }
-  }
-
-  return discovery_filter;
-}
-
 void StopDiscoverySession(
     std::unique_ptr<device::BluetoothDiscoverySession> discovery_session) {
   // Nothing goes wrong if the discovery session fails to stop, and we don't
@@ -568,4 +536,45 @@ void BluetoothDeviceChooserController::PostErrorCallback(
   }
 }
 
+// static
+std::unique_ptr<device::BluetoothDiscoveryFilter>
+BluetoothDeviceChooserController::ComputeScanFilter(
+    const base::Optional<
+        std::vector<blink::mojom::WebBluetoothLeScanFilterPtr>>& filters) {
+  // There isn't much support for GATT over BR/EDR from neither platforms nor
+  // devices so performing a Dual scan will find devices that the API is not
+  // able to interact with. To avoid wasting power and confusing users with
+  // devices they are not able to interact with, we only perform an LE Scan.
+  auto discovery_filter = std::make_unique<device::BluetoothDiscoveryFilter>(
+      device::BLUETOOTH_TRANSPORT_LE);
+
+  if (filters) {
+    for (const auto& filter : filters.value()) {
+      device::BluetoothDiscoveryFilter::DeviceInfoFilter device_filter;
+      // Keep track of whether this filter can be converted accurately.
+      bool has_supported_fields = false;
+      if (filter->services) {
+        device_filter.uuids =
+            base::flat_set<device::BluetoothUUID>(filter->services.value());
+        has_supported_fields = true;
+      }
+      if (filter->name) {
+        device_filter.name = filter->name.value();
+        has_supported_fields = true;
+      }
+
+      // If we don't have any supported fields in this filter then we cannot
+      // filter any devices as we don't want to filter out devices which would
+      // have passed these unsupported filter criteria.
+      if (!has_supported_fields) {
+        discovery_filter->ClearDeviceFilters();
+        return discovery_filter;
+      }
+      discovery_filter->AddDeviceFilter(device_filter);
+    }
+  }
+
+  return discovery_filter;
+}
+
 }  // namespace content

content/browser/bluetooth/bluetooth_device_chooser_controller.h

@@ -20,6 +20,7 @@ namespace device {
 class BluetoothAdapter;
 class BluetoothDevice;
 class BluetoothDiscoverySession;
+class BluetoothDiscoveryFilter;
 }
 
 namespace content {
@@ -94,6 +95,10 @@ class CONTENT_EXPORT BluetoothDeviceChooserController final {
       TestScanDurationSetting setting =
           TestScanDurationSetting::IMMEDIATE_TIMEOUT);
 
+  static std::unique_ptr<device::BluetoothDiscoveryFilter> ComputeScanFilter(
+      const base::Optional<
+          std::vector<blink::mojom::WebBluetoothLeScanFilterPtr>>& filters);
+
  private:
   // Populates the chooser with the GATT connected devices.
   void PopulateConnectedDevices();

content/browser/bluetooth/bluetooth_device_chooser_controller_unittest.cc

@@ -3,8 +3,13 @@
 // found in the LICENSE file.
 
 #include "content/browser/bluetooth/bluetooth_device_chooser_controller.h"
+#include "device/bluetooth/bluetooth_discovery_filter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+constexpr char kHeartRateUUIDString[] = "0000180d-0000-1000-8000-00805f9b34fb";
+constexpr char kBatteryServiceUUIDString[] =
+    "0000180f-0000-1000-8000-00805f9b34fb";
+
 namespace content {
 
 class BluetoothDeviceChooserControllerTest : public testing::Test {};
@@ -36,4 +41,129 @@ TEST_F(BluetoothDeviceChooserControllerTest, CalculateSignalStrengthLevel) {
       4, BluetoothDeviceChooserController::CalculateSignalStrengthLevel(127));
 }
 
+TEST_F(BluetoothDeviceChooserControllerTest, ComputeScanFilterTest) {
+  // No supported OS level filters have a name prefix filter. Since filters are
+  // unioned we must not filter any devices if we have a name prefix filter.
+  {
+    auto filter = blink::mojom::WebBluetoothLeScanFilter::New();
+    filter->name_prefix = "test name prefix";
+
+    std::vector<blink::mojom::WebBluetoothLeScanFilterPtr> scan_filters;
+    scan_filters.emplace_back(std::move(filter));
+
+    std::unique_ptr<device::BluetoothDiscoveryFilter> resulting_filter =
+        BluetoothDeviceChooserController::ComputeScanFilter(
+            std::move(scan_filters));
+
+    const base::flat_set<device::BluetoothDiscoveryFilter::DeviceInfoFilter>*
+        device_info_filters = resulting_filter->GetDeviceFilters();
+
+    EXPECT_TRUE(device_info_filters->empty());
+  }
+  // Ensures ComputeScanFilter adds all UUIDs to the BluetoothDiscoveryFilter
+  // that it creates
+  {
+    std::vector<device::BluetoothUUID> services;
+    services.emplace_back(device::BluetoothUUID(kHeartRateUUIDString));
+    services.emplace_back(device::BluetoothUUID(kBatteryServiceUUIDString));
+
+    auto filter = blink::mojom::WebBluetoothLeScanFilter::New();
+    filter->services = std::move(services);
+
+    std::vector<blink::mojom::WebBluetoothLeScanFilterPtr> scan_filters;
+    scan_filters.emplace_back(std::move(filter));
+
+    std::unique_ptr<device::BluetoothDiscoveryFilter> resulting_filter =
+        BluetoothDeviceChooserController::ComputeScanFilter(
+            std::move(scan_filters));
+
+    const base::flat_set<device::BluetoothDiscoveryFilter::DeviceInfoFilter>*
+        device_info_filters = resulting_filter->GetDeviceFilters();
+
+    EXPECT_TRUE(device_info_filters->begin()->uuids.contains(
+        device::BluetoothUUID(kHeartRateUUIDString)));
+    EXPECT_TRUE(device_info_filters->begin()->uuids.contains(
+        device::BluetoothUUID(kBatteryServiceUUIDString)));
+    EXPECT_TRUE(device_info_filters->begin()->name.empty());
+  }
+  // Ensures ComputeScanFilter adds the correct name to the
+  // BluetoothDiscoveryFilter that it creates
+  {
+    auto filter = blink::mojom::WebBluetoothLeScanFilter::New();
+    filter->name = "test name";
+
+    std::vector<blink::mojom::WebBluetoothLeScanFilterPtr> scan_filters;
+    scan_filters.emplace_back(std::move(filter));
+
+    std::unique_ptr<device::BluetoothDiscoveryFilter> resulting_filter =
+        BluetoothDeviceChooserController::ComputeScanFilter(
+            std::move(scan_filters));
+
+    const base::flat_set<device::BluetoothDiscoveryFilter::DeviceInfoFilter>*
+        device_info_filters = resulting_filter->GetDeviceFilters();
+
+    EXPECT_TRUE(device_info_filters->begin()->uuids.empty());
+    EXPECT_EQ(device_info_filters->begin()->name, "test name");
+  }
+  // Ensures ComputeScanFilter adds both the name and the UUIDs to the
+  // BluetoothDiscoveryFilter that it creates
+  {
+    std::vector<device::BluetoothUUID> services;
+    services.emplace_back(device::BluetoothUUID(kHeartRateUUIDString));
+    services.emplace_back(device::BluetoothUUID(kBatteryServiceUUIDString));
+
+    auto filter = blink::mojom::WebBluetoothLeScanFilter::New();
+    filter->services = std::move(services);
+    filter->name = "test name";
+
+    std::vector<blink::mojom::WebBluetoothLeScanFilterPtr> scan_filters;
+    scan_filters.emplace_back(std::move(filter));
+
+    std::unique_ptr<device::BluetoothDiscoveryFilter> resulting_filter =
+        BluetoothDeviceChooserController::ComputeScanFilter(
+            std::move(scan_filters));
+
+    const base::flat_set<device::BluetoothDiscoveryFilter::DeviceInfoFilter>*
+        device_info_filters = resulting_filter->GetDeviceFilters();
+
+    EXPECT_TRUE(device_info_filters->begin()->uuids.contains(
+        device::BluetoothUUID(kHeartRateUUIDString)));
+    EXPECT_TRUE(device_info_filters->begin()->uuids.contains(
+        device::BluetoothUUID(kBatteryServiceUUIDString)));
+    EXPECT_EQ(device_info_filters->begin()->name, "test name");
+  }
+  // Ensures we will not filter any devices if we have at least one
+  // name_prefix only filter even with other filters present.
+  {
+    std::vector<device::BluetoothUUID> services;
+    services.emplace_back(device::BluetoothUUID(kHeartRateUUIDString));
+    std::vector<device::BluetoothUUID> services2;
+    services2.emplace_back(device::BluetoothUUID(kBatteryServiceUUIDString));
+
+    auto filter = blink::mojom::WebBluetoothLeScanFilter::New();
+    filter->services = std::move(services);
+    filter->name = "test name";
+
+    auto prefix_filter = blink::mojom::WebBluetoothLeScanFilter::New();
+    prefix_filter->name_prefix = "test name prefix";
+
+    auto filter2 = blink::mojom::WebBluetoothLeScanFilter::New();
+    filter2->services = std::move(services2);
+    filter2->name = "test name2";
+
+    std::vector<blink::mojom::WebBluetoothLeScanFilterPtr> scan_filters;
+    scan_filters.emplace_back(std::move(filter));
+    scan_filters.emplace_back(std::move(prefix_filter));
+
+    std::unique_ptr<device::BluetoothDiscoveryFilter> resulting_filter =
+        BluetoothDeviceChooserController::ComputeScanFilter(
+            std::move(scan_filters));
+
+    const base::flat_set<device::BluetoothDiscoveryFilter::DeviceInfoFilter>*
+        device_info_filters = resulting_filter->GetDeviceFilters();
+
+    EXPECT_TRUE(device_info_filters->empty());
+  }
+}
+
 }  // namespace content
\ No newline at end of file

device/bluetooth/bluetooth_discovery_filter.cc

@@ -95,6 +95,10 @@ BluetoothDiscoveryFilter::GetDeviceFilters() const {
   return &device_filters_;
 }
 
+void BluetoothDiscoveryFilter::ClearDeviceFilters() {
+  device_filters_.clear();
+}
+
 void BluetoothDiscoveryFilter::CopyFrom(
     const BluetoothDiscoveryFilter& filter) {
   transport_ = filter.transport_;

device/bluetooth/bluetooth_discovery_filter.h

@@ -93,6 +93,8 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDiscoveryFilter {
   // Returns true if all fields in filter are empty
   bool IsDefault() const;
 
+  void ClearDeviceFilters();
+
   // Returns result of merging two filters together. If at least one of the
   // filters is NULL this will return an empty filter
   static std::unique_ptr<device::BluetoothDiscoveryFilter> Merge(