WebBluetooth fixes for the wpt tests #13918

zakorgy · 2016-10-25T14:28:41Z

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.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
There are tests for these changes OR

This change is

highfive · 2016-10-25T14:28:46Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/bluetoothdevice.rs, components/script/dom/webidls/TestRunner.webidl, components/script/dom/webidls/Window.webidl, components/script/dom/window.rs, components/script/dom/testrunner.rs, components/script/dom/bluetooth.rs, components/script/dom/webidls/BluetoothDevice.webidl, components/script/dom/mod.rs, components/script/dom/bluetoothremotegattserver.rs, components/script/dom/bluetoothremotegattservice.rs, components/script/dom/bluetoothremotegattdescriptor.rs, components/script/dom/bluetoothremotegattcharacteristic.rs
@KiChjang: components/script/dom/bluetoothdevice.rs, components/net_traits/bluetooth_scanfilter.rs, components/net_traits/bluetooth_scanfilter.rs, components/script/dom/webidls/TestRunner.webidl, components/script/dom/webidls/Window.webidl, components/net/lib.rs, components/net/bluetooth_thread.rs, components/script/dom/window.rs, components/script/dom/testrunner.rs, components/script/dom/bluetooth.rs, components/net/Cargo.toml, components/script/dom/webidls/BluetoothDevice.webidl, components/script/dom/mod.rs, components/script/dom/bluetoothremotegattserver.rs, components/net/bluetooth_test.rs, components/script/dom/bluetoothremotegattservice.rs, components/net_traits/bluetooth_thread.rs, components/net_traits/bluetooth_thread.rs, components/script/dom/bluetoothremotegattdescriptor.rs, components/script/dom/bluetoothremotegattcharacteristic.rs

zakorgy · 2016-10-25T14:28:58Z

r? @jdm

jdm

These changes are largely fine.

jdm · 2016-10-31T19:23:18Z

components/net_traits/bluetooth_scanfilter.rs

-    pub fn get_name(&self) -> &str {
-        &self.name
+    pub fn get_name(&self) -> Option<&str> {
+        match &self.name {


self.name.as_ref().map(|s| s.as_str())

jdm · 2016-10-31T19:23:18Z

components/net_traits/bluetooth_scanfilter.rs

         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
+         self.get_name().unwrap_or("").len() > MAX_NAME_LENGTH ||


These lines should be unindented by one, since they are not part of the subexpression.

jdm · 2016-10-31T19:23:18Z

components/net/bluetooth_thread.rs

@@ -12,6 +12,7 @@ use net_traits::bluetooth_thread::{BluetoothDescriptorMsg, BluetoothDescriptorsM
 use net_traits::bluetooth_thread::{BluetoothDeviceMsg, BluetoothError, BluetoothMethodMsg};
 use net_traits::bluetooth_thread::{BluetoothResult, BluetoothServiceMsg, BluetoothServicesMsg};
 use rand::{self, Rng};
+use script::bluetooth_blacklist::{uuid_is_blacklisted, Blacklist};


These helpers need to move into net_traits instead.

jdm · 2016-10-31T19:23:18Z

components/net/Cargo.toml

@@ -34,6 +34,7 @@ plugins = {path = "../plugins"}
 profile_traits = {path = "../profile_traits"}
 rand = "0.3"
 rustc-serialize = "0.3"
+script = {path = "../script"}


We need to find a way not to do this; it's useful for net to be able to build without depending on script.

jdm · 2016-11-02T14:25:36Z

components/servo/Cargo.lock

@@ -1526,6 +1525,7 @@ dependencies = [
 "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
 "msg 0.0.1",
 "num-traits 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)",
+ "regex 0.1.76 (registry+https://github.com/rust-lang/crates.io-index)",


We'll need to run ./mach cargo-update -p net_traits to ensure that ports/cef/Cargo.lock is updated too.

jdm · 2016-11-02T14:25:58Z

Other than that, this is good to merge after #13612.


        Return with InvalidStateError if a Bluetooth id is not cached.


        Check if the BluetoothDevice is connected, when calling getGATT funct…

…ions.


        Removing uuids field from BluetoothDevice with the related test cases.


        Accepting empty deviceName, when requesting a BluetoothDevice.


        Return with NotFoundError insted of TypeError, when adapter is not fo…

…und.


        Return with NotFoundError when the requested deviceName/deviceNamePre…

…fix is longer than 29 bytes.


        Remove not allowed services when calling getPrimaryServices.


        Update the value of the Characteristic/Descriptor when calling writeV…

…alue.


        Blacklisted items are removed when calling getServices/Characteristic…

…s/Descriptors.


        Return the same JS object for the same Bluetooth item.


        Move bluetooth-helper.js

zakorgy · 2016-11-07T14:43:36Z

The last commit solves #14112.

jdm · 2016-11-07T15:02:00Z

@bors-servo: r+

bors-servo · 2016-11-07T15:02:05Z

📌 Commit 91a0c4d has been approved by jdm

bors-servo · 2016-11-07T15:50:14Z

⌛ Testing commit 91a0c4d with merge eb2484f...


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

WebBluetooth fixes for the wpt tests  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. ---  - [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   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13918)

bors-servo · 2016-11-07T16:53:13Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

highfive assigned SimonSapin Oct 25, 2016

highfive added the S-awaiting-review label Oct 25, 2016

highfive assigned jdm and unassigned SimonSapin Oct 25, 2016

jdm requested changes Oct 31, 2016

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Oct 31, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 2, 2016

fokinv force-pushed the szeged:wpt-error-fixes branch from 8d73598 to 503202f Nov 2, 2016

jdm added S-blocked-on-external and removed S-awaiting-review labels Nov 2, 2016

jdm requested changes Nov 2, 2016

View changes

jdm added the S-needs-code-changes label Nov 2, 2016

fokinv force-pushed the szeged:wpt-error-fixes branch from 503202f to 50556d8 Nov 2, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 2, 2016

jdm added S-needs-rebase and removed S-awaiting-review labels Nov 2, 2016

zakorgy and others added 8 commits Oct 6, 2016

Return with InvalidStateError if a Bluetooth id is not cached.

11dbb71

Check if the BluetoothDevice is connected, when calling getGATT funct…

b72dcf3

…ions.

Removing uuids field from BluetoothDevice with the related test cases.

2b4829b

Accepting empty deviceName, when requesting a BluetoothDevice.

4ed461c

Return with NotFoundError insted of TypeError, when adapter is not fo…

98f2233

…und.

Return with NotFoundError when the requested deviceName/deviceNamePre…

baa024e

…fix is longer than 29 bytes.

Remove not allowed services when calling getPrimaryServices.

0eeb56b

Update the value of the Characteristic/Descriptor when calling writeV…

dd733f6

…alue.

fokinv and others added 2 commits Oct 10, 2016

Blacklisted items are removed when calling getServices/Characteristic…

d30bcbd

…s/Descriptors.

Return the same JS object for the same Bluetooth item.

Loading status checks…

e8c1c98

zakorgy force-pushed the szeged:wpt-error-fixes branch from 50556d8 to e8c1c98 Nov 4, 2016

highfive added the S-awaiting-review label Nov 4, 2016

dati91 mentioned this pull request Nov 7, 2016

Move bluetooth-helpers.js into mozilla test directory #14112

Closed

jdm removed S-awaiting-review S-needs-rebase labels Nov 7, 2016

Move bluetooth-helper.js

Loading status checks…

91a0c4d

zakorgy changed the title ~~[WIP] WebBluetooth fixes for the wpt tests~~ WebBluetooth fixes for the wpt tests Nov 7, 2016

highfive added the S-awaiting-merge label Nov 7, 2016

jdm removed the S-blocked-on-external label Nov 7, 2016

bors-servo merged commit 91a0c4d into servo:master Nov 7, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Nov 7, 2016

This was referenced Nov 7, 2016

Update web-platform-tests to revision c75cae4d1fcd0ed472b9110b14f6a8b5e2ce2759 #14107

Closed

Webbluetooth Async behaviour #13909

Merged

Enable cors and redirect fetch tests. #14070

Closed

servo / servo

WebBluetooth fixes for the wpt tests #13918

WebBluetooth fixes for the wpt tests #13918

zakorgy commented Oct 25, 2016 •

edited by larsbergstrom

highfive commented Oct 25, 2016

zakorgy commented Oct 25, 2016

jdm left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdm commented Nov 2, 2016

zakorgy commented Nov 7, 2016

jdm commented Nov 7, 2016

bors-servo commented Nov 7, 2016

bors-servo commented Nov 7, 2016

bors-servo commented Nov 7, 2016

servo / servo

Join GitHub today

WebBluetooth fixes for the wpt tests #13918

WebBluetooth fixes for the wpt tests #13918

Conversation

zakorgy commented Oct 25, 2016 • edited by larsbergstrom

highfive commented Oct 25, 2016

zakorgy commented Oct 25, 2016

jdm left a comment

This comment has been minimized.

jdm Oct 31, 2016

This comment has been minimized.

jdm Oct 31, 2016

This comment has been minimized.

jdm Oct 31, 2016

This comment has been minimized.

jdm Oct 31, 2016

This comment has been minimized.

jdm Nov 2, 2016

jdm commented Nov 2, 2016

zakorgy commented Nov 7, 2016

jdm commented Nov 7, 2016

bors-servo commented Nov 7, 2016

bors-servo commented Nov 7, 2016

bors-servo commented Nov 7, 2016

zakorgy commented Oct 25, 2016 •

edited by larsbergstrom