ZacharyZhang-NY/ELY-Browser
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fold WebSurfaceStore into a single PerTabSurface owner

Browse Source 
Eleven parallel BTreeMaps (click_points, hover_points,
pending_scroll_deltas, scroll_offsets, typed_texts, viewport_bounds,
viewport_sizes, pending_viewport_sizes, states, etc.) made every
input bug a coordination problem across 11 disjoint maps with no
compile-time guarantee they stayed in sync. Collapse them into
BTreeMap<TabId, PerTabSurface>: one owner per tab, one lookup per
input event. keyboard_focus stays at the store level because only
one tab in the window can hold focus at a time.

Public API and behavior are unchanged; the previously-fixed
"scroll keeps keyboard_focus" semantics are preserved. cargo test
ely_browser_core + cargo test ely_app --bin ely_app green.
This commit is contained in:
ZacharyZhang-NY
2026-05-10 01:08:03 -04:00
parent 21da001bf5
commit dc06d5590d
2 changed files with 139 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ely_app/src/shell/web_surface.rs
+80 -86
View File
@@ -7,50 +7,35 @@ use crate::services::ProfileDataMode;
use super::{ use super::{
web_surface_frame::WebSurfaceFrame, web_surface_frame::WebSurfaceFrame,
web_surface_geometry::{ web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize},
WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize,
},
web_surface_permissions::WebSurfaceSitePermission, web_surface_permissions::WebSurfaceSitePermission,
web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame}, web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame},
web_surface_state::{ web_surface_state::{
WebSurfaceClickState, WebSurfaceKeyboardFocusState, WebSurfacePendingInput, PerTabSurface, WebSurfaceClickState, WebSurfaceKeyboardFocusState, WebSurfacePendingInput,
WebSurfaceScrollState, WebSurfaceState, WebSurfaceTextInputState, WebSurfaceScrollState, WebSurfaceState, WebSurfaceTextInputState,
}, },
}; };
pub(super) struct WebSurfaceStore { pub(super) struct WebSurfaceStore {
runtime: WebSurfaceRuntime, runtime: WebSurfaceRuntime,
pending_viewport_sizes: BTreeMap<TabId, WebSurfaceSize>, /// Single owner of every per-tab invariant. See [`PerTabSurface`].
click_points: BTreeMap<TabId, WebSurfaceClickState>, surfaces: BTreeMap<TabId, PerTabSurface>,
hover_points: BTreeMap<TabId, WebSurfaceClickPoint>, /// Singleton because only one tab at a time holds keyboard focus
/// across the whole window. Lives on the store, not per-tab.
keyboard_focus: Option<WebSurfaceKeyboardFocusState>, keyboard_focus: Option<WebSurfaceKeyboardFocusState>,
pending_scroll_deltas: BTreeMap<TabId, WebSurfaceScrollDelta>,
scroll_offsets: BTreeMap<TabId, WebSurfaceScrollState>,
typed_texts: BTreeMap<TabId, WebSurfaceTextInputState>,
viewport_bounds: BTreeMap<TabId, Bounds<Pixels>>,
viewport_sizes: BTreeMap<TabId, WebSurfaceSize>,
states: BTreeMap<TabId, WebSurfaceState>,
} }
impl WebSurfaceStore { impl WebSurfaceStore {
pub(super) fn new() -> Self { pub(super) fn new() -> Self {
Self { Self {
runtime: WebSurfaceRuntime::new(), runtime: WebSurfaceRuntime::new(),
pending_viewport_sizes: BTreeMap::new(), surfaces: BTreeMap::new(),
click_points: BTreeMap::new(),
hover_points: BTreeMap::new(),
keyboard_focus: None, keyboard_focus: None,
pending_scroll_deltas: BTreeMap::new(),
scroll_offsets: BTreeMap::new(),
typed_texts: BTreeMap::new(),
viewport_bounds: BTreeMap::new(),
viewport_sizes: BTreeMap::new(),
states: BTreeMap::new(),
} }
} }
pub(super) fn state(&self, tab_id: &TabId) -> Option<&WebSurfaceState> { pub(super) fn state(&self, tab_id: &TabId) -> Option<&WebSurfaceState> {
self.states.get(tab_id) self.surfaces.get(tab_id).and_then(|surface| surface.state.as_ref())
} }
pub(super) fn ensure_surface( pub(super) fn ensure_surface(
@@ -64,7 +49,8 @@ impl WebSurfaceStore {
} }
let requested_url = tab.url().as_str().to_string(); let requested_url = tab.url().as_str().to_string();
let Some(size) = self.viewport_sizes.get(tab.id()).copied() else { let Some(size) = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size)
else {
return; return;
}; };
let input = self.take_pending_input(tab.id(), requested_url.as_str()); let input = self.take_pending_input(tab.id(), requested_url.as_str());
@@ -76,20 +62,17 @@ impl WebSurfaceStore {
let Some(frame) = result.frame else { let Some(frame) = result.frame else {
return; return;
}; };
self.states.insert(tab.id().clone(), WebSurfaceState::Ready(frame)); self.surface_mut(tab.id()).state = Some(WebSurfaceState::Ready(frame));
} }
Ok(result) if result.started_loading => { Ok(result) if result.started_loading => {
self.states.insert( self.surface_mut(tab.id()).state = Some(WebSurfaceState::Loading {
tab.id().clone(),
WebSurfaceState::Loading {
requested_url: result.requested_url, requested_url: result.requested_url,
previous_frame, previous_frame,
}, });
);
} }
Ok(_) => {} Ok(_) => {}
Err(message) => { Err(message) => {
self.states.insert(tab.id().clone(), WebSurfaceState::Failed { message }); self.surface_mut(tab.id()).state = Some(WebSurfaceState::Failed { message });
} }
} }
} }
@@ -101,11 +84,11 @@ impl WebSurfaceStore {
for frame in frames { for frame in frames {
match frame { match frame {
WebSurfaceRuntimeFrame::Ready { tab_id, frame } => { WebSurfaceRuntimeFrame::Ready { tab_id, frame } => {
self.states.insert(tab_id, WebSurfaceState::Ready(frame)); self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(frame));
changed = true; changed = true;
} }
WebSurfaceRuntimeFrame::Failed { tab_id, message } => { WebSurfaceRuntimeFrame::Failed { tab_id, message } => {
self.states.insert(tab_id, WebSurfaceState::Failed { message }); self.surface_mut(&tab_id).state = Some(WebSurfaceState::Failed { message });
changed = true; changed = true;
} }
} }
@@ -124,26 +107,26 @@ impl WebSurfaceStore {
return false; return false;
}; };
let state = self let surface = self.surface_mut(tab_id);
.scroll_offsets let scroll = surface
.entry(tab_id.clone()) .scroll_offset
.or_insert_with(|| WebSurfaceScrollState::new(requested_url.to_string())); .get_or_insert_with(|| WebSurfaceScrollState::new(requested_url.to_string()));
if state.requested_url != requested_url { if scroll.requested_url != requested_url {
*state = WebSurfaceScrollState::new(requested_url.to_string()); *scroll = WebSurfaceScrollState::new(requested_url.to_string());
} }
state.offset = state.offset.scrolled_by(delta); scroll.offset = scroll.offset.scrolled_by(delta);
self.pending_scroll_deltas surface.pending_scroll_delta = Some(match surface.pending_scroll_delta {
.entry(tab_id.clone()) Some(current) => current.combined_with(delta),
.and_modify(|current| *current = current.combined_with(delta)) None => delta,
.or_insert(delta); });
// Drop any buffered click — its viewport coordinates were // Drop any buffered click — its viewport coordinates were
// captured against the pre-scroll page, so applying it after // captured against the pre-scroll page, so applying it after
// the scroll would land on the wrong DOM element. Keep // the scroll would land on the wrong DOM element. Keep
// `keyboard_focus` and `typed_texts` though: Servo maintains // `keyboard_focus` and `typed_text` though: Servo maintains
// its own DOM focus across scrolls, so a focused input keeps // its own DOM focus across scrolls, so a focused input keeps
// accepting the user's keystrokes after they wheel-scroll. // accepting the user's keystrokes after they wheel-scroll.
self.click_points.remove(tab_id); surface.click_point = None;
true true
} }
@@ -151,26 +134,27 @@ impl WebSurfaceStore {
let Some(size) = WebSurfaceSize::from_bounds(bounds) else { let Some(size) = WebSurfaceSize::from_bounds(bounds) else {
return false; return false;
}; };
self.viewport_bounds.insert(tab_id.clone(), bounds); let surface = self.surface_mut(tab_id);
surface.viewport_bounds = Some(bounds);
let Some(current_size) = self.viewport_sizes.get(tab_id).copied() else { let Some(current_size) = surface.viewport_size else {
self.viewport_sizes.insert(tab_id.clone(), size); surface.viewport_size = Some(size);
self.pending_viewport_sizes.remove(tab_id); surface.pending_viewport_size = None;
return true; return true;
}; };
if current_size == size { if current_size == size {
self.pending_viewport_sizes.remove(tab_id); surface.pending_viewport_size = None;
return false; return false;
} }
if self.pending_viewport_sizes.get(tab_id) != Some(&size) { if surface.pending_viewport_size != Some(size) {
self.pending_viewport_sizes.insert(tab_id.clone(), size); surface.pending_viewport_size = Some(size);
return false; return false;
} }
self.pending_viewport_sizes.remove(tab_id); surface.pending_viewport_size = None;
self.viewport_sizes.insert(tab_id.clone(), size); surface.viewport_size = Some(size);
true true
} }
@@ -179,13 +163,15 @@ impl WebSurfaceStore {
tab_id: &TabId, tab_id: &TabId,
position: Point<Pixels>, position: Point<Pixels>,
) -> bool { ) -> bool {
let Some(bounds) = self.viewport_bounds.get(tab_id).copied() else { let surface = self.surfaces.get_mut(tab_id).filter(|surface| surface.viewport_bounds.is_some());
let Some(surface) = surface else {
return false; return false;
}; };
let bounds = surface.viewport_bounds.expect("viewport_bounds checked above");
let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position) else { let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position) else {
return false; return false;
}; };
self.hover_points.insert(tab_id.clone(), point); surface.hover_point = Some(point);
true true
} }
@@ -195,16 +181,24 @@ impl WebSurfaceStore {
requested_url: &str, requested_url: &str,
position: Point<Pixels>, position: Point<Pixels>,
) -> bool { ) -> bool {
let Some(bounds) = self.viewport_bounds.get(tab_id).copied() else { let Some(bounds) =
self.surfaces.get(tab_id).and_then(|surface| surface.viewport_bounds)
else {
return false; return false;
}; };
let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position) else { let Some(point) = WebSurfaceClickPoint::from_window_position(bounds, position) else {
return false; return false;
}; };
let scroll_offset = self
.surfaces
.get(tab_id)
.map(|surface| surface.scroll_offset_for(requested_url))
.unwrap_or_default();
let state = WebSurfaceClickState { let state = WebSurfaceClickState {
requested_url: requested_url.to_string(), requested_url: requested_url.to_string(),
scroll_offset: self.scroll_offset_for(tab_id, requested_url), scroll_offset,
point, point,
}; };
self.keyboard_focus = Some(WebSurfaceKeyboardFocusState { self.keyboard_focus = Some(WebSurfaceKeyboardFocusState {
@@ -213,8 +207,9 @@ impl WebSurfaceStore {
scroll_offset: state.scroll_offset, scroll_offset: state.scroll_offset,
click_point: state.point, click_point: state.point,
}); });
self.typed_texts.remove(tab_id); let surface = self.surface_mut(tab_id);
self.click_points.insert(tab_id.clone(), state); surface.typed_text = None;
surface.click_point = Some(state);
true true
} }
@@ -234,21 +229,24 @@ impl WebSurfaceStore {
return false; return false;
} }
let entry = let scroll_offset = focus.scroll_offset;
self.typed_texts.entry(tab_id.clone()).or_insert_with(|| WebSurfaceTextInputState { let click_point = focus.click_point;
let surface = self.surface_mut(tab_id);
let entry = surface.typed_text.get_or_insert_with(|| WebSurfaceTextInputState {
requested_url: requested_url.to_string(), requested_url: requested_url.to_string(),
scroll_offset: focus.scroll_offset, scroll_offset,
click_point: focus.click_point, click_point,
text: String::new(), text: String::new(),
}); });
if entry.requested_url != requested_url if entry.requested_url != requested_url
|| entry.scroll_offset != focus.scroll_offset || entry.scroll_offset != scroll_offset
|| entry.click_point != focus.click_point || entry.click_point != click_point
{ {
*entry = WebSurfaceTextInputState { *entry = WebSurfaceTextInputState {
requested_url: requested_url.to_string(), requested_url: requested_url.to_string(),
scroll_offset: focus.scroll_offset, scroll_offset,
click_point: focus.click_point, click_point,
text: String::new(), text: String::new(),
}; };
} }
@@ -262,22 +260,22 @@ impl WebSurfaceStore {
tab_id: &TabId, tab_id: &TabId,
requested_url: &str, requested_url: &str,
) -> WebSurfacePendingInput { ) -> WebSurfacePendingInput {
let scroll_offset = self.scroll_offset_for(tab_id, requested_url); let surface = self.surface_mut(tab_id);
let scroll_delta = self.pending_scroll_deltas.remove(tab_id); let scroll_offset = surface.scroll_offset_for(requested_url);
let click_point = self let scroll_delta = surface.pending_scroll_delta.take();
.click_points let click_point = surface
.remove(tab_id) .click_point
.take()
.filter(|state| { .filter(|state| {
state.requested_url == requested_url && state.scroll_offset == scroll_offset state.requested_url == requested_url && state.scroll_offset == scroll_offset
}) })
.map(|state| state.point); .map(|state| state.point);
let typed_text = self let typed_text = surface
.typed_texts .typed_text
.remove(tab_id) .take()
.filter(|state| state.requested_url == requested_url) .filter(|state| state.requested_url == requested_url)
.map(|state| state.text); .map(|state| state.text);
let hover_point = surface.hover_point.take();
let hover_point = self.hover_points.remove(tab_id);
WebSurfacePendingInput { scroll_offset, scroll_delta, click_point, hover_point, typed_text } WebSurfacePendingInput { scroll_offset, scroll_delta, click_point, hover_point, typed_text }
} }
@@ -288,7 +286,7 @@ impl WebSurfaceStore {
requested_url: &str, requested_url: &str,
zoom_percent: u16, zoom_percent: u16,
) -> Option<WebSurfaceFrame> { ) -> Option<WebSurfaceFrame> {
match self.states.get(tab_id) { match self.surfaces.get(tab_id).and_then(|surface| surface.state.as_ref()) {
Some(WebSurfaceState::Ready(frame)) Some(WebSurfaceState::Ready(frame))
if frame.requested_url == requested_url && frame.zoom_percent() == zoom_percent => if frame.requested_url == requested_url && frame.zoom_percent() == zoom_percent =>
{ {
@@ -304,12 +302,8 @@ impl WebSurfaceStore {
} }
} }
fn scroll_offset_for(&self, tab_id: &TabId, requested_url: &str) -> WebSurfaceScrollOffset { fn surface_mut(&mut self, tab_id: &TabId) -> &mut PerTabSurface {
self.scroll_offsets self.surfaces.entry(tab_id.clone()).or_insert_with(PerTabSurface::new)
.get(tab_id)
.filter(|state| state.requested_url == requested_url)
.map(|state| state.offset)
.unwrap_or_default()
} }
} }
crates/ely_app/src/shell/web_surface_state.rs
+54 -2
View File
@@ -1,8 +1,11 @@
use ely_domain::TabId; use ely_domain::TabId;
use gpui::{Bounds, Pixels};
use super::{ use super::{
web_surface_frame::WebSurfaceFrame, web_surface_frame::WebSurfaceFrame,
web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollOffset}, web_surface_geometry::{
WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize,
},
}; };
pub(super) struct WebSurfaceScrollState { pub(super) struct WebSurfaceScrollState {
@@ -41,7 +44,7 @@ pub(super) struct WebSurfaceTextInputState {
#[derive(Clone, Debug, Eq, PartialEq)] #[derive(Clone, Debug, Eq, PartialEq)]
pub(super) struct WebSurfacePendingInput { pub(super) struct WebSurfacePendingInput {
pub(super) scroll_offset: WebSurfaceScrollOffset, pub(super) scroll_offset: WebSurfaceScrollOffset,
pub(super) scroll_delta: Option<super::web_surface_geometry::WebSurfaceScrollDelta>, pub(super) scroll_delta: Option<WebSurfaceScrollDelta>,
pub(super) click_point: Option<WebSurfaceClickPoint>, pub(super) click_point: Option<WebSurfaceClickPoint>,
pub(super) hover_point: Option<WebSurfaceClickPoint>, pub(super) hover_point: Option<WebSurfaceClickPoint>,
pub(super) typed_text: Option<String>, pub(super) typed_text: Option<String>,
@@ -52,3 +55,52 @@ pub(super) enum WebSurfaceState {
Ready(WebSurfaceFrame), Ready(WebSurfaceFrame),
Failed { message: String }, Failed { message: String },
} }
/// All per-tab surface invariants in one owner.
///
/// Replaces the previous 11 parallel `BTreeMap<TabId, _>` fields on
/// `WebSurfaceStore`. Every per-tab field lives here so a single
/// lookup yields the full input/render context for that tab; the
/// remaining cross-tab state (the runtime backend and the singular
/// `keyboard_focus` pointer to the active tab) stays on the store.
///
/// All inputs that depend on a measured viewport take their
/// pre-conditions from this struct (`viewport_bounds`,
/// `viewport_size`), so "viewport not ready" turns into a guard at
/// the call site instead of a `get` on a parallel map that may or
/// may not be populated.
pub(super) struct PerTabSurface {
pub(super) viewport_bounds: Option<Bounds<Pixels>>,
pub(super) viewport_size: Option<WebSurfaceSize>,
pub(super) pending_viewport_size: Option<WebSurfaceSize>,
pub(super) hover_point: Option<WebSurfaceClickPoint>,
pub(super) click_point: Option<WebSurfaceClickState>,
pub(super) pending_scroll_delta: Option<WebSurfaceScrollDelta>,
pub(super) scroll_offset: Option<WebSurfaceScrollState>,
pub(super) typed_text: Option<WebSurfaceTextInputState>,
pub(super) state: Option<WebSurfaceState>,
}
impl PerTabSurface {
pub(super) fn new() -> Self {
Self {
viewport_bounds: None,
viewport_size: None,
pending_viewport_size: None,
hover_point: None,
click_point: None,
pending_scroll_delta: None,
scroll_offset: None,
typed_text: None,
state: None,
}
}
pub(super) fn scroll_offset_for(&self, requested_url: &str) -> WebSurfaceScrollOffset {
self.scroll_offset
.as_ref()
.filter(|state| state.requested_url == requested_url)
.map(|state| state.offset)
.unwrap_or_default()
}
}