bluetooth: Web Bluetooth can filter by empty device names, doesn't leak MACs.

content/browser/bluetooth/bluetooth_device_chooser_controller.cc

Index: content/browser/bluetooth/bluetooth_device_chooser_controller.cc
diff --git a/content/browser/bluetooth/bluetooth_device_chooser_controller.cc b/content/browser/bluetooth/bluetooth_device_chooser_controller.cc
index 4214b01ae96ff992ac2640434940729eac1dc934..390fca9833779132b0e864c29d459117fe9c5309 100644
--- a/content/browser/bluetooth/bluetooth_device_chooser_controller.cc
+++ b/content/browser/bluetooth/bluetooth_device_chooser_controller.cc
@@ -69,10 +69,11 @@ bool IsEmptyOrInvalidFilter(
 
   // The renderer will never send a name or a name_prefix longer than
   // kMaxLengthForDeviceName.
-  if (!filter->name.is_null() && filter->name.size() > kMaxLengthForDeviceName)
+  if (filter->name.size() > kMaxLengthForDeviceName)

[ortuno, 2016/07/31 18:00:05]

I don't think we should remove the is_null. Once we move to
base::Optional<std::string> this will become filter->name->size() and will
DCHECK when there is no name.

[scheib, 2016/08/02 03:24:33]

Done.

     return true;
-  if (!filter->name_prefix.is_null() &&
-      filter->name_prefix.size() > kMaxLengthForDeviceName)
+  if (!filter->name_prefix.is_null() && filter->name_prefix.size() == 0)
+    return true;
+  if (filter->name_prefix.size() > kMaxLengthForDeviceName)

[ortuno, 2016/07/31 18:00:05]

Same here. I don't think we should remove the check of is_null.

[scheib, 2016/08/02 03:24:33]

Done.

     return true;
 
   return false;
@@ -88,19 +89,21 @@ bool HasEmptyOrInvalidFilter(
 
 bool MatchesFilter(const device::BluetoothDevice& device,
                    const blink::mojom::WebBluetoothScanFilterPtr& filter) {
-  DCHECK(!IsEmptyOrInvalidFilter(filter));
-
-  // TODO(615720): Use the upcoming GetName (was GetDeviceName).
-  const std::string device_name = base::UTF16ToUTF8(device.GetNameForDisplay());
+  CHECK(!IsEmptyOrInvalidFilter(filter));

[ortuno, 2016/07/31 18:00:05]

Not sure if we actually need this CHECK. We already crash the renderer on
invalid filters.

[scheib, 2016/08/02 03:24:33]

Ok, removing the DCHECK here. MatchesFilters calls it, as does GetDevice().

 
-  if (!filter->name.is_null() && (device_name != filter->name)) {
-    return false;
+  if (!filter->name.is_null()) {
+    if (!device.GetName())
+      return false;
+    if (filter->name != device.GetName().value())
+      return false;
   }
 
-  if (!filter->name_prefix.is_null() &&
-      (!base::StartsWith(device_name, filter->name_prefix.get(),
-                         base::CompareCase::SENSITIVE))) {
-    return false;
+  if (filter->name_prefix.size()) {

[ortuno, 2016/07/31 18:00:05]

Same here. I don't think we should change the is_null check.

[scheib, 2016/08/02 03:24:33]

Done.

+    if (!device.GetName())
+      return false;
+    if (!base::StartsWith(device.GetName().value(), filter->name_prefix.get(),
+                          base::CompareCase::SENSITIVE))
+      return false;
   }
 
   if (!filter->services.is_null()) {