Rename ScriptChan in constellation to EventLoop by asajeffrey · Pull Request #14260 · servo/servo

asajeffrey · 2016-11-17T14:42:06Z

We currently have a type ScriptChan in the constellation, which is named after its implementation rather than its semantics. In the spec, the nearest concept seems to be event loop https://html.spec.whatwg.org/multipage/#event-loop.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because renaming.

This change is

nox · 2016-11-17T14:42:50Z

r? @Ms2ger

KiChjang · 2016-11-17T19:32:32Z

Huh... not sure how this ties into the task queue concept as well, for which we have an assortment of task sources under script/task_sources.

asajeffrey · 2016-11-17T19:40:06Z

@KiChjang I don't think the constellation needs to know about task queues, just about event loops. Could be wrong! We do have a general issue about naming, because some things are needed in script, some in the constellation, and some in both.

KiChjang · 2016-11-17T19:44:44Z

@asajeffrey Oh, I meant that whether it'd be useful to store a Vec of TaskSources or whatnot in the event loop, as per spec.

asajeffrey · 2016-11-17T19:45:43Z

@KiChjang good question, I think not, since they're needed by script and not the constellation.

Ms2ger · 2016-11-18T17:01:30Z

I'm going to wait until #13996 lands.

bors-servo · 2016-11-21T23:22:03Z

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

bors-servo · 2016-11-23T00:01:18Z

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

bors-servo · 2016-12-01T09:37:35Z

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

Ms2ger · 2016-12-06T23:51:48Z

Rebase, make it build, and ping me :)

asajeffrey · 2016-12-07T01:15:55Z

@Ms2ger it builds again now.

bors-servo · 2016-12-14T20:14:51Z

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

jdm · 2016-12-15T19:24:05Z

@Ms2ger Review ping.

Ms2ger

r+ with nits.

Ms2ger · 2016-12-15T22:19:05Z

components/constellation/event_loop.rs

+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use ipc_channel::ipc::IpcSender;


Some module-wide docs, please.

Ms2ger · 2016-12-15T22:19:59Z

components/constellation/pipeline.rs

    fn notify_visibility(&self) {
-        self.script_chan.send(ConstellationControlMsg::ChangeFrameVisibilityStatus(self.id, self.visible))
+        self.event_loop.send(ConstellationControlMsg::ChangeFrameVisibilityStatus(self.id, self.visible))
                        .expect("Pipeline script chan");


Fix the indentation

asajeffrey · 2016-12-15T22:59:24Z

@bors-servo r=Ms2ger

bors-servo · 2016-12-15T22:59:25Z

📌 Commit e945e38 has been approved by Ms2ger

bors-servo · 2016-12-15T23:02:38Z

⌛ Testing commit e945e38 with merge 91a2e76...

Rename ScriptChan in constellation to EventLoop  We currently have a type `ScriptChan` in the constellation, which is named after its implementation rather than its semantics. In the spec, the nearest concept seems to be event loop https://html.spec.whatwg.org/multipage/#event-loop. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because renaming.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14260)

bors-servo · 2016-12-15T23:54:37Z

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

asajeffrey added A-constellation Involves the constellation I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring. labels Nov 17, 2016

highfive assigned metajack Nov 17, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 17, 2016

highfive assigned Ms2ger and unassigned metajack Nov 17, 2016

highfive added the S-needs-rebase There are merge conflict errors. label Nov 21, 2016

asajeffrey force-pushed the constellation-event-loops branch from cae98f4 to 46a666b Compare November 21, 2016 23:39

asajeffrey removed the S-needs-rebase There are merge conflict errors. label Nov 21, 2016

highfive added the S-needs-rebase There are merge conflict errors. label Nov 23, 2016

asajeffrey force-pushed the constellation-event-loops branch from 46a666b to 4ae4fb0 Compare November 23, 2016 15:18

asajeffrey removed the S-needs-rebase There are merge conflict errors. label Nov 23, 2016

highfive added the S-needs-rebase There are merge conflict errors. label Dec 1, 2016

asajeffrey force-pushed the constellation-event-loops branch from 4ae4fb0 to cdfe6da Compare December 3, 2016 17:43

asajeffrey removed the S-needs-rebase There are merge conflict errors. label Dec 3, 2016

Ms2ger added S-fails-travis and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 6, 2016

asajeffrey force-pushed the constellation-event-loops branch from cdfe6da to 3f4e52e Compare December 7, 2016 01:12

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 7, 2016

jdm removed the S-fails-travis label Dec 7, 2016

asajeffrey force-pushed the constellation-event-loops branch from 3f4e52e to b634987 Compare December 7, 2016 01:39

highfive added the S-needs-rebase There are merge conflict errors. label Dec 14, 2016

asajeffrey force-pushed the constellation-event-loops branch from b634987 to 5e9b35e Compare December 15, 2016 01:14

asajeffrey removed the S-needs-rebase There are merge conflict errors. label Dec 15, 2016

Ms2ger reviewed Dec 15, 2016

View reviewed changes

Renamed ScriptChan in constellation to EventLoop.

e945e38

asajeffrey force-pushed the constellation-event-loops branch from 5e9b35e to e945e38 Compare December 15, 2016 22:58

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 15, 2016

bors-servo merged commit e945e38 into servo:master Dec 15, 2016

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 15, 2016

bors-servo mentioned this pull request Dec 15, 2016

Implement browsing context discarding #14559

Merged

4 tasks

Uh oh!

Conversation

asajeffrey commented Nov 17, 2016 • edited by larsbergstrom Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nox commented Nov 17, 2016

Uh oh!

KiChjang commented Nov 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asajeffrey commented Nov 17, 2016

Uh oh!

KiChjang commented Nov 17, 2016

Uh oh!

asajeffrey commented Nov 17, 2016

Uh oh!

Ms2ger commented Nov 18, 2016

Uh oh!

bors-servo commented Nov 21, 2016

Uh oh!

bors-servo commented Nov 23, 2016

Uh oh!

bors-servo commented Dec 1, 2016

Uh oh!

Ms2ger commented Dec 6, 2016

Uh oh!

asajeffrey commented Dec 7, 2016

Uh oh!

bors-servo commented Dec 14, 2016

Uh oh!

jdm commented Dec 15, 2016

Uh oh!

Ms2ger left a comment

Choose a reason for hiding this comment

Uh oh!

Ms2ger Dec 15, 2016

Choose a reason for hiding this comment

Uh oh!

asajeffrey Dec 15, 2016

Choose a reason for hiding this comment

Uh oh!

Ms2ger Dec 15, 2016

Choose a reason for hiding this comment

Uh oh!

asajeffrey Dec 15, 2016

Choose a reason for hiding this comment

Uh oh!

asajeffrey commented Dec 15, 2016

Uh oh!

bors-servo commented Dec 15, 2016

Uh oh!

bors-servo commented Dec 15, 2016

Uh oh!

bors-servo commented Dec 15, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

asajeffrey commented Nov 17, 2016 •

edited by larsbergstrom

Loading

KiChjang commented Nov 17, 2016 •

edited

Loading