minibrowser: Add loading spinner (#31713)

* minibrowser: Rename "history_changed" flag to "need_update"

There are other data points in the toolbar that might need to be
updated. This commit prepares for that by generaliziing the
"history_changed" flag to a more generic "need_update" flag.

Signed-off-by: Frederik Reiter <hi@frereit.de>

* minibrowser: Add spinner to indicate loading status of the webview

Signed-off-by: Frederik Reiter <hi@frereit.de>

---------

Signed-off-by: Frederik Reiter <hi@frereit.de>

ports/servoshell/app.rs

@@ -51,7 +51,7 @@ enum PumpResult {
     /// The caller should shut down Servo and its related context.
     Shutdown,
     Continue {
-        history_changed: bool,
+        update: bool,
         present: Present,
     },
 }
@@ -298,14 +298,11 @@ impl App {
                         minibrowser.context.destroy();
                     }
                 },
-                PumpResult::Continue {
+                PumpResult::Continue { update, present } => {
-                    history_changed,
+                    if update {
-                    present,
-                } => {
-                    if history_changed {
                         if let Some(mut minibrowser) = app.minibrowser() {
                             let webviews = &mut app.webviews.borrow_mut();
-                            if minibrowser.update_location_in_toolbar(webviews) {
+                            if minibrowser.update_webview_data(webviews) {
                                 // Update the minibrowser immediately. While we could update by requesting a
                                 // redraw, doing so would delay the location update by two frames.
                                 minibrowser.update(
@@ -432,12 +429,12 @@ impl App {
         let mut embedder_messages = self.servo.as_mut().unwrap().get_events();
         let mut need_resize = false;
         let mut need_present = false;
-        let mut history_changed = false;
+        let mut need_update = false;
         loop {
             // Consume and handle those embedder messages.
             let servo_event_response = webviews.handle_servo_events(embedder_messages);
             need_present |= servo_event_response.need_present;
-            history_changed |= servo_event_response.history_changed;
+            need_update |= servo_event_response.need_update;
 
             // Route embedder events from the WebViewManager to the relevant Servo components,
             // receives and collects embedder messages from various Servo components,
@@ -467,7 +464,7 @@ impl App {
         };
 
         PumpResult::Continue {
-            history_changed,
+            update: need_update,
             present,
         }
     }

ports/servoshell/minibrowser.rs

@@ -7,7 +7,7 @@ use std::num::NonZeroU32;
 use std::sync::Arc;
 use std::time::Instant;
 
-use egui::{CentralPanel, Frame, Key, Modifiers, PaintCallback, TopBottomPanel};
+use egui::{CentralPanel, Color32, Frame, Key, Modifiers, PaintCallback, Spinner, TopBottomPanel};
 use egui_glow::CallbackFn;
 use egui_winit::EventResponse;
 use euclid::{Length, Point2D, Scale};
@@ -24,7 +24,7 @@ use crate::egui_glue::EguiGlow;
 use crate::events_loop::EventsLoop;
 use crate::geometry::winit_position_to_euclid_point;
 use crate::parser::location_bar_input_to_url;
-use crate::webview::WebViewManager;
+use crate::webview::{LoadStatus, WebViewManager};
 use crate::window_trait::WindowPortsMethods;
 
 pub struct Minibrowser {
@@ -42,6 +42,8 @@ pub struct Minibrowser {
 
     /// Whether the location has been edited by the user without clicking Go.
     location_dirty: Cell<bool>,
+
+    load_status: LoadStatus,
 }
 
 pub enum MinibrowserEvent {
@@ -83,6 +85,7 @@ impl Minibrowser {
             last_mouse_position: None,
             location: RefCell::new(initial_url.to_string()),
             location_dirty: false.into(),
+            load_status: LoadStatus::LoadComplete,
         }
     }
 
@@ -162,6 +165,16 @@ impl Minibrowser {
                                     location_dirty.set(false);
                                 }
 
+                                match self.load_status {
+                                    LoadStatus::LoadStart => {
+                                        ui.add(Spinner::new().color(Color32::GRAY));
+                                    },
+                                    LoadStatus::HeadParsed => {
+                                        ui.add(Spinner::new().color(Color32::WHITE));
+                                    },
+                                    LoadStatus::LoadComplete => { /* No Spinner */ },
+                                }
+
                                 let location_field = ui.add_sized(
                                     ui.available_size(),
                                     egui::TextEdit::singleline(&mut *location.borrow_mut()),
@@ -314,4 +327,28 @@ impl Minibrowser {
             _ => false,
         }
     }
+
+    /// Updates the spinner from the given [WebViewManager], returning true iff it has changed
+    /// (needing an egui update).
+    pub fn update_spinner_in_toolbar(
+        &mut self,
+        browser: &mut WebViewManager<dyn WindowPortsMethods>,
+    ) -> bool {
+        let need_update = browser.load_status() != self.load_status;
+        self.load_status = browser.load_status();
+        return need_update;
+    }
+
+    /// Updates all fields taken from the given [WebViewManager], such as the location field.
+    /// Returns true iff the egui needs an update.
+    pub fn update_webview_data(
+        &mut self,
+        browser: &mut WebViewManager<dyn WindowPortsMethods>,
+    ) -> bool {
+        // Note: We must use the "bitwise OR" (|) operator here instead of "logical OR" (||)
+        //       because logical OR would short-circuit if any of the functions return true.
+        //       We want to ensure that all functions are called. The "bitwise OR" operator
+        //       does not short-circuit.
+        return self.update_location_in_toolbar(browser) | self.update_spinner_in_toolbar(browser);
+    }
 }

ports/servoshell/webview.rs

@@ -56,6 +56,7 @@ pub struct WebViewManager<Window: WindowPortsMethods + ?Sized> {
     clipboard: Option<Clipboard>,
     gamepad: Option<Gilrs>,
     shutdown_requested: bool,
+    load_status: LoadStatus,
 }
 
 #[derive(Debug)]
@@ -63,7 +64,14 @@ pub struct WebView {}
 
 pub struct ServoEventResponse {
     pub need_present: bool,
-    pub history_changed: bool,
+    pub need_update: bool,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum LoadStatus {
+    HeadParsed,
+    LoadStart,
+    LoadComplete,
 }
 
 impl<Window> WebViewManager<Window>
@@ -95,6 +103,7 @@ where
             },
             event_queue: Vec::new(),
             shutdown_requested: false,
+            load_status: LoadStatus::LoadComplete,
         }
     }
 
@@ -106,6 +115,10 @@ where
         self.current_url_string.as_deref()
     }
 
+    pub fn load_status(&self) -> LoadStatus {
+        self.load_status
+    }
+
     pub fn get_events(&mut self) -> Vec<EmbedderEvent> {
         std::mem::take(&mut self.event_queue)
     }
@@ -409,8 +422,8 @@ where
         &mut self,
         events: Drain<'_, (Option<WebViewId>, EmbedderMsg)>,
     ) -> ServoEventResponse {
-        let mut need_present = false;
+        let mut need_present = self.load_status != LoadStatus::LoadComplete;
-        let mut history_changed = false;
+        let mut need_update = false;
         for (webview_id, msg) in events {
             if let Some(webview_id) = webview_id {
                 trace_embedder_msg!(msg, "{webview_id} {msg:?}");
@@ -594,21 +607,24 @@ where
                     // FIXME: show favicons in the UI somehow
                 },
                 EmbedderMsg::HeadParsed => {
-                    // FIXME: surface the loading state in the UI somehow
+                    self.load_status = LoadStatus::HeadParsed;
+                    need_update = true;
                 },
                 EmbedderMsg::HistoryChanged(urls, current) => {
                     self.current_url = Some(urls[current].clone());
                     self.current_url_string = Some(urls[current].clone().into_string());
-                    history_changed = true;
+                    need_update = true;
                 },
                 EmbedderMsg::SetFullscreenState(state) => {
                     self.window.set_fullscreen(state);
                 },
                 EmbedderMsg::LoadStart => {
-                    // FIXME: surface the loading state in the UI somehow
+                    self.load_status = LoadStatus::LoadStart;
+                    need_update = true;
                 },
                 EmbedderMsg::LoadComplete => {
-                    // FIXME: surface the loading state in the UI somehow
+                    self.load_status = LoadStatus::LoadComplete;
+                    need_update = true;
                 },
                 EmbedderMsg::Shutdown => {
                     self.shutdown_requested = true;
@@ -680,7 +696,7 @@ where
 
         ServoEventResponse {
             need_present,
-            history_changed,
+            need_update,
         }
     }
 }