Commit Servo loaded URLs to tabs

crates/ely_app/src/shell/chrome/topbar.rs

@@ -2,9 +2,9 @@ use ely_browser_core::BrowserSnapshot;
 use ely_design_system::{colors, spacing};
 use ely_domain::{BrowserTab, ThemeMode};
 use gpui::{
-    AnyElement, BoxShadow, Context, FontWeight, InteractiveElement, IntoElement, ParentElement,
+    AnyElement, BoxShadow, Context, Focusable, FontWeight, InteractiveElement, IntoElement,
-    SharedString, StatefulInteractiveElement, Styled, div, hsla, point, prelude::FluentBuilder, px,
+    ParentElement, SharedString, StatefulInteractiveElement, Styled, Window, div, hsla, point,
-    rgb, rgba,
+    prelude::FluentBuilder, px, rgb, rgba,
 };
 use gpui_component::{IconName, input::Input};
 
@@ -16,6 +16,7 @@ pub(crate) fn render_topbar(
     snapshot: &BrowserSnapshot,
     active_tab: &BrowserTab,
     sidebar_collapsed: bool,
+    window: &mut Window,
     cx: &mut Context<ElyShell>,
 ) -> AnyElement {
     div()
@@ -42,7 +43,7 @@ pub(crate) fn render_topbar(
             cx,
             |shell, window, cx| shell.navigate_active_tab_forward(window, cx),
         ))
-        .child(render_omnibar(shell, active_tab, cx))
+        .child(render_omnibar(shell, active_tab, window, cx))
         .child(render_topbar_action("share-url", IconName::Copy, cx, |shell, window, cx| {
             shell.copy_active_tab_url(window, cx)
         }))
@@ -64,14 +65,14 @@ pub(crate) fn render_topbar(
 fn render_omnibar(
     shell: &mut ElyShell,
     active_tab: &BrowserTab,
+    window: &mut Window,
     cx: &mut Context<ElyShell>,
 ) -> AnyElement {
     let favorite_active = active_tab.flags().favorite;
     let favorite_icon = if favorite_active { IconName::Star } else { IconName::StarOff };
-    let input_value = shell.command_input.read(cx).value().to_string();
     let active_url = active_tab.url().as_str().to_string();
-    let show_styled =
+    let command_focused = shell.command_input.read(cx).focus_handle(cx).is_focused(window);
-        !input_value.is_empty() && input_value == active_url && active_url != "ely://new-tab";
+    let show_styled = !command_focused && active_url != "ely://new-tab";
     let secure = active_url.starts_with("https://") || active_url.starts_with("ely://");
 
     div()

crates/ely_app/src/shell/render.rs

@@ -17,10 +17,10 @@ use super::sidebar::collapsed_sidebar_active;
 use super::{ElyShell, ShellState};
 
 impl Render for ElyShell {
-    fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         match &self.state {
             ShellState::Ready(core) => match (core.snapshot(), core.active_tab().cloned()) {
-                (Ok(snapshot), Ok(active_tab)) => self.render_browser(snapshot, active_tab, cx),
+                (Ok(snapshot), Ok(active_tab)) => self.render_browser(snapshot, active_tab, window, cx),
                 (Err(error), _) | (_, Err(error)) => render_error(error.to_string()),
             },
             ShellState::StartupError(message) => render_error(message.clone()),
@@ -33,6 +33,7 @@ impl ElyShell {
         &mut self,
         snapshot: BrowserSnapshot,
         active_tab: BrowserTab,
+        window: &mut Window,
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let sidebar_width = match active_sidebar_width(&snapshot) {
@@ -88,7 +89,7 @@ impl ElyShell {
                         sidebar_hidden,
                         cx,
                     ))
-                    .child(self.render_main_pane(&snapshot, &active_tab, sidebar_collapsed, cx)),
+                    .child(self.render_main_pane(&snapshot, &active_tab, sidebar_collapsed, window, cx)),
             )
             .when(hover_expanded, |el| {
                 el.child(self.render_hidden_sidebar_overlay(&snapshot, cx))
@@ -144,6 +145,7 @@ impl ElyShell {
         snapshot: &BrowserSnapshot,
         active_tab: &BrowserTab,
         sidebar_collapsed: bool,
+        window: &mut Window,
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let panel_color = panel_bg(snapshot);
@@ -159,7 +161,7 @@ impl ElyShell {
             .border_color(rgba(MAIN_PANE_HIGHLIGHT_BORDER))
             .shadow(panel_shadow())
             .overflow_hidden()
-            .child(render_topbar_chrome(self, snapshot, active_tab, sidebar_collapsed, cx))
+            .child(render_topbar_chrome(self, snapshot, active_tab, sidebar_collapsed, window, cx))
             .child(
                 div()
                     .flex_1()

crates/ely_app/src/shell/web_surface.rs

@@ -9,7 +9,7 @@ use super::{
     web_surface_frame::WebSurfaceFrame,
     web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize},
     web_surface_permissions::WebSurfaceSitePermission,
-    web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame},
+    web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame, WebSurfaceUrlChange},
     web_surface_state::{
         PerTabSurface, WebSurfaceClickState, WebSurfaceInputOutcome, WebSurfaceKeyboardFocusState,
         WebSurfacePendingInput, WebSurfaceScrollState, WebSurfaceState, WebSurfaceTextInputState,
@@ -39,58 +39,62 @@ impl WebSurfaceStore {
         tab: &BrowserTab,
         profile_data_mode: ProfileDataMode,
         permissions: &[WebSurfaceSitePermission],
-    ) {
+    ) -> Option<WebSurfaceUrlChange> {
         if !is_external_web_url(tab.url().as_str()) {
-            return;
+            return None;
         }
 
         let requested_url = tab.url().as_str().to_string();
-        let Some(size) = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size)
+        let size = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size)?;
-        else {
-            return;
-        };
         let input = self.take_pending_input(tab.id(), requested_url.as_str());
         let previous_frame =
             self.previous_ready_frame(tab.id(), requested_url.as_str(), tab.zoom_percent());
 
         match self.runtime.ensure_tab(tab, size, profile_data_mode, permissions, input) {
             Ok(result) if result.frame.is_some() => {
+                let url_change = result.url_change;
                 let Some(frame) = result.frame else {
-                    return;
+                    return url_change;
                 };
                 self.surface_mut(tab.id()).state = Some(WebSurfaceState::Ready(frame));
+                url_change
             }
             Ok(result) if result.started_loading => {
                 self.surface_mut(tab.id()).state = Some(WebSurfaceState::Loading {
                     requested_url: result.requested_url,
                     previous_frame,
                 });
+                result.url_change
             }
-            Ok(_) => {}
+            Ok(result) => result.url_change,
             Err(message) => {
                 self.surface_mut(tab.id()).state = Some(WebSurfaceState::Failed { message });
+                None
             }
         }
     }
 
-    pub(super) fn tick(&mut self) -> bool {
+    pub(super) fn tick(&mut self) -> WebSurfaceTickResult {
         let frames = self.runtime.tick();
-        let mut changed = false;
+        let mut result = WebSurfaceTickResult::default();
 
         for frame in frames {
             match frame {
-                WebSurfaceRuntimeFrame::Ready { tab_id, frame } => {
+                WebSurfaceRuntimeFrame::Ready { tab_id, frame, url_change } => {
-                    self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(frame));
+                    self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(*frame));
-                    changed = true;
+                    result.changed = true;
+                    if let Some(url_change) = url_change {
+                        result.url_changes.push(url_change);
+                    }
                 }
                 WebSurfaceRuntimeFrame::Failed { tab_id, message } => {
                     self.surface_mut(&tab_id).state = Some(WebSurfaceState::Failed { message });
-                    changed = true;
+                    result.changed = true;
                 }
             }
         }
 
-        changed
+        result
     }
 
     pub(super) fn record_scroll_delta(
@@ -340,6 +344,12 @@ pub(super) fn is_external_web_url(url: &str) -> bool {
     url.starts_with("https://") || url.starts_with("http://")
 }
 
+#[derive(Default)]
+pub(super) struct WebSurfaceTickResult {
+    pub(super) changed: bool,
+    pub(super) url_changes: Vec<WebSurfaceUrlChange>,
+}
+
 #[cfg(test)]
 #[path = "web_surface_tests.rs"]
 mod tests;

crates/ely_app/src/shell/web_surface_controller.rs

@@ -1,5 +1,5 @@
 use ely_browser_core::BrowserSnapshot;
-use ely_domain::{BrowserTab, ProfileKind, TabId};
+use ely_domain::{BrowserTab, ProfileKind, TabId, UrlText};
 use gpui::{AnyElement, Bounds, Context, Pixels, Point};
 
 use crate::services::ProfileDataMode;
@@ -7,6 +7,7 @@ use crate::services::ProfileDataMode;
 use super::{
     ElyShell,
     web_surface_permissions::web_surface_site_permissions_for_tab,
+    web_surface_runtime::{WebSurfaceUrlChange, WebSurfaceUrlChangeKind},
     web_surface_state::{WebSurfaceInputOutcome, WebSurfaceState},
     web_surface_view::{
         render_failed_web_surface, render_loading_web_surface, render_ready_web_surface,
@@ -26,7 +27,12 @@ impl ElyShell {
         };
 
         let permissions = web_surface_site_permissions_for_tab(tab, snapshot);
-        self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions);
+        if let Some(url_change) =
+            self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions)
+            && self.apply_web_surface_url_change(url_change)
+        {
+            cx.notify();
+        }
 
         match self.web_surfaces.state(tab.id()) {
             Some(WebSurfaceState::Ready(frame)) => {
@@ -45,7 +51,12 @@ impl ElyShell {
     }
 
     pub(super) fn tick_external_web_surfaces(&mut self) -> bool {
-        self.web_surfaces.tick()
+        let result = self.web_surfaces.tick();
+        let mut url_changed = false;
+        for url_change in result.url_changes {
+            url_changed |= self.apply_web_surface_url_change(url_change);
+        }
+        result.changed || url_changed
     }
 
     pub(super) fn record_external_web_viewport(
@@ -144,6 +155,26 @@ impl ElyShell {
     }
 }
 
+impl ElyShell {
+    fn apply_web_surface_url_change(&mut self, change: WebSurfaceUrlChange) -> bool {
+        let Ok(url) = UrlText::parse(change.loaded_url) else {
+            return false;
+        };
+        let super::ShellState::Ready(core) = &mut self.state else {
+            return false;
+        };
+
+        match change.kind {
+            WebSurfaceUrlChangeKind::UserInitiated => {
+                core.navigate_tab_to_loaded_url(&change.tab_id, url).is_ok_and(|changed| changed)
+            }
+            WebSurfaceUrlChangeKind::Observed => {
+                core.replace_tab_loaded_url(&change.tab_id, url).is_ok_and(|changed| changed)
+            }
+        }
+    }
+}
+
 fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option<ProfileDataMode> {
     snapshot.profiles.iter().find(|profile| profile.id() == tab.profile_id()).map(|profile| {
         match profile.kind() {

crates/ely_app/src/shell/web_surface_frame.rs

@@ -200,6 +200,10 @@ impl WebSurfaceFrame {
         self.zoom_percent
     }
 
+    pub(super) fn loaded_url(&self) -> Option<&str> {
+        self.loaded_url.as_deref()
+    }
+
     #[cfg(all(test, feature = "live-site-smoke"))]
     pub(super) fn click_point(&self) -> Option<WebSurfaceClickPoint> {
         self.click_point

crates/ely_app/src/shell/web_surface_runtime.rs

@@ -47,10 +47,17 @@ impl WebSurfaceRuntime {
         }
         let (scroll_delta_x, scroll_delta_y, scroll_point_x, scroll_point_y) =
             scroll_wire_fields(input.scroll_delta, input.scroll_point)?;
+        let user_navigation_input = input_requests_history_navigation(&input);
 
         let session = sessions.entry(tab.id().clone()).or_insert_with(WebSurfaceSession::default);
         let next_scroll_offset = input.scroll_offset;
         let started_loading = session.started_loading(&requested_url, size, zoom_percent);
+        if started_loading {
+            session.pending_user_navigation = false;
+        }
+        if user_navigation_input {
+            session.pending_user_navigation = true;
+        }
         let frame = client
             .ensure(ServoLiveEnsureRequest {
                 tab_id: tab.id().as_str().to_string(),
@@ -82,17 +89,20 @@ impl WebSurfaceRuntime {
                 .map_err(|error| error.to_string())
             })
             .transpose()?;
+        let url_change = frame
+            .as_ref()
+            .and_then(|frame| session.url_change_for(tab.id(), requested_url.as_str(), frame));
 
         session.requested_url = requested_url.clone();
         session.size = size;
         session.zoom_percent = zoom_percent;
         session.scroll_offset = next_scroll_offset;
 
-        Ok(WebSurfaceEnsureResult { requested_url, started_loading, frame })
+        Ok(WebSurfaceEnsureResult { requested_url, started_loading, frame, url_change })
     }
 
     pub(super) fn tick(&mut self) -> Vec<WebSurfaceRuntimeFrame> {
-        let (state, sessions) = (&mut self.state, &self.sessions);
+        let (state, sessions) = (&mut self.state, &mut self.sessions);
         let RuntimeState::Ready { client, .. } = state else {
             return Vec::new();
         };
@@ -107,7 +117,14 @@ impl WebSurfaceRuntime {
                     frame,
                 ) {
                     Ok(frame) => {
-                        frames.push(WebSurfaceRuntimeFrame::Ready { tab_id: tab_id.clone(), frame })
+                        let requested_url = session.requested_url.clone();
+                        let url_change =
+                            session.url_change_for(tab_id, requested_url.as_str(), &frame);
+                        frames.push(WebSurfaceRuntimeFrame::Ready {
+                            tab_id: tab_id.clone(),
+                            frame: Box::new(frame),
+                            url_change,
+                        })
                     }
                     Err(error) => frames.push(WebSurfaceRuntimeFrame::Failed {
                         tab_id: tab_id.clone(),
@@ -185,6 +202,7 @@ struct WebSurfaceSession {
     size: WebSurfaceSize,
     zoom_percent: u16,
     scroll_offset: WebSurfaceScrollOffset,
+    pending_user_navigation: bool,
 }
 
 impl WebSurfaceSession {
@@ -198,19 +216,57 @@ impl WebSurfaceSession {
             || self.size != size
             || self.zoom_percent != zoom_percent
     }
+
+    fn url_change_for(
+        &mut self,
+        tab_id: &TabId,
+        requested_url: &str,
+        frame: &WebSurfaceFrame,
+    ) -> Option<WebSurfaceUrlChange> {
+        let loaded_url = frame.loaded_url()?;
+        if loaded_url == requested_url {
+            return None;
+        }
+
+        let kind = if self.pending_user_navigation {
+            WebSurfaceUrlChangeKind::UserInitiated
+        } else {
+            WebSurfaceUrlChangeKind::Observed
+        };
+        self.pending_user_navigation = false;
+        Some(WebSurfaceUrlChange {
+            tab_id: tab_id.clone(),
+            loaded_url: loaded_url.to_string(),
+            kind,
+        })
+    }
 }
 
 pub(super) struct WebSurfaceEnsureResult {
     pub(super) requested_url: String,
     pub(super) started_loading: bool,
     pub(super) frame: Option<WebSurfaceFrame>,
+    pub(super) url_change: Option<WebSurfaceUrlChange>,
 }
 
 pub(super) enum WebSurfaceRuntimeFrame {
-    Ready { tab_id: TabId, frame: WebSurfaceFrame },
+    Ready { tab_id: TabId, frame: Box<WebSurfaceFrame>, url_change: Option<WebSurfaceUrlChange> },
     Failed { tab_id: TabId, message: String },
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(super) struct WebSurfaceUrlChange {
+    pub(super) tab_id: TabId,
+    pub(super) loaded_url: String,
+    pub(super) kind: WebSurfaceUrlChangeKind,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub(super) enum WebSurfaceUrlChangeKind {
+    UserInitiated,
+    Observed,
+}
+
 fn config_dir_for_scope(
     scope: &WebSurfaceRuntimeScope,
 ) -> Result<(PathBuf, Option<PathBuf>), String> {
@@ -245,6 +301,11 @@ fn scroll_wire_fields(
     }
 }
 
+fn input_requests_history_navigation(input: &WebSurfacePendingInput) -> bool {
+    input.click_point.is_some()
+        || input.typed_text.as_deref().is_some_and(|text| text.contains('\n'))
+}
+
 impl From<&WebSurfaceSitePermission> for ServoLiveSitePermission {
     fn from(permission: &WebSurfaceSitePermission) -> Self {
         Self::new(

crates/ely_browser_core/src/state/tabs.rs

@@ -23,19 +23,26 @@ impl BrowserCore {
     /// no new tab is created and the active tab id is unchanged.
     pub fn navigate_active_tab(&mut self, url: UrlText) -> Result<(), CoreError> {
         let active_id = self.active_tab_id.clone();
-        let tab_index = self
+        self.update_tab_url(&active_id, url, TabUrlUpdate::PushHistory)?;
-            .tabs
-            .iter()
-            .position(|tab| tab.id() == &active_id)
-            .ok_or_else(|| CoreError::TabNotFound { id: active_id.clone() })?;
-        self.tabs[tab_index].navigate_to(url);
-        self.tabs[tab_index].mark_ready();
-        let snapshot_tab = self.tabs[tab_index].clone();
-        self.record_history_entry(&snapshot_tab);
-        self.record_tab_activity(&active_id, SystemTime::now());
         Ok(())
     }
 
+    pub fn navigate_tab_to_loaded_url(
+        &mut self,
+        tab_id: &TabId,
+        url: UrlText,
+    ) -> Result<bool, CoreError> {
+        self.update_tab_url(tab_id, url, TabUrlUpdate::PushHistory)
+    }
+
+    pub fn replace_tab_loaded_url(
+        &mut self,
+        tab_id: &TabId,
+        url: UrlText,
+    ) -> Result<bool, CoreError> {
+        self.update_tab_url(tab_id, url, TabUrlUpdate::PreserveHistory)
+    }
+
     pub fn navigate_active_tab_back(&mut self) -> Result<bool, CoreError> {
         self.navigate_active_tab_history(TabHistoryDirection::Back)
     }
@@ -377,6 +384,32 @@ impl BrowserCore {
         self.tabs.get_mut(active_index).ok_or(CoreError::MissingActiveTab)
     }
 
+    fn update_tab_url(
+        &mut self,
+        tab_id: &TabId,
+        url: UrlText,
+        update: TabUrlUpdate,
+    ) -> Result<bool, CoreError> {
+        let tab_index = self
+            .tabs
+            .iter()
+            .position(|tab| tab.id() == tab_id)
+            .ok_or_else(|| CoreError::TabNotFound { id: tab_id.clone() })?;
+        if self.tabs[tab_index].url() == &url {
+            return Ok(false);
+        }
+
+        match update {
+            TabUrlUpdate::PushHistory => self.tabs[tab_index].navigate_to(url),
+            TabUrlUpdate::PreserveHistory => self.tabs[tab_index].set_url(url),
+        }
+        self.tabs[tab_index].mark_ready();
+        let snapshot_tab = self.tabs[tab_index].clone();
+        self.record_history_entry(&snapshot_tab);
+        self.record_tab_activity(tab_id, SystemTime::now());
+        Ok(true)
+    }
+
     fn navigate_active_tab_history(
         &mut self,
         direction: TabHistoryDirection,
@@ -459,3 +492,8 @@ enum TabHistoryDirection {
     Back,
     Forward,
 }
+
+enum TabUrlUpdate {
+    PushHistory,
+    PreserveHistory,
+}

crates/ely_browser_core/tests/tab_navigation.rs

@@ -40,6 +40,34 @@ fn new_navigation_clears_forward_stack() -> Result<(), Box<dyn Error>> {
     Ok(())
 }
 
+#[test]
+fn observed_loaded_url_replaces_active_url_without_back_stack() -> Result<(), Box<dyn Error>> {
+    let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
+    let tab_id = core.active_tab()?.id().clone();
+
+    assert!(core.replace_tab_loaded_url(&tab_id, UrlText::parse("https://example.com/")?)?);
+
+    assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/");
+    assert!(!core.active_tab()?.can_navigate_back());
+    assert!(!core.active_tab()?.can_navigate_forward());
+    Ok(())
+}
+
+#[test]
+fn user_loaded_url_enters_active_back_stack() -> Result<(), Box<dyn Error>> {
+    let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
+    core.navigate_active_tab(UrlText::parse("https://example.com/a")?)?;
+    let tab_id = core.active_tab()?.id().clone();
+
+    assert!(core.navigate_tab_to_loaded_url(&tab_id, UrlText::parse("https://example.com/b")?)?);
+
+    assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/b");
+    assert!(core.active_tab()?.can_navigate_back());
+    assert!(core.navigate_active_tab_back()?);
+    assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/a");
+    Ok(())
+}
+
 #[test]
 fn empty_history_navigation_keeps_active_url() -> Result<(), Box<dyn Error>> {
     let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;