Skip to content

Conversation

@mrobinson
Copy link
Member

@mrobinson mrobinson commented Nov 22, 2025

It used to be that the winit event loop for different platforms was
initialized differently. That isn't the case any longer, so we do not
need platform-specific code here.

In addition, I think that the slightly-odd EventsLoop name was chosen
to not conflict with winit's EventLoop. Instead I've renamed this to
ServoShellEventLoop and removed one level of nesting in the data structure,
which simplifies things a bit.

Testing: This should not change behavior, so the WPT should cover it.

@mrobinson mrobinson requested a review from atbrakhi as a code owner November 22, 2025 10:41
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 22, 2025
Copy link
Member

@mukilan mukilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

}
impl HeadedEventLoopWaker {
fn new(events_loop: &winit::event_loop::EventLoop<AppEvent>) -> HeadedEventLoopWaker {
fn new(events_loop: &EventLoop<AppEvent>) -> HeadedEventLoopWaker {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn new(events_loop: &EventLoop<AppEvent>) -> HeadedEventLoopWaker {
fn new(event_loop: &EventLoop<AppEvent>) -> HeadedEventLoopWaker {

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Nov 22, 2025
@mrobinson mrobinson force-pushed the remove-code-duplication-event-loop branch from fc5f79e to 91e03c2 Compare November 22, 2025 11:22
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 22, 2025
@mrobinson mrobinson enabled auto-merge November 22, 2025 11:22
@mrobinson mrobinson added this pull request to the merge queue Nov 22, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Nov 22, 2025
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 22, 2025
@jdm jdm added S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 22, 2025
…rvoEventLoop`

It used to be that the winit event loop for different platforms was
initialized differently. That isn't the case any longer, so we do not
need platform-specific code here.

In addition, I think that the slightly-odd `EventsLoop` name was chosen
to not conflict with winit's `EventLoop`. Instead I've renamed this to
`ServoEventLoop` and removed one level of nesting in the data structure,
which simplifies things a bit.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@mrobinson mrobinson force-pushed the remove-code-duplication-event-loop branch from 91e03c2 to 6a18a72 Compare November 22, 2025 14:41
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-rebase There are merge conflict errors. labels Nov 22, 2025
@mrobinson mrobinson enabled auto-merge November 22, 2025 14:41
@mrobinson mrobinson added this pull request to the merge queue Nov 22, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 22, 2025
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 22, 2025
@mrobinson mrobinson added this pull request to the merge queue Nov 22, 2025
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 22, 2025
Merged via the queue into servo:main with commit 1f6aa6d Nov 22, 2025
41 checks passed
@mrobinson mrobinson deleted the remove-code-duplication-event-loop branch November 22, 2025 17:06
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants