Switch from rust-websocket to ws (fixes #14517) #16012

nox · 2017-03-17T14:41:15Z

This change is

highfive · 2017-03-17T14:41:17Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/lib.rs, components/script/Cargo.toml, components/script/dom/websocket.rs
@KiChjang: components/net/Cargo.toml, components/script/lib.rs, components/net/lib.rs, components/net/http_loader.rs, components/script/Cargo.toml, components/net/websocket_loader.rs, components/script/dom/websocket.rs, components/net_traits/Cargo.toml, components/net_traits/Cargo.toml, components/net_traits/lib.rs, components/net_traits/lib.rs

highfive · 2017-03-17T14:41:19Z

Warning

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

nox · 2017-03-17T14:41:21Z

@bors-servo try

bors-servo · 2017-03-17T14:41:30Z

⌛ Trying commit fcd195d with merge 0606e1f...


        Auto merge of #16012 - servo:ws, r=<try>

(Do not merge) Switch from rust-websocket to ws (fixes #14517)

bors-servo · 2017-03-17T15:04:17Z

💔 Test failed - android

nox · 2017-03-17T15:05:19Z

error[E0432]: unresolved import `libc::EPOLLRDHUP`
 --> /home/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/mio-0.6.4/src/sys/unix/epoll.rs:8:32
  |
8 | use libc::{EPOLLERR, EPOLLHUP, EPOLLRDHUP, EPOLLONESHOT};
  |                                ^^^^^^^^^^ no `EPOLLRDHUP` in the root. Did you mean to use `EPOLLHUP`?

error[E0432]: unresolved import `libc::EPOLLONESHOT`
 --> /home/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/mio-0.6.4/src/sys/unix/epoll.rs:8:44
  |
8 | use libc::{EPOLLERR, EPOLLHUP, EPOLLRDHUP, EPOLLONESHOT};
  |                                            ^^^^^^^^^^^^ no `EPOLLONESHOT` in the root

nox · 2017-03-17T15:06:19Z

Seems like this is fixed in mio 0.6.5, but I'll check the other job results before retrying.

nox · 2017-03-17T15:07:26Z

I'm also really confused as to why this didn't fail in the past.

bors-servo · 2017-03-17T15:20:41Z

☔ The latest upstream changes (presumably #16011) made this pull request unmergeable. Please resolve the merge conflicts.

nox · 2017-03-17T15:55:34Z

housleyjk/ws-rs#103

nox · 2017-03-19T13:45:55Z

@bors-servo try

bors-servo · 2017-03-19T13:46:00Z

⌛ Trying commit dc82ca5 with merge 34e99b4...


        Auto merge of #16012 - servo:ws, r=<try>

(Do not merge) Switch from rust-websocket to ws (fixes #14517)  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16012)

bors-servo · 2017-03-19T14:13:21Z

💔 Test failed - linux-rel-wpt

staabm · 2017-03-20T13:55:48Z

nit: this PR changes from servo-websocket to ws not rust-websocket as the PR-title mentions

jdm · 2017-03-20T14:06:50Z

servo-websocket is just a fork that we made last week.

nox · 2017-03-21T08:53:15Z

@bors-servo try

bors-servo · 2017-03-21T08:53:18Z

⌛ Trying commit 9c304c5 with merge 3b2cddb...


        Switch from rust-websocket to ws (fixes #14517)

WIP

nox

Addressed your comments.

nox · 2017-03-22T16:12:01Z

components/net/websocket_loader.rs

+        let protocol_in_use = try!(res.protocol());
+        if let Some(protocol_name) = protocol_in_use {
+            let protocol_name = protocol_name.to_lowercase();
+            if self.protocols.is_empty() && !self.protocols.iter().any(|p| protocol_name == (*p).to_lowercase()) {


nox · 2017-03-22T16:12:01Z

components/net/websocket_loader.rs

-    Ok((headers, sender, receiver))
+    fn on_open(&mut self, shake: Handshake) -> WebSocketResult<()> {
+        let headers = WebSocketHeaders::new(shake.response.headers().clone());
+        set_cookies_from_headers(self.resource_url, &headers.as_hyper_headers(), self.cookie_jar);


This was the reason the expectations for /websockets/cookies/007.html changed, thanks for noticing.

nox · 2017-03-22T16:12:01Z

components/net/websocket_loader.rs

+            "".to_owned()
+        } else {
+            reason.to_string()
+        };


Filed housleyjk/ws-rs#120.

nox · 2017-03-22T16:12:01Z

components/net/websocket_loader.rs

        };

-        let initiated_close = Arc::new(AtomicBool::new(false));


AFAICT, this is already handled on ws' side.

nox · 2017-03-22T16:12:40Z

This is still blocked on housleyjk/ws-rs#120.

bors-servo · 2017-03-23T00:12:52Z

☔ The latest upstream changes (presumably #16088) made this pull request unmergeable. Please resolve the merge conflicts.

nox · 2017-03-26T16:23:42Z

I'm going to switch to tungstenite instead.


        Replace servo-websocket by ws