Finish up the implementation of EventSource #13774

KiChjang · 2016-10-14T19:20:50Z

Full implementation of EventSource, complete with closing and reopening streams.

Fixes #8925.

This change is

highfive · 2016-10-14T19:20:55Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/htmllinkelement.rs, components/script/dom/bindings/trace.rs, components/script/fetch.rs, components/script/dom/globalscope.rs, components/script/dom/xmlhttprequest.rs, components/script/network_listener.rs, components/script/dom/workerglobalscope.rs, components/script/task_source/networking.rs, components/script/dom/websocket.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/htmlmediaelement.rs, components/script/dom/htmlimageelement.rs, components/script/dom/webidls/EventSource.webidl, components/script/dom/eventsource.rs, components/script/dom/window.rs, components/script/script_thread.rs
@KiChjang: components/script/dom/htmllinkelement.rs, components/script/dom/bindings/trace.rs, components/script/fetch.rs, components/script/dom/globalscope.rs, components/script/dom/xmlhttprequest.rs, components/script/network_listener.rs, components/script/dom/workerglobalscope.rs, components/script/task_source/networking.rs, components/script/dom/websocket.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/htmlmediaelement.rs, components/script/dom/htmlimageelement.rs, components/script/dom/webidls/EventSource.webidl, components/script/dom/eventsource.rs, components/script/dom/window.rs, components/script/script_thread.rs, components/net_traits/request.rs, components/net_traits/request.rs

highfive · 2016-10-14T19:20:56Z

Warning

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

KiChjang · 2016-10-14T19:21:03Z

@bors-servo try

bors-servo · 2016-10-14T19:21:07Z

⌛ Trying commit 650383d with merge b3f92ec...


        Auto merge of #13774 - KiChjang:event-source-constructor, r=<try>

Implement EventSource constructor, parse metadata in EventSource response It still doesn't parse the event stream correctly, but it does announce and fail the connection correctly. Re-establishing the connection is not supported yet. Partial #8925.

bors-servo · 2016-10-14T20:09:17Z

💔 Test failed - linux-rel-wpt

KiChjang · 2016-10-14T20:16:19Z

I believe there's still too many timeouts in EventSource and it's not worthwhile to enable it just yet.

SimonSapin · 2016-10-17T12:40:45Z

r? @frewsxcv

frewsxcv · 2016-10-17T12:58:40Z

./components/script/task_source/networking.rs:19: tab on line

frewsxcv · 2016-10-17T13:00:43Z

components/script/dom/eventsource.rs

 #[derive(JSTraceable, PartialEq, Copy, Clone, Debug, HeapSizeOf)]
-enum EventSourceReadyState {
+enum ReadyState {


Can you add a comment here?

// https://html.spec.whatwg.org/multipage/#dom-eventsource-readystate

frewsxcv · 2016-10-17T13:14:54Z

I can look through this, though I don't have much time over the next few days to do an in-depth review of this. Might be better to find someone else for the r+.

Is it true that there are not test expectation differences or have they not been updated?

KiChjang · 2016-10-17T17:51:21Z

@frewsxcv They haven't been updated yet, since I'm not entirely sure that it's worth enabling the tests again due to the amount of expected timeouts in the tests.

frewsxcv · 2016-10-17T20:11:17Z

Considering websockets will be partially supported after this PR, would it make sense to enable them?

jdm · 2016-10-18T04:54:39Z

How are websockets related to EventSource? We already support websockets in Servo.

frewsxcv · 2016-10-18T13:02:13Z

Blah, my bad, I was thinking "SSE" but said "websockets".

KiChjang · 2016-10-27T09:14:54Z

@bors-servo try

bors-servo · 2016-10-27T09:15:00Z

⌛ Trying commit 555d576 with merge c6338be...


        Auto merge of #13774 - KiChjang:event-source-constructor, r=<try>

Implement EventSource constructor, parse metadata in EventSource response It still doesn't parse the event stream correctly, but it does announce and fail the connection correctly. Re-establishing the connection is not supported yet. Partial #8925.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13774)

bors-servo · 2016-10-27T09:31:28Z

💔 Test failed - linux-rel-css

bors-servo · 2016-11-10T01:10:18Z

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

KiChjang · 2016-11-10T21:45:45Z

Crap, something went wrong during rebase, will fix soon.

bors-servo · 2016-11-10T22:56:42Z

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


        Implement the constructor for EventSource


        Properly implement TaskSource for NetworkingTaskSource


        Parse metadata in EventSource


        Remove preferences in EventSource.webidl


        Implement reestablishing the connection for EventSource

And remove unneeded fields in EventSourceContext


        Add GenerationId to EventSource


        Interpret event stream


        Use a timer callback when re-establishing a connection


        Update WPT expectations

jdm · 2016-11-12T14:08:34Z

@bors-servo: r+

bors-servo · 2016-11-12T14:08:37Z

📌 Commit a39d1fa has been approved by jdm

bors-servo · 2016-11-12T14:08:39Z

⌛ Testing commit a39d1fa with merge 579ab2d...


        Auto merge of #13774 - KiChjang:event-source-constructor, r=jdm

Finish up the implementation of EventSource Full implementation of EventSource, complete with closing and reopening streams. Fixes #8925.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13774)

bors-servo · 2016-11-12T15:08:04Z

☀️ 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 14, 2016

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

highfive added the S-tests-failed label Oct 14, 2016

KiChjang force-pushed the KiChjang:event-source-constructor branch from 650383d to c81ef97 Oct 15, 2016

highfive removed the S-tests-failed label Oct 15, 2016

KiChjang force-pushed the KiChjang:event-source-constructor branch from 1446403 to 555d576 Oct 16, 2016

highfive assigned frewsxcv and unassigned SimonSapin Oct 17, 2016

frewsxcv reviewed Oct 17, 2016

View changes

highfive added the S-tests-failed label Oct 27, 2016

KiChjang force-pushed the KiChjang:event-source-constructor branch from 555d576 to d6bfc10 Oct 27, 2016

highfive removed the S-tests-failed label Oct 27, 2016

highfive added the S-needs-rebase label Nov 10, 2016

KiChjang force-pushed the KiChjang:event-source-constructor branch from a64cdd7 to 7a9097f Nov 10, 2016

highfive added the S-needs-rebase label Nov 10, 2016

KiChjang force-pushed the KiChjang:event-source-constructor branch from 7a9097f to 501175d Nov 11, 2016

KiChjang removed the S-needs-rebase label Nov 11, 2016

KiChjang force-pushed the KiChjang:event-source-constructor branch from 501175d to 4018b4b Nov 11, 2016

KiChjang added 9 commits Oct 5, 2016

Implement the constructor for EventSource

d99d26c

Properly implement TaskSource for NetworkingTaskSource

72cb856

Parse metadata in EventSource

388c3bf

Remove preferences in EventSource.webidl

9afc17c

Implement reestablishing the connection for EventSource

2924726

And remove unneeded fields in EventSourceContext

Add GenerationId to EventSource

c198bfa

Interpret event stream

0b32b62

Use a timer callback when re-establishing a connection

a5c2c0b

Update WPT expectations

Loading status checks…

a39d1fa

KiChjang force-pushed the KiChjang:event-source-constructor branch from 4018b4b to a39d1fa Nov 11, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Nov 12, 2016

bors-servo merged commit a39d1fa into servo:master Nov 12, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

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

homu Test successful
Details

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

bors-servo mentioned this pull request Nov 12, 2016

Implement synchronous about:blank loading. #13996

Merged

KiChjang deleted the KiChjang:event-source-constructor branch Nov 12, 2016

servo / servo

Finish up the implementation of EventSource #13774

Finish up the implementation of EventSource #13774

KiChjang commented Oct 14, 2016 •

edited

highfive commented Oct 14, 2016

highfive commented Oct 14, 2016

KiChjang commented Oct 14, 2016

bors-servo commented Oct 14, 2016

bors-servo commented Oct 14, 2016

KiChjang commented Oct 14, 2016

SimonSapin commented Oct 17, 2016

frewsxcv commented Oct 17, 2016

This comment has been minimized.

frewsxcv commented Oct 17, 2016

KiChjang commented Oct 17, 2016

frewsxcv commented Oct 17, 2016

jdm commented Oct 18, 2016

frewsxcv commented Oct 18, 2016

KiChjang commented Oct 27, 2016

bors-servo commented Oct 27, 2016

bors-servo commented Oct 27, 2016

bors-servo commented Nov 10, 2016

KiChjang commented Nov 10, 2016

bors-servo commented Nov 10, 2016

jdm commented Nov 12, 2016

bors-servo commented Nov 12, 2016

bors-servo commented Nov 12, 2016

bors-servo commented Nov 12, 2016

servo / servo

Join GitHub today

Finish up the implementation of EventSource #13774

Finish up the implementation of EventSource #13774

Conversation

KiChjang commented Oct 14, 2016 • edited

highfive commented Oct 14, 2016

highfive commented Oct 14, 2016

KiChjang commented Oct 14, 2016

bors-servo commented Oct 14, 2016

bors-servo commented Oct 14, 2016

KiChjang commented Oct 14, 2016

SimonSapin commented Oct 17, 2016

frewsxcv commented Oct 17, 2016

This comment has been minimized.

frewsxcv Oct 17, 2016 • edited

frewsxcv commented Oct 17, 2016

KiChjang commented Oct 17, 2016

frewsxcv commented Oct 17, 2016

jdm commented Oct 18, 2016

frewsxcv commented Oct 18, 2016

KiChjang commented Oct 27, 2016

bors-servo commented Oct 27, 2016

bors-servo commented Oct 27, 2016

bors-servo commented Nov 10, 2016

KiChjang commented Nov 10, 2016

bors-servo commented Nov 10, 2016

jdm commented Nov 12, 2016

bors-servo commented Nov 12, 2016

bors-servo commented Nov 12, 2016

bors-servo commented Nov 12, 2016

KiChjang commented Oct 14, 2016 •

edited

frewsxcv Oct 17, 2016 •

edited