servoshell: Extract duplicate WebDriver methods to common struct (servo#40186)

atbrakhi · web-flow · commit 33c086d8dc35 · 2025-10-29T18:58:20.000Z
There are several code duplicates in servoshell. In this PR, we extract
identical WebDriver support methods from the desktop and EGL
implementations into a shared `RunningAppStateBase` trait in the new
`running_app_state`.
This eliminates duplicate code while maintaining the same functionality
across all platforms.

This is the first step in a larger refactoring effort to consolidate
shared code between desktop and EGL implementations.

Testing: No functional changes. Existing tests should test this.

---------

Signed-off-by: atbrakhi &lt;atbrakhi@igalia.com&gt;
diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs
@@ -40,6 +40,7 @@ use crate::desktop::tracing::trace_winit_event;
 use crate::desktop::window_trait::WindowPortsMethods;
 use crate::parser::{get_default_url, location_bar_input_to_url};
 use crate::prefs::ServoShellPreferences;
+use crate::running_app_state::RunningAppStateTrait;
 
 pub struct App {
     opts: Opts,
diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs
@@ -20,8 +20,8 @@ use servo::{
     AllowOrDenyRequest, AuthenticationRequest, EmbedderControl, EmbedderControlId,
     GamepadHapticEffectType, InputEvent, InputEventId, InputEventResult, JSValue, LoadStatus,
     PermissionRequest, Servo, ServoDelegate, ServoError, SimpleDialog, TraversalId,
-    WebDriverCommandMsg, WebDriverJSResult, WebDriverLoadStatus, WebDriverSenders,
-    WebDriverUserPrompt, WebView, WebViewBuilder, WebViewDelegate,
+    WebDriverCommandMsg, WebDriverLoadStatus, WebDriverUserPrompt, WebView, WebViewBuilder,
+    WebViewDelegate,
 };
 use url::Url;
 
@@ -30,6 +30,7 @@ use super::dialog::Dialog;
 use super::gamepad::GamepadSupport;
 use super::window_trait::WindowPortsMethods;
 use crate::prefs::ServoShellPreferences;
+use crate::running_app_state::{RunningAppStateBase, RunningAppStateTrait};
 
 pub(crate) enum AppState {
     Initializing,
@@ -38,6 +39,7 @@ pub(crate) enum AppState {
 }
 
 pub(crate) struct RunningAppState {
+    base: RunningAppStateBase,
     /// A handle to the Servo instance of the [`RunningAppState`]. This is not stored inside
     /// `inner` so that we can keep a reference to Servo in order to spin the event loop,
     /// which will in turn call delegates doing a mutable borrow on `inner`.
@@ -48,7 +50,6 @@ pub(crate) struct RunningAppState {
     /// A [`Receiver`] for receiving commands from a running WebDriver server, if WebDriver
     /// was enabled.
     webdriver_receiver: Option<Receiver<WebDriverCommandMsg>>,
-    webdriver_senders: RefCell<WebDriverSenders>,
     inner: RefCell<RunningAppStateInner>,
 }
 
@@ -108,6 +109,16 @@ impl Drop for RunningAppState {
     }
 }
 
+impl RunningAppStateTrait for RunningAppState {
+    fn base(&self) -> &RunningAppStateBase {
+        &self.base
+    }
+
+    fn base_mut(&mut self) -> &mut RunningAppStateBase {
+        &mut self.base
+    }
+}
+
 impl RunningAppState {
     pub fn new(
         servo: Servo,
@@ -122,10 +133,10 @@ impl RunningAppState {
             None
         };
         RunningAppState {
+            base: RunningAppStateBase::new(),
             servo,
             servoshell_preferences,
             webdriver_receiver,
-            webdriver_senders: RefCell::default(),
             inner: RefCell::new(RunningAppStateInner {
                 webviews: HashMap::default(),
                 creation_order: Default::default(),
@@ -420,6 +431,7 @@ impl RunningAppState {
         // Dialogs block the page load, so need need to notify WebDriver
         let webview_id = webview.id();
         if let Some(sender) = self
+            .base()
             .webdriver_senders
             .borrow_mut()
             .load_status_senders
@@ -457,37 +469,6 @@ impl RunningAppState {
             .position(|webview| webview.0 == focused_id)
     }
 
-    pub(crate) fn set_pending_traversal(
-        &self,
-        traversal_id: TraversalId,
-        sender: GenericSender<WebDriverLoadStatus>,
-    ) {
-        self.webdriver_senders
-            .borrow_mut()
-            .pending_traversals
-            .insert(traversal_id, sender);
-    }
-
-    pub(crate) fn set_load_status_sender(
-        &self,
-        webview_id: WebViewId,
-        sender: GenericSender<WebDriverLoadStatus>,
-    ) {
-        self.webdriver_senders
-            .borrow_mut()
-            .load_status_senders
-            .insert(webview_id, sender);
-    }
-
-    pub(crate) fn set_script_command_interrupt_sender(
-        &self,
-        sender: Option<IpcSender<WebDriverJSResult>>,
-    ) {
-        self.webdriver_senders
-            .borrow_mut()
-            .script_evaluation_interrupt_sender = sender;
-    }
-
     /// Interrupt any ongoing WebDriver-based script evaluation.
     ///
     /// From <https://w3c.github.io/webdriver/#dfn-execute-a-function-body>:
@@ -499,6 +480,7 @@ impl RunningAppState {
     /// >  other steps of this algorithm in parallel.
     fn interrupt_webdriver_script_evaluation(&self) {
         if let Some(sender) = &self
+            .base()
             .webdriver_senders
             .borrow()
             .script_evaluation_interrupt_sender
@@ -511,13 +493,6 @@ impl RunningAppState {
         }
     }
 
-    pub(crate) fn remove_load_status_sender(&self, webview_id: WebViewId) {
-        self.webdriver_senders
-            .borrow_mut()
-            .load_status_senders
-            .remove(&webview_id);
-    }
-
     /// Return a list of all webviews that have favicons that have not yet been loaded by egui.
     pub(crate) fn take_pending_favicon_loads(&self) -> Vec<WebViewId> {
         mem::take(&mut self.inner_mut().pending_favicon_loads)
@@ -619,7 +594,7 @@ impl WebViewDelegate for RunningAppState {
     }
 
     fn notify_traversal_complete(&self, _webview: servo::WebView, traversal_id: TraversalId) {
-        let mut webdriver_state = self.webdriver_senders.borrow_mut();
+        let mut webdriver_state = self.base().webdriver_senders.borrow_mut();
         if let Entry::Occupied(entry) = webdriver_state.pending_traversals.entry(traversal_id) {
             let sender = entry.remove();
             let _ = sender.send(WebDriverLoadStatus::Complete);
@@ -713,6 +688,7 @@ impl WebViewDelegate for RunningAppState {
 
         if status == LoadStatus::Complete {
             if let Some(sender) = self
+                .base()
                 .webdriver_senders
                 .borrow_mut()
                 .load_status_senders
diff --git a/ports/servoshell/egl/app_state.rs b/ports/servoshell/egl/app_state.rs
@@ -32,6 +32,7 @@ use url::Url;
 
 use crate::egl::host_trait::HostTrait;
 use crate::prefs::ServoShellPreferences;
+use crate::running_app_state::{RunningAppStateBase, RunningAppStateTrait};
 
 #[derive(Clone, Debug)]
 pub struct Coordinates {
@@ -72,6 +73,7 @@ impl ServoWindowCallbacks {
 }
 
 pub struct RunningAppState {
+    base: RunningAppStateBase,
     servo: Servo,
     rendering_context: Rc<WindowRenderingContext>,
     callbacks: Rc<ServoWindowCallbacks>,
@@ -82,7 +84,6 @@ pub struct RunningAppState {
     /// A [`Receiver`] for receiving commands from a running WebDriver server, if WebDriver
     /// was enabled.
     webdriver_receiver: Option<Receiver<WebDriverCommandMsg>>,
-    webdriver_senders: RefCell<WebDriverSenders>,
 }
 
 struct RunningAppStateInner {
@@ -172,6 +173,7 @@ impl WebViewDelegate for RunningAppState {
 
         if load_status == LoadStatus::Complete {
             if let Some(sender) = self
+                .base()
                 .webdriver_senders
                 .borrow_mut()
                 .load_status_senders
@@ -226,7 +228,7 @@ impl WebViewDelegate for RunningAppState {
     }
 
     fn notify_traversal_complete(&self, _webview: servo::WebView, traversal_id: TraversalId) {
-        let mut webdriver_state = self.webdriver_senders.borrow_mut();
+        let mut webdriver_state = self.base().webdriver_senders.borrow_mut();
         if let std::collections::hash_map::Entry::Occupied(entry) =
             webdriver_state.pending_traversals.entry(traversal_id)
         {
@@ -367,6 +369,16 @@ impl RefreshDriver for VsyncRefreshDriver {
     }
 }
 
+impl RunningAppStateTrait for RunningAppState {
+    fn base(&self) -> &RunningAppStateBase {
+        &self.base
+    }
+
+    fn base_mut(&mut self) -> &mut RunningAppStateBase {
+        &mut self.base
+    }
+}
+
 #[allow(unused)]
 impl RunningAppState {
     pub(super) fn new(
@@ -391,13 +403,13 @@ impl RunningAppState {
         }));
 
         let app_state = Rc::new(Self {
+            base: RunningAppStateBase::new(),
             rendering_context,
             servo,
             callbacks,
             refresh_driver,
             servoshell_preferences,
             webdriver_receiver,
-            webdriver_senders: RefCell::default(),
             inner: RefCell::new(RunningAppStateInner {
                 need_present: false,
                 context_menu_sender: None,
@@ -415,26 +427,6 @@ impl RunningAppState {
         app_state
     }
 
-    pub(crate) fn set_script_command_interrupt_sender(
-        &self,
-        sender: Option<IpcSender<WebDriverJSResult>>,
-    ) {
-        self.webdriver_senders
-            .borrow_mut()
-            .script_evaluation_interrupt_sender = sender;
-    }
-
-    pub(crate) fn set_pending_traversal(
-        &self,
-        traversal_id: TraversalId,
-        sender: GenericSender<WebDriverLoadStatus>,
-    ) {
-        self.webdriver_senders
-            .borrow_mut()
-            .pending_traversals
-            .insert(traversal_id, sender);
-    }
-
     pub fn webviews(&self) -> Vec<(WebViewId, WebView)> {
         let inner = self.inner();
         inner
@@ -772,23 +764,6 @@ impl RunningAppState {
         }
     }
 
-    pub(crate) fn set_load_status_sender(
-        &self,
-        webview_id: WebViewId,
-        sender: GenericSender<WebDriverLoadStatus>,
-    ) {
-        self.webdriver_senders
-            .borrow_mut()
-            .load_status_senders
-            .insert(webview_id, sender);
-    }
-
-    pub(crate) fn remove_load_status_sender(&self, webview_id: WebViewId) {
-        self.webdriver_senders
-            .borrow_mut()
-            .load_status_senders
-            .remove(&webview_id);
-    }
     /// Touch event: press down
     pub fn touch_down(&self, x: f32, y: f32, pointer_id: i32) {
         self.active_webview()
diff --git a/ports/servoshell/lib.rs b/ports/servoshell/lib.rs
@@ -21,6 +21,7 @@ mod parser;
 mod prefs;
 #[cfg(not(any(target_os = "android", target_env = "ohos")))]
 mod resources;
+mod running_app_state;
 
 pub mod platform {
     #[cfg(target_os = "macos")]
diff --git a/ports/servoshell/running_app_state.rs b/ports/servoshell/running_app_state.rs
@@ -0,0 +1,70 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+//! Shared state and methods for desktop and EGL implementations.
+
+use std::cell::RefCell;
+
+use servo::base::generic_channel::GenericSender;
+use servo::base::id::WebViewId;
+use servo::ipc_channel::ipc::IpcSender;
+use servo::{TraversalId, WebDriverJSResult, WebDriverLoadStatus, WebDriverSenders};
+
+pub struct RunningAppStateBase {
+    pub(crate) webdriver_senders: RefCell<WebDriverSenders>,
+}
+
+impl RunningAppStateBase {
+    pub fn new() -> Self {
+        Self {
+            webdriver_senders: RefCell::default(),
+        }
+    }
+}
+
+pub trait RunningAppStateTrait {
+    fn base(&self) -> &RunningAppStateBase;
+
+    #[allow(dead_code)]
+    fn base_mut(&mut self) -> &mut RunningAppStateBase;
+
+    fn set_pending_traversal(
+        &self,
+        traversal_id: TraversalId,
+        sender: GenericSender<WebDriverLoadStatus>,
+    ) {
+        self.base()
+            .webdriver_senders
+            .borrow_mut()
+            .pending_traversals
+            .insert(traversal_id, sender);
+    }
+
+    fn set_load_status_sender(
+        &self,
+        webview_id: WebViewId,
+        sender: GenericSender<WebDriverLoadStatus>,
+    ) {
+        self.base()
+            .webdriver_senders
+            .borrow_mut()
+            .load_status_senders
+            .insert(webview_id, sender);
+    }
+
+    fn remove_load_status_sender(&self, webview_id: WebViewId) {
+        self.base()
+            .webdriver_senders
+            .borrow_mut()
+            .load_status_senders
+            .remove(&webview_id);
+    }
+
+    fn set_script_command_interrupt_sender(&self, sender: Option<IpcSender<WebDriverJSResult>>) {
+        self.base()
+            .webdriver_senders
+            .borrow_mut()
+            .script_evaluation_interrupt_sender = sender;
+    }
+}
