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

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

Re: discussion in https://crbug.com/653718

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org

Review-Url: https://codereview.chromium.org/2642123003
Cr-Commit-Position: refs/heads/master@{#447395}
Committed: https://chromium.googlesource.com/chromium/src/+/715c254e7e980472c07a29b2a0efdbf2ef8f469e

[jeffcarp, 2017-01-20 00:27:10]

Description was changed from

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

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org
==========

to

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

Re: discussion in https://crbug.com/653718

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org
==========

[scheib, 2017-01-20 17:54:24]

Thanks, LGTM with small fix:

https://codereview.chromium.org/2642123003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html
(right):

https://codereview.chromium.org/2642123003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html:15:
'acquire a name longer than 249 bytes.',
this line's 249 and the two below in test should be 248.

[jeffcarp, 2017-01-21 00:11:41]

https://codereview.chromium.org/2642123003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html
(right):

https://codereview.chromium.org/2642123003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html:15:
'acquire a name longer than 249 bytes.',
On 2017/01/20 at 17:54:24, scheib wrote:
> this line's 249 and the two below in test should be 248.

Thanks for catching that, fixed

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

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
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.

[Jeffrey Yasskin, 2017-01-21 00:18:41]

Description was changed from

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

Re: discussion in https://crbug.com/653718

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org
==========

to

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

Re: discussion in https://crbug.com/653718

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org
==========

[Jeffrey Yasskin, 2017-01-21 00:20:14]

Attila, FYI.

[jeffcarp, 2017-01-21 00:20:35]

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
On 2017/01/21 at 00:14:05, jeffcarp wrote:
> 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.

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]

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
On 2017/01/21 00:20:34, jeffcarp wrote:
> On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > 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.
> 
> 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...

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:22:14]

The CQ bit was checked by scheib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 18:22:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scheib, 2017-01-23 18:31:48]

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
On 2017/01/21 00:20:34, jeffcarp wrote:
> On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > 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.
> 
> 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...

Switching the expectations to TypeError looks correct to me.

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
On 2017/01/23 17:16:14, Jeffrey Yasskin wrote:
> On 2017/01/21 00:20:34, jeffcarp wrote:
> > On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > > 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.
> > 
> > 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...
> 
> 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.

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;	
  }
"""

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

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
On 2017/01/23 18:31:47, scheib wrote:
> On 2017/01/23 17:16:14, Jeffrey Yasskin wrote:
> > On 2017/01/21 00:20:34, jeffcarp wrote:
> > > On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > > > 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.
> > > 
> > > 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...
> > 
> > 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.
> 
> 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;	
>   }
> """

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

[commit-bot: I haz the power, 2017-01-23 20:00:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 20:00:18]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

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

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):

https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
On 2017/01/23 18:55:21, Jeffrey Yasskin wrote:
> On 2017/01/23 18:31:47, scheib wrote:
> > On 2017/01/23 17:16:14, Jeffrey Yasskin wrote:
> > > On 2017/01/21 00:20:34, jeffcarp wrote:
> > > > On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > > > > 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.
> > > > 
> > > > 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...
> > > 
> > > 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.
> > 
> > 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;	
> >   }
> > """
> 
> I think TypeError is correct when nameLength > kMaxDeviceNameLength, and that
> NotFoundError would be wrong.

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?

[Jeffrey Yasskin, 2017-01-25 22:09:16]

On 2017/01/23 20:10:25, scheib wrote:
>
https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):
> 
>
https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
> On 2017/01/23 18:55:21, Jeffrey Yasskin wrote:
> > On 2017/01/23 18:31:47, scheib wrote:
> > > On 2017/01/23 17:16:14, Jeffrey Yasskin wrote:
> > > > On 2017/01/21 00:20:34, jeffcarp wrote:
> > > > > On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > > > > > 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.
> > > > > 
> > > > > 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...
> > > > 
> > > > 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.
> > > 
> > > 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;	
> > >   }
> > > """
> > 
> > I think TypeError is correct when nameLength > kMaxDeviceNameLength, and
that
> > NotFoundError would be wrong.
> 
> 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?

Sorry, yes, I meant that if none of the tests' lengths changed then the names
with lengths between 29 and 248 would change from a NotFoundError to success,
and lengths greater than 248 would stay TypeError. But Vince told me you've also
updated the lengths, so it sounds right for some of the NotFoundErrors to turn
into TypeError.

[jeffcarp, 2017-01-27 05:37:46]

On 2017/01/25 at 22:09:16, jyasskin wrote:
> On 2017/01/23 20:10:25, scheib wrote:
> >
https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):
> > 
> >
https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
> > On 2017/01/23 18:55:21, Jeffrey Yasskin wrote:
> > > On 2017/01/23 18:31:47, scheib wrote:
> > > > On 2017/01/23 17:16:14, Jeffrey Yasskin wrote:
> > > > > On 2017/01/21 00:20:34, jeffcarp wrote:
> > > > > > On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > > > > > > 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.
> > > > > > 
> > > > > > 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...
> > > > > 
> > > > > 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.
> > > > 
> > > > 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;	
> > > >   }
> > > > """
> > > 
> > > I think TypeError is correct when nameLength > kMaxDeviceNameLength, and
that
> > > NotFoundError would be wrong.
> > 
> > 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?
> 
> Sorry, yes, I meant that if none of the tests' lengths changed then the names
with lengths between 29 and 248 would change from a NotFoundError to success,
and lengths greater than 248 would stay TypeError. But Vince told me you've also
updated the lengths, so it sounds right for some of the NotFoundErrors to turn
into TypeError.

Updated the tests to expect TypeErrors (this also changes all their expected
messages to 'A device name can't be longer than 248 bytes.')

[jeffcarp, 2017-01-31 01:51:21]

On 2017/01/27 at 05:37:46, jeffcarp wrote:
> On 2017/01/25 at 22:09:16, jyasskin wrote:
> > On 2017/01/23 20:10:25, scheib wrote:
> > >
https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (left):
> > > 
> > >
https://codereview.chromium.org/2642123003/diff/20001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:79: }
> > > On 2017/01/23 18:55:21, Jeffrey Yasskin wrote:
> > > > On 2017/01/23 18:31:47, scheib wrote:
> > > > > On 2017/01/23 17:16:14, Jeffrey Yasskin wrote:
> > > > > > On 2017/01/21 00:20:34, jeffcarp wrote:
> > > > > > > On 2017/01/21 at 00:14:05, jeffcarp wrote:
> > > > > > > > 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.
> > > > > > > 
> > > > > > > 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...
> > > > > > 
> > > > > > 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.
> > > > > 
> > > > > 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;	
> > > > >   }
> > > > > """
> > > > 
> > > > I think TypeError is correct when nameLength > kMaxDeviceNameLength, and
that
> > > > NotFoundError would be wrong.
> > > 
> > > 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?
> > 
> > Sorry, yes, I meant that if none of the tests' lengths changed then the
names with lengths between 29 and 248 would change from a NotFoundError to
success, and lengths greater than 248 would stay TypeError. But Vince told me
you've also updated the lengths, so it sounds right for some of the
NotFoundErrors to turn into TypeError.
> 
> Updated the tests to expect TypeErrors (this also changes all their expected
messages to 'A device name can't be longer than 248 bytes.')

Does this still look OK?

[scheib, 2017-01-31 02:37:43]

LGTM

[jeffcarp, 2017-01-31 22:10:01]

The CQ bit was checked by jeffcarp@chromium.org

[commit-bot: I haz the power, 2017-01-31 22:10:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 01:13:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485900601812450,
"parent_rev": "bd13a9f7838046525e55350a7e491e014235aea1", "commit_rev":
"715c254e7e980472c07a29b2a0efdbf2ef8f469e"}

[commit-bot: I haz the power, 2017-02-01 01:14:30]

Description was changed from

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

Re: discussion in https://crbug.com/653718

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org
==========

to

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

Re: discussion in https://crbug.com/653718

BUG=653718
R=scheib@chromium.org,jyasskin@chromium.org

Review-Url: https://codereview.chromium.org/2642123003
Cr-Commit-Position: refs/heads/master@{#447395}
Committed:
https://chromium.googlesource.com/chromium/src/+/715c254e7e980472c07a29b2a0ef...
==========

[commit-bot: I haz the power, 2017-02-01 01:14:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/715c254e7e980472c07a29b2a0ef...