From 18fe57ddc65f2b0d53ae4b1363a499216493ac50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 10 Jul 2026 16:28:06 -0400 Subject: [PATCH] fix(servo): reconcile the loading state through redirects and pushState --- crates/ely_servo_host/src/runtime.rs | 13 ++++++- crates/ely_servo_host/src/runtime_webview.rs | 38 ++++++++++++++++++-- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index cc0d6cc..41e43c1 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -192,9 +192,14 @@ impl ServoHost for SoftwareServoHost { .ok_or_else(|| ServoHostError::WebViewNotFound { id: request.webview_id.clone() })?; let requested_url = url.to_string(); + let current_url = webview.current_url(); let has_loaded_page = - matches!(webview.current_url().as_deref(), Some(value) if value != "about:blank"); + matches!(current_url.as_deref(), Some(value) if value != "about:blank"); let should_create_initial_document = webview.requested_url.is_none() && !has_loaded_page; + // A same-URL reload does not fire a URL change, so only a navigation + // to a different URL arms the pending-navigation hold; reloads defer + // to Servo's own load-status transitions. + let navigates_to_new_url = current_url.as_deref() != Some(requested_url.as_str()); webview.delegate.set_state(WebViewState::Loading); if should_create_initial_document { @@ -212,6 +217,12 @@ impl ServoHost for SoftwareServoHost { } else { webview.webview.load(url); } + // Hold `Loading` until Servo reports the URL actually changed, so a + // redirect reconciles instead of pinning the surface (a stale + // previous-page `Complete` cannot clear it). + if navigates_to_new_url { + webview.delegate.arm_pending_navigation(); + } webview.requested_url = Some(requested_url); Ok(()) } diff --git a/crates/ely_servo_host/src/runtime_webview.rs b/crates/ely_servo_host/src/runtime_webview.rs index d6ec87a..e16b7e8 100644 --- a/crates/ely_servo_host/src/runtime_webview.rs +++ b/crates/ely_servo_host/src/runtime_webview.rs @@ -43,9 +43,7 @@ impl HostWebView { return state; } - if let Some(requested_url) = &self.requested_url - && self.current_url().as_deref() != Some(requested_url.as_str()) - { + if self.delegate.awaiting_url_change() { return WebViewState::Loading; } @@ -69,6 +67,7 @@ pub(super) struct HostWebViewDelegate { title: RefCell>, has_pending_frame: Cell, has_pending_metadata: Cell, + awaiting_url_change: Cell, } impl HostWebViewDelegate { @@ -81,6 +80,7 @@ impl HostWebViewDelegate { title: RefCell::new(None), has_pending_frame: Cell::new(false), has_pending_metadata: Cell::new(false), + awaiting_url_change: Cell::new(false), } } @@ -88,6 +88,18 @@ impl HostWebViewDelegate { self.state.replace(state); } + /// Called at navigation time. The surface stays `Loading` until Servo + /// reports a URL change, which fires for the target *and any redirect + /// or pushState*. A stale `Complete` from the previous page never + /// changes the URL, so it cannot end the loading state early. + pub(super) fn arm_pending_navigation(&self) { + self.awaiting_url_change.set(true); + } + + pub(super) fn awaiting_url_change(&self) -> bool { + self.awaiting_url_change.get() + } + fn state(&self) -> WebViewState { self.state.borrow().clone() } @@ -103,6 +115,7 @@ impl HostWebViewDelegate { fn record_url_change(&self, url: String) { self.url.replace(Some(url)); self.has_pending_metadata.set(true); + self.awaiting_url_change.set(false); } fn record_title_change(&self, title: Option) { @@ -234,4 +247,23 @@ mod tests { assert!(title.len() <= super::MAX_PAGE_TITLE_BYTES); assert!(title.ends_with('…')); } + + #[test] + fn only_a_url_change_ends_a_pending_navigation() { + let delegate = HostWebViewDelegate::new(ProfileId::new(), PermissionStore::default()); + assert!(!delegate.awaiting_url_change()); + + delegate.arm_pending_navigation(); + assert!(delegate.awaiting_url_change()); + + // A late `Complete` from the previous page must not end the load; + // otherwise the surface would present the old page as finished. + delegate.record_load_status(servo::LoadStatus::Complete); + assert!(delegate.awaiting_url_change()); + + // The real navigation — or a redirect / pushState target — changes + // the URL, which is what actually reconciles the loading state. + delegate.record_url_change("https://www.example.com/".to_string()); + assert!(!delegate.awaiting_url_change()); + } }