bluetooth: Implement hardware filtering on macOS

bluetooth: Implement hardware filtering on macOS

Passes the UUIDs in the filters to mac's function to scan for device.
This allows for a more efficient scan of devices.

BUG=638704

[ortuno, 2016-08-19 20:54:18]

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

[commit-bot: I haz the power, 2016-08-19 20:55:14]

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-08-19 21:14:49]

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

[commit-bot: I haz the power, 2016-08-19 21:14:50]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ortuno, 2016-08-23 01:21:44]

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

[commit-bot: I haz the power, 2016-08-23 01:22:22]

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-08-23 02:56:48]

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

[commit-bot: I haz the power, 2016-08-23 02:56:49]

Dry run: This issue passed the CQ dry run.

[ortuno, 2016-08-23 02:59:43]

ortuno@chromium.org changed reviewers:
+ jyasskin@chromium.org

[ortuno, 2016-08-23 02:59:44]

jyasskin: PTAL

[Jeffrey Yasskin, 2016-08-23 21:37:04]

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_mac.mm (right):

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_mac.mm:300: BluetoothDiscoveryFilter*
discovery_filter,
If we're guaranteed this is never null, can this become a const
BluetoothDiscoveryFilter& for all adapters?

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_mac.mm:360: BluetoothDiscoveryFilter*
discovery_filter) {
Similarly here, change the interface's signature if the argument can't be null.

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right):

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_mac_unittest.mm:39: const char*
kTestStringUUIDHeartRate = "0000180d-0000-1000-8000-00805f9b34fb";
Use either "const char* const kTestString" or "const char kTestString[]" to make
the constant actually constant.

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_mac_unittest.mm:371: // Remove one discovery
sesssion. We should not have called stop
sp: sesssion

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_mac_unittest.mm:371: // Remove one discovery
sesssion. We should not have called stop
Is "have called" the right tense here? It looks like the removal hasn't happened
yet.

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_low_energy_discovery_manager_mac.h (right):

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_low_energy_discovery_manager_mac.h:48: // no other
filter is scanning for those uuids.
We don't have the concept of a filter in this class's interface, so you should
describe the condition some other way. Maybe say that StartDiscovery() adds the
UUIDs to a multiset of active UUIDs,and RemoveDiscoveryUUIDs() removes them, and
discovery will be active for any UUID in that multiset?

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_low_energy_discovery_manager_mac.h:91: // List of
service UUIDs to scan for to the number of filters that have that
s/list/map/?

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_low_energy_discovery_manager_mac.mm (right):

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_low_energy_discovery_manager_mac.mm:26:
std::set<BluetoothUUID> services_uuids) {
Since this doesn't modify the input argument, it should probably take a const
std::set<BluetoothUUID>& instead of by-value. Also, a vector<> or
unordered_set<> would be more efficient: do they make sense as the argument
type?

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_low_energy_discovery_manager_mac.mm:40: if
(should_update_scan || !discovering_) {
If we're not discovering, but there are no services to scan for, shouldn't we
keep not discovering?

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_low_energy_discovery_manager_mac.mm:110:
TryStartDiscovery();
Shouldn't RemoveDiscoveryUUIDs stop discovery when no UUIDs are left?

[ortuno, 2016-08-25 17:58:31]

I'm closing this for now. I realized that more refactoring is needed and
whatever refactor we do with mac will be eventually replaced by the cross
platform implementation.

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_mac.mm (right):

https://codereview.chromium.org/2253223002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_mac.mm:300: BluetoothDiscoveryFilter*
discovery_filter,
On 2016/08/23 at 21:37:04, Jeffrey Yasskin wrote:
> If we're guaranteed this is never null, can this become a const
BluetoothDiscoveryFilter& for all adapters?

Ah my bad. BluetoothDiscoveryFilter is not guaranteed to be there because of
StartDiscoverySession. (Which just calls
StartDiscoverySessionWithFilter(nullptr...).)