bluetooth: Implement requestDevice by name or name prefix

content/browser/bluetooth/bluetooth_dispatcher_host.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // ID Not In Map Note:
 // A service, characteristic, or descriptor ID not in the corresponding
 // BluetoothDispatcherHost map [service_to_device_, characteristic_to_service_,
 // descriptor_to_characteristic_] implies a hostile renderer because a renderer
 // obtains the corresponding ID from this class and it will be added to the map
 // at that time.
@@ skipping 29 matching lines @@
 
 namespace {
 
 // TODO(ortuno): Once we have a chooser for scanning, a way to control that
 // chooser from tests, and the right callback for discovered services we should
 // delete these constants.
 // https://crbug.com/436280 and https://crbug.com/484504
 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;
     }
   }
   return false;
 }
 
 WebBluetoothError TranslateConnectError(
     device::BluetoothDevice::ConnectErrorCode error_code) {
   switch (error_code) {
     case device::BluetoothDevice::ERROR_UNKNOWN:
@@ skipping 355 matching lines @@
     int request_id,
     int frame_routing_id,
     const std::vector<BluetoothScanFilter>& filters,
     const std::vector<BluetoothUUID>& optional_services) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::REQUEST_DEVICE);
   RecordRequestDeviceArguments(filters, optional_services);
 
   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();
     VLOG(1) << "\t]";
   }
 
   VLOG(1) << "requestDevice called with the following optional services: ";
   for (const BluetoothUUID& service : optional_services)
     VLOG(1) << "\t" << service.value();
 
@@ skipping 20 matching lines @@
 
   if (!adapter_->IsPresent()) {
     VLOG(1) << "Bluetooth Adapter not present. Can't serve requestDevice.";
     RecordRequestDeviceOutcome(
         UMARequestDeviceOutcome::BLUETOOTH_ADAPTER_NOT_PRESENT);
     Send(new BluetoothMsg_RequestDeviceError(
         thread_id, request_id, WebBluetoothError::NoBluetoothAdapter));
     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(
       thread_id, request_id, filters, optional_services);
   int chooser_id = request_device_sessions_.Add(session);
 
   BluetoothChooser::EventHandler chooser_event_handler =
       base::Bind(&BluetoothDispatcherHost::OnBluetoothChooserEvent,
                  weak_ptr_on_ui_thread_, chooser_id);
   if (WebContents* web_contents =
@@ skipping 640 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   NOTIMPLEMENTED();
 }
 
 void BluetoothDispatcherHost::ShowBluetoothAdapterOffLink() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   NOTIMPLEMENTED();
 }
 
 }  // namespace content