perf(app): suppress repeated web metadata updates
This commit is contained in:
@@ -10,7 +10,6 @@ use super::{
|
|||||||
web_surface_cadence::IDLE_POLL_INTERVAL,
|
web_surface_cadence::IDLE_POLL_INTERVAL,
|
||||||
web_surface_frame::WebSurfaceFrame,
|
web_surface_frame::WebSurfaceFrame,
|
||||||
web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize},
|
web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize},
|
||||||
web_surface_metadata::WebSurfacePageMetadata,
|
|
||||||
web_surface_permissions::WebSurfaceSitePermission,
|
web_surface_permissions::WebSurfaceSitePermission,
|
||||||
web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame},
|
web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame},
|
||||||
web_surface_state::{
|
web_surface_state::{
|
||||||
@@ -118,9 +117,8 @@ impl WebSurfaceStore {
|
|||||||
}
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
result
|
let metadata = self.surface_mut(&tab_id).changed_page_metadata(&tab_id, &frame);
|
||||||
.page_metadata
|
result.page_metadata.extend(metadata);
|
||||||
.extend(WebSurfacePageMetadata::from_frame(&tab_id, &frame));
|
|
||||||
if self
|
if self
|
||||||
.surfaces
|
.surfaces
|
||||||
.get(&tab_id)
|
.get(&tab_id)
|
||||||
|
|||||||
@@ -2,6 +2,30 @@ use ely_domain::TabId;
|
|||||||
|
|
||||||
use super::web_surface_frame::WebSurfaceFrame;
|
use super::web_surface_frame::WebSurfaceFrame;
|
||||||
|
|
||||||
|
#[derive(Clone, Debug, Default, Eq, PartialEq)]
|
||||||
|
pub(super) struct WebSurfaceMetadataTracker {
|
||||||
|
last_title: Option<String>,
|
||||||
|
last_loaded_url: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl WebSurfaceMetadataTracker {
|
||||||
|
pub(super) fn changed_metadata_for(
|
||||||
|
&mut self,
|
||||||
|
tab_id: &TabId,
|
||||||
|
frame: &WebSurfaceFrame,
|
||||||
|
) -> Option<WebSurfacePageMetadata> {
|
||||||
|
let title = frame.title().map(str::to_string);
|
||||||
|
let loaded_url = frame.loaded_url().map(str::to_string);
|
||||||
|
if self.last_title == title && self.last_loaded_url == loaded_url {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
self.last_title = title.clone();
|
||||||
|
self.last_loaded_url = loaded_url.clone();
|
||||||
|
WebSurfacePageMetadata::from_parts(tab_id, title, loaded_url)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// One page's worth of metadata observed in a Ready frame. The
|
/// One page's worth of metadata observed in a Ready frame. The
|
||||||
/// controller applies these to the `BrowserTab` after the frame has
|
/// controller applies these to the `BrowserTab` after the frame has
|
||||||
/// been swapped into the surface state. Title and favicon are
|
/// been swapped into the surface state. Title and favicon are
|
||||||
@@ -15,10 +39,13 @@ pub(super) struct WebSurfacePageMetadata {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl WebSurfacePageMetadata {
|
impl WebSurfacePageMetadata {
|
||||||
pub(super) fn from_frame(tab_id: &TabId, frame: &WebSurfaceFrame) -> Option<Self> {
|
fn from_parts(
|
||||||
let title = frame.title().map(str::to_string);
|
tab_id: &TabId,
|
||||||
let favicon_url = frame
|
title: Option<String>,
|
||||||
.loaded_url()
|
loaded_url: Option<String>,
|
||||||
|
) -> Option<Self> {
|
||||||
|
let favicon_url = loaded_url
|
||||||
|
.as_deref()
|
||||||
.and_then(|loaded| ely_domain::UrlText::parse(loaded).ok())
|
.and_then(|loaded| ely_domain::UrlText::parse(loaded).ok())
|
||||||
.and_then(|url| url.favicon_url());
|
.and_then(|url| url.favicon_url());
|
||||||
if title.is_none() && favicon_url.is_none() {
|
if title.is_none() && favicon_url.is_none() {
|
||||||
@@ -27,3 +54,41 @@ impl WebSurfacePageMetadata {
|
|||||||
Some(Self { tab_id: tab_id.clone(), title, favicon_url })
|
Some(Self { tab_id: tab_id.clone(), title, favicon_url })
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use std::error::Error;
|
||||||
|
|
||||||
|
use ely_domain::TabId;
|
||||||
|
|
||||||
|
use super::*;
|
||||||
|
use crate::{
|
||||||
|
services::servo_live::ServoLiveFrame,
|
||||||
|
shell::{web_surface_frame::WebSurfaceFrame, web_surface_geometry::WebSurfaceScrollOffset},
|
||||||
|
};
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn repeated_frame_metadata_emits_once() -> Result<(), Box<dyn Error>> {
|
||||||
|
let tab_id = TabId::new();
|
||||||
|
let frame = frame()?;
|
||||||
|
let mut tracker = WebSurfaceMetadataTracker::default();
|
||||||
|
|
||||||
|
let first = tracker.changed_metadata_for(&tab_id, &frame);
|
||||||
|
let second = tracker.changed_metadata_for(&tab_id, &frame);
|
||||||
|
|
||||||
|
assert!(first.is_some());
|
||||||
|
assert_eq!(second, None);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn frame() -> Result<WebSurfaceFrame, Box<dyn Error>> {
|
||||||
|
let rgba_pixel = vec![255u8, 255, 255, 255];
|
||||||
|
let live = ServoLiveFrame::for_test(1, 1, rgba_pixel);
|
||||||
|
Ok(WebSurfaceFrame::from_live_frame(
|
||||||
|
"https://example.com/".to_string(),
|
||||||
|
WebSurfaceScrollOffset::default(),
|
||||||
|
100,
|
||||||
|
live,
|
||||||
|
)?)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -176,7 +176,7 @@ fn identical_ready_software_frame_keeps_tick_unchanged() -> Result<(), String> {
|
|||||||
let second_tick = store.tick(&visible_tabs);
|
let second_tick = store.tick(&visible_tabs);
|
||||||
|
|
||||||
assert!(!second_tick.changed);
|
assert!(!second_tick.changed);
|
||||||
assert_eq!(second_tick.page_metadata.len(), 1);
|
assert_eq!(second_tick.page_metadata.len(), 0);
|
||||||
assert_eq!(second_tick.url_changes.len(), 1);
|
assert_eq!(second_tick.url_changes.len(), 1);
|
||||||
assert_eq!(REPEATED_FRAME_ENSURE_COUNT.load(Ordering::SeqCst), 2);
|
assert_eq!(REPEATED_FRAME_ENSURE_COUNT.load(Ordering::SeqCst), 2);
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ use super::{
|
|||||||
web_surface_geometry::{
|
web_surface_geometry::{
|
||||||
WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize,
|
WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize,
|
||||||
},
|
},
|
||||||
web_surface_metadata::WebSurfacePageMetadata,
|
web_surface_metadata::{WebSurfaceMetadataTracker, WebSurfacePageMetadata},
|
||||||
web_surface_permissions::WebSurfaceSitePermission,
|
web_surface_permissions::WebSurfaceSitePermission,
|
||||||
web_surface_runtime::WebSurfaceUrlChange,
|
web_surface_runtime::WebSurfaceUrlChange,
|
||||||
};
|
};
|
||||||
@@ -137,6 +137,7 @@ pub(super) struct PerTabSurface {
|
|||||||
pub(super) typed_text: Option<WebSurfaceTextInputState>,
|
pub(super) typed_text: Option<WebSurfaceTextInputState>,
|
||||||
pub(super) state: Option<WebSurfaceState>,
|
pub(super) state: Option<WebSurfaceState>,
|
||||||
last_input_flushed_at: Option<Instant>,
|
last_input_flushed_at: Option<Instant>,
|
||||||
|
metadata_tracker: WebSurfaceMetadataTracker,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl PerTabSurface {
|
impl PerTabSurface {
|
||||||
@@ -155,6 +156,7 @@ impl PerTabSurface {
|
|||||||
typed_text: None,
|
typed_text: None,
|
||||||
state: None,
|
state: None,
|
||||||
last_input_flushed_at: None,
|
last_input_flushed_at: None,
|
||||||
|
metadata_tracker: WebSurfaceMetadataTracker::default(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -195,6 +197,14 @@ impl PerTabSurface {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn changed_page_metadata(
|
||||||
|
&mut self,
|
||||||
|
tab_id: &TabId,
|
||||||
|
frame: &WebSurfaceFrame,
|
||||||
|
) -> Option<WebSurfacePageMetadata> {
|
||||||
|
self.metadata_tracker.changed_metadata_for(tab_id, frame)
|
||||||
|
}
|
||||||
|
|
||||||
fn has_pending_input(&self) -> bool {
|
fn has_pending_input(&self) -> bool {
|
||||||
self.hover_point.is_some()
|
self.hover_point.is_some()
|
||||||
|| self.click_point.is_some()
|
|| self.click_point.is_some()
|
||||||
|
|||||||
Reference in New Issue
Block a user