script: Do not prioritize *update-the-rendering* in ScriptThread message loop (#34539)

Instead run *update the rendering* at the end of every process of
gathering messages sent to the `ScriptThread`. This ensures that the
thread is in a consistent state when the update is finally run and
prevents running more than one instance of *update the rendering* per
spin of the message loop.

In addition:

- Move the *run the resize steps* implementation to `Window` and ensure
  that the realm is active when it is run.
- Profile the queueing of the resize message instead of handling it. I
  think this makes more sense as the profiling seems to be targeting
  message handling and not *update the rendering*. Additionally, it's
  difficult to profile from the context of `Window`.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2024-12-10 09:54:42 +01:00 committed by GitHub
parent bc2def0582
commit 2b8a8f7498
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 75 additions and 103 deletions

View file

@ -117,7 +117,7 @@ use crate::dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration
use crate::dom::customelementregistry::CustomElementRegistry; use crate::dom::customelementregistry::CustomElementRegistry;
use crate::dom::document::{AnimationFrameCallback, Document, ReflowTriggerCondition}; use crate::dom::document::{AnimationFrameCallback, Document, ReflowTriggerCondition};
use crate::dom::element::Element; use crate::dom::element::Element;
use crate::dom::event::{Event, EventStatus}; use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus};
use crate::dom::eventtarget::EventTarget; use crate::dom::eventtarget::EventTarget;
use crate::dom::globalscope::GlobalScope; use crate::dom::globalscope::GlobalScope;
use crate::dom::hashchangeevent::HashChangeEvent; use crate::dom::hashchangeevent::HashChangeEvent;
@ -136,6 +136,7 @@ use crate::dom::screen::Screen;
use crate::dom::selection::Selection; use crate::dom::selection::Selection;
use crate::dom::storage::Storage; use crate::dom::storage::Storage;
use crate::dom::testrunner::TestRunner; use crate::dom::testrunner::TestRunner;
use crate::dom::types::UIEvent;
use crate::dom::webglrenderingcontext::WebGLCommandSender; use crate::dom::webglrenderingcontext::WebGLCommandSender;
#[cfg(feature = "webgpu")] #[cfg(feature = "webgpu")]
use crate::dom::webgpu::identityhub::IdentityHub; use crate::dom::webgpu::identityhub::IdentityHub;
@ -144,7 +145,7 @@ use crate::dom::worklet::Worklet;
use crate::dom::workletglobalscope::WorkletGlobalScopeType; use crate::dom::workletglobalscope::WorkletGlobalScopeType;
use crate::layout_image::fetch_image_for_layout; use crate::layout_image::fetch_image_for_layout;
use crate::microtask::MicrotaskQueue; use crate::microtask::MicrotaskQueue;
use crate::realms::InRealm; use crate::realms::{enter_realm, InRealm};
use crate::script_runtime::{ use crate::script_runtime::{
CanGc, CommonScriptMsg, JSContext, Runtime, ScriptChan, ScriptPort, ScriptThreadEventCategory, CanGc, CommonScriptMsg, JSContext, Runtime, ScriptChan, ScriptPort, ScriptThreadEventCategory,
}; };
@ -2482,9 +2483,52 @@ impl Window {
self.parent_info.is_none() self.parent_info.is_none()
} }
/// An implementation of:
/// <https://drafts.csswg.org/cssom-view/#document-run-the-resize-steps>
///
/// Returns true if there were any pending resize events.
pub(crate) fn run_the_resize_steps(&self, can_gc: CanGc) -> bool {
let Some((new_size, size_type)) = self.take_unhandled_resize_event() else {
return false;
};
if self.window_size() == new_size {
return false;
}
let _realm = enter_realm(self);
debug!(
"Resizing Window for pipeline {:?} from {:?} to {new_size:?}",
self.pipeline_id(),
self.window_size(),
);
self.set_window_size(new_size);
// TODO: This should just trigger a pending reflow instead of forcing one now.
self.force_reflow(ReflowGoal::Full, ReflowReason::WindowResize, None);
// http://dev.w3.org/csswg/cssom-view/#resizing-viewports
if size_type == WindowSizeType::Resize {
let uievent = UIEvent::new(
self,
DOMString::from("resize"),
EventBubbles::DoesNotBubble,
EventCancelable::NotCancelable,
Some(self),
0i32,
can_gc,
);
uievent.upcast::<Event>().fire(self.upcast(), can_gc);
}
true
}
/// Evaluate media query lists and report changes /// Evaluate media query lists and report changes
/// <https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes> /// <https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes>
pub fn evaluate_media_queries_and_report_changes(&self, can_gc: CanGc) { pub fn evaluate_media_queries_and_report_changes(&self, can_gc: CanGc) {
let _realm = enter_realm(self);
rooted_vec!(let mut mql_list); rooted_vec!(let mut mql_list);
self.media_query_lists.for_each(|mql| { self.media_query_lists.for_each(|mql| {
if let MediaQueryListMatchState::Changed = mql.evaluate_changes() { if let MediaQueryListMatchState::Changed = mql.evaluate_changes() {

View file

@ -126,7 +126,6 @@ use crate::dom::document::{
Document, DocumentSource, FocusType, HasBrowsingContext, IsHTMLDocument, TouchEventResult, Document, DocumentSource, FocusType, HasBrowsingContext, IsHTMLDocument, TouchEventResult,
}; };
use crate::dom::element::Element; use crate::dom::element::Element;
use crate::dom::event::{Event, EventBubbles, EventCancelable};
use crate::dom::globalscope::GlobalScope; use crate::dom::globalscope::GlobalScope;
use crate::dom::htmlanchorelement::HTMLAnchorElement; use crate::dom::htmlanchorelement::HTMLAnchorElement;
use crate::dom::htmliframeelement::HTMLIFrameElement; use crate::dom::htmliframeelement::HTMLIFrameElement;
@ -136,7 +135,6 @@ use crate::dom::performanceentry::PerformanceEntry;
use crate::dom::performancepainttiming::PerformancePaintTiming; use crate::dom::performancepainttiming::PerformancePaintTiming;
use crate::dom::serviceworker::TrustedServiceWorkerAddress; use crate::dom::serviceworker::TrustedServiceWorkerAddress;
use crate::dom::servoparser::{ParserContext, ServoParser}; use crate::dom::servoparser::{ParserContext, ServoParser};
use crate::dom::uievent::UIEvent;
#[cfg(feature = "webgpu")] #[cfg(feature = "webgpu")]
use crate::dom::webgpu::identityhub::IdentityHub; use crate::dom::webgpu::identityhub::IdentityHub;
use crate::dom::window::{ReflowReason, Window}; use crate::dom::window::{ReflowReason, Window};
@ -1333,19 +1331,6 @@ impl ScriptThread {
debug!("Stopped script thread."); debug!("Stopped script thread.");
} }
/// <https://drafts.csswg.org/cssom-view/#document-run-the-resize-steps>
fn run_the_resize_steps(
&self,
id: PipelineId,
size: WindowSizeData,
size_type: WindowSizeType,
can_gc: CanGc,
) {
self.profile_event(ScriptThreadEventCategory::Resize, Some(id), || {
self.handle_resize_event(id, size, size_type, can_gc);
});
}
/// Process a compositor mouse move event. /// Process a compositor mouse move event.
fn process_mouse_move_event( fn process_mouse_move_event(
&self, &self,
@ -1592,10 +1577,8 @@ impl ScriptThread {
// TODO(#31665): Implement the "run the scroll steps" from // TODO(#31665): Implement the "run the scroll steps" from
// https://drafts.csswg.org/cssom-view/#document-run-the-scroll-steps. // https://drafts.csswg.org/cssom-view/#document-run-the-scroll-steps.
if let Some((size, size_type)) = document.window().take_unhandled_resize_event() { // > 8. For each doc of docs, run the resize steps for doc. [CSSOMVIEW]
// Resize steps. if document.window().run_the_resize_steps(can_gc) {
self.run_the_resize_steps(pipeline_id, size, size_type, can_gc);
// Evaluate media queries and report changes. // Evaluate media queries and report changes.
document document
.window() .window()
@ -1690,13 +1673,14 @@ impl ScriptThread {
// Queues a task to update the rendering. // Queues a task to update the rendering.
// <https://html.spec.whatwg.org/multipage/#event-loop-processing-model:queue-a-global-task> // <https://html.spec.whatwg.org/multipage/#event-loop-processing-model:queue-a-global-task>
//
// Note: The specification says to queue a task using the navigable's active
// window, but then updates the rendering for all documents.
let _ = rendering_task_source.queue_with_canceller( let _ = rendering_task_source.queue_with_canceller(
task!(update_the_rendering: move || { task!(update_the_rendering: move || {
// Note: spec says to queue a task using the navigable's active window, // This task is empty because any new IPC messages in the ScriptThread trigger a
// but then updates the rendering for all docs in the same event-loop. // rendering update, unless animations are running, in which case updates are driven
with_script_thread(|script_thread| { // by the compositor.
script_thread.update_the_rendering(false, CanGc::note());
})
}), }),
&canceller, &canceller,
); );
@ -1749,11 +1733,8 @@ impl ScriptThread {
} }
}, },
}; };
// Prioritize only a single update of the rendering;
// others will run together with the other sequential tasks.
let mut rendering_update_already_prioritized = false;
let mut compositor_requested_update_the_rendering = false;
let mut compositor_requested_update_the_rendering = false;
loop { loop {
debug!("Handling event: {event:?}"); debug!("Handling event: {event:?}");
@ -1829,29 +1810,14 @@ impl ScriptThread {
self.handle_event(id, event) self.handle_event(id, event)
}, },
FromScript(MainThreadScriptMsg::Common(CommonScriptMsg::Task( FromScript(MainThreadScriptMsg::Common(CommonScriptMsg::Task(
category, _,
task, _,
pipeline_id, _,
TaskSourceName::Rendering, TaskSourceName::Rendering,
))) => { ))) => {
if rendering_update_already_prioritized { // Instead of interleaving any number of update the rendering tasks with other
// If we've already updated the rendering, // message handling, we run those steps only once at the end of each call of
// run this task along with the other non-prioritized ones. // this function.
sequential.push(FromScript(MainThreadScriptMsg::Common(
CommonScriptMsg::Task(
category,
task,
pipeline_id,
TaskSourceName::Rendering,
),
)));
} else {
// Run the "update the rendering" task.
task.run_box();
// Always perform a microtrask checkpoint after running a task.
self.perform_a_microtask_checkpoint(can_gc);
rendering_update_already_prioritized = true;
}
}, },
FromScript(MainThreadScriptMsg::Inactive) => { FromScript(MainThreadScriptMsg::Inactive) => {
// An event came-in from a document that is not fully-active, it has been stored by the task-queue. // An event came-in from a document that is not fully-active, it has been stored by the task-queue.
@ -2876,18 +2842,20 @@ impl ScriptThread {
size: WindowSizeData, size: WindowSizeData,
size_type: WindowSizeType, size_type: WindowSizeType,
) { ) {
let window = self.documents.borrow().find_window(id); self.profile_event(ScriptThreadEventCategory::Resize, Some(id), || {
if let Some(ref window) = window { let window = self.documents.borrow().find_window(id);
self.rendering_opportunity(id); if let Some(ref window) = window {
window.add_resize_event(size, size_type); self.rendering_opportunity(id);
return; window.add_resize_event(size, size_type);
} return;
let mut loads = self.incomplete_loads.borrow_mut(); }
if let Some(ref mut load) = loads.iter_mut().find(|load| load.pipeline_id == id) { let mut loads = self.incomplete_loads.borrow_mut();
load.window_size = size; if let Some(ref mut load) = loads.iter_mut().find(|load| load.pipeline_id == id) {
return; load.window_size = size;
} return;
warn!("resize sent to nonexistent pipeline"); }
warn!("resize sent to nonexistent pipeline");
})
} }
// exit_fullscreen creates a new JS promise object, so we need to have entered a realm // exit_fullscreen creates a new JS promise object, so we need to have entered a realm
@ -4032,46 +4000,6 @@ impl ScriptThread {
load_data.url = ServoUrl::parse("about:blank").unwrap(); load_data.url = ServoUrl::parse("about:blank").unwrap();
} }
fn handle_resize_event(
&self,
pipeline_id: PipelineId,
new_size: WindowSizeData,
size_type: WindowSizeType,
can_gc: CanGc,
) {
let document = match self.documents.borrow().find_document(pipeline_id) {
Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
};
let window = document.window();
if window.window_size() == new_size {
return;
}
debug!(
"resizing pipeline {:?} from {:?} to {:?}",
pipeline_id,
window.window_size(),
new_size
);
window.set_window_size(new_size);
window.force_reflow(ReflowGoal::Full, ReflowReason::WindowResize, None);
// http://dev.w3.org/csswg/cssom-view/#resizing-viewports
if size_type == WindowSizeType::Resize {
let uievent = UIEvent::new(
window,
DOMString::from("resize"),
EventBubbles::DoesNotBubble,
EventCancelable::NotCancelable,
Some(window),
0i32,
can_gc,
);
uievent.upcast::<Event>().fire(window.upcast(), can_gc);
}
}
/// Instructs the constellation to fetch the document that will be loaded. Stores the InProgressLoad /// Instructs the constellation to fetch the document that will be loaded. Stores the InProgressLoad
/// argument until a notification is received that the fetch is complete. /// argument until a notification is received that the fetch is complete.
fn pre_page_load(&self, mut incomplete: InProgressLoad, load_data: LoadData) { fn pre_page_load(&self, mut incomplete: InProgressLoad, load_data: LoadData) {