bluetooth: Implement requestDevice by name or name prefix

content/browser/bluetooth/bluetooth_dispatcher_host.cc

Index: content/browser/bluetooth/bluetooth_dispatcher_host.cc
diff --git a/content/browser/bluetooth/bluetooth_dispatcher_host.cc b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
index d10d56e1266d48b1f242041cffe9db05d790b299..b176917dd12a203417c028f68481575d0c149aa8 100644
--- a/content/browser/bluetooth/bluetooth_dispatcher_host.cc
+++ b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
@@ -47,27 +47,63 @@ namespace {
 const int kDelayTime = 5;         // 5 seconds for scanning and discovering
 const int kTestingDelayTime = 0;  // No need to wait during tests
 
+bool IsEmptyFilter(const content::BluetoothScanFilter& filter) {
+  return filter.name.empty() && filter.namePrefix.empty() &&
+         filter.services.empty();
+}
+
+bool HasEmptyFilter(const std::vector<content::BluetoothScanFilter>& filters) {
+  if (filters.empty()) {
+    return true;
+  }
+  for (const content::BluetoothScanFilter& filter : filters) {
+    if (IsEmptyFilter(filter)) {
+      return true;
+    }
+  }
+  return false;

[palmer, 2015/10/27 21:43:18]

Nit: Maybe this is cleaner/more clear:

return filters.empty() ? true
                       : filters.end() != std::find_if(filters.begin(),
                                                       filters.end(),
                                                       IsEmptyFilter);

[ortuno, 2015/10/28 17:53:25]

Done.

+}
+
 // Defined at
 // https://webbluetoothchrome.github.io/web-bluetooth/#dfn-matches-a-filter
-bool MatchesFilter(const std::set<BluetoothUUID>& device_uuids,
+bool MatchesFilter(const device::BluetoothDevice& device,
                    const content::BluetoothScanFilter& filter) {
-  if (filter.services.empty())
-    return false;
-  for (const BluetoothUUID& service : filter.services) {
-    if (!ContainsKey(device_uuids, service)) {
+  DCHECK(!IsEmptyFilter(filter));
+
+  const std::string device_name = base::UTF16ToUTF8(device.GetName());
+
+  if (!filter.name.empty()) {

[palmer, 2015/10/27 21:43:18]

Nit: I think you can combine these 2 conditions into 1, right?

if (!filter.name.empty() && device_name != filter_name)
  return false;

Same thing with lines 81 and 82 below.

[ortuno, 2015/10/28 17:53:25]

Done.

+    if (device_name != filter.name) {
       return false;
     }
   }
+
+  if (!filter.namePrefix.empty()) {
+    if (!base::StartsWith(device_name, filter.namePrefix,
+                          base::CompareCase::SENSITIVE)) {
+      return false;
+    }
+  }
+
+  if (!filter.services.empty()) {
+    const std::vector<BluetoothUUID>& device_uuid_list = device.GetUUIDs();

[palmer, 2015/10/27 21:43:18]

This whole block is a good use for auto:

const auto& device_uuid_list = device.GetUUIDs();
et c. ...

Also I think you can shorten the code by using a reutnr-if-empty,
otherwise-return-find_if strategy like above in the function |HasEmptyFilter|.

[ortuno, 2015/10/28 17:53:25]

Done.
I don't think we can use that pattern here since we need to check that every
service in the filter is in the device. I think converting the service uuids
into a set and then checking of all services are in that set is also a fairly
efficient way of doing this.

+    const std::set<BluetoothUUID> device_uuids(device_uuid_list.begin(),
+                                               device_uuid_list.end());
+    for (const BluetoothUUID& service : filter.services) {
+      if (!ContainsKey(device_uuids, service)) {
+        return false;
+      }
+    }
+  }
+
   return true;
 }
 
 bool MatchesFilters(const device::BluetoothDevice& device,
                     const std::vector<content::BluetoothScanFilter>& filters) {
-  const std::vector<BluetoothUUID>& device_uuid_list = device.GetUUIDs();
-  const std::set<BluetoothUUID> device_uuids(device_uuid_list.begin(),
-                                             device_uuid_list.end());
+  DCHECK(!HasEmptyFilter(filters));
   for (const content::BluetoothScanFilter& filter : filters) {
-    if (MatchesFilter(device_uuids, filter)) {
+    if (MatchesFilter(device, filter)) {
       return true;
     }
   }
@@ -443,6 +479,9 @@ void BluetoothDispatcherHost::OnRequestDevice(
 
   VLOG(1) << "requestDevice called with the following filters: ";
   for (const BluetoothScanFilter& filter : filters) {
+    VLOG(1) << "Name: " << filter.name;

[palmer, 2015/10/27 21:43:18]

Are name and namePrefix ever going to be used for anything else?

[palmer, 2015/10/27 21:46:56]

Also, do they have any length, lexical, or syntactic constraints that it makes
sense to enforce?

[ortuno, 2015/10/28 17:53:25]

We don't have any plans to use it beyond what's implemented here i.e. filtering.

On 2015/10/27 at 21:46:56, palmer wrote:
Good point. Device names are UTF-8 strings with a max length of 248 bytes; we
should enforce that. I've raised the issue in the Web Bluetooth Spec [1].

[1]
https://github.com/WebBluetoothCG/web-bluetooth/pull/174#discussion_r43203535

[Jeffrey Yasskin, 2015/10/30 22:04:44]

We can enforce a maximum of 248 UTF-8 code units in the spec (since I want to
allow UAs to connect to devices to decide if filters match), but in current
Chrome, we can auto-reject the promise if it goes over the maximum amount of
name that can be sent in an advertising packet: 29 bytes unless my math is
wrong.

[ortuno, 2015/10/30 22:36:02]

Yup. It's 29 if you don't count the flags which are almost always there.

This sounds good to me. On the browser side we can make sure the name(Prefix)
that we get from the renderer is never greater than 26 and if so crash the
renderer.

palmer: does this sound good to you?

+    VLOG(1) << "Name Prefix: " << filter.namePrefix;
+    VLOG(1) << "Services:";
     VLOG(1) << "\t[";
     for (const BluetoothUUID& service : filter.services)
       VLOG(1) << "\t\t" << service.value();
@@ -483,6 +522,12 @@ void BluetoothDispatcherHost::OnRequestDevice(
     return;
   }
 
+  // The renderer should never send empty filters.
+  if (HasEmptyFilter(filters)) {
+    bad_message::ReceivedBadMessage(this, bad_message::BDH_EMPTY_FILTERS);
+    return;
+  }
+
   // Create storage for the information that backs the chooser, and show the
   // chooser.
   RequestDeviceSession* const session = new RequestDeviceSession(