Annotations for WebBluetooth webidl functions #57

zakorgy · 2016-08-15T13:32:22Z

Step annotations for the WebBluetooth webidl functions, excluding requestDevice and unimplmeneted methods.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix Step annotations for files related to WebBluetooth servo/servo#12614

dati91 · 2016-08-16T08:59:06Z

components/net/bluetooth_thread.rs

        if !self.allowed_services.get(&device_id).map_or(false, |s| s.contains(&uuid)) {
            return drop(sender.send(Err(BluetoothError::Security)));
        }
+
+        // Step 2.4.


you lost me here :(

The idea is this will fix the bad behavior of the bloom filter in parallel traversal.

Move to Cargo workspaces  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14381)

Stylo: implement column-rule-style Implemented `column-rule-style` in stylo. - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix servo#13868 - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a style addition and I added the screenshot. Screenshot: ![screenshot from 2016-11-27 21-59-38](https://cloud.githubusercontent.com/assets/597469/20652731/0a7ccbfc-b4ff-11e6-8e4c-1547897bcf25.png)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14383)

dati91 · 2016-11-28T13:44:57Z

components/bluetooth/lib.rs

@@ -754,6 +782,10 @@ impl BluetoothManager {
            None => return drop(sender.send(Err(BluetoothError::NotFound))),
        };
        let services = primary_service.get_includes(device).unwrap_or(vec!());
+
+        // TODO: Step 7: If its argument is empty, throws a NotFoundError.


hmm how do we missed this step? we don't have a test for this or what?

I think this step here is just an early return, if the services vec is empty, the function will return still with a NotFoundError at the end of the function.
In other functions e.g. in get_primary_service there is a check here too, I just missed that step here.

dati91 · 2016-11-28T13:45:14Z

components/bluetooth/lib.rs

@@ -789,6 +823,10 @@ impl BluetoothManager {
            None => return drop(sender.send(Err(BluetoothError::NotFound))),
        };
        let services = primary_service.get_includes(device).unwrap_or(vec!());
+
+        // TODO: Step 7: If its argument is empty, throws a NotFoundError.


Same as above

dati91 · 2016-11-28T13:48:47Z

components/bluetooth/lib.rs

        match result {
            Some(v) => match v {
+                // (Characteristic) Step 7.5.3.
+                // (Descriptor) Step 7.4.3.
+                // Note: There is no need to send back the value, because both algorithm returns with undefined.


Yes we do, because we need to set the new value here.

dati91 · 2016-11-28T13:52:38Z

components/script/dom/bluetooth.rs

            name_prefix.to_string()
        },
        None => String::new(),
    };

-    // Step 2.4.6 - 2.4.7
+    // Step 6 -7.


missing whitespace

dati91 · 2016-11-28T13:56:53Z

components/script/dom/bluetoothremotegattcharacteristic.rs

+        // TODO: Step 5: Implement the `connection-checking-wrapper` algorithm for BluetoothRemoteGATTServer.
+
+        // Step 5.1.
+        // Note: May make this check in the lower level too.


Because the properties link in ReadValue Step 5.1. points here not to the BluetoothCharacteristicProperties

dati91 · 2016-11-28T13:58:21Z

components/script/dom/bluetoothremotegattcharacteristic.rs

+                // TODO: Step 5.5.1: Implement activeAlgorithms internal slot for BluetoothRemoteGATTServer.
+
+                // Step 5.5.2.
+                // TODO(#5014): Replace ByteString with ArrayBuffer when ArrayBuffer is implemented.


Replace ByteString with ArrayBuffer when it is implemented.

dati91 · 2016-11-28T14:04:35Z

components/script/dom/bluetoothremotegattcharacteristic.rs

+                // TODO: Step 7.5.1: Implement activeAlgorithms internal slot for BluetoothRemoteGATTServer.
+
+                // Step 7.5.2.
+                // TODO(#5014): Replace ByteString with a DataView wrapping a new ArrayBuffer


needs wording

Fix a typo in components/bluetooth_traits/lib.rs  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14392)

@SimonSapin

Fix the bloom filter stuff.  I think I got the numbers right, want to do a try run before just in case. r? @SimonSapin  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14353)

… uploads as an artefact

@metajack

…ergstrom,jdm Add ./mach filter-intermittents for catching intermittents on CI cc @metajack The plan here is to run this on the error summary of the logs after each WPT/CSS run, as `./mach filter-intermittents wpt-errorsummary.log --output filtered-errorsummary.log --auth /path/to/authfile` The `filtered-errorsummary.log` file will be a buildbot artifact for this run, and can be used for updating test results. We change WPT/CSS runs to not cause test failures on Buildbot; instead; the test failure will be caused by this job. We should at some point add a separate highfive task which ccs the appropriate bugs and tracks consistent failures. (We can change mach filter-intermittents to output a second file of intermittents matched to bug numbers to make this easy) A small issue with this is that this simple task might mask failures of the WPT harness itself. We have a separate test that tests if the test harness works though. (This test will fail if the log files it requires don't exist, perhaps that's enough to solve the problem) r? @larsbergstrom  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14331)

@KiChjang

Add a log of only intermittents r? @KiChjang  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14400)

@aneeshusa

…,aneeshusa,jgraham Use Selenium for Gecko performance test  We run the `etc/ci/performance` test on Gecko for comparison, but the old add-on approach doesn't work anymore. I'm using selenium to drive Gecko instead. Also did some refactoring and fixed some nits I saw along the way. r? @aneeshusa ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix servo#14148 (github issue number if applicable).  - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14194)

Add event target for bluetooth  Add support for event listeners in webbluetooth. ---  - [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   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14387)

dati91 reviewed Aug 16, 2016
View reviewed changes

shinglyu and others added 12 commits November 14, 2016 11:43

Use selenium to drive gecko test

6110017

Various easy nits

d8b862e

Space nit

9f0b685

Remove a level of unnessary function call

d9a7221

Use resource manager in gecko_driver and nits

ecfabb6

Add ./mach filter-intermittents for catching intermittents on CI

4b4421c

style: Introduce StyleBloom

84a50ed

The idea is this will fix the bad behavior of the bloom filter in parallel traversal.

style: Enable the bloom filter recovering.

de1a3d8

Implemented colum-rule-style

255d497

Move to Cargo workspaces

dfb35db

Add event target for bluetooth

d9c7ffb

zakorgy force-pushed the annotations branch from 68dd438 to c86e572 Compare November 28, 2016 10:11

dati91 reviewed Nov 28, 2016

View reviewed changes

Zakor Gyula and others added 3 commits November 28, 2016 15:49

Fix a typo in bluetooth_traits/lib.rs

f416759

Run filter-intermittents on buildbot

4f021d3

bors-servo and others added 18 commits November 28, 2016 10:30

Use jdm's tracker

c0c65ea

Use log- prefix for filtered log arg so that buildbot picks it up and…

f42ce6d

… uploads as an artefact

Add a log of only intermittents

c23ea67

Move convert_request_device_options into request_bluetooth_devices.

809d461

Update requestDevice step annotations.

80bdf0c

Gatt function

024ce04

GATTServer functions

9a14eb6

Service functions

ed30495

Characteristic functions

3a5acb7

Descriptor functions

0d0e110

fixup! Update requestDevice step annotations.

11ca3fb

fixup! Descriptor functions

c9a4104

fixup! Characteristic functions

dedceeb

zakorgy force-pushed the annotations branch from c86e572 to dedceeb Compare November 29, 2016 08:54

zakorgy merged commit dedceeb into szeged:annotations Nov 29, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Annotations for WebBluetooth webidl functions #57

Annotations for WebBluetooth webidl functions #57

zakorgy commented Aug 15, 2016

dati91 Aug 16, 2016

dati91 Nov 28, 2016

zakorgy Nov 28, 2016

dati91 Nov 28, 2016

dati91 Nov 28, 2016

dati91 Nov 28, 2016

dati91 Nov 28, 2016

zakorgy Nov 28, 2016

dati91 Nov 28, 2016

dati91 Nov 28, 2016

Annotations for WebBluetooth webidl functions #57

Annotations for WebBluetooth webidl functions #57

Conversation

zakorgy commented Aug 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment