servoshell: Remove code duplication in EventsLoop and rename to ServoEventLoop (#40815)

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.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

ports/servoshell/desktop/app.rs

@@ -23,13 +23,14 @@ use servo::{EventLoopWaker, WebDriverCommandMsg, WebDriverUserPromptAction};
 use url::Url;
 use winit::application::ApplicationHandler;
 use winit::event::WindowEvent;
-use winit::event_loop::{ActiveEventLoop, ControlFlow};
+use winit::event_loop::{ActiveEventLoop, ControlFlow, EventLoopProxy};
 use winit::window::WindowId;
 
 use super::app_state::AppState;
-use super::events_loop::{AppEvent, EventLoopProxy, EventsLoop};
+use super::event_loop::AppEvent;
 use super::{headed_window, headless_window};
 use crate::desktop::app_state::RunningAppState;
+use crate::desktop::event_loop::ServoShellEventLoop;
 use crate::desktop::protocols;
 use crate::desktop::tracing::trace_winit_event;
 use crate::desktop::window_trait::WindowPortsMethods;
@@ -43,7 +44,7 @@ pub struct App {
     servoshell_preferences: ServoShellPreferences,
     suspended: Cell<bool>,
     waker: Box<dyn EventLoopWaker>,
-    proxy: Option<EventLoopProxy>,
+    proxy: Option<EventLoopProxy<AppEvent>>,
     initial_url: ServoUrl,
     t_start: Instant,
     t: Instant,
@@ -70,7 +71,7 @@ impl App {
         opts: Opts,
         preferences: Preferences,
         servo_shell_preferences: ServoShellPreferences,
-        events_loop: &EventsLoop,
+        event_loop: &ServoShellEventLoop,
     ) -> Self {
         let initial_url = get_default_url(
             servo_shell_preferences.url.as_deref(),
@@ -86,8 +87,8 @@ impl App {
             servoshell_preferences: servo_shell_preferences,
             suspended: Cell::new(false),
             windows: HashMap::new(),
-            waker: events_loop.create_event_loop_waker(),
-            proxy: events_loop.event_loop_proxy(),
+            waker: event_loop.create_event_loop_waker(),
+            proxy: event_loop.event_loop_proxy(),
             initial_url: initial_url.clone(),
             t_start: t,
             t,

ports/servoshell/desktop/cli.rs

@@ -5,7 +5,7 @@
 use std::{env, panic};
 
 use crate::desktop::app::App;
-use crate::desktop::events_loop::EventsLoop;
+use crate::desktop::event_loop::ServoShellEventLoop;
 use crate::panic_hook;
 use crate::prefs::{ArgumentParsingResult, parse_command_line_arguments};
 
@@ -35,8 +35,10 @@ pub fn main() {
     crate::init_tracing(servoshell_preferences.tracing_filter.as_deref());
 
     let clean_shutdown = servoshell_preferences.clean_shutdown;
-    let event_loop =
-        EventsLoop::new(servoshell_preferences.headless).expect("Failed to create events loop");
+    let event_loop = match servoshell_preferences.headless {
+        true => ServoShellEventLoop::headless(),
+        false => ServoShellEventLoop::headed(),
+    };
 
     {
         let mut app = App::new(opts, preferences, servoshell_preferences, &event_loop);

ports/servoshell/desktop/event_loop.rs

@@ -9,13 +9,10 @@ use std::time;
 
 use log::warn;
 use servo::EventLoopWaker;
-use winit::error::EventLoopError;
-use winit::event_loop::EventLoop as WinitEventLoop;
+use winit::event_loop::{EventLoop, EventLoop as WinitEventLoop, EventLoopProxy};
 
 use super::app::App;
 
-pub type EventLoopProxy = winit::event_loop::EventLoopProxy<AppEvent>;
-
 #[derive(Debug)]
 pub enum AppEvent {
     /// Another process or thread has kicked the OS event loop with EventLoopWaker.
@@ -29,70 +26,58 @@ impl From<egui_winit::accesskit_winit::Event> for AppEvent {
     }
 }
 
-/// The real or fake OS event loop.
-#[allow(dead_code)]
+/// A headed or headless event loop. Headless event loops are necessary for environments without a
+/// display server. Ideally, we could use the headed winit event loop in both modes, but on Linux,
+/// the event loop requires a display server, which prevents running servoshell in a console.
 #[allow(clippy::large_enum_variant)]
-enum EventLoop {
+pub(crate) enum ServoShellEventLoop {
     /// A real Winit windowing event loop.
-    Winit(winit::event_loop::EventLoop<AppEvent>),
+    Winit(EventLoop<AppEvent>),
     /// A fake event loop which contains a signalling flag used to ensure
     /// that pending events get processed in a timely fashion, and a condition
     /// variable to allow waiting on that flag changing state.
     Headless(Arc<(Mutex<bool>, Condvar)>),
 }
 
-pub struct EventsLoop(EventLoop);
+impl ServoShellEventLoop {
+    pub(crate) fn headless() -> ServoShellEventLoop {
+        ServoShellEventLoop::Headless(Arc::new((Mutex::new(false), Condvar::new())))
+    }
 
-impl EventsLoop {
-    // Ideally, we could use the winit event loop in both modes,
-    // but on Linux, the event loop requires a X11 server.
-    #[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
-    pub fn new(_headless: bool) -> Result<EventsLoop, EventLoopError> {
-        Ok(EventsLoop(EventLoop::Winit(
-            WinitEventLoop::with_user_event().build()?,
-        )))
-    }
-    #[cfg(any(target_os = "linux", target_os = "windows"))]
-    pub fn new(headless: bool) -> Result<EventsLoop, EventLoopError> {
-        Ok(EventsLoop(if headless {
-            EventLoop::Headless(Arc::new((Mutex::new(false), Condvar::new())))
-        } else {
-            EventLoop::Winit(WinitEventLoop::with_user_event().build()?)
-        }))
-    }
-    #[cfg(target_os = "macos")]
-    pub fn new(headless: bool) -> Result<EventsLoop, EventLoopError> {
-        Ok(EventsLoop(if headless {
-            EventLoop::Headless(Arc::new((Mutex::new(false), Condvar::new())))
-        } else {
-            EventLoop::Winit(WinitEventLoop::with_user_event().build()?)
-        }))
+    pub(crate) fn headed() -> ServoShellEventLoop {
+        ServoShellEventLoop::Winit(
+            WinitEventLoop::with_user_event()
+                .build()
+                .expect("Could not start winit event loop"),
+        )
     }
 }
 
-impl EventsLoop {
-    pub(crate) fn event_loop_proxy(&self) -> Option<EventLoopProxy> {
-        match self.0 {
-            EventLoop::Winit(ref events_loop) => Some(events_loop.create_proxy()),
-            EventLoop::Headless(..) => None,
+impl ServoShellEventLoop {
+    pub(crate) fn event_loop_proxy(&self) -> Option<EventLoopProxy<AppEvent>> {
+        match self {
+            ServoShellEventLoop::Winit(event_loop) => Some(event_loop.create_proxy()),
+            ServoShellEventLoop::Headless(..) => None,
         }
     }
 
     pub fn create_event_loop_waker(&self) -> Box<dyn EventLoopWaker> {
-        match self.0 {
-            EventLoop::Winit(ref events_loop) => Box::new(HeadedEventLoopWaker::new(events_loop)),
-            EventLoop::Headless(ref data) => Box::new(HeadlessEventLoopWaker(data.clone())),
+        match self {
+            ServoShellEventLoop::Winit(event_loop) => {
+                Box::new(HeadedEventLoopWaker::new(event_loop))
+            },
+            ServoShellEventLoop::Headless(data) => Box::new(HeadlessEventLoopWaker(data.clone())),
         }
     }
 
     pub fn run_app(self, app: &mut App) {
-        match self.0 {
-            EventLoop::Winit(events_loop) => {
-                events_loop
+        match self {
+            ServoShellEventLoop::Winit(event_loop) => {
+                event_loop
                     .run_app(app)
                     .expect("Failed while running events loop");
             },
-            EventLoop::Headless(ref data) => {
+            ServoShellEventLoop::Headless(ref data) => {
                 let (flag, condvar) = &**data;
 
                 app.init(None);
@@ -124,11 +109,11 @@ impl EventsLoop {
 }
 
 struct HeadedEventLoopWaker {
-    proxy: Arc<Mutex<winit::event_loop::EventLoopProxy<AppEvent>>>,
+    proxy: Arc<Mutex<EventLoopProxy<AppEvent>>>,
 }
 impl HeadedEventLoopWaker {
-    fn new(events_loop: &winit::event_loop::EventLoop<AppEvent>) -> HeadedEventLoopWaker {
-        let proxy = Arc::new(Mutex::new(events_loop.create_proxy()));
+    fn new(event_loop: &EventLoop<AppEvent>) -> HeadedEventLoopWaker {
+        let proxy = Arc::new(Mutex::new(event_loop.create_proxy()));
         HeadedEventLoopWaker { proxy }
     }
 }

ports/servoshell/desktop/gui.rs

@@ -22,12 +22,12 @@ use servo::{
     Image, LoadStatus, OffscreenRenderingContext, PixelFormat, PrefValue, RenderingContext, WebView,
 };
 use winit::event::{ElementState, MouseButton, WindowEvent};
-use winit::event_loop::ActiveEventLoop;
+use winit::event_loop::{ActiveEventLoop, EventLoopProxy};
 use winit::window::Window;
 
 use super::app_state::RunningAppState;
-use super::events_loop::EventLoopProxy;
 use super::geometry::winit_position_to_euclid_point;
+use crate::desktop::event_loop::AppEvent;
 use crate::prefs::{EXPERIMENTAL_PREFS, ServoShellPreferences};
 use crate::running_app_state::RunningAppStateTrait;
 
@@ -97,7 +97,7 @@ impl Gui {
     pub fn new(
         winit_window: &Window,
         event_loop: &ActiveEventLoop,
-        event_loop_proxy: EventLoopProxy,
+        event_loop_proxy: EventLoopProxy<AppEvent>,
         rendering_context: Rc<OffscreenRenderingContext>,
         initial_url: ServoUrl,
         preferences: &ServoShellPreferences,

ports/servoshell/desktop/headed_window.rs

@@ -36,7 +36,7 @@ use winit::dpi::{LogicalPosition, LogicalSize, PhysicalPosition, PhysicalSize};
 use winit::event::{
     ElementState, Ime, KeyEvent, MouseButton, MouseScrollDelta, TouchPhase, WindowEvent,
 };
-use winit::event_loop::ActiveEventLoop;
+use winit::event_loop::{ActiveEventLoop, EventLoopProxy};
 use winit::keyboard::{Key as LogicalKey, ModifiersState, NamedKey as WinitNamedKey};
 #[cfg(target_os = "linux")]
 use winit::platform::wayland::WindowAttributesExtWayland;
@@ -54,7 +54,7 @@ use super::keyutils::{CMD_OR_ALT, keyboard_event_from_winit};
 use super::window_trait::{LINE_HEIGHT, LINE_WIDTH, WindowPortsMethods};
 use crate::desktop::accelerated_gl_media::setup_gl_accelerated_media;
 use crate::desktop::app::PumpResult;
-use crate::desktop::events_loop::{AppEvent, EventLoopProxy};
+use crate::desktop::event_loop::AppEvent;
 use crate::desktop::gui::{Gui, GuiCommand};
 use crate::desktop::keyutils::CMD_OR_CONTROL;
 use crate::desktop::window_trait::MIN_WINDOW_INNER_SIZE;
@@ -108,7 +108,7 @@ impl Window {
     pub fn new(
         servoshell_preferences: &ServoShellPreferences,
         event_loop: &ActiveEventLoop,
-        event_loop_proxy: EventLoopProxy,
+        event_loop_proxy: EventLoopProxy<AppEvent>,
         initial_url: ServoUrl,
     ) -> Window {
         let no_native_titlebar = servoshell_preferences.no_native_titlebar;

ports/servoshell/desktop/headless_window.rs

@@ -128,7 +128,7 @@ impl WindowPortsMethods for Window {
     #[cfg(feature = "webxr")]
     fn new_glwindow(
         &self,
-        _events_loop: &winit::event_loop::ActiveEventLoop,
+        _event_loop: &winit::event_loop::ActiveEventLoop,
     ) -> Rc<dyn servo::webxr::glwindow::GlWindow> {
         unimplemented!()
     }

ports/servoshell/desktop/mod.rs

@@ -9,7 +9,7 @@ pub(crate) mod app;
 mod app_state;
 pub(crate) mod cli;
 mod dialog;
-pub(crate) mod events_loop;
+pub(crate) mod event_loop;
 mod gamepad;
 pub mod geometry;
 mod gui;

ports/servoshell/desktop/tracing.rs

@@ -31,7 +31,7 @@ pub(crate) trait LogTarget {
 
 mod from_winit {
     use super::LogTarget;
-    use crate::desktop::events_loop::AppEvent;
+    use crate::desktop::event_loop::AppEvent;
 
     macro_rules! target {
         ($($name:literal)+) => {

ports/servoshell/desktop/window_trait.rs

@@ -17,7 +17,7 @@ use servo::{
 use winit::event::WindowEvent;
 
 use super::app_state::RunningAppState;
-use crate::desktop::events_loop::AppEvent;
+use crate::desktop::event_loop::AppEvent;
 
 // This should vary by zoom level and maybe actual text size (focused or under cursor)
 pub(crate) const LINE_HEIGHT: f32 = 76.0;