WebBluetooth fixes for the wpt tests by zakorgy · Pull Request #13918 · servo/servo

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:15:52Z

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:16:27Z

components/net_traits/bluetooth_scanfilter.rs

         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:22:15Z

components/net/bluetooth_thread.rs

 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:22:41Z

components/net/Cargo.toml

 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:19:19Z

components/servo/Cargo.lock

 "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.

…ions.

…und.

…fix is longer than 29 bytes.

…alue.

…s/Descriptors.

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...

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 There is new code that needs to be reviewed. label Oct 25, 2016

highfive assigned jdm and unassigned SimonSapin Oct 25, 2016

jdm requested changes Oct 31, 2016

View reviewed changes

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 31, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Nov 2, 2016

fokinv force-pushed the wpt-error-fixes branch from 8d73598 to 503202f Compare November 2, 2016 14:17

jdm added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 2, 2016

jdm requested changes Nov 2, 2016

View reviewed changes

jdm added the S-needs-code-changes Changes have not yet been made that were requested by a reviewer. label Nov 2, 2016

fokinv force-pushed the wpt-error-fixes branch from 503202f to 50556d8 Compare November 2, 2016 14:33

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Nov 2, 2016

jdm added S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 2, 2016

zakorgy and others added 8 commits November 4, 2016 15:22

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 November 4, 2016 15:36

Blacklisted items are removed when calling getServices/Characteristic…

d30bcbd

…s/Descriptors.

Return the same JS object for the same Bluetooth item.

e8c1c98

zakorgy force-pushed the wpt-error-fixes branch from 50556d8 to e8c1c98 Compare November 4, 2016 14:52

highfive added the S-awaiting-review There is new code that needs to be reviewed. 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 There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Nov 7, 2016

Move bluetooth-helper.js

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 The PR is in the process of compiling and running tests on the automated CI. label Nov 7, 2016

jdm removed the S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. label Nov 7, 2016

bors-servo merged commit 91a0c4d into servo:master Nov 7, 2016

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. 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

Uh oh!

Conversation

zakorgy commented Oct 25, 2016 • edited by larsbergstrom Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highfive commented Oct 25, 2016

Uh oh!

zakorgy commented Oct 25, 2016

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

jdm Oct 31, 2016

Choose a reason for hiding this comment

Uh oh!

jdm Oct 31, 2016

Choose a reason for hiding this comment

Uh oh!

jdm Oct 31, 2016

Choose a reason for hiding this comment

Uh oh!

jdm Oct 31, 2016

Choose a reason for hiding this comment

Uh oh!

jdm Nov 2, 2016

Choose a reason for hiding this comment

Uh oh!

jdm commented Nov 2, 2016

Uh oh!

zakorgy commented Nov 7, 2016

Uh oh!

jdm commented Nov 7, 2016

Uh oh!

bors-servo commented Nov 7, 2016

Uh oh!

bors-servo commented Nov 7, 2016

Uh oh!

bors-servo commented Nov 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zakorgy commented Oct 25, 2016 •

edited by larsbergstrom

Loading