Auto merge of #13184 - szeged:requestdevice-refactor, r=jdm

Webbluetooth requestDevice refactor and update

<!-- Please describe your changes on the following line: -->
Refactor requestDevice function according to the specification changes.
1. Moved the `request_bluetooth_devices` algorithm out from the `requestDevice` function.
2. Two new members in `BluetoothRequestDeviceFilter` and one new member in `RequestDeviceOptions`.
3. Also added annotations to the related functions.
Related links:
https://webbluetoothcg.github.io/web-bluetooth/#device-discovery,
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice, https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices,
https://webbluetoothcg.github.io/web-bluetooth/#matches-a-filter

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #12614

<!-- Either: -->
- [x] These changes do not require tests because , there is no WebBluetooth Test API implementation yet.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13184)
<!-- Reviewable:end -->

components/net/bluetooth_thread.rs

@@ -84,17 +84,20 @@ impl BluetoothThreadFactory for IpcSender<BluetoothMethodMsg> {
     }
 }
 
+// https://webbluetoothcg.github.io/web-bluetooth/#matches-a-filter
 fn matches_filter(device: &BluetoothDevice, filter: &BluetoothScanfilter) -> bool {
     if filter.is_empty_or_invalid() {
         return false;
     }
 
+    // Step 1.
     if !filter.get_name().is_empty() {
         if device.get_name().ok() != Some(filter.get_name().to_string()) {
             return false;
         }
     }
 
+    // Step 2.
     if !filter.get_name_prefix().is_empty() {
         if let Ok(device_name) = device.get_name() {
             if !device_name.starts_with(filter.get_name_prefix()) {
@@ -105,6 +108,7 @@ fn matches_filter(device: &BluetoothDevice, filter: &BluetoothScanfilter) -> boo
         }
     }
 
+    // Step 3.
     if !filter.get_services().is_empty() {
         if let Ok(device_uuids) = device.get_uuids() {
             for service in filter.get_services() {
@@ -114,7 +118,25 @@ fn matches_filter(device: &BluetoothDevice, filter: &BluetoothScanfilter) -> boo
             }
         }
     }
-    return true;
+
+// Step 4.
+// TODO: Implement get_manufacturer_data in device crate.
+//    if let Some(manufacturer_id) = filter.get_manufacturer_id() {
+//        if !device.get_manufacturer_data().contains_key(manufacturer_id) {
+//            return false;
+//        }
+//    }
+//
+// Step 5.
+// TODO: Implement get_device_data in device crate.
+//    if !filter.get_service_data_uuid().is_empty() {
+//        if !device.get_service_data().contains_key(filter.get_service_data_uuid()) {
+//            return false;
+//        }
+//    }
+
+    // Step 6.
+    true
 }
 
 fn matches_filters(device: &BluetoothDevice, filters: &BluetoothScanfilterSequence) -> bool {
@@ -427,6 +449,7 @@ impl BluetoothManager {
 
     // Methods
 
+    // https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices
     fn request_device(&mut self,
                       options: RequestDeviceoptions,
                       sender: IpcSender<BluetoothResult<BluetoothDeviceMsg>>) {
@@ -437,10 +460,19 @@ impl BluetoothManager {
             }
             let _ = session.stop_discovery();
         }
-        let devices = self.get_and_cache_devices(&mut adapter);
+
-        let matched_devices: Vec<BluetoothDevice> = devices.into_iter()
+        // Step 6.
-                                                           .filter(|d| matches_filters(d, options.get_filters()))
+        // Note: There is no requiredServiceUUIDS, we scan for all devices.
-                                                           .collect();
+        let mut matched_devices = self.get_and_cache_devices(&mut adapter);
+
+        // Step 7.
+        if !options.is_accepting_all_devices() {
+            matched_devices = matched_devices.into_iter()
+                                             .filter(|d| matches_filters(d, options.get_filters()))
+                                             .collect();
+        }
+
+        // Step 8.
         if let Some(address) = self.select_device(matched_devices) {
             let device_id = match self.address_to_id.get(&address) {
                 Some(id) => id.clone(),

components/net_traits/bluetooth_scanfilter.rs

@@ -28,14 +28,23 @@ pub struct BluetoothScanfilter {
     name: String,
     name_prefix: String,
     services: ServiceUUIDSequence,
+    manufacturer_id: Option<u16>,
+    service_data_uuid: String,
 }
 
 impl BluetoothScanfilter {
-    pub fn new(name: String, name_prefix: String, services: Vec<String>) -> BluetoothScanfilter {
+    pub fn new(name: String,
+               name_prefix: String,
+               services: Vec<String>,
+               manufacturer_id: Option<u16>,
+               service_data_uuid: String)
+               -> BluetoothScanfilter {
         BluetoothScanfilter {
             name: name,
             name_prefix: name_prefix,
             services: ServiceUUIDSequence::new(services),
+            manufacturer_id: manufacturer_id,
+            service_data_uuid: service_data_uuid,
         }
     }
 
@@ -51,8 +60,20 @@ impl BluetoothScanfilter {
         &self.services.0
     }
 
+    pub fn get_manufacturer_id(&self) -> Option<u16> {
+        self.manufacturer_id
+    }
+
+    pub fn get_service_data_uuid(&self) -> &str {
+        &self.service_data_uuid
+    }
+
     pub fn is_empty_or_invalid(&self) -> bool {
-        (self.name.is_empty() && self.name_prefix.is_empty() && self.get_services().is_empty()) ||
+        (self.name.is_empty() &&
+         self.name_prefix.is_empty() &&
+         self.get_services().is_empty() &&
+         self.manufacturer_id.is_none() &&
+         self.service_data_uuid.is_empty()) ||
         self.name.len() > MAX_NAME_LENGTH ||
         self.name_prefix.len() > MAX_NAME_LENGTH
     }
@@ -67,7 +88,6 @@ impl BluetoothScanfilterSequence {
     }
 
     pub fn has_empty_or_invalid_filter(&self) -> bool {
-        self.0.is_empty() ||
         self.0.iter().any(BluetoothScanfilter::is_empty_or_invalid)
     }
 
@@ -78,6 +98,10 @@ impl BluetoothScanfilterSequence {
     fn get_services_set(&self) -> HashSet<String> {
         self.iter().flat_map(|filter| filter.services.get_services_set()).collect()
     }
+
+    fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
 }
 
 #[derive(Deserialize, Serialize)]
@@ -103,4 +127,8 @@ impl RequestDeviceoptions {
     pub fn get_services_set(&self) -> HashSet<String> {
         &self.filters.get_services_set() | &self.optional_services.get_services_set()
     }
+
+    pub fn is_accepting_all_devices(&self) -> bool {
+        self.filters.is_empty()
+    }
 }

components/script/dom/bluetooth.rs

@@ -4,8 +4,7 @@
 
 use bluetooth_blacklist::{Blacklist, uuid_is_blacklisted};
 use core::clone::Clone;
-use dom::bindings::codegen::Bindings::BluetoothBinding;
+use dom::bindings::codegen::Bindings::BluetoothBinding::{self, BluetoothMethods, BluetoothRequestDeviceFilter};
-use dom::bindings::codegen::Bindings::BluetoothBinding::{BluetoothMethods, BluetoothScanFilter};
 use dom::bindings::codegen::Bindings::BluetoothBinding::RequestDeviceOptions;
 use dom::bindings::error::Error::{self, Security, Type};
 use dom::bindings::error::Fallible;
@@ -15,13 +14,13 @@ use dom::bindings::reflector::{Reflectable, Reflector, reflect_dom_object};
 use dom::bindings::str::DOMString;
 use dom::bluetoothadvertisingdata::BluetoothAdvertisingData;
 use dom::bluetoothdevice::BluetoothDevice;
-use dom::bluetoothuuid::BluetoothUUID;
+use dom::bluetoothuuid::{BluetoothServiceUUID, BluetoothUUID};
 use ipc_channel::ipc::{self, IpcSender};
 use net_traits::bluetooth_scanfilter::{BluetoothScanfilter, BluetoothScanfilterSequence};
 use net_traits::bluetooth_scanfilter::{RequestDeviceoptions, ServiceUUIDSequence};
 use net_traits::bluetooth_thread::{BluetoothError, BluetoothMethodMsg};
 
-const FILTER_EMPTY_ERROR: &'static str = "'filters' member must be non - empty to find any devices.";
+const FILTER_EMPTY_ERROR: &'static str = "'filters' member, if present, must be nonempty to find any devices.";
 const FILTER_ERROR: &'static str = "A filter must restrict the devices in some way.";
 const FILTER_NAME_TOO_LONG_ERROR: &'static str = "A 'name' or 'namePrefix' can't be longer then 29 bytes.";
 // 248 is the maximum number of UTF-8 code units in a Bluetooth Device Name.
@@ -31,9 +30,11 @@ const MAX_DEVICE_NAME_LENGTH: usize = 248;
 // The length and identifier of the length field take 2 bytes.
 // That leaves 29 bytes for the name.
 const MAX_FILTER_NAME_LENGTH: usize = 29;
-const NAME_PREFIX_ERROR: &'static str = "'namePrefix', if present, must be non - empty.";
+const NAME_PREFIX_ERROR: &'static str = "'namePrefix', if present, must be nonempty.";
 const NAME_TOO_LONG_ERROR: &'static str = "A device name can't be longer than 248 bytes.";
 const SERVICE_ERROR: &'static str = "'services', if present, must contain at least one service.";
+const OPTIONS_ERROR: &'static str = "Fields of 'options' conflict with each other.
+ Either 'acceptAllDevices' member must be true, or 'filters' member must be set to a value.";
 
 // https://webbluetoothcg.github.io/web-bluetooth/#bluetooth
 #[dom_struct]
@@ -59,80 +60,196 @@ impl Bluetooth {
         let global_ref = global_root.r();
         global_ref.as_window().bluetooth_thread()
     }
+
+    // https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices
+    fn request_bluetooth_devices(&self,
+                                 filters: &Option<Vec<BluetoothRequestDeviceFilter>>,
+                                 optional_services: &Option<Vec<BluetoothServiceUUID>>)
+                                 -> Fallible<Root<BluetoothDevice>> {
+        // TODO: Step 1: Triggered by user activation.
+
+        // Step 2.
+        let option = try!(convert_request_device_options(self.global().r(), filters, optional_services));
+
+        // TODO: Step 3-5: Implement the permission API.
+
+        // Note: Steps 6-8 are implemented in
+        // components/net/bluetooth_thread.rs in request_device function.
+        let (sender, receiver) = ipc::channel().unwrap();
+        self.get_bluetooth_thread().send(BluetoothMethodMsg::RequestDevice(option, sender)).unwrap();
+        let device = receiver.recv().unwrap();
+
+        // TODO: Step 9-10: Implement the permission API.
+
+        // Step 11: This step is optional.
+
+        // Step 12-13.
+        match device {
+            Ok(device) => {
+                let ad_data = BluetoothAdvertisingData::new(self.global().r(),
+                                                            device.appearance,
+                                                            device.tx_power,
+                                                            device.rssi);
+                Ok(BluetoothDevice::new(self.global().r(),
+                                        DOMString::from(device.id),
+                                        device.name.map(DOMString::from),
+                                        &ad_data))
+            },
+            Err(error) => {
+                Err(Error::from(error))
+            },
+        }
+
+    }
 }
 
-fn canonicalize_filter(filter: &BluetoothScanFilter, global: GlobalRef) -> Fallible<BluetoothScanfilter> {
+// https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices
-    if filter.services.is_none() && filter.name.is_none() && filter.namePrefix.is_none() {
+fn convert_request_device_options(global: GlobalRef,
-        return Err(Type(FILTER_ERROR.to_owned()));
+                                  filters: &Option<Vec<BluetoothRequestDeviceFilter>>,
+                                  optional_services: &Option<Vec<BluetoothServiceUUID>>)
+                                  -> Fallible<RequestDeviceoptions> {
+    // Step 2.2: There is no requiredServiceUUIDS, we scan for all devices.
+    let mut uuid_filters = vec!();
+
+    if let &Some(ref filters) = filters {
+        // Step 2.1.
+        if filters.is_empty()  {
+            return Err(Type(FILTER_EMPTY_ERROR.to_owned()));
+        }
+
+        // Step 2.3: There is no requiredServiceUUIDS, we scan for all devices.
+
+        // Step 2.4.
+        for filter in filters {
+            // Step 2.4.8.
+            uuid_filters.push(try!(canonicalize_filter(&filter, global)));
+        }
     }
 
-    let mut services_vec = vec!();
+    let mut optional_services_uuids = vec!();
-    if let Some(ref services) = filter.services {
+    if let &Some(ref opt_services) = optional_services {
-        if services.is_empty() {
+        for opt_service in opt_services {
-            return Err(Type(SERVICE_ERROR.to_owned()));
+            // Step 2.5 - 2.6.
+            let uuid = try!(BluetoothUUID::GetService(global, opt_service.clone())).to_string();
+
+            // Step 2.7.
+            // Note: What we are doing here is adding the not blacklisted UUIDs to the result vector,
+            // insted of removing them from an already filled vector.
+            if !uuid_is_blacklisted(uuid.as_ref(), Blacklist::All) {
+                optional_services_uuids.push(uuid);
+            }
         }
-        for service in services {
+    }
-            let uuid = try!(BluetoothUUID::GetService(global, service.clone())).to_string();
+
+    Ok(RequestDeviceoptions::new(BluetoothScanfilterSequence::new(uuid_filters),
+                                 ServiceUUIDSequence::new(optional_services_uuids)))
+}
+
+// https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices
+fn canonicalize_filter(filter: &BluetoothRequestDeviceFilter, global: GlobalRef) -> Fallible<BluetoothScanfilter> {
+    // Step 2.4.1.
+    if filter.services.is_none() &&
+       filter.name.is_none() &&
+       filter.namePrefix.is_none() &&
+       filter.manufacturerId.is_none() &&
+       filter.serviceDataUUID.is_none() {
+           return Err(Type(FILTER_ERROR.to_owned()));
+    }
+
+    // Step 2.4.2: There is no empty canonicalizedFilter member,
+    // we create a BluetoothScanfilter instance at the end of the function.
+
+    // Step 2.4.3.
+    let services_vec = match filter.services {
+        Some(ref services) => {
+            // Step 2.4.3.1.
+            if services.is_empty() {
+                return Err(Type(SERVICE_ERROR.to_owned()));
+            }
+
+            let mut services_vec = vec!();
+
+            for service in services {
+                // Step 2.4.3.2 - 2.4.3.3.
+                let uuid = try!(BluetoothUUID::GetService(global, service.clone())).to_string();
+
+                // Step 2.4.3.4.
+                if uuid_is_blacklisted(uuid.as_ref(), Blacklist::All) {
+                    return Err(Security)
+                }
+
+                services_vec.push(uuid);
+            }
+            // Step 2.4.3.5.
+            services_vec
+            // Step 2.4.3.6: There is no requiredServiceUUIDS, we scan for all devices.
+        },
+        None => vec!(),
+    };
+
+    // Step 2.4.4.
+    let name = match filter.name {
+        Some(ref name) => {
+            // Step 2.4.4.1.
+            // Note: DOMString::len() gives back the size in bytes.
+            if name.len() > MAX_DEVICE_NAME_LENGTH {
+                return Err(Type(NAME_TOO_LONG_ERROR.to_owned()));
+            }
+            if name.len() > MAX_FILTER_NAME_LENGTH {
+                return Err(Type(FILTER_NAME_TOO_LONG_ERROR.to_owned()));
+            }
+
+            // Step 2.4.4.2.
+            name.to_string()
+        },
+        None => String::new(),
+    };
+
+    // Step 2.4.5.
+    let name_prefix = match filter.namePrefix {
+        Some(ref name_prefix) => {
+            // Step 2.4.5.1.
+            if name_prefix.is_empty() {
+                return Err(Type(NAME_PREFIX_ERROR.to_owned()));
+            }
+            if name_prefix.len() > MAX_DEVICE_NAME_LENGTH {
+                return Err(Type(NAME_TOO_LONG_ERROR.to_owned()));
+            }
+            if name_prefix.len() > MAX_FILTER_NAME_LENGTH {
+                return Err(Type(FILTER_NAME_TOO_LONG_ERROR.to_owned()));
+            }
+
+            // Step 2.4.5.2.
+            name_prefix.to_string()
+        },
+        None => String::new(),
+    };
+
+    // Step 2.4.6.
+    let manufacturer_id = filter.manufacturerId;
+
+    // Step 2.4.7.
+    let service_data_uuid = match filter.serviceDataUUID {
+        Some(ref service_data_uuid) => {
+            // Step 2.4.7.1 - 2.4.7.2.
+            let uuid = try!(BluetoothUUID::GetService(global, service_data_uuid.clone())).to_string();
+
+            // Step 2.4.7.3.
             if uuid_is_blacklisted(uuid.as_ref(), Blacklist::All) {
                 return Err(Security)
             }
-            services_vec.push(uuid);
-        }
-    }
 
-    let mut name = String::new();
+            // Step 2.4.7.4.
-    if let Some(ref filter_name) = filter.name {
+            uuid
-        //NOTE: DOMString::len() gives back the size in bytes
+        },
-        if filter_name.len() > MAX_DEVICE_NAME_LENGTH {
+        None => String::new(),
-            return Err(Type(NAME_TOO_LONG_ERROR.to_owned()));
+    };
-        }
-        if filter_name.len() > MAX_FILTER_NAME_LENGTH {
-            return Err(Type(FILTER_NAME_TOO_LONG_ERROR.to_owned()));
-        }
-        name = filter_name.to_string();
-    }
 
-    let mut name_prefix = String::new();
+    Ok(BluetoothScanfilter::new(name,
-    if let Some(ref filter_name_prefix) = filter.namePrefix {
+                                name_prefix,
-        if filter_name_prefix.is_empty() {
+                                services_vec,
-            return Err(Type(NAME_PREFIX_ERROR.to_owned()));
+                                manufacturer_id,
-        }
+                                service_data_uuid))
-        if filter_name_prefix.len() > MAX_DEVICE_NAME_LENGTH {
-            return Err(Type(NAME_TOO_LONG_ERROR.to_owned()));
-        }
-        if filter_name_prefix.len() > MAX_FILTER_NAME_LENGTH {
-            return Err(Type(FILTER_NAME_TOO_LONG_ERROR.to_owned()));
-        }
-        name_prefix = filter_name_prefix.to_string();
-    }
-
-    Ok(BluetoothScanfilter::new(name, name_prefix, services_vec))
-}
-
-fn convert_request_device_options(options: &RequestDeviceOptions,
-                                  global: GlobalRef)
-                                  -> Fallible<RequestDeviceoptions> {
-    if options.filters.is_empty() {
-        return Err(Type(FILTER_EMPTY_ERROR.to_owned()));
-    }
-
-    let mut filters = vec!();
-    for filter in &options.filters {
-        filters.push(try!(canonicalize_filter(&filter, global)));
-    }
-
-    let mut optional_services = vec!();
-    if let Some(ref opt_services) = options.optionalServices {
-        for opt_service in opt_services {
-            let uuid = try!(BluetoothUUID::GetService(global, opt_service.clone())).to_string();
-            if !uuid_is_blacklisted(uuid.as_ref(), Blacklist::All) {
-                optional_services.push(uuid);
-            }
-        }
-    }
-
-    Ok(RequestDeviceoptions::new(BluetoothScanfilterSequence::new(filters),
-                                 ServiceUUIDSequence::new(optional_services)))
 }
 
 impl From<BluetoothError> for Error {
@@ -150,24 +267,18 @@ impl From<BluetoothError> for Error {
 impl BluetoothMethods for Bluetooth {
     // https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice
     fn RequestDevice(&self, option: &RequestDeviceOptions) -> Fallible<Root<BluetoothDevice>> {
-        let (sender, receiver) = ipc::channel().unwrap();
+        // Step 1.
-        let option = try!(convert_request_device_options(option, self.global().r()));
+        // TODO(#4282): Reject promise.
-        self.get_bluetooth_thread().send(BluetoothMethodMsg::RequestDevice(option, sender)).unwrap();
+        if (option.filters.is_some() && option.acceptAllDevices) ||
-        let device = receiver.recv().unwrap();
+           (option.filters.is_none() && !option.acceptAllDevices) {
-        match device {
+            return Err(Type(OPTIONS_ERROR.to_owned()));
-            Ok(device) => {
-                let ad_data = BluetoothAdvertisingData::new(self.global().r(),
-                                                            device.appearance,
-                                                            device.tx_power,
-                                                            device.rssi);
-                Ok(BluetoothDevice::new(self.global().r(),
-                                        DOMString::from(device.id),
-                                        device.name.map(DOMString::from),
-                                        &ad_data))
-            },
-            Err(error) => {
-                Err(Error::from(error))
-            },
         }
+        // Step 2.
+        if !option.acceptAllDevices {
+            return self.request_bluetooth_devices(&option.filters, &option.optionalServices);
+        }
+
+        self.request_bluetooth_devices(&None, &option.optionalServices)
+        // TODO(#4282): Step 3-5: Reject and resolve promise.
     }
 }

components/script/dom/webidls/Bluetooth.webidl

@@ -4,24 +4,31 @@
 
 // https://webbluetoothcg.github.io/web-bluetooth/#bluetooth
 
-dictionary BluetoothScanFilter {
+dictionary BluetoothRequestDeviceFilter {
   sequence<BluetoothServiceUUID> services;
   DOMString name;
   DOMString namePrefix;
+  unsigned short manufacturerId;
+  BluetoothServiceUUID serviceDataUUID;
 };
 
 dictionary RequestDeviceOptions {
-  required sequence<BluetoothScanFilter> filters;
+  sequence<BluetoothRequestDeviceFilter> filters;
   sequence<BluetoothServiceUUID> optionalServices /*= []*/;
+  boolean acceptAllDevices = false;
 };
 
 [Pref="dom.bluetooth.enabled", Exposed=(Window,Worker)]
 interface Bluetooth {
-    // Promise<BluetoothDevice> requestDevice(RequestDeviceOptions options);
+//  [SecureContext]
-    [Throws]
+//  readonly attribute BluetoothDevice? referringDevice;
-    BluetoothDevice requestDevice(RequestDeviceOptions options);
+//  [SecureContext]
+//  Promise<BluetoothDevice> requestDevice(RequestDeviceOptions options);
+  [Throws]
+  BluetoothDevice requestDevice(optional RequestDeviceOptions options);
 };
 
 // Bluetooth implements EventTarget;
+// Bluetooth implements BluetoothDeviceEventHandlers;
 // Bluetooth implements CharacteristicEventHandlers;
 // Bluetooth implements ServiceEventHandlers;

tests/html/bluetooth/bluetooth_request_all_devices.html

@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<title>Request All Devices</title>
+<body>
+    <button type="button" onclick="onButtonClick()">Request All Devices</button>
+    <pre id="log"></pre>
+    <script src="bluetooth_functions.js"></script>
+    <script>
+    function onButtonClick() {
+        clear();
+        var options = {optionalServices: ['generic_access'], acceptAllDevices:true};
+        try {
+            log('Requesting Bluetooth Device...');
+            var bluetooth = window.navigator.bluetooth;
+            var device = bluetooth.requestDevice(options);
+
+            log('Connecting to GATT Server on device...');
+            var server = device.gatt.connect();
+
+            log('Getting Generic Access Service...');
+            var service = server.getPrimaryService('generic_access');
+        } catch(err) {
+            log(err);
+        }
+    }
+    </script>
+</body>
+</html>