Rename ScriptChan in constellation to EventLoop #14260

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

            warn!("Sending mozbrowser event to script failed ({}).", e);
        }
    }

    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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename ScriptChan in constellation to EventLoop #14260

Rename ScriptChan in constellation to EventLoop #14260

asajeffrey commented Nov 17, 2016 •

edited by larsbergstrom

nox commented Nov 17, 2016

KiChjang commented Nov 17, 2016 •

edited

asajeffrey commented Nov 17, 2016

KiChjang commented Nov 17, 2016

asajeffrey commented Nov 17, 2016

Ms2ger commented Nov 18, 2016

bors-servo commented Nov 21, 2016

bors-servo commented Nov 23, 2016

bors-servo commented Dec 1, 2016

Ms2ger commented Dec 6, 2016

asajeffrey commented Dec 7, 2016

bors-servo commented Dec 14, 2016

jdm commented Dec 15, 2016

Ms2ger left a comment

Ms2ger Dec 15, 2016

asajeffrey Dec 15, 2016

Ms2ger Dec 15, 2016

asajeffrey Dec 15, 2016

asajeffrey commented Dec 15, 2016

bors-servo commented Dec 15, 2016

bors-servo commented Dec 15, 2016

bors-servo commented Dec 15, 2016

Rename ScriptChan in constellation to EventLoop #14260

Rename ScriptChan in constellation to EventLoop #14260

Conversation

asajeffrey commented Nov 17, 2016 • edited by larsbergstrom

nox commented Nov 17, 2016

KiChjang commented Nov 17, 2016 • edited

asajeffrey commented Nov 17, 2016

KiChjang commented Nov 17, 2016

asajeffrey commented Nov 17, 2016

Ms2ger commented Nov 18, 2016

bors-servo commented Nov 21, 2016

bors-servo commented Nov 23, 2016

bors-servo commented Dec 1, 2016

Ms2ger commented Dec 6, 2016

asajeffrey commented Dec 7, 2016

bors-servo commented Dec 14, 2016

jdm commented Dec 15, 2016

Ms2ger left a comment

Choose a reason for hiding this comment

Ms2ger Dec 15, 2016

Choose a reason for hiding this comment

asajeffrey Dec 15, 2016

Choose a reason for hiding this comment

Ms2ger Dec 15, 2016

Choose a reason for hiding this comment

asajeffrey Dec 15, 2016

Choose a reason for hiding this comment

asajeffrey commented Dec 15, 2016

bors-servo commented Dec 15, 2016

bors-servo commented Dec 15, 2016

bors-servo commented Dec 15, 2016

asajeffrey commented Nov 17, 2016 •

edited by larsbergstrom

KiChjang commented Nov 17, 2016 •

edited