Extend WebBluetooth with discovery session and dialog

[dati91]

Thank you for contributing to Servo! Please replace each [ ] by [X] when the step is complete, and replace __ with appropriate data:

• ./mach build -d does not report any errors
• ./mach test-tidy --faster does not report any errors
• These changes fix #__ (github issue number if applicable).

Either:

• There are tests for these changes OR
• These changes do not require tests because the there are no webbluetooth tests ye.

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

---

This change is

[highfive]

Heads up! This PR modifies the following files:

• @KiChjang: components/net/bluetooth_thread.rs

[highfive]

Warning

• These commits modify net code, but no tests are modified. Please consider adding a test!

[wafflespeanut]

r? @jdm

[jdm]

-S-awaiting-review +S-needs-code-changes

Previously, Wafflespeanut (Ravi Shankar) wrote…

r? @jdm

---

Reviewed 1 of 1 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

components/net/bluetooth_thread.rs, line 32 [r1] (raw file):

const VALUE_ERROR: &'static str = "No characteristic or descriptor found with that id";
// The discovery session needs some time to find any nearby devices
const DISCOVERY_TIMEOUT: u64 = 1500;

Let's call this DISCOVERY_TIMEOUT_MS for clarity.

---

components/net/bluetooth_thread.rs, line 382 [r1] (raw file):

                                     rssi: device.get_rssi().ok().map(|p| p as i8),
                                 });
                session.map(|ref s| s.stop_discovery());

Let's use if let.

---

components/net/bluetooth_thread.rs, line 386 [r1] (raw file):

            }
        }
        session.map(|ref s| s.stop_discovery());

Could we avoid the duplication by calling stop_discovery before the for?

---

components/net/bluetooth_thread.rs, line 236 [r2] (raw file):

            // The device string format will be "Address|Name".
            // We need the first part, therefore we reverse the split and use the last item.
            return device.rsplit("|").map(|s| s.to_string()).collect::<Vec<String>>().pop();

Isn't this equivalent to calling first() on the result of the map?

---

Comments from Reviewable

[dati91]

components/net/bluetooth_thread.rs, line 236 [r2] (raw file):

Previously, jdm (Josh Matthews) wrote…

Isn't this equivalent to calling first() on the result of the map?

`first()` and `get(0)` will give you `Option<&T>`, while `pop()` will give `Option`

---

Comments from Reviewable

[highfive]

New code was committed to pull request.

[jdm]

Go ahead and squash the review comment changes into the second commit.
-S-awaiting-review +S-needs-code-changes +S-needs-squash

Previously, highfive wrote…

New code was committed to pull request.

---

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

components/net/bluetooth_thread.rs, line 236 [r2] (raw file):

Previously, dati91 (Attila Dusnoki) wrote…

first() and get(0) will give you Option<&T>, while pop() will give Option<T>

I think this would be clearer as `device.split("|").first().map(|s| s.to_string())`, in that case.

---

Comments from Reviewable

[highfive]

New code was committed to pull request.

[KiChjang]

-S-awaiting-review -S-needs-squash +S-needs-code-changes

Previously, highfive wrote…

New code was committed to pull request.

---

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions.

---

components/net/bluetooth_thread.rs, line 235 [r5] (raw file):

                                                           Some(dialog_rows)) {
            // The device string format will be "Address|Name". We need the first part of it.
            return device.split("|").collect::<Vec<&str>>().first().map(|s| s.to_string());

device.split("|").next().map(|s| s.to_string())

---

Comments from Reviewable

[highfive]

New code was committed to pull request.

[jdm]

@bors-servo: r+

[bors-servo]

📌 Commit 6048f0f has been approved by jdm

[bors-servo]

⌛ Testing commit 6048f0f with merge c4bb543...

[bors-servo]

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