bluetooth: Implement acceptAllDevices

bluetooth: Implement acceptAllDevices

See https://webbluetoothcg.github.io/web-bluetooth/#dom-requestdeviceoptions-acceptalldevices

BUG=654918

[fbeaufortchromium, 2016-10-26 14:52:21]

Patchset #2 (id:20001) has been deleted

[fbeaufortchromium, 2016-10-26 14:52:32]

Patchset #2 (id:40001) has been deleted

[fbeaufortchromium, 2016-10-26 14:52:40]

Patchset #2 (id:60001) has been deleted

[fbeaufortchromium, 2016-10-26 14:55:38]

Description was changed from

==========
bluetooth: Implement acceptAllDevices

BUG=654918
==========

to

==========
bluetooth: Implement acceptAllDevices

TEST=https://beaufortfrancois.github.io/sandbox/web-bluetooth/accept-all-devices

BUG=654918
==========

[François Beaufort, 2016-10-27 10:13:29]

Patchset #5 (id:140001) has been deleted

[François Beaufort, 2016-10-27 10:13:45]

Patchset #1 (id:1) has been deleted

[François Beaufort, 2016-10-27 10:14:01]

Patchset #1 (id:80001) has been deleted

[François Beaufort, 2016-10-27 10:14:12]

Patchset #1 (id:100001) has been deleted

[François Beaufort, 2016-10-27 10:14:28]

Patchset #1 (id:120001) has been deleted

[François Beaufort, 2016-10-27 11:37:15]

Patchset #1 (id:160001) has been deleted

[fbeaufortchromium, 2016-10-27 13:31:38]

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

[commit-bot: I haz the power, 2016-10-27 13:31:58]

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, 2016-10-27 13:40:52]

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

[commit-bot: I haz the power, 2016-10-27 13:40:53]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[François Beaufort, 2016-10-27 15:46:42]

beaufort.francois@gmail.com changed reviewers:
+ ortuno@chromium.org
- fbeaufort@chromium.org

[François Beaufort, 2016-10-27 15:46:43]

Hello Gio,

Can you have a look at this CL?

[ortuno, 2016-10-31 00:00:03]

Please remove the TEST line. That should only be used when manual testing is
required and if used it should contain specific instructions about how to test
the patch e.g. http://crrev.com/1858293002

https://codereview.chromium.org/2449813002/diff/180001/content/browser/blueto...
File content/browser/bluetooth/bluetooth_device_chooser_controller.cc (right):

https://codereview.chromium.org/2449813002/diff/180001/content/browser/blueto...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:95: bool
HasEmptyOrInvalidFilter(
Add the same check as the one on the renderer side:

if (filters.is_null()) {
  return false;
}

if (!filters.is_null() && filters.empty()) {
  return true;
}
return filters.end() != std::find_if(...);

https://codereview.chromium.org/2449813002/diff/180001/content/browser/blueto...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:247: if
(!options->accept_all_devices &&
You can remove the accept_all_devices check. See comment for
HasEmptyOrInvalidFilter also fix the comment.

https://codereview.chromium.org/2449813002/diff/180001/content/browser/blueto...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:257: if
(!options_->accept_all_devices &&
!options_->filters.is_null()

https://codereview.chromium.org/2449813002/diff/180001/content/browser/blueto...
content/browser/bluetooth/bluetooth_device_chooser_controller.cc:359: if
(chooser_.get() && (options_->accept_all_devices ||
options_->filters.is_null() || ...

https://codereview.chromium.org/2449813002/diff/180001/content/renderer/bluet...
File content/renderer/bluetooth/bluetooth_type_converters.cc (right):

https://codereview.chromium.org/2449813002/diff/180001/content/renderer/bluet...
content/renderer/bluetooth/bluetooth_type_converters.cc:40: options->filters =
mojo::Array<blink::mojom::WebBluetoothScanFilterPtr>::From(
if (web_options.hasFilters) {
  options->filters = mojo::Array<...>::From(...);
}

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/acceptAllDevices/blacklisted-service-in-filter.html
(right):

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/acceptAllDevices/blacklisted-service-in-filter.html:11:
filters: [{services: ['human_interface_device']}]}));
Also add a test for the fifth case in
https://webbluetoothcg.github.io/web-bluetooth/#example-disallowed-filters

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filters-member.html
(right):

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filters-member.html:14:
</script>
There are more tests in this directory that will need to include the
acceptAllDevices option e.g.
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/bluetooth....
Please take a close look at all the tests in requestDevice/ and modify the
relevant ones to include this option.

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp (right):

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp:105:
result.acceptAllDevices = options.acceptAllDevices();
In order to debug the spec and make the implementation easier to follow, I like
to follow the spec as closely as possible. For this reason could you implement
the logic as is in Step 1 of
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice and
make sure each of the paths is properly tested?

if (options.hasFilters() && options.acceptAllDevices()) {
  // reject with TypeError.
  return;
}

if (!options.hasFilters() && !options.acceptAllDevices()) {
  // reject with TypeError.
  return;
}

if (options.hasFilters() && options.filters.isEmpty()) {
  // reject with TypeError.
  return;
}

result.hasFilters = options.hasFilters();
if (options.hasFilters()) {
  Vector<WebBluetoothScanFilter> filters;
  // ...
}

...

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/pub...
File
third_party/WebKit/public/platform/modules/bluetooth/WebRequestDeviceOptions.h
(right):

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/pub...
third_party/WebKit/public/platform/modules/bluetooth/WebRequestDeviceOptions.h:34:
bool acceptAllDevices;
See comment in web_bluetooth.mojom. Move this before filters and call it
hasFilters.

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom
(right):

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/pub...
third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom:80:
array<WebBluetoothScanFilter> filters;
Make this optional.

https://codereview.chromium.org/2449813002/diff/180001/third_party/WebKit/pub...
third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom:82:
bool accept_all_devices;
I don't think we need this one. We want this part of the implementation to match
https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices which
just uses options.filters.

[François Beaufort, 2016-11-02 15:44:42]

Description was changed from

==========
bluetooth: Implement acceptAllDevices

TEST=https://beaufortfrancois.github.io/sandbox/web-bluetooth/accept-all-devices

BUG=654918
==========

to

==========
bluetooth: Implement acceptAllDevices

See
https://webbluetoothcg.github.io/web-bluetooth/#dom-requestdeviceoptions-acce...

BUG=654918
==========