Auto merge of #13918 - szeged:wpt-error-fixes, r=jdm

WebBluetooth fixes for the wpt tests

<!-- Please describe your changes on the following line: -->

Note: depends on #13612
WebBluetooth fixes for the failing wpt tests, excluding the `disconnect-during` tests cases, due to the lack of the event handling in the current implementation.
---

<!-- 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] There are tests for these changes OR

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/13918)

<!-- Reviewable:end -->

components/bluetooth/lib.rs

@@ -19,6 +19,7 @@ use bluetooth_traits::{BluetoothCharacteristicMsg, BluetoothCharacteristicsMsg};
 use bluetooth_traits::{BluetoothDescriptorMsg, BluetoothDescriptorsMsg};
 use bluetooth_traits::{BluetoothDeviceMsg, BluetoothError, BluetoothMethodMsg};
 use bluetooth_traits::{BluetoothResult, BluetoothServiceMsg, BluetoothServicesMsg};
+use bluetooth_traits::blacklist::{uuid_is_blacklisted, Blacklist};
 use bluetooth_traits::scanfilter::{BluetoothScanfilter, BluetoothScanfilterSequence, RequestDeviceoptions};
 use device::bluetooth::{BluetoothAdapter, BluetoothDevice, BluetoothGATTCharacteristic};
 use device::bluetooth::{BluetoothGATTDescriptor, BluetoothGATTService};
@@ -31,10 +32,6 @@ use std::thread;
 use std::time::Duration;
 use util::thread::spawn_named;
 
-const ADAPTER_ERROR: &'static str = "No adapter found";
-
-const ADAPTER_NOT_POWERED_ERROR: &'static str = "Bluetooth adapter not powered";
-
 // A transaction not completed within 30 seconds shall time out. Such a transaction shall be considered to have failed.
 // https://www.bluetooth.org/DocMan/handlers/DownloadDoc.ashx?doc_id=286439 (Vol. 3, page 480)
 const MAXIMUM_TRANSACTION_TIME: u8 = 30;
@@ -75,11 +72,11 @@ macro_rules! get_adapter_or_return_error(
         match $bl_manager.get_or_create_adapter() {
             Some(adapter) => {
                 if !adapter.is_powered().unwrap_or(false) {
-                    return drop($sender.send(Err(BluetoothError::Type(ADAPTER_NOT_POWERED_ERROR.to_string()))))
+                    return drop($sender.send(Err(BluetoothError::NotFound)))
                 }
                 adapter
             },
-            None => return drop($sender.send(Err(BluetoothError::Type(ADAPTER_ERROR.to_string())))),
+            None => return drop($sender.send(Err(BluetoothError::NotFound))),
         }
     );
 );
@@ -106,8 +103,8 @@ fn matches_filter(device: &BluetoothDevice, filter: &BluetoothScanfilter) -> boo
     }
 
     // Step 1.
-    if !filter.get_name().is_empty() {
-        if device.get_name().ok() != Some(filter.get_name().to_string()) {
+    if let Some(name) = filter.get_name() {
+        if device.get_name().ok() != Some(name.to_string()) {
             return false;
         }
     }
@@ -628,6 +625,9 @@ impl BluetoothManager {
                            device_id: String,
                            uuid: String,
                            sender: IpcSender<BluetoothResult<BluetoothServiceMsg>>) {
+        if !self.cached_devices.contains_key(&device_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = get_adapter_or_return_error!(self, sender);
         if !self.allowed_services.get(&device_id).map_or(false, |s| s.contains(&uuid)) {
             return drop(sender.send(Err(BluetoothError::Security)));
@@ -654,6 +654,9 @@ impl BluetoothManager {
                             device_id: String,
                             uuid: Option<String>,
                             sender: IpcSender<BluetoothResult<BluetoothServicesMsg>>) {
+        if !self.cached_devices.contains_key(&device_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = get_adapter_or_return_error!(self, sender);
         let services = match uuid {
             Some(ref id) => {
@@ -679,6 +682,10 @@ impl BluetoothManager {
                 }
             }
         }
+        services_vec.retain(|s| !uuid_is_blacklisted(&s.uuid, Blacklist::All) &&
+                                self.allowed_services
+                                    .get(&device_id)
+                                    .map_or(false, |uuids| uuids.contains(&s.uuid)));
         if services_vec.is_empty() {
             return drop(sender.send(Err(BluetoothError::NotFound)));
         }
@@ -690,9 +697,12 @@ impl BluetoothManager {
                             service_id: String,
                             uuid: String,
                             sender: IpcSender<BluetoothResult<BluetoothServiceMsg>>) {
+        if !self.cached_services.contains_key(&service_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = match self.get_or_create_adapter() {
             Some(a) => a,
-            None => return drop(sender.send(Err(BluetoothError::Type(ADAPTER_ERROR.to_string())))),
+            None => return drop(sender.send(Err(BluetoothError::NotFound))),
         };
         let device = match self.device_from_service_id(&service_id) {
             Some(device) => device,
@@ -721,9 +731,12 @@ impl BluetoothManager {
                              service_id: String,
                              uuid: Option<String>,
                              sender: IpcSender<BluetoothResult<BluetoothServicesMsg>>) {
+        if !self.cached_services.contains_key(&service_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = match self.get_or_create_adapter() {
             Some(a) => a,
-            None => return drop(sender.send(Err(BluetoothError::Type(ADAPTER_ERROR.to_string())))),
+            None => return drop(sender.send(Err(BluetoothError::NotFound))),
         };
         let device = match self.device_from_service_id(&service_id) {
             Some(device) => device,
@@ -747,6 +760,7 @@ impl BluetoothManager {
         if let Some(uuid) = uuid {
             services_vec.retain(|ref s| s.uuid == uuid);
         }
+        services_vec.retain(|s| !uuid_is_blacklisted(&s.uuid, Blacklist::All));
         if services_vec.is_empty() {
             return drop(sender.send(Err(BluetoothError::NotFound)));
         }
@@ -758,6 +772,9 @@ impl BluetoothManager {
                           service_id: String,
                           uuid: String,
                           sender: IpcSender<BluetoothResult<BluetoothCharacteristicMsg>>) {
+        if !self.cached_services.contains_key(&service_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = get_adapter_or_return_error!(self, sender);
         let characteristics = self.get_gatt_characteristics_by_uuid(&mut adapter, &service_id, &uuid);
         if characteristics.is_empty() {
@@ -789,6 +806,9 @@ impl BluetoothManager {
                            service_id: String,
                            uuid: Option<String>,
                            sender: IpcSender<BluetoothResult<BluetoothCharacteristicsMsg>>) {
+        if !self.cached_services.contains_key(&service_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = get_adapter_or_return_error!(self, sender);
         let characteristics = match uuid {
             Some(id) => self.get_gatt_characteristics_by_uuid(&mut adapter, &service_id, &id),
@@ -817,6 +837,7 @@ impl BluetoothManager {
                                 });
             }
         }
+        characteristics_vec.retain(|c| !uuid_is_blacklisted(&c.uuid, Blacklist::All));
         if characteristics_vec.is_empty() {
             return drop(sender.send(Err(BluetoothError::NotFound)));
         }
@@ -828,6 +849,9 @@ impl BluetoothManager {
                       characteristic_id: String,
                       uuid: String,
                       sender: IpcSender<BluetoothResult<BluetoothDescriptorMsg>>) {
+        if !self.cached_characteristics.contains_key(&characteristic_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = get_adapter_or_return_error!(self, sender);
         let descriptors = self.get_gatt_descriptors_by_uuid(&mut adapter, &characteristic_id, &uuid);
         if descriptors.is_empty() {
@@ -848,6 +872,9 @@ impl BluetoothManager {
                        characteristic_id: String,
                        uuid: Option<String>,
                        sender: IpcSender<BluetoothResult<BluetoothDescriptorsMsg>>) {
+        if !self.cached_characteristics.contains_key(&characteristic_id) {
+            return drop(sender.send(Err(BluetoothError::InvalidState)));
+        }
         let mut adapter = get_adapter_or_return_error!(self, sender);
         let descriptors = match uuid {
             Some(id) => self.get_gatt_descriptors_by_uuid(&mut adapter, &characteristic_id, &id),
@@ -865,6 +892,7 @@ impl BluetoothManager {
                                      });
             }
         }
+        descriptors_vec.retain(|d| !uuid_is_blacklisted(&d.uuid, Blacklist::All));
         if descriptors_vec.is_empty() {
             return drop(sender.send(Err(BluetoothError::NotFound)));
         }
@@ -879,7 +907,7 @@ impl BluetoothManager {
             value = self.get_gatt_descriptor(&mut adapter, &id)
                         .map(|d| d.read_value().unwrap_or(vec![]));
         }
-        let _ = sender.send(value.ok_or(BluetoothError::NotSupported));
+        let _ = sender.send(value.ok_or(BluetoothError::InvalidState));
     }
 
     fn write_value(&mut self, id: String, value: Vec<u8>, sender: IpcSender<BluetoothResult<bool>>) {
@@ -895,7 +923,7 @@ impl BluetoothManager {
                 Ok(_) => Ok(true),
                 Err(_) => return drop(sender.send(Err(BluetoothError::NotSupported))),
             },
-            None => return drop(sender.send(Err(BluetoothError::NotSupported))),
+            None => return drop(sender.send(Err(BluetoothError::InvalidState))),
         };
         let _ = sender.send(message);
     }

components/bluetooth_traits/Cargo.toml

@@ -11,5 +11,7 @@ path = "lib.rs"
 
 [dependencies]
 ipc-channel = "0.5"
+regex = "0.1.43"
 serde = "0.8"
 serde_derive = "0.8"
+util = {path = "../util"}

components/bluetooth_traits/lib.rs

@@ -5,9 +5,12 @@
 #![feature(proc_macro)]
 
 extern crate ipc_channel;
+extern crate regex;
 #[macro_use]
 extern crate serde_derive;
+extern crate util;
 
+pub mod blacklist;
 pub mod scanfilter;
 
 use ipc_channel::ipc::IpcSender;
@@ -20,6 +23,7 @@ pub enum BluetoothError {
     NotFound,
     NotSupported,
     Security,
+    InvalidState,
 }
 
 #[derive(Deserialize, Serialize)]

components/bluetooth_traits/scanfilter.rs

@@ -25,15 +25,15 @@ impl ServiceUUIDSequence {
 
 #[derive(Deserialize, Serialize)]
 pub struct BluetoothScanfilter {
-    name: String,
+    name: Option<String>,
     name_prefix: String,
     services: ServiceUUIDSequence,
     manufacturer_id: Option<u16>,
     service_data_uuid: String,
 }
 
 impl BluetoothScanfilter {
-    pub fn new(name: String,
+    pub fn new(name: Option<String>,
                name_prefix: String,
                services: Vec<String>,
                manufacturer_id: Option<u16>,
@@ -48,8 +48,8 @@ impl BluetoothScanfilter {
         }
     }
 
-    pub fn get_name(&self) -> &str {
-        &self.name
+    pub fn get_name(&self) -> Option<&str> {
+        self.name.as_ref().map(|s| s.as_str())
     }
 
     pub fn get_name_prefix(&self) -> &str {
@@ -69,12 +69,12 @@ impl BluetoothScanfilter {
     }
 
     pub fn is_empty_or_invalid(&self) -> bool {
-        (self.name.is_empty() &&
+        (self.name.is_none() &&
          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.get_name().unwrap_or("").len() > MAX_NAME_LENGTH ||
         self.name_prefix.len() > MAX_NAME_LENGTH
     }
 }

components/script/dom/bluetooth.rs

@@ -2,30 +2,34 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use bluetooth_blacklist::{Blacklist, uuid_is_blacklisted};
 use bluetooth_traits::{BluetoothError, BluetoothMethodMsg};
+use bluetooth_traits::blacklist::{Blacklist, uuid_is_blacklisted};
 use bluetooth_traits::scanfilter::{BluetoothScanfilter, BluetoothScanfilterSequence};
 use bluetooth_traits::scanfilter::{RequestDeviceoptions, ServiceUUIDSequence};
 use core::clone::Clone;
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::BluetoothBinding::{self, BluetoothMethods, BluetoothRequestDeviceFilter};
 use dom::bindings::codegen::Bindings::BluetoothBinding::RequestDeviceOptions;
-use dom::bindings::error::Error::{self, Security, Type};
+use dom::bindings::error::Error::{self, NotFound, Security, Type};
 use dom::bindings::error::Fallible;
-use dom::bindings::js::Root;
+use dom::bindings::js::{JS, MutHeap, Root};
 use dom::bindings::reflector::{Reflectable, Reflector, reflect_dom_object};
 use dom::bindings::str::DOMString;
 use dom::bluetoothadvertisingdata::BluetoothAdvertisingData;
 use dom::bluetoothdevice::BluetoothDevice;
+use dom::bluetoothremotegattcharacteristic::BluetoothRemoteGATTCharacteristic;
+use dom::bluetoothremotegattdescriptor::BluetoothRemoteGATTDescriptor;
+use dom::bluetoothremotegattservice::BluetoothRemoteGATTService;
 use dom::bluetoothuuid::{BluetoothServiceUUID, BluetoothUUID};
 use dom::globalscope::GlobalScope;
 use dom::promise::Promise;
 use ipc_channel::ipc::{self, IpcSender};
 use js::conversions::ToJSValConvertible;
+use std::collections::HashMap;
 use std::rc::Rc;
 
 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.
 const MAX_DEVICE_NAME_LENGTH: usize = 248;
 // A device name can never be longer than 29 bytes.
@@ -43,12 +47,20 @@ const OPTIONS_ERROR: &'static str = "Fields of 'options' conflict with each othe
 #[dom_struct]
 pub struct Bluetooth {
     reflector_: Reflector,
+    device_instance_map: DOMRefCell<HashMap<String, MutHeap<JS<BluetoothDevice>>>>,
+    service_instance_map: DOMRefCell<HashMap<String, MutHeap<JS<BluetoothRemoteGATTService>>>>,
+    characteristic_instance_map: DOMRefCell<HashMap<String, MutHeap<JS<BluetoothRemoteGATTCharacteristic>>>>,
+    descriptor_instance_map: DOMRefCell<HashMap<String, MutHeap<JS<BluetoothRemoteGATTDescriptor>>>>,
 }
 
 impl Bluetooth {
     pub fn new_inherited() -> Bluetooth {
         Bluetooth {
             reflector_: Reflector::new(),
+            device_instance_map: DOMRefCell::new(HashMap::new()),
+            service_instance_map: DOMRefCell::new(HashMap::new()),
+            characteristic_instance_map: DOMRefCell::new(HashMap::new()),
+            descriptor_instance_map: DOMRefCell::new(HashMap::new()),
         }
     }
 
@@ -58,6 +70,19 @@ impl Bluetooth {
                            BluetoothBinding::Wrap)
     }
 
+    pub fn get_service_map(&self) -> &DOMRefCell<HashMap<String, MutHeap<JS<BluetoothRemoteGATTService>>>> {
+        &self.service_instance_map
+    }
+
+    pub fn get_characteristic_map(&self)
+            -> &DOMRefCell<HashMap<String, MutHeap<JS<BluetoothRemoteGATTCharacteristic>>>> {
+        &self.characteristic_instance_map
+    }
+
+    pub fn get_descriptor_map(&self) -> &DOMRefCell<HashMap<String, MutHeap<JS<BluetoothRemoteGATTDescriptor>>>> {
+        &self.descriptor_instance_map
+    }
+
     fn get_bluetooth_thread(&self) -> IpcSender<BluetoothMethodMsg> {
         self.global().as_window().bluetooth_thread()
     }
@@ -103,15 +128,21 @@ impl Bluetooth {
         // Step 12-13.
         match device {
             Ok(device) => {
-                let global = self.global();
-                let ad_data = BluetoothAdvertisingData::new(&global,
+                let mut device_instance_map = self.device_instance_map.borrow_mut();
+                if let Some(existing_device) = device_instance_map.get(&device.id.clone()) {
+                    return Ok(existing_device.get());
+                }
+                let ad_data = BluetoothAdvertisingData::new(&self.global(),
                                                             device.appearance,
                                                             device.tx_power,
                                                             device.rssi);
-                Ok(BluetoothDevice::new(&global,
-                                        DOMString::from(device.id),
-                                        device.name.map(DOMString::from),
-                                        &ad_data))
+                let bt_device = BluetoothDevice::new(&self.global(),
+                                                     DOMString::from(device.id.clone()),
+                                                     device.name.map(DOMString::from),
+                                                     &ad_data,
+                                                     &self);
+                device_instance_map.insert(device.id, MutHeap::new(&bt_device));
+                Ok(bt_device)
             },
             Err(error) => {
                 Err(Error::from(error))
@@ -213,13 +244,13 @@ fn canonicalize_filter(filter: &BluetoothRequestDeviceFilter) -> Fallible<Blueto
                 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()));
+                return Err(NotFound);
             }
 
             // Step 2.4.4.2.
-            name.to_string()
+            Some(name.to_string())
         },
-        None => String::new(),
+        None => None,
     };
 
     // Step 2.4.5.
@@ -233,7 +264,7 @@ fn canonicalize_filter(filter: &BluetoothRequestDeviceFilter) -> Fallible<Blueto
                 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()));
+                return Err(NotFound);
             }
 
             // Step 2.4.5.2.
@@ -289,6 +320,7 @@ impl From<BluetoothError> for Error {
             BluetoothError::NotFound => Error::NotFound,
             BluetoothError::NotSupported => Error::NotSupported,
             BluetoothError::Security => Error::Security,
+            BluetoothError::InvalidState => Error::InvalidState,
         }
     }
 }