diff options
author | Martin Robinson <mrobinson@igalia.com> | 2024-12-13 14:25:47 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-13 13:25:47 +0000 |
commit | 471d3572b77be4df31ce5d6a20d522eb9ffdbe7f (patch) | |
tree | 4bffa0479bbf86a153d57ff0d921bcd78d8efdae /components/script | |
parent | 682eba9f7425fb4478207372f5f9e550b467bf7b (diff) | |
download | servo-471d3572b77be4df31ce5d6a20d522eb9ffdbe7f.tar.gz servo-471d3572b77be4df31ce5d6a20d522eb9ffdbe7f.zip |
script: No longer do explicit reflows for display (#34599)
These all happen now in *update the rendering*, typically after the
message that triggered this code is processed, though in two cases
reflow needs to be triggered explicitly. This makes `ReflowReason`
redundant though perhaps `ReflowCondition` can be expanded later to give
more insight into why the page is dirty.
- Handling of the "reflow timer" concept has been explained a bit more via
data structures and rustdoc comments.
- Theme changes are cleaned up a little to simplify what happens during
reflow and to avoid unecessary reflows when the theme doesn't change.
Notably, layout queries and scrolling still trigger normal reflows and
don't update the rendering. This needs more investigation as it's
unclear to me currently whether or not they should update the rendering
and simply delay event dispatch or only reflow.
In general, this is a simplfication of the code.
Fixes #31871.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Diffstat (limited to 'components/script')
-rw-r--r-- | components/script/dom/activation.rs | 18 | ||||
-rw-r--r-- | components/script/dom/document.rs | 63 | ||||
-rw-r--r-- | components/script/dom/element.rs | 14 | ||||
-rw-r--r-- | components/script/dom/htmlbodyelement.rs | 11 | ||||
-rw-r--r-- | components/script/dom/servoparser/mod.rs | 4 | ||||
-rw-r--r-- | components/script/dom/window.rs | 221 | ||||
-rw-r--r-- | components/script/script_thread.rs | 20 |
7 files changed, 165 insertions, 186 deletions
diff --git a/components/script/dom/activation.rs b/components/script/dom/activation.rs index 8f325fb4e89..2d78f505214 100644 --- a/components/script/dom/activation.rs +++ b/components/script/dom/activation.rs @@ -2,14 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use script_layout_interface::ReflowGoal; - use crate::dom::element::Element; use crate::dom::event::Event; use crate::dom::eventtarget::EventTarget; use crate::dom::htmlinputelement::InputActivationState; -use crate::dom::node::window_from_node; -use crate::dom::window::ReflowReason; use crate::script_runtime::CanGc; /// Trait for elements with defined activation behavior @@ -35,23 +31,9 @@ pub trait Activatable { // https://html.spec.whatwg.org/multipage/#concept-selector-active fn enter_formal_activation_state(&self) { self.as_element().set_active_state(true); - - let win = window_from_node(self.as_element()); - win.reflow( - ReflowGoal::Full, - ReflowReason::ElementStateChanged, - CanGc::note(), - ); } fn exit_formal_activation_state(&self) { self.as_element().set_active_state(false); - - let win = window_from_node(self.as_element()); - win.reflow( - ReflowGoal::Full, - ReflowReason::ElementStateChanged, - CanGc::note(), - ); } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 550832a47bb..84441a3d239 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -47,7 +47,7 @@ use num_traits::ToPrimitive; use percent_encoding::percent_decode; use profile_traits::ipc as profile_ipc; use profile_traits::time::{TimerMetadata, TimerMetadataFrameType, TimerMetadataReflowType}; -use script_layout_interface::{PendingRestyle, ReflowGoal, TrustedNodeAddress}; +use script_layout_interface::{PendingRestyle, TrustedNodeAddress}; use script_traits::{ AnimationState, AnimationTickType, CompositorEvent, DocumentActivity, MouseButton, MouseEventType, ScriptMsg, TouchEventType, TouchId, UntrustedNodeAddress, WheelDelta, @@ -180,7 +180,7 @@ use crate::dom::webglrenderingcontext::WebGLRenderingContext; #[cfg(feature = "webgpu")] use crate::dom::webgpu::gpucanvascontext::GPUCanvasContext; use crate::dom::wheelevent::WheelEvent; -use crate::dom::window::{ReflowReason, Window}; +use crate::dom::window::Window; use crate::dom::windowproxy::WindowProxy; use crate::dom::xpathevaluator::XPathEvaluator; use crate::fetch::FetchCanceller; @@ -335,8 +335,6 @@ pub struct Document { loader: DomRefCell<DocumentLoader>, /// The current active HTML parser, to allow resuming after interruptions. current_parser: MutNullableDom<ServoParser>, - /// When we should kick off a reflow. This happens during parsing. - reflow_timeout: Cell<Option<Instant>>, /// The cached first `base` element with an `href` attribute. base_element: MutNullableDom<HTMLBaseElement>, /// This field is set to the document itself for inert documents. @@ -705,7 +703,7 @@ impl Document { self.activity.get() != DocumentActivity::Inactive } - pub fn set_activity(&self, activity: DocumentActivity, can_gc: CanGc) { + pub fn set_activity(&self, activity: DocumentActivity) { // This function should only be called on documents with a browsing context assert!(self.has_browsing_context); if activity == self.activity.get() { @@ -727,11 +725,6 @@ impl Document { self.title_changed(); self.dirty_all_nodes(); - self.window().reflow( - ReflowGoal::Full, - ReflowReason::CachedPageNeededReflow, - can_gc, - ); self.window().resume(); media.resume(&client_context_id); @@ -919,36 +912,6 @@ impl Document { node.dirty(NodeDamage::OtherNodeDamage); } - /// Reflows and disarms the timer if the reflow timer has expired. - pub fn reflow_if_reflow_timer_expired(&self, can_gc: CanGc) { - let Some(reflow_timeout) = self.reflow_timeout.get() else { - return; - }; - - if Instant::now() < reflow_timeout { - return; - } - - self.reflow_timeout.set(None); - self.window - .reflow(ReflowGoal::Full, ReflowReason::RefreshTick, can_gc); - } - - /// Schedules a reflow to be kicked off at the given [`Duration`] in the future. This reflow - /// happens even if the event loop is busy. This is used to display initial page content during - /// parsing. - pub fn set_reflow_timeout(&self, duration: Duration) { - let new_reflow_time = Instant::now() + duration; - - // Do not schedule a timeout that is longer than the one that is currently queued. - if matches!(self.reflow_timeout.get(), Some(existing_timeout) if existing_timeout < new_reflow_time) - { - return; - } - - self.reflow_timeout.set(Some(new_reflow_time)) - } - /// Remove any existing association between the provided id and any elements in this document. pub fn unregister_element_id(&self, to_unregister: &Element, id: Atom) { self.document_or_shadow_root @@ -1037,7 +1000,7 @@ impl Document { let target = self.find_fragment_node(fragment); // Step 1 - self.set_target_element(target.as_deref(), can_gc); + self.set_target_element(target.as_deref()); let point = target .as_ref() @@ -2257,16 +2220,10 @@ impl Document { self.process_deferred_scripts(); }, LoadType::PageSource(_) => { + // We finished loading the page, so if the `Window` is still waiting for + // the first layout, allow it. if self.has_browsing_context && self.is_fully_active() { - // Note: if the document is not fully active, layout will have exited already. - // The underlying problem might actually be that layout exits while it should be kept alive. - // See https://github.com/servo/servo/issues/22507 - - // Disarm the reflow timer and trigger the initial reflow. - self.reflow_timeout.set(None); - self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage); - self.window - .reflow(ReflowGoal::Full, ReflowReason::FirstLoad, can_gc); + self.window().allow_layout_if_necessary(can_gc); } // Deferred scripts have to wait for page to finish loading, @@ -3385,7 +3342,6 @@ impl Document { running_animation_callbacks: Cell::new(false), loader: DomRefCell::new(doc_loader), current_parser: Default::default(), - reflow_timeout: Cell::new(None), base_element: Default::default(), appropriate_template_contents_owner_document: Default::default(), pending_restyles: DomRefCell::new(HashMap::new()), @@ -3831,7 +3787,7 @@ impl Document { self.policy_container.borrow().get_referrer_policy() } - pub fn set_target_element(&self, node: Option<&Element>, can_gc: CanGc) { + pub fn set_target_element(&self, node: Option<&Element>) { if let Some(ref element) = self.target_element.get() { element.set_target_state(false); } @@ -3841,9 +3797,6 @@ impl Document { if let Some(ref element) = self.target_element.get() { element.set_target_state(true); } - - self.window - .reflow(ReflowGoal::Full, ReflowReason::ElementStateChanged, can_gc); } pub fn incr_ignore_destructive_writes_counter(&self) { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index f28070a7bf7..44838256cf5 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -26,7 +26,6 @@ use js::jsval::JSVal; use js::rust::HandleObject; use net_traits::request::CorsSettings; use net_traits::ReferrerPolicy; -use script_layout_interface::ReflowGoal; use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint}; use selectors::bloom::{BloomFilter, BLOOM_HASH_MASK}; use selectors::matching::{ElementSelectorFlags, MatchingContext}; @@ -149,7 +148,6 @@ use crate::dom::text::Text; use crate::dom::validation::Validatable; use crate::dom::validitystate::ValidationFlags; use crate::dom::virtualmethods::{vtable_for, VirtualMethods}; -use crate::dom::window::ReflowReason; use crate::script_runtime::CanGc; use crate::script_thread::ScriptThread; use crate::stylesheet_loader::StylesheetOwner; @@ -4429,11 +4427,6 @@ impl TaskOnce for ElementPerformFullscreenEnter { // Step 7.5 element.set_fullscreen_state(true); document.set_fullscreen_element(Some(&element)); - document.window().reflow( - ReflowGoal::Full, - ReflowReason::ElementStateChanged, - CanGc::note(), - ); // Step 7.6 document @@ -4467,13 +4460,6 @@ impl TaskOnce for ElementPerformFullscreenExit { // TODO Step 9.1-5 // Step 9.6 element.set_fullscreen_state(false); - - document.window().reflow( - ReflowGoal::Full, - ReflowReason::ElementStateChanged, - CanGc::note(), - ); - document.set_fullscreen_element(None); // Step 9.8 diff --git a/components/script/dom/htmlbodyelement.rs b/components/script/dom/htmlbodyelement.rs index 5bb650973c7..0b749b779a5 100644 --- a/components/script/dom/htmlbodyelement.rs +++ b/components/script/dom/htmlbodyelement.rs @@ -2,8 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use std::time::Duration; - use dom_struct::dom_struct; use embedder_traits::EmbedderMsg; use html5ever::{local_name, namespace_url, ns, LocalName, Prefix}; @@ -26,9 +24,6 @@ use crate::dom::node::{document_from_node, window_from_node, BindContext, Node}; use crate::dom::virtualmethods::VirtualMethods; use crate::script_runtime::CanGc; -/// How long we should wait before performing the initial reflow after `<body>` is parsed. -const INITIAL_REFLOW_DELAY: Duration = Duration::from_millis(200); - #[dom_struct] pub struct HTMLBodyElement { htmlelement: HTMLElement, @@ -158,11 +153,9 @@ impl VirtualMethods for HTMLBodyElement { } let window = window_from_node(self); - let document = window.Document(); - document.set_reflow_timeout(INITIAL_REFLOW_DELAY); + window.prevent_layout_until_load_event(); if window.is_top_level() { - let msg = EmbedderMsg::HeadParsed; - window.send_to_embedder(msg); + window.send_to_embedder(EmbedderMsg::HeadParsed); } } diff --git a/components/script/dom/servoparser/mod.rs b/components/script/dom/servoparser/mod.rs index aa181a3e074..2a2b19ad061 100644 --- a/components/script/dom/servoparser/mod.rs +++ b/components/script/dom/servoparser/mod.rs @@ -618,7 +618,9 @@ impl ServoParser { assert!(!self.suspended.get()); assert!(!self.aborted.get()); - self.document.reflow_if_reflow_timer_expired(can_gc); + self.document + .window() + .reflow_if_reflow_timer_expired(can_gc); let script = match feed(&self.tokenizer) { TokenizerResult::Done => return, TokenizerResult::Script(script) => script, diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 0c2f8ef57ed..3b7bc966534 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -13,7 +13,7 @@ use std::ptr::NonNull; use std::rc::Rc; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::time::{Duration, Instant}; use app_units::Au; use backtrace::Backtrace; @@ -72,6 +72,7 @@ use style::media_queries; use style::parser::ParserContext as CssParserContext; use style::properties::style_structs::Font; use style::properties::PropertyId; +use style::queries::values::PrefersColorScheme; use style::selector_parser::PseudoElement; use style::str::HTML_SPACE_CHARACTERS; use style::stylesheets::{CssRuleType, Origin, UrlExtraData}; @@ -167,20 +168,36 @@ enum WindowState { Zombie, // Pipeline is closed, but the window hasn't been GCed yet. } -/// Extra information concerning the reason for reflowing. -#[derive(Debug, MallocSizeOf)] -pub enum ReflowReason { - CachedPageNeededReflow, - ElementStateChanged, - FirstLoad, - Query, - RefreshTick, - RequestAnimationFrame, - ScrollFromScript, - UpdateTheRendering, - Viewport, - WorkletLoaded, - ThemeChange, +/// How long we should wait before performing the initial reflow after `<body>` is parsed, +/// assuming that `<body>` take this long to parse. +const INITIAL_REFLOW_DELAY: Duration = Duration::from_millis(200); + +/// During loading and parsing, layouts are suppressed to avoid flashing incomplete page +/// contents. +/// +/// Exceptions: +/// - Parsing the body takes so long, that layouts are no longer suppressed in order +/// to show the user that the page is loading. +/// - Script triggers a layout query or scroll event in which case, we want to layout +/// but not display the contents. +/// +/// For more information see: <https://github.com/servo/servo/pull/6028>. +#[derive(Clone, Copy, MallocSizeOf)] +enum LayoutBlocker { + /// The first load event hasn't been fired and we have not started to parse the `<body>` yet. + WaitingForParse, + /// The body is being parsed the `<body>` starting at the `Instant` specified. + Parsing(Instant), + /// The body finished parsing and the `load` event has been fired or parsing took so + /// long, that we are going to do layout anyway. Note that subsequent changes to the body + /// can trigger parsing again, but the `Window` stays in this state. + FiredLoadEventOrParsingTimerExpired, +} + +impl LayoutBlocker { + fn layout_blocked(&self) -> bool { + !matches!(self, Self::FiredLoadEventOrParsingTimerExpired) + } } #[dom_struct] @@ -226,7 +243,7 @@ pub struct Window { /// Platform theme. #[no_trace] - theme: Cell<Theme>, + theme: Cell<PrefersColorScheme>, /// Parent id associated with this page, if any. #[no_trace] @@ -255,10 +272,11 @@ pub struct Window { #[no_trace] page_clip_rect: Cell<UntypedRect<Au>>, - /// Flag to suppress reflows. The first reflow will come either with - /// RefreshTick or with FirstLoad. Until those first reflows, we want to - /// suppress others like MissingExplicitReflow. - suppress_reflow: Cell<bool>, + /// See the documentation for [`LayoutBlocker`]. Essentially, this flag prevents + /// layouts from happening before the first load event, apart from a few exceptional + /// cases. + #[no_trace] + layout_blocker: Cell<LayoutBlocker>, /// A channel for communicating results of async scripts back to the webdriver server #[ignore_malloc_size_of = "channels are hard"] @@ -1769,7 +1787,6 @@ impl Window { scroll_id, scroll_offset: Vector2D::new(-x, -y), }), - ReflowReason::ScrollFromScript, can_gc, ); } @@ -1809,41 +1826,30 @@ impl Window { ScriptThread::handle_tick_all_animations_for_testing(pipeline_id); } - pub(crate) fn reflows_suppressed(&self) -> bool { - self.suppress_reflow.get() - } - /// Reflows the page unconditionally if possible and not suppressed. This method will wait for /// the layout to complete. If there is no window size yet, the page is presumed invisible and /// no reflow is performed. If reflow is suppressed, no reflow will be performed for ForDisplay /// goals. /// /// Returns true if layout actually happened, false otherwise. + /// + /// NOTE: This method should almost never be called directly! Layout and rendering updates should + /// happen as part of the HTML event loop via *update the rendering*. #[allow(unsafe_code)] - pub(crate) fn force_reflow( + fn force_reflow( &self, reflow_goal: ReflowGoal, - reason: ReflowReason, condition: Option<ReflowTriggerCondition>, ) -> bool { self.Document().ensure_safe_to_run_script_or_layout(); - // Check if we need to unsuppress reflow. Note that this needs to be - // *before* any early bailouts, or reflow might never be unsuppresed! - match reason { - ReflowReason::FirstLoad | ReflowReason::RefreshTick => self.suppress_reflow.set(false), - _ => (), - } - let for_display = reflow_goal.needs_display(); + // If layouts are blocked, we block all layouts that are for display only. Other + // layouts (for queries and scrolling) are not blocked, as they do not display + // anything and script excpects the layout to be up-to-date after they run. + let layout_blocked = self.layout_blocker.get().layout_blocked(); let pipeline_id = self.upcast::<GlobalScope>().pipeline_id(); - - // If this was just a normal reflow (not triggered by script which forces reflow), - // and the document is still suppressing reflows, exit early. - if reflow_goal == ReflowGoal::Full && self.suppress_reflow.get() { - debug!( - "Suppressing reflow pipeline {} for reason {:?} before FirstLoad or RefreshTick", - pipeline_id, reason - ); + if reflow_goal == ReflowGoal::UpdateTheRendering && layout_blocked { + debug!("Suppressing pre-load-event reflow pipeline {pipeline_id}"); return false; } @@ -1860,8 +1866,7 @@ impl Window { debug!("Not invalidating cached layout values for paint-only reflow."); } - debug!("script: performing reflow for reason {:?}", reason); - + debug!("script: performing reflow for goal {reflow_goal:?}"); let marker = if self.need_emit_timeline_marker(TimelineMarkerType::Reflow) { Some(TimelineMarker::start("Reflow".to_owned())) } else { @@ -1873,7 +1878,7 @@ impl Window { // On debug mode, print the reflow event information. if self.relayout_event { - debug_reflow_events(pipeline_id, &reflow_goal, &reason); + debug_reflow_events(pipeline_id, &reflow_goal); } let document = self.Document(); @@ -1882,6 +1887,7 @@ impl Window { // If this reflow is for display, ensure webgl canvases are composited with // up-to-date contents. + let for_display = reflow_goal.needs_display(); if for_display { #[cfg(feature = "webgpu")] document.flush_dirty_webgpu_canvases(); @@ -1896,11 +1902,6 @@ impl Window { .or_else(|| document.GetDocumentElement()) .map(|root| root.upcast::<Node>().to_trusted_node_address()); - let theme = match self.theme.get() { - Theme::Light => style::queries::values::PrefersColorScheme::Light, - Theme::Dark => style::queries::values::PrefersColorScheme::Dark, - }; - // Send new document and relevant styles to layout. let reflow = ScriptReflow { reflow_info: Reflow { @@ -1917,7 +1918,7 @@ impl Window { pending_restyles, animation_timeline_value: document.current_animation_timeline_value(), animations: document.animations().sets.clone(), - theme, + theme: self.theme.get(), }; self.layout.borrow_mut().reflow(reflow); @@ -1977,26 +1978,25 @@ impl Window { true } - /// Reflows the page if it's possible to do so and the page is dirty. + /// Reflows the page if it's possible to do so and the page is dirty. Returns true if layout + /// actually happened, false otherwise. /// - /// Returns true if layout actually happened, false otherwise. - /// This return value is useful for script queries, that wait for a lock - /// that layout might hold if the first layout hasn't happened yet (which - /// may happen in the only case a query reflow may bail out, that is, if the - /// viewport size is not present). See #11223 for an example of that. - pub fn reflow(&self, reflow_goal: ReflowGoal, reason: ReflowReason, can_gc: CanGc) -> bool { + /// NOTE: This method should almost never be called directly! Layout and rendering updates + /// should happen as part of the HTML event loop via *update the rendering*. Currerntly, the + /// only exceptions are script queries and scroll requests. + pub(crate) fn reflow(&self, reflow_goal: ReflowGoal, can_gc: CanGc) -> bool { // Fetch the pending web fonts before layout, in case a font loads during // the layout. let pending_web_fonts = self.layout.borrow().waiting_for_web_fonts_to_load(); self.Document().ensure_safe_to_run_script_or_layout(); - let for_display = reflow_goal == ReflowGoal::Full; let mut issued_reflow = false; let condition = self.Document().needs_reflow(); - if !for_display || condition.is_some() { + let updating_the_rendering = reflow_goal == ReflowGoal::UpdateTheRendering; + if !updating_the_rendering || condition.is_some() { debug!("Reflowing document ({:?})", self.pipeline_id()); - issued_reflow = self.force_reflow(reflow_goal, reason, condition); + issued_reflow = self.force_reflow(reflow_goal, condition); // We shouldn't need a reflow immediately after a // reflow, except if we're waiting for a deferred paint. @@ -2004,16 +2004,16 @@ impl Window { assert!( { condition.is_none() || - (!for_display && + (!updating_the_rendering && condition == Some(ReflowTriggerCondition::PaintPostponed)) || - self.suppress_reflow.get() + self.layout_blocker.get().layout_blocked() }, "condition was {:?}", condition ); } else { debug!( - "Document ({:?}) doesn't need reflow - skipping it (reason {reason:?})", + "Document ({:?}) doesn't need reflow - skipping it (goal {reflow_goal:?})", self.pipeline_id() ); } @@ -2044,7 +2044,7 @@ impl Window { // When all these conditions are met, notify the constellation // that this pipeline is ready to write the image (from the script thread // perspective at least). - if self.prepare_for_screenshot && for_display { + if self.prepare_for_screenshot && updating_the_rendering { // Checks if the html element has reftest-wait attribute present. // See http://testthewebforward.org/docs/reftests.html // and https://web-platform-tests.org/writing-tests/crashtest.html @@ -2076,6 +2076,66 @@ impl Window { issued_reflow } + /// If parsing has taken a long time and reflows are still waiting for the `load` event, + /// start allowing them. See <https://github.com/servo/servo/pull/6028>. + pub(crate) fn reflow_if_reflow_timer_expired(&self, can_gc: CanGc) { + // Only trigger a long parsing time reflow if we are in the first parse of `<body>` + // and it started more than `INITIAL_REFLOW_DELAY` ago. + if !matches!( + self.layout_blocker.get(), + LayoutBlocker::Parsing(instant) if instant + INITIAL_REFLOW_DELAY < Instant::now() + ) { + return; + } + self.allow_layout_if_necessary(can_gc); + } + + /// Block layout for this `Window` until parsing is done. If parsing takes a long time, + /// we want to layout anyway, so schedule a moment in the future for when layouts are + /// allowed even though parsing isn't finished and we havne't sent a load event. + pub(crate) fn prevent_layout_until_load_event(&self) { + // If we have already started parsing or have already fired a load event, then + // don't delay the first layout any longer. + if !matches!(self.layout_blocker.get(), LayoutBlocker::WaitingForParse) { + return; + } + + self.layout_blocker + .set(LayoutBlocker::Parsing(Instant::now())); + } + + /// Inform the [`Window`] that layout is allowed either because `load` has happened + /// or because parsing the `<body>` took so long that we cannot wait any longer. + pub(crate) fn allow_layout_if_necessary(&self, can_gc: CanGc) { + if matches!( + self.layout_blocker.get(), + LayoutBlocker::FiredLoadEventOrParsingTimerExpired + ) { + return; + } + + self.layout_blocker + .set(LayoutBlocker::FiredLoadEventOrParsingTimerExpired); + self.Document().set_needs_paint(true); + + // We do this immediately instead of scheduling a future task, because this can + // happen if parsing is taking a very long time, which means that the + // `ScriptThread` is busy doing the parsing and not doing layouts. + // + // TOOD(mrobinson): It's expected that this is necessary when in the process of + // parsing, as we need to interrupt it to update contents, but why is this + // necessary when parsing finishes? Not doing the synchronous update in that case + // causes iframe tests to become flaky. It seems there's an issue with the timing of + // iframe size updates. + // + // See <https://github.com/servo/servo/issues/14719> + self.reflow(ReflowGoal::UpdateTheRendering, can_gc); + } + + pub(crate) fn layout_blocked(&self) -> bool { + self.layout_blocker.get().layout_blocked() + } + /// If writing a screenshot, synchronously update the layout epoch that it set /// in the constellation. pub(crate) fn update_constellation_epoch(&self) { @@ -2095,11 +2155,7 @@ impl Window { } pub fn layout_reflow(&self, query_msg: QueryMsg, can_gc: CanGc) -> bool { - self.reflow( - ReflowGoal::LayoutQuery(query_msg), - ReflowReason::Query, - can_gc, - ) + self.reflow(ReflowGoal::LayoutQuery(query_msg), can_gc) } pub fn resolved_font_style_query( @@ -2368,8 +2424,18 @@ impl Window { self.window_size.get() } - pub fn set_theme(&self, theme: Theme) { - self.theme.set(theme); + /// Handle a theme change request, triggering a reflow is any actual change occured. + pub(crate) fn handle_theme_change(&self, new_theme: Theme) { + let new_theme = match new_theme { + Theme::Light => PrefersColorScheme::Light, + Theme::Dark => PrefersColorScheme::Dark, + }; + + if self.theme.get() == new_theme { + return; + } + self.theme.set(new_theme); + self.Document().set_needs_paint(true); } pub fn get_url(&self) -> ServoUrl { @@ -2717,7 +2783,7 @@ impl Window { unhandled_resize_event: Default::default(), window_size: Cell::new(window_size), current_viewport: Cell::new(initial_viewport.to_untyped()), - suppress_reflow: Cell::new(true), + layout_blocker: Cell::new(LayoutBlocker::WaitingForParse), current_state: Cell::new(WindowState::Alive), devtools_marker_sender: Default::default(), devtools_markers: Default::default(), @@ -2747,7 +2813,7 @@ impl Window { throttled: Cell::new(false), layout_marker: DomRefCell::new(Rc::new(Cell::new(true))), current_event: DomRefCell::new(None), - theme: Cell::new(Theme::Light), + theme: Cell::new(PrefersColorScheme::Light), }); unsafe { WindowBinding::Wrap(JSContext::from_ptr(runtime.cx()), win) } @@ -2825,10 +2891,9 @@ fn should_move_clip_rect(clip_rect: UntypedRect<Au>, new_viewport: UntypedRect<f (clip_rect.max_y() - new_viewport.max_y()).abs() <= viewport_scroll_margin.height } -fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal, reason: &ReflowReason) { +fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal) { let goal_string = match *reflow_goal { - ReflowGoal::Full => "\tFull", - ReflowGoal::TickAnimations => "\tTickAnimations", + ReflowGoal::UpdateTheRendering => "\tFull", ReflowGoal::UpdateScrollNode(_) => "\tUpdateScrollNode", ReflowGoal::LayoutQuery(ref query_msg) => match *query_msg { QueryMsg::ContentBox => "\tContentBoxQuery", @@ -2846,7 +2911,7 @@ fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal, reason: &Reflow }, }; - println!("**** pipeline={}\t{}\t{:?}", id, goal_string, reason); + println!("**** pipeline={id}\t{goal_string}"); } impl Window { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 095d3eefbf6..4730b48600d 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -137,7 +137,7 @@ use crate::dom::serviceworker::TrustedServiceWorkerAddress; use crate::dom::servoparser::{ParserContext, ServoParser}; #[cfg(feature = "webgpu")] use crate::dom::webgpu::identityhub::IdentityHub; -use crate::dom::window::{ReflowReason, Window}; +use crate::dom::window::Window; use crate::dom::windowproxy::{CreatorBrowsingContextInfo, WindowProxy}; use crate::dom::worker::TrustedWorkerAddress; use crate::dom::worklet::WorkletThreadPool; @@ -1625,7 +1625,7 @@ impl ScriptThread { // > doc and its node navigable to reflect the current state. let window = document.window(); if document.is_fully_active() { - window.reflow(ReflowGoal::Full, ReflowReason::UpdateTheRendering, can_gc); + window.reflow(ReflowGoal::UpdateTheRendering, can_gc); } // TODO: Process top layer removals according to @@ -1660,7 +1660,7 @@ impl ScriptThread { } let Some((_, document)) = self.documents.borrow().iter().find(|(_, document)| { - !document.window().reflows_suppressed() && document.needs_reflow().is_some() + !document.window().layout_blocked() && document.needs_reflow().is_some() }) else { return; }; @@ -2254,7 +2254,7 @@ impl ScriptThread { self.handle_resize_inactive_msg(id, new_size) }, ConstellationControlMsg::ThemeChange(_, theme) => { - self.handle_theme_change(theme); + self.handle_theme_change_msg(theme); }, ConstellationControlMsg::GetTitle(pipeline_id) => { self.handle_get_title_msg(pipeline_id) @@ -2846,12 +2846,10 @@ impl ScriptThread { }) } - fn handle_theme_change(&self, theme: Theme) { - let docs = self.documents.borrow(); - for (_, document) in docs.iter() { - let window = document.window(); - window.set_theme(theme); - window.force_reflow(ReflowGoal::Full, ReflowReason::ThemeChange, None); + /// Handle changes to the theme, triggering reflow if the theme actually changed. + fn handle_theme_change_msg(&self, theme: Theme) { + for (_, document) in self.documents.borrow().iter() { + document.window().handle_theme_change(theme); } } @@ -2972,7 +2970,7 @@ impl ScriptThread { ); let document = self.documents.borrow().find_document(id); if let Some(document) = document { - document.set_activity(activity, CanGc::note()); + document.set_activity(activity); return; } let mut loads = self.incomplete_loads.borrow_mut(); |