Bump Bluetooth max filter name length from 29 bytes to 240 bytes

Bump Bluetooth max filter name length from 29 bytes to 240 bytes

A device can't advertise a name longer than 29 bytes, but it can acquire a name via gap.device_name that is longer than 29 bytes, limited by the maximum EIR packet size of 240 bytes.

BUG=653718
Review-Url: https://codereview.chromium.org/2615273002
Cr-Commit-Position: refs/heads/master@{#444551}
Committed: https://chromium.googlesource.com/chromium/src/+/97239d883153487960a88a907740755e5459b2a6

[jeffcarp, 2017-01-09 18:45:19]

jeffcarp@chromium.org changed reviewers:
+ scheib@chromium.org

[jeffcarp, 2017-01-09 18:45:20]

I've done everything except getting the newly added layout test working (testing
names 29<x<240 bytes). Publishing now to get feedback.

https://codereview.chromium.org/2615273002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):

https://codereview.chromium.org/2615273002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:83: constexpr
size_t kMaxLengthForDeviceName = 240;
In my head it made more sense to limit it to 240 bytes (not the 512 that was
discussed in the issue) since there's no way the name could be longer than 240.
Let me know if you prefer 512.

https://codereview.chromium.org/2615273002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py
(right):

https://codereview.chromium.org/2615273002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py:37:
'a_device_name_that_is_longer_than_29_bytes_but_shorter_than_240_bytes',
I'm still trying to figure out how to create the correct mock for the new layout
test below.

[scheib, 2017-01-09 19:26:50]

Thanks!

https://codereview.chromium.org/2615273002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):

https://codereview.chromium.org/2615273002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:83: constexpr
size_t kMaxLengthForDeviceName = 240;
On 2017/01/09 18:45:20, jeffcarp wrote:
> In my head it made more sense to limit it to 240 bytes (not the 512 that was
> discussed in the issue) since there's no way the name could be longer than
240.
> Let me know if you prefer 512.

I think this is OK too, practically I doubt devices will be hitting this limit.
Please cite the spec sections and concepts that brought us to this number,
though.

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

https://codereview.chromium.org/2615273002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:31: const size_t
kMaxFilterNameLength = 240;
Description here is good, please add spec sections that we used to look this up,
though.

https://codereview.chromium.org/2615273002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py
(right):

https://codereview.chromium.org/2615273002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py:37:
'a_device_name_that_is_longer_than_29_bytes_but_shorter_than_240_bytes',
On 2017/01/09 18:45:20, jeffcarp wrote:
> I'm still trying to figure out how to create the correct mock for the new
layout
> test below.

They are created in
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h. Copy
paste one , such as UnicodeDeviceAdapter.

[jeffcarp, 2017-01-11 01:47:27]

https://codereview.chromium.org/2615273002/diff/60001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):

https://codereview.chromium.org/2615273002/diff/60001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:85: // to the
max EIR packet size of 240 bytes. See Core Spec 5.0, vol 3, C, 8.1.2.
Added spec citation here and copied over the comment from
Bluetooth.kMaxFilterNameLength.

https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/device-name-longer-than-29-bytes.html
(right):

https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/device-name-longer-than-29-bytes.html:15:
</script>
Got the new layout test working. lmk if this works (also is canonicalizeFilter
the right place for this?)

https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (right):

https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:31: // See Core Spec
5.0, vol 3, C, 8.1.2.
Added spec citation here.

https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py
(right):

https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py:37:
'a_device_name_that_is_longer_than_29_bytes_but_shorter_than_240_bytes',
Not sure if edits in this file are still necessary. Just a remnant from trying
to get the device mock to work.

[scheib, 2017-01-11 03:04:06]

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

[commit-bot: I haz the power, 2017-01-11 03:05:15]

Dry run: 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-01-11 04:01:09]

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

[commit-bot: I haz the power, 2017-01-11 04:01:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jeffcarp, 2017-01-12 00:08:49]

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

[commit-bot: I haz the power, 2017-01-12 00:09:42]

Dry run: 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-01-12 01:39:29]

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

[commit-bot: I haz the power, 2017-01-12 01:39:31]

Dry run: This issue passed the CQ dry run.

[jeffcarp, 2017-01-18 19:37:31]

On 2017/01/11 at 01:47:27, jeffcarp wrote:
>
https://codereview.chromium.org/2615273002/diff/60001/content/browser/bluetoo...
> File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):
> 
>
https://codereview.chromium.org/2615273002/diff/60001/content/browser/bluetoo...
> content/browser/bluetooth/bluetooth_device_chooser_controller.cc:85: // to the
max EIR packet size of 240 bytes. See Core Spec 5.0, vol 3, C, 8.1.2.
> Added spec citation here and copied over the comment from
Bluetooth.kMaxFilterNameLength.
> 
>
https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/device-name-longer-than-29-bytes.html
(right):
> 
>
https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/device-name-longer-than-29-bytes.html:15:
</script>
> Got the new layout test working. lmk if this works (also is canonicalizeFilter
the right place for this?)
> 
>
https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (right):
> 
>
https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:31: // See Core Spec
5.0, vol 3, C, 8.1.2.
> Added spec citation here.
> 
>
https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py
(right):
> 
>
https://codereview.chromium.org/2615273002/diff/60001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py:37:
'a_device_name_that_is_longer_than_29_bytes_but_shorter_than_240_bytes',
> Not sure if edits in this file are still necessary. Just a remnant from trying
to get the device mock to work.

Tests are passing now, this should be ready for a final review.

[scheib, 2017-01-18 21:31:43]

Rutabaga LGTM

[jeffcarp, 2017-01-18 22:09:45]

The CQ bit was checked by jeffcarp@chromium.org

[commit-bot: I haz the power, 2017-01-18 22:10:20]

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-01-18 23:54:19]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484777385224310,
"parent_rev": "f7734aba4cc737cdb271dd31867baad2efb83fbb", "commit_rev":
"97239d883153487960a88a907740755e5459b2a6"}

[commit-bot: I haz the power, 2017-01-18 23:54:55]

Description was changed from

==========
Bump Bluetooth max filter name length from 29 bytes to 240 bytes

A device can't advertise a name longer than 29 bytes, but it can acquire a name
via gap.device_name that is longer than 29 bytes, limited by the maximum EIR
packet size of 240 bytes.

BUG=653718
==========

to

==========
Bump Bluetooth max filter name length from 29 bytes to 240 bytes

A device can't advertise a name longer than 29 bytes, but it can acquire a name
via gap.device_name that is longer than 29 bytes, limited by the maximum EIR
packet size of 240 bytes.

BUG=653718

Review-Url: https://codereview.chromium.org/2615273002
Cr-Commit-Position: refs/heads/master@{#444551}
Committed:
https://chromium.googlesource.com/chromium/src/+/97239d883153487960a88a907740...
==========

[commit-bot: I haz the power, 2017-01-18 23:54:56]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/97239d883153487960a88a907740...