compositing: Don't reset scroll positions when receiving a new frame tree (#39515)

In the past, all scroll offsets were reset when navigating between
pages,
and layouts probably didn't preserve their scroll positions between back
and forward operations. Whatever the reason, the compositor was still
trying to
reset these positions right after a load.

This is an issue, because a page can request a scroll during a load, and
this
scroll might be requested from `<iframe>` content. In that case, the
scroll
positions should be the ones that layout reflects and not cleared from
the
Compositor. This reset operation had the potential to stomp on scroll
positions
set during load.

It's quite likely that the Compositor shouldn't be trying to reset them
at all,
so just stop doing that. This removes a source of intermittency when
running
tests that set scroll positions.

Testing: Once #39475 lands after this, its test should always pass.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Oriol Brufau 2025-09-26 19:14:45 +02:00 committed by GitHub
parent 461c2427f3
commit d29ad5b9c3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -21,7 +21,7 @@ use embedder_traits::{
use euclid::{Point2D, Scale, Vector2D}; use euclid::{Point2D, Scale, Vector2D};
use log::{debug, warn}; use log::{debug, warn};
use malloc_size_of::MallocSizeOf; use malloc_size_of::MallocSizeOf;
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::FxHashMap;
use servo_geometry::DeviceIndependentPixel; use servo_geometry::DeviceIndependentPixel;
use style_traits::{CSSPixel, PinchZoomFactor}; use style_traits::{CSSPixel, PinchZoomFactor};
use webrender_api::units::{DeviceIntPoint, DevicePixel, DevicePoint, DeviceRect, LayoutVector2D}; use webrender_api::units::{DeviceIntPoint, DevicePixel, DevicePoint, DeviceRect, LayoutVector2D};
@ -180,8 +180,6 @@ impl WebViewRenderer {
} }
self.set_frame_tree_on_pipeline_details(frame_tree, None); self.set_frame_tree_on_pipeline_details(frame_tree, None);
self.reset_scroll_tree_for_unattached_pipelines(frame_tree);
self.send_scroll_positions_to_layout_for_pipeline(pipeline_id);
} }
pub(crate) fn send_scroll_positions_to_layout_for_pipeline(&self, pipeline_id: PipelineId) { pub(crate) fn send_scroll_positions_to_layout_for_pipeline(&self, pipeline_id: PipelineId) {
@ -219,32 +217,6 @@ impl WebViewRenderer {
} }
} }
pub(crate) fn reset_scroll_tree_for_unattached_pipelines(
&mut self,
frame_tree: &SendableFrameTree,
) {
// TODO(mrobinson): Eventually this can selectively preserve the scroll trees
// state for some unattached pipelines in order to preserve scroll position when
// navigating backward and forward.
fn collect_pipelines(
pipelines: &mut FxHashSet<PipelineId>,
frame_tree: &SendableFrameTree,
) {
pipelines.insert(frame_tree.pipeline.id);
for kid in &frame_tree.children {
collect_pipelines(pipelines, kid);
}
}
let mut attached_pipelines: FxHashSet<PipelineId> = FxHashSet::default();
collect_pipelines(&mut attached_pipelines, frame_tree);
self.pipelines
.iter_mut()
.filter(|(id, _)| !attached_pipelines.contains(id))
.for_each(|(_, details)| details.scroll_tree.reset_all_scroll_offsets());
}
/// Sets or unsets the animations-running flag for the given pipeline. Returns /// Sets or unsets the animations-running flag for the given pipeline. Returns
/// true if the pipeline has started animating. /// true if the pipeline has started animating.
pub(crate) fn change_pipeline_running_animations_state( pub(crate) fn change_pipeline_running_animations_state(