Bluetooth: Make maximum possible name filter 248 bytes, not 240

third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp

 // Copyright 2015 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.
 
 #include "modules/bluetooth/Bluetooth.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
 #include "core/frame/LocalFrame.h"
 #include "modules/bluetooth/BluetoothDevice.h"
 #include "modules/bluetooth/BluetoothError.h"
 #include "modules/bluetooth/BluetoothRemoteGATTCharacteristic.h"
 #include "modules/bluetooth/BluetoothUUID.h"
 #include "modules/bluetooth/RequestDeviceOptions.h"
 #include "platform/UserGestureIndicator.h"
 #include "public/platform/InterfaceProvider.h"
 #include <memory>
 #include <utility>
 
 namespace blink {
 
 namespace {
-// A name coming from an adv packet is max 29 bytes (adv packet max size
-// 31 bytes - 2 byte length field), but the name can also be acquired via
-// gap.device_name, so it is limited to the max EIR packet size of 240 bytes.
-// See Core Spec 5.0, vol 3, C, 8.1.2.
-const size_t kMaxFilterNameLength = 240;
-const char kFilterNameTooLong[] =
-    "A 'name' or 'namePrefix' longer than 240 bytes results in no devices "
-    "being found, because a device can't acquire a name longer than 240 bytes.";
 // Per the Bluetooth Spec: The name is a user-friendly name associated with the
 // device and consists of a maximum of 248 bytes coded according to the UTF-8
 // standard.
 const size_t kMaxDeviceNameLength = 248;
 const char kDeviceNameTooLong[] =
     "A device name can't be longer than 248 bytes.";
 }  // namespace
 
 static void canonicalizeFilter(
     const BluetoothScanFilterInit& filter,
@@ skipping 20 matching lines @@
       canonicalizedFilter->services->push_back(validatedService);
     }
   }
 
   if (filter.hasName()) {
     size_t nameLength = filter.name().utf8().length();
     if (nameLength > kMaxDeviceNameLength) {
       exceptionState.throwTypeError(kDeviceNameTooLong);
       return;
     }
-    if (nameLength > kMaxFilterNameLength) {
-      exceptionState.throwDOMException(NotFoundError, kFilterNameTooLong);
-      return;
-    }

[jeffcarp, 2017/01/21 00:14:05]

Because of this change and the one below, the layout tests are now failing with
'expected "NotFoundError" but got "TypeError"'. What is the correct behavior
here now that the filter length checking and name length checking are the same
thing? I'll see if there's a spec that defines this.

[jeffcarp, 2017/01/21 00:20:34]

The WebBluetooth spec doesn't seem to explicitly mention using NotFoundError
when the filter name is too long. It does say to throw a TypeError if the name
is more than 248 bytes here:
https://webbluetoothcg.github.io/web-bluetooth/#bluetoothlescanfilterinit-can...

[Jeffrey Yasskin, 2017/01/23 17:16:14]

Yep, as discussed in https://github.com/WebBluetoothCG/web-bluetooth/issues/296,
the UA can return a NotFoundError eagerly if it can prove that no device would
show up in the chooser, and so the user would have to cancel and cause a
NotFoundError regardless. You found the right reference for returning a
TypeError for >248-byte names.

I would expect this change to cause 'expected "NotFoundError" but succeeded'
failures, but not to replace NotFoundErrors with TypeErrors, since all the
TypeErrors are caused by the if above.

[scheib, 2017/01/23 18:31:47]

Thanks Jeffrey. So Jeff, please just update these error blocks to use the
DOMException(NotFoundError ...

"""
if (filter.hasName()) {
  size_t nameLength = filter.name().utf8().length();
  if (nameLength > kMaxDeviceNameLength) {
    exceptionState.throwDOMException(NotFoundError, kFilterNameTooLong);	
    return;	
  }
"""

[scheib, 2017/01/23 18:31:47]

Switching the expectations to TypeError looks correct to me.

[Jeffrey Yasskin, 2017/01/23 18:55:21]

I think TypeError is correct when nameLength > kMaxDeviceNameLength, and that
NotFoundError would be wrong.

[scheib, 2017/01/23 20:10:25]

Jeffrey, I'm left confused by what you think is best then. Maybe mostly from
your 2017/01/23 17:16:14 comment.

From your last comment I expect us to no longer have any name length related
NotFoundErrors, and that we'll be changing tests over to TypeErrors.

If that's not accurate, would you more clearly state for Jeff which error types
you expect?

     canonicalizedFilter->name = filter.name();
   }
 
   if (filter.hasNamePrefix()) {
     size_t namePrefixLength = filter.namePrefix().utf8().length();
     if (namePrefixLength > kMaxDeviceNameLength) {
       exceptionState.throwTypeError(kDeviceNameTooLong);
       return;
     }
-    if (namePrefixLength > kMaxFilterNameLength) {
-      exceptionState.throwDOMException(NotFoundError, kFilterNameTooLong);
-      return;
-    }
     if (filter.namePrefix().length() == 0) {
       exceptionState.throwTypeError(
           "'namePrefix', if present, must me non-empty.");
       return;
     }
     canonicalizedFilter->name_prefix = filter.namePrefix();
   }
 }
 
 static void convertRequestDeviceOptions(
@@ skipping 191 matching lines @@
   BluetoothDevice* device = m_deviceInstanceMap.get(id);
   if (!device) {
     device = BluetoothDevice::take(resolver, std::move(devicePtr), this);
     auto result = m_deviceInstanceMap.add(id, device);
     DCHECK(result.isNewEntry);
   }
   return device;
 }
 
 }  // namespace blink