Only access hit test items for the current epoch in the compositor (#30491)

When display lists update quickly, a hit test result might be returned
for a previous display list / list of hit test items. When that happens,
ignore the hit test result.

This fixes a crash, but there might be situations where we can do
something better, such as wait for display list processing to finish
before performing the hit test. A future change might do this for events
like mouse clicks and touch events that should never be thrown away.
Ultimately, the best thing is likely moving hit testing back to layout
or script so a valid hit test can always be performed against the
current DOM.

Fixes #29796.
This commit is contained in:
Martin Robinson 2023-10-04 18:33:18 +02:00 committed by GitHub
parent 38a325cc1c
commit ce183d8581
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 3 deletions

View file

@ -18,7 +18,7 @@ use crossbeam_channel::Sender;
use embedder_traits::Cursor;
use euclid::{Point2D, Rect, Scale, Transform3D, Vector2D};
use fnv::{FnvHashMap, FnvHashSet};
use gfx_traits::{Epoch, FontData};
use gfx_traits::{Epoch, FontData, WebRenderEpochToU16};
#[cfg(feature = "gl")]
use image::{DynamicImage, ImageFormat};
use ipc_channel::ipc;
@ -283,6 +283,10 @@ struct PipelineDetails {
/// The pipeline associated with this PipelineDetails object.
pipeline: Option<CompositionPipeline>,
/// The epoch of the most recent display list for this pipeline. Note that this display
/// list might not be displayed, as WebRender processes display lists asynchronously.
most_recent_display_list_epoch: Option<WebRenderEpoch>,
/// Whether animations are running
animations_running: bool,
@ -305,6 +309,7 @@ impl PipelineDetails {
fn new() -> PipelineDetails {
PipelineDetails {
pipeline: None,
most_recent_display_list_epoch: None,
animations_running: false,
animation_callbacks_running: false,
visible: true,
@ -696,6 +701,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
let pipeline_id = display_list_info.pipeline_id;
let details = self.pipeline_details(PipelineId::from_webrender(pipeline_id));
details.most_recent_display_list_epoch = Some(display_list_info.epoch);
details.hit_test_items = display_list_info.hit_test_info;
details.install_new_scroll_tree(display_list_info.scroll_tree);
@ -1134,6 +1140,14 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
None => return None,
};
// If the epoch in the tag does not match the current epoch of the pipeline,
// then the hit test is against an old version of the display list and we
// should ignore this hit test for now.
match details.most_recent_display_list_epoch {
Some(epoch) if epoch.as_u16() == item.tag.1 => {},
_ => return None,
}
let info = &details.hit_test_items[item.tag.0 as usize];
Some(CompositorHitTestResult {
pipeline_id,

View file

@ -31,6 +31,19 @@ impl Into<WebRenderEpoch> for Epoch {
}
}
pub trait WebRenderEpochToU16 {
fn as_u16(&self) -> u16;
}
impl WebRenderEpochToU16 for WebRenderEpoch {
/// The value of this [`Epoch`] as a u16 value. Note that if this Epoch's
/// value is more than u16::MAX, then the return value will be modulo
/// u16::MAX.
fn as_u16(&self) -> u16 {
(self.0 % u16::MAX as u32) as u16
}
}
/// A unique ID for every stacking context.
#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
pub struct StackingContextId(

View file

@ -7,6 +7,7 @@
// This might be achieved by sharing types between WR and Servo display lists, or
// completely converting layout to directly generate WebRender display lists, for example.
use gfx_traits::WebRenderEpochToU16;
use log::trace;
use msg::constellation_msg::PipelineId;
use script_traits::compositor::{CompositorDisplayListInfo, ScrollTreeNodeId, ScrollableNodeInfo};
@ -190,7 +191,7 @@ impl DisplayItem {
clip_id: ClipId::ClipChain(current_clip_chain_id),
flags: PrimitiveFlags::default(),
},
(hit_test_index as u64, 0u16),
(hit_test_index as u64, state.compositor_info.epoch.as_u16()),
);
};

View file

@ -9,6 +9,7 @@ use embedder_traits::Cursor;
use euclid::{Point2D, SideOffsets2D, Size2D};
use fnv::FnvHashMap;
use gfx::text::glyph::GlyphStore;
use gfx_traits::WebRenderEpochToU16;
use msg::constellation_msg::BrowsingContextId;
use net_traits::image_cache::UsePlaceholder;
use script_traits::compositor::{CompositorDisplayListInfo, ScrollTreeNodeId};
@ -180,7 +181,10 @@ impl<'a> DisplayListBuilder<'a> {
Some(cursor(inherited_ui.cursor.keyword, auto_cursor)),
self.current_scroll_node_id,
);
Some((hit_test_index as u64, 0u16))
Some((
hit_test_index as u64,
self.display_list.compositor_info.epoch.as_u16(),
))
}
}