From 99eb457fc7afb0cd4178ae955d0d998e253b25f2 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 16 Jan 2018 18:47:05 +0100 Subject: [PATCH] Update WebRender This allows servo to use the ExternalScrollId API from WebRender fixing some issues related to duplicate scroll root ids. Fixes #17176. Fixes #19287. Fixes #19648. --- Cargo.lock | 4 +-- components/compositing/compositor.rs | 14 +++----- components/gfx/display_list/mod.rs | 18 ++++------ components/gfx_traits/lib.rs | 27 +++++++-------- components/layout/display_list/builder.rs | 25 ++++++-------- .../layout/display_list/webrender_helpers.rs | 7 ++-- components/layout/query.rs | 25 +++++++------- components/layout_thread/lib.rs | 33 ++++++++---------- components/malloc_size_of/lib.rs | 4 +-- components/msg/constellation_msg.rs | 4 +++ components/script/dom/document.rs | 4 +-- components/script/dom/window.rs | 34 +++++++++++-------- components/script_layout_interface/message.rs | 7 ++-- components/script_layout_interface/rpc.rs | 8 ++--- .../script_layout_interface/wrapper_traits.rs | 6 ++-- components/script_traits/lib.rs | 4 +-- tests/wpt/mozilla/meta/MANIFEST.json | 25 ++++++++++++++ .../tests/mozilla/duplicated_scroll_ids.html | 13 +++++++ .../mozilla/duplicated_scroll_ids_ref.html | 11 ++++++ 19 files changed, 154 insertions(+), 119 deletions(-) create mode 100644 tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids.html create mode 100644 tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids_ref.html diff --git a/Cargo.lock b/Cargo.lock index cc9ea3941b5..5e22965bdfc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3309,7 +3309,7 @@ dependencies = [ [[package]] name = "webrender" version = "0.57.0" -source = "git+https://github.com/servo/webrender#c956e149f23a9088758338990fd65f0250638352" +source = "git+https://github.com/servo/webrender#47f5afa5c011080ee787c1170038de1e5fc28155" dependencies = [ "app_units 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3337,7 +3337,7 @@ dependencies = [ [[package]] name = "webrender_api" version = "0.57.0" -source = "git+https://github.com/servo/webrender#c956e149f23a9088758338990fd65f0250638352" +source = "git+https://github.com/servo/webrender#47f5afa5c011080ee787c1170038de1e5fc28155" dependencies = [ "app_units 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index e15f3ff2603..58f7358045b 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -1171,19 +1171,15 @@ impl IOCompositor { fn send_viewport_rects(&self) { let mut scroll_states_per_pipeline = HashMap::new(); for scroll_layer_state in self.webrender_api.get_scroll_node_state(self.webrender_document) { - if scroll_layer_state.id.external_id().is_none() && - !scroll_layer_state.id.is_root_scroll_node() { - continue; - } - let scroll_state = ScrollState { - scroll_root_id: scroll_layer_state.id, + scroll_id: scroll_layer_state.id, scroll_offset: scroll_layer_state.scroll_offset.to_untyped(), }; - scroll_states_per_pipeline.entry(scroll_layer_state.id.pipeline_id()) - .or_insert(vec![]) - .push(scroll_state); + scroll_states_per_pipeline + .entry(scroll_layer_state.id.pipeline_id()) + .or_insert(vec![]) + .push(scroll_state); } for (pipeline_id, scroll_states) in scroll_states_per_pipeline { diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs index a3abba8fd29..5d41ffa8a03 100644 --- a/components/gfx/display_list/mod.rs +++ b/components/gfx/display_list/mod.rs @@ -31,11 +31,11 @@ use std::fmt; use std::sync::Arc; use text::TextRun; use text::glyph::ByteIndex; -use webrender_api::{BorderRadius, BorderWidths, BoxShadowClipMode, ClipId, ColorF, ExtendMode}; -use webrender_api::{FilterOp, GradientStop, ImageBorder, ImageKey, ImageRendering, LayoutPoint}; -use webrender_api::{LayoutRect, LayoutSize, LayoutVector2D, LineStyle, LocalClip, MixBlendMode}; -use webrender_api::{NormalBorder, ScrollPolicy, ScrollSensitivity, StickyOffsetBounds}; -use webrender_api::TransformStyle; +use webrender_api::{BorderRadius, BorderWidths, BoxShadowClipMode, ColorF, ExtendMode}; +use webrender_api::{ExternalScrollId, FilterOp, GradientStop, ImageBorder, ImageKey}; +use webrender_api::{ImageRendering, LayoutPoint, LayoutRect, LayoutSize, LayoutVector2D}; +use webrender_api::{LineStyle, LocalClip, MixBlendMode, NormalBorder, ScrollPolicy}; +use webrender_api::{ScrollSensitivity, StickyOffsetBounds, TransformStyle}; pub use style::dom::OpaqueNode; @@ -350,7 +350,7 @@ pub struct StickyFrameData { #[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)] pub enum ClipScrollNodeType { - ScrollFrame(ScrollSensitivity), + ScrollFrame(ScrollSensitivity, ExternalScrollId), StickyFrame(StickyFrameData), Clip, } @@ -358,10 +358,6 @@ pub enum ClipScrollNodeType { /// Defines a clip scroll node. #[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)] pub struct ClipScrollNode { - /// The WebRender clip id of this scroll root based on the source of this clip - /// and information about the fragment. - pub id: Option, - /// The index of the parent of this ClipScrollNode. pub parent_index: ClipScrollNodeIndex, @@ -1099,7 +1095,7 @@ impl WebRenderImageInfo { } /// The type of the scroll offset list. This is only populated if WebRender is in use. -pub type ScrollOffsetMap = HashMap>; +pub type ScrollOffsetMap = HashMap>; pub trait SimpleMatrixDetection { diff --git a/components/gfx_traits/lib.rs b/components/gfx_traits/lib.rs index 4500a3816da..247396ca04e 100644 --- a/components/gfx_traits/lib.rs +++ b/components/gfx_traits/lib.rs @@ -56,10 +56,10 @@ int_range_index! { struct ByteIndex(isize) } -/// The type of fragment that a stacking context represents. +/// The type of fragment that a scroll root is created for. /// /// This can only ever grow to maximum 4 entries. That's because we cram the value of this enum -/// into the lower 2 bits of the `StackingContextId`, which otherwise contains a 32-bit-aligned +/// into the lower 2 bits of the `ScrollRootId`, which otherwise contains a 32-bit-aligned /// heap address. #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)] pub enum FragmentType { @@ -71,23 +71,20 @@ pub enum FragmentType { AfterPseudoContent, } -/// The next ID that will be used for a special stacking context. +/// The next ID that will be used for a special scroll root id. /// -/// A special stacking context is a stacking context that is one of (a) the outer stacking context -/// of an element with `overflow: scroll`; (b) generated content; (c) both (a) and (b). -static NEXT_SPECIAL_STACKING_CONTEXT_ID: AtomicUsize = ATOMIC_USIZE_INIT; +/// A special scroll root is a scroll root that is created for generated content. +static NEXT_SPECIAL_SCROLL_ROOT_ID: AtomicUsize = ATOMIC_USIZE_INIT; -/// If none of the bits outside this mask are set, the stacking context is a special stacking -/// context. -/// +/// If none of the bits outside this mask are set, the scroll root is a special scroll root. /// Note that we assume that the top 16 bits of the address space are unused on the platform. -const SPECIAL_STACKING_CONTEXT_ID_MASK: usize = 0xffff; +const SPECIAL_SCROLL_ROOT_ID_MASK: usize = 0xffff; -/// Returns a new stacking context ID for a special stacking context. +/// Returns a new scroll root ID for a scroll root. fn next_special_id() -> usize { // We shift this left by 2 to make room for the fragment type ID. - ((NEXT_SPECIAL_STACKING_CONTEXT_ID.fetch_add(1, Ordering::SeqCst) + 1) << 2) & - SPECIAL_STACKING_CONTEXT_ID_MASK + ((NEXT_SPECIAL_SCROLL_ROOT_ID.fetch_add(1, Ordering::SeqCst) + 1) << 2) & + SPECIAL_SCROLL_ROOT_ID_MASK } pub fn combine_id_with_fragment_type(id: usize, fragment_type: FragmentType) -> usize { @@ -99,8 +96,8 @@ pub fn combine_id_with_fragment_type(id: usize, fragment_type: FragmentType) -> } } -pub fn node_id_from_clip_id(id: usize) -> Option { - if (id & !SPECIAL_STACKING_CONTEXT_ID_MASK) != 0 { +pub fn node_id_from_scroll_id(id: usize) -> Option { + if (id & !SPECIAL_SCROLL_ROOT_ID_MASK) != 0 { return Some((id & !3) as usize); } None diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index bb60018b389..347d180c89d 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -73,8 +73,8 @@ use style_traits::CSSPixel; use style_traits::ToCss; use style_traits::cursor::CursorKind; use table_cell::CollapsedBordersForCell; -use webrender_api::{self, BorderSide, BoxShadowClipMode, ClipId, ClipMode, ColorF}; -use webrender_api::{ComplexClipRegion, FilterOp, ImageBorder, ImageRendering, LayoutRect}; +use webrender_api::{self, BorderSide, BoxShadowClipMode, ClipMode, ColorF, ComplexClipRegion}; +use webrender_api::{ExternalScrollId, FilterOp, ImageBorder, ImageRendering, LayoutRect}; use webrender_api::{LayoutSize, LayoutVector2D, LineStyle, LocalClip, NinePatchDescriptor}; use webrender_api::{NormalBorder, ScrollPolicy, ScrollSensitivity, StickyOffsetBounds}; @@ -243,11 +243,13 @@ impl StackingContextCollectionState { // This is just a dummy node to take up a slot in the array. WebRender // takes care of adding this root node and it can be ignored during DL conversion. let root_node = ClipScrollNode { - id: Some(ClipId::root_scroll_node(pipeline_id.to_webrender())), parent_index: ClipScrollNodeIndex(0), clip: ClippingRegion::from_rect(&TypedRect::zero()), content_rect: LayoutRect::zero(), - node_type: ClipScrollNodeType::ScrollFrame(ScrollSensitivity::ScriptAndInputEvents), + node_type: ClipScrollNodeType::ScrollFrame( + ScrollSensitivity::ScriptAndInputEvents, + pipeline_id.root_scroll_id(), + ), }; StackingContextCollectionState { @@ -2666,7 +2668,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow { }; let new_clip_scroll_index = state.add_clip_scroll_node(ClipScrollNode { - id: None, parent_index: self.clipping_and_scrolling().scrolling, clip: ClippingRegion::from_rect(border_box), content_rect: LayoutRect::zero(), @@ -2699,12 +2700,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow { return; } - // If we already have a scroll root for this flow, just return. This can happen - // when fragments map to more than one flow, such as in the case of table - // wrappers. We just accept the first scroll root in that case. - let new_clip_scroll_node_id = - ClipId::new(self.fragment.unique_id(), state.pipeline_id.to_webrender()); - let sensitivity = if StyleOverflow::Hidden == self.fragment.style.get_box().overflow_x && StyleOverflow::Hidden == self.fragment.style.get_box().overflow_y { @@ -2723,12 +2718,15 @@ impl BlockFlowDisplayListBuilding for BlockFlow { let content_size = self.base.overflow.scroll.origin + self.base.overflow.scroll.size; let content_size = Size2D::new(content_size.x, content_size.y); + let external_id = ExternalScrollId( + self.fragment.unique_id(), + state.pipeline_id.to_webrender() + ); let new_clip_scroll_index = state.add_clip_scroll_node(ClipScrollNode { - id: Some(new_clip_scroll_node_id), parent_index: self.clipping_and_scrolling().scrolling, clip: clip, content_rect: Rect::new(content_box.origin, content_size).to_layout(), - node_type: ClipScrollNodeType::ScrollFrame(sensitivity), + node_type: ClipScrollNodeType::ScrollFrame(sensitivity, external_id), }); let new_clipping_and_scrolling = ClippingAndScrolling::simple(new_clip_scroll_index); @@ -2777,7 +2775,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow { preserved_state.push_clip(state, &clip_rect, self.positioning()); let new_index = state.add_clip_scroll_node(ClipScrollNode { - id: None, parent_index: self.clipping_and_scrolling().scrolling, clip: ClippingRegion::from_rect(&clip_rect), content_rect: LayoutRect::zero(), // content_rect isn't important for clips. diff --git a/components/layout/display_list/webrender_helpers.rs b/components/layout/display_list/webrender_helpers.rs index a4d221f3d12..31147bf4199 100644 --- a/components/layout/display_list/webrender_helpers.rs +++ b/components/layout/display_list/webrender_helpers.rs @@ -296,16 +296,15 @@ impl WebRenderDisplayItemConverter for DisplayItem { let webrender_id = match node.node_type { ClipScrollNodeType::Clip => builder.define_clip_with_parent( - node.id, parent_id, item_rect, node.clip.get_complex_clips(), None, ), - ClipScrollNodeType::ScrollFrame(scroll_sensitivity) => builder + ClipScrollNodeType::ScrollFrame(scroll_sensitivity, external_id) => builder .define_scroll_frame_with_parent( - node.id, parent_id, + Some(external_id), node.content_rect, node.clip.main.to_layout(), node.clip.get_complex_clips(), @@ -316,7 +315,6 @@ impl WebRenderDisplayItemConverter for DisplayItem { // TODO: Add define_sticky_frame_with_parent to WebRender. builder.push_clip_id(parent_id); let id = builder.define_sticky_frame( - node.id, item_rect, sticky_data.margins, sticky_data.vertical_offset_bounds, @@ -328,7 +326,6 @@ impl WebRenderDisplayItemConverter for DisplayItem { }, }; - debug_assert!(node.id.is_none() || node.id == Some(webrender_id)); clip_ids[item.node_index.0] = Some(webrender_id); }, } diff --git a/components/layout/query.rs b/components/layout/query.rs index c5da8694077..2df1fc99e53 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -16,7 +16,7 @@ use ipc_channel::ipc::IpcSender; use msg::constellation_msg::PipelineId; use opaque_node::OpaqueNodeMethods; use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC}; -use script_layout_interface::rpc::{NodeGeometryResponse, NodeScrollRootIdResponse}; +use script_layout_interface::rpc::{NodeGeometryResponse, NodeScrollIdResponse}; use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse, StyleResponse}; use script_layout_interface::rpc::TextIndexResponse; use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode}; @@ -34,7 +34,7 @@ use style::logical_geometry::{WritingMode, BlockFlowDirection, InlineBaseDirecti use style::properties::{style_structs, PropertyId, PropertyDeclarationId, LonghandId}; use style::selector_parser::PseudoElement; use style_traits::ToCss; -use webrender_api::ClipId; +use webrender_api::ExternalScrollId; use wrapper::LayoutNodeLayoutData; /// Mutable data belonging to the LayoutThread. @@ -56,8 +56,8 @@ pub struct LayoutThreadData { /// A queued response for the client {top, left, width, height} of a node in pixels. pub client_rect_response: Rect, - /// A queued response for the scroll root id for a given node. - pub scroll_root_id_response: Option, + /// A queued response for the scroll id for a given node. + pub scroll_id_response: Option, /// A queued response for the scroll {top, left, width, height} of a node in pixels. pub scroll_area_response: Rect, @@ -131,10 +131,10 @@ impl LayoutRPC for LayoutRPCImpl { } } - fn node_scroll_root_id(&self) -> NodeScrollRootIdResponse { - NodeScrollRootIdResponse(self.0.lock() - .unwrap().scroll_root_id_response - .expect("scroll_root_id is not correctly fetched")) + fn node_scroll_id(&self) -> NodeScrollIdResponse { + NodeScrollIdResponse(self.0.lock() + .unwrap().scroll_id_response + .expect("scroll id is not correctly fetched")) } /// Retrieves the resolved value for a CSS style property. @@ -611,11 +611,12 @@ pub fn process_node_geometry_request(requested_node: N, layout_ro iterator.client_rect } -pub fn process_node_scroll_root_id_request(id: PipelineId, - requested_node: N) - -> ClipId { +pub fn process_node_scroll_id_request( + id: PipelineId, + requested_node: N +) -> ExternalScrollId { let layout_node = requested_node.to_threadsafe(); - layout_node.generate_scroll_root_id(id) + layout_node.generate_scroll_id(id) } pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layout_root: &mut Flow) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 5606b8782f7..efc1489eae3 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -59,7 +59,7 @@ use gfx::display_list::{OpaqueNode, WebRenderImageInfo}; use gfx::font; use gfx::font_cache_thread::FontCacheThread; use gfx::font_context; -use gfx_traits::{Epoch, node_id_from_clip_id}; +use gfx_traits::{Epoch, node_id_from_scroll_id}; use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; use ipc_channel::router::ROUTER; use layout::animation; @@ -76,7 +76,7 @@ use layout::layout_debug; use layout::parallel; use layout::query::{LayoutRPCImpl, LayoutThreadData, process_content_box_request, process_content_boxes_request}; use layout::query::{process_node_geometry_request, process_node_scroll_area_request}; -use layout::query::{process_node_scroll_root_id_request, process_offset_parent_query, process_resolved_style_request}; +use layout::query::{process_node_scroll_id_request, process_offset_parent_query, process_resolved_style_request}; use layout::query::process_style_query; use layout::sequential; use layout::traversal::{ComputeStackingRelativePositions, PreorderFlowTraversal, RecalcStyleAndConstructFlows}; @@ -517,7 +517,7 @@ impl LayoutThread { content_box_response: None, content_boxes_response: Vec::new(), client_rect_response: Rect::zero(), - scroll_root_id_response: None, + scroll_id_response: None, scroll_area_response: Rect::zero(), resolved_style_response: String::new(), offset_parent_response: OffsetParentResponse::empty(), @@ -700,13 +700,13 @@ impl LayoutThread { } Msg::UpdateScrollStateFromScript(state) => { let mut rw_data = possibly_locked_rw_data.lock(); - rw_data.scroll_offsets.insert(state.scroll_root_id, state.scroll_offset); + rw_data.scroll_offsets.insert(state.scroll_id, state.scroll_offset); let point = Point2D::new(-state.scroll_offset.x, -state.scroll_offset.y); let mut txn = webrender_api::Transaction::new(); txn.scroll_node_with_id( webrender_api::LayoutPoint::from_untyped(&point), - state.scroll_root_id, + webrender_api::IdType::ExternalScrollId(state.scroll_id), webrender_api::ScrollClamping::ToContentBounds ); self.webrender_api.send_transaction(self.webrender_document, txn); @@ -1091,8 +1091,8 @@ impl LayoutThread { ReflowGoal::NodeScrollGeometryQuery(_) => { rw_data.scroll_area_response = Rect::zero(); }, - ReflowGoal::NodeScrollRootIdQuery(_) => { - rw_data.scroll_root_id_response = None; + ReflowGoal::NodeScrollIdQuery(_) => { + rw_data.scroll_id_response = None; }, ReflowGoal::ResolvedStyleQuery(_, _, _) => { rw_data.resolved_style_response = String::new(); @@ -1375,10 +1375,9 @@ impl LayoutThread { let node = unsafe { ServoLayoutNode::new(&node) }; rw_data.scroll_area_response = process_node_scroll_area_request(node, root_flow); }, - ReflowGoal::NodeScrollRootIdQuery(node) => { + ReflowGoal::NodeScrollIdQuery(node) => { let node = unsafe { ServoLayoutNode::new(&node) }; - rw_data.scroll_root_id_response = Some(process_node_scroll_root_id_request(self.id, - node)); + rw_data.scroll_id_response = Some(process_node_scroll_id_request(self.id, node)); }, ReflowGoal::ResolvedStyleQuery(node, ref pseudo, ref property) => { let node = unsafe { ServoLayoutNode::new(&node) }; @@ -1430,16 +1429,14 @@ impl LayoutThread { let mut rw_data = possibly_locked_rw_data.lock(); let mut script_scroll_states = vec![]; let mut layout_scroll_states = HashMap::new(); - for new_scroll_state in &new_scroll_states { - let offset = new_scroll_state.scroll_offset; - layout_scroll_states.insert(new_scroll_state.scroll_root_id, offset); + for new_state in &new_scroll_states { + let offset = new_state.scroll_offset; + layout_scroll_states.insert(new_state.scroll_id, offset); - if new_scroll_state.scroll_root_id.is_root_scroll_node() { + if new_state.scroll_id.is_root() { script_scroll_states.push((UntrustedNodeAddress::from_id(0), offset)) - } else if let Some(id) = new_scroll_state.scroll_root_id.external_id() { - if let Some(node_id) = node_id_from_clip_id(id as usize) { - script_scroll_states.push((UntrustedNodeAddress::from_id(node_id), offset)) - } + } else if let Some(node_id) = node_id_from_scroll_id(new_state.scroll_id.0 as usize) { + script_scroll_states.push((UntrustedNodeAddress::from_id(node_id), offset)) } } let _ = self.script_chan diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index f76ae42c8fa..d4c2656ae86 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -823,14 +823,14 @@ malloc_size_of_is_0!(webrender_api::BoxShadowClipMode); #[cfg(feature = "servo")] malloc_size_of_is_0!(webrender_api::ClipAndScrollInfo); #[cfg(feature = "servo")] -malloc_size_of_is_0!(webrender_api::ClipId); -#[cfg(feature = "servo")] malloc_size_of_is_0!(webrender_api::ColorF); #[cfg(feature = "servo")] malloc_size_of_is_0!(webrender_api::ExtendMode); #[cfg(feature = "servo")] malloc_size_of_is_0!(webrender_api::FilterOp); #[cfg(feature = "servo")] +malloc_size_of_is_0!(webrender_api::ExternalScrollId); +#[cfg(feature = "servo")] malloc_size_of_is_0!(webrender_api::GradientStop); #[cfg(feature = "servo")] malloc_size_of_is_0!(webrender_api::ImageBorder); diff --git a/components/msg/constellation_msg.rs b/components/msg/constellation_msg.rs index eb2c1b74f19..6f9e89036a2 100644 --- a/components/msg/constellation_msg.rs +++ b/components/msg/constellation_msg.rs @@ -261,6 +261,10 @@ impl PipelineId { webrender_api::ClipId::root_scroll_node(self.to_webrender()) } + pub fn root_scroll_id(&self) -> webrender_api::ExternalScrollId { + webrender_api::ExternalScrollId(0, self.to_webrender()) + } + pub fn root_clip_and_scroll_info(&self) -> webrender_api::ClipAndScrollInfo { webrender_api::ClipAndScrollInfo::simple(self.root_scroll_node()) } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 436d4e71d1a..0ed2c25ff4b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -141,7 +141,6 @@ use time; use timers::OneshotTimerCallback; use url::Host; use url::percent_encoding::percent_decode; -use webrender_api::ClipId; /// The number of times we are allowed to see spurious `requestAnimationFrame()` calls before /// falling back to fake ones. @@ -730,11 +729,10 @@ impl Document { if let Some((x, y)) = point { // Step 3 let global_scope = self.window.upcast::(); - let webrender_pipeline_id = global_scope.pipeline_id().to_webrender(); self.window.update_viewport_for_scroll(x, y); self.window.perform_a_scroll(x, y, - ClipId::root_scroll_node(webrender_pipeline_id), + global_scope.pipeline_id().root_scroll_id(), ScrollBehavior::Instant, target.r()); } diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index e9055f54ea0..c92a04c7cd4 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -72,7 +72,7 @@ use script_layout_interface::{TrustedNodeAddress, PendingImageState}; use script_layout_interface::message::{Msg, Reflow, ReflowGoal, ScriptReflow}; use script_layout_interface::reporter::CSSErrorReporter; use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC}; -use script_layout_interface::rpc::{NodeScrollRootIdResponse, ResolvedStyleResponse, TextIndexResponse}; +use script_layout_interface::rpc::{NodeScrollIdResponse, ResolvedStyleResponse, TextIndexResponse}; use script_runtime::{CommonScriptMsg, ScriptChan, ScriptPort, ScriptThreadEventCategory, Runtime}; use script_thread::{ImageCacheMsg, MainThreadScriptChan, MainThreadScriptMsg}; use script_thread::{ScriptThread, SendableMainThreadScriptChan}; @@ -120,7 +120,7 @@ use timers::{IsInterval, TimerCallback}; use tinyfiledialogs::{self, MessageBoxIcon}; use url::Position; use webdriver_handlers::jsval_to_webdriver; -use webrender_api::{ClipId, DocumentId}; +use webrender_api::{ExternalScrollId, DocumentId}; use webvr_traits::WebVRMsg; /// Current state of the window object @@ -1141,7 +1141,7 @@ impl Window { self.update_viewport_for_scroll(x, y); self.perform_a_scroll(x, y, - global_scope.pipeline_id().root_scroll_node(), + global_scope.pipeline_id().root_scroll_id(), behavior, None); } @@ -1150,14 +1150,14 @@ impl Window { pub fn perform_a_scroll(&self, x: f32, y: f32, - scroll_root_id: ClipId, + scroll_id: ExternalScrollId, _behavior: ScrollBehavior, _element: Option<&Element>) { // TODO Step 1 // TODO(mrobinson, #18709): Add smooth scrolling support to WebRender so that we can // properly process ScrollBehavior here. self.layout_chan.send(Msg::UpdateScrollStateFromScript(ScrollState { - scroll_root_id: scroll_root_id, + scroll_id, scroll_offset: Vector2D::new(-x, -y), })).unwrap(); } @@ -1412,13 +1412,17 @@ impl Window { } // https://drafts.csswg.org/cssom-view/#element-scrolling-members - pub fn scroll_node(&self, - node: &Node, - x_: f64, - y_: f64, - behavior: ScrollBehavior) { - if !self.reflow(ReflowGoal::NodeScrollRootIdQuery(node.to_trusted_node_address()), - ReflowReason::Query) { + pub fn scroll_node( + &self, + node: &Node, + x_: f64, + y_: f64, + behavior: ScrollBehavior + ) { + if !self.reflow( + ReflowGoal::NodeScrollIdQuery(node.to_trusted_node_address()), + ReflowReason::Query + ) { return; } @@ -1428,12 +1432,12 @@ impl Window { self.scroll_offsets.borrow_mut().insert(node.to_untrusted_node_address(), Vector2D::new(x_ as f32, y_ as f32)); - let NodeScrollRootIdResponse(scroll_root_id) = self.layout_rpc.node_scroll_root_id(); + let NodeScrollIdResponse(scroll_id) = self.layout_rpc.node_scroll_id(); // Step 12 self.perform_a_scroll(x_.to_f32().unwrap_or(0.0f32), y_.to_f32().unwrap_or(0.0f32), - scroll_root_id, + scroll_id, behavior, None); } @@ -1888,7 +1892,7 @@ fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal, reason: &Reflow ReflowGoal::NodesFromPointQuery(..) => "\tNodesFromPointQuery", ReflowGoal::NodeGeometryQuery(_n) => "\tNodeGeometryQuery", ReflowGoal::NodeScrollGeometryQuery(_n) => "\tNodeScrollGeometryQuery", - ReflowGoal::NodeScrollRootIdQuery(_n) => "\tNodeScrollRootIdQuery", + ReflowGoal::NodeScrollIdQuery(_n) => "\tNodeScrollIdQuery", ReflowGoal::ResolvedStyleQuery(_, _, _) => "\tResolvedStyleQuery", ReflowGoal::OffsetParentQuery(_n) => "\tOffsetParentQuery", ReflowGoal::StyleQuery(_n) => "\tStyleQuery", diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 6f2ab91420d..f53022d5286 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -113,7 +113,7 @@ pub enum ReflowGoal { TickAnimations, ContentBoxQuery(TrustedNodeAddress), ContentBoxesQuery(TrustedNodeAddress), - NodeScrollRootIdQuery(TrustedNodeAddress), + NodeScrollIdQuery(TrustedNodeAddress), NodeGeometryQuery(TrustedNodeAddress), NodeScrollGeometryQuery(TrustedNodeAddress), ResolvedStyleQuery(TrustedNodeAddress, Option, PropertyId), @@ -132,7 +132,7 @@ impl ReflowGoal { ReflowGoal::TickAnimations | ReflowGoal::Full => true, ReflowGoal::ContentBoxQuery(_) | ReflowGoal::ContentBoxesQuery(_) | ReflowGoal::NodeGeometryQuery(_) | ReflowGoal::NodeScrollGeometryQuery(_) | - ReflowGoal::NodeScrollRootIdQuery(_) | + ReflowGoal::NodeScrollIdQuery(_) | ReflowGoal::ResolvedStyleQuery(..) | ReflowGoal::OffsetParentQuery(_) | ReflowGoal::StyleQuery(_) => false, } @@ -145,8 +145,7 @@ impl ReflowGoal { ReflowGoal::StyleQuery(_) | ReflowGoal::TextIndexQuery(..) | ReflowGoal::ContentBoxQuery(_) | ReflowGoal::ContentBoxesQuery(_) | ReflowGoal::NodeGeometryQuery(_) | ReflowGoal::NodeScrollGeometryQuery(_) | - ReflowGoal::NodeScrollRootIdQuery(_) | - ReflowGoal::ResolvedStyleQuery(..) | + ReflowGoal::NodeScrollIdQuery(_) | ReflowGoal::ResolvedStyleQuery(..) | ReflowGoal::OffsetParentQuery(_) => false, ReflowGoal::NodesFromPointQuery(..) | ReflowGoal::Full | ReflowGoal::TickAnimations => true, diff --git a/components/script_layout_interface/rpc.rs b/components/script_layout_interface/rpc.rs index 3988711eeaa..8902c7d0466 100644 --- a/components/script_layout_interface/rpc.rs +++ b/components/script_layout_interface/rpc.rs @@ -8,7 +8,7 @@ use script_traits::UntrustedNodeAddress; use servo_arc::Arc; use style::properties::ComputedValues; use style::properties::longhands::overflow_x; -use webrender_api::ClipId; +use webrender_api::ExternalScrollId; /// Synchronous messages that script can send to layout. /// @@ -27,8 +27,8 @@ pub trait LayoutRPC { fn node_geometry(&self) -> NodeGeometryResponse; /// Requests the scroll geometry of this node. Used by APIs such as `scrollTop`. fn node_scroll_area(&self) -> NodeGeometryResponse; - /// Requests the scroll root id of this node. Used by APIs such as `scrollTop` - fn node_scroll_root_id(&self) -> NodeScrollRootIdResponse; + /// Requests the scroll id of this node. Used by APIs such as `scrollTop` + fn node_scroll_id(&self) -> NodeScrollIdResponse; /// Query layout for the resolved value of a given CSS property fn resolved_style(&self) -> ResolvedStyleResponse; fn offset_parent(&self) -> OffsetParentResponse; @@ -51,7 +51,7 @@ pub struct NodeGeometryResponse { pub struct NodeOverflowResponse(pub Option>); -pub struct NodeScrollRootIdResponse(pub ClipId); +pub struct NodeScrollIdResponse(pub ExternalScrollId); pub struct ResolvedStyleResponse(pub String); diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 0b04d4256a6..a3365c3c6e5 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -25,7 +25,7 @@ use style::font_metrics::ServoMetricsProvider; use style::properties::ComputedValues; use style::selector_parser::{PseudoElement, PseudoElementCascadeType, SelectorImpl}; use style::stylist::RuleInclusion; -use webrender_api::ClipId; +use webrender_api::ExternalScrollId; #[derive(Clone, Copy, Debug, PartialEq)] pub enum PseudoElementType { @@ -269,9 +269,9 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo self.get_pseudo_element_type().fragment_type() } - fn generate_scroll_root_id(&self, pipeline_id: PipelineId) -> ClipId { + fn generate_scroll_id(&self, pipeline_id: PipelineId) -> ExternalScrollId { let id = combine_id_with_fragment_type(self.opaque().id(), self.fragment_type()); - ClipId::new(id as u64, pipeline_id.to_webrender()) + ExternalScrollId(id as u64, pipeline_id.to_webrender()) } } diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index e330c49d441..a8ef74c56c3 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -70,7 +70,7 @@ use style_traits::CSSPixel; use style_traits::SpeculativePainter; use style_traits::cursor::CursorKind; use webdriver_msg::{LoadStatus, WebDriverScriptCommand}; -use webrender_api::{ClipId, DevicePixel, DocumentId, ImageKey}; +use webrender_api::{ExternalScrollId, DevicePixel, DocumentId, ImageKey}; use webvr_traits::{WebVREvent, WebVRMsg}; pub use script_msg::{LayoutMsg, ScriptMsg, EventResult, LogEntry}; @@ -719,7 +719,7 @@ pub enum AnimationTickType { #[derive(Clone, Copy, Debug, Deserialize, Serialize)] pub struct ScrollState { /// The ID of the scroll root. - pub scroll_root_id: ClipId, + pub scroll_id: ExternalScrollId, /// The scrolling offset of this stacking context. pub scroll_offset: Vector2D, } diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 9197fcdd90d..2fbfc9f584e 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -7517,6 +7517,18 @@ {} ] ], + "mozilla/duplicated_scroll_ids.html": [ + [ + "/_mozilla/mozilla/duplicated_scroll_ids.html", + [ + [ + "/_mozilla/mozilla/duplicated_scroll_ids_ref.html", + "==" + ] + ], + {} + ] + ], "mozilla/iframe/resize_after_load.html": [ [ "/_mozilla/mozilla/iframe/resize_after_load.html", @@ -11040,6 +11052,11 @@ {} ] ], + "mozilla/duplicated_scroll_ids_ref.html": [ + [ + {} + ] + ], "mozilla/form_submit_about_frame.html": [ [ {} @@ -66229,6 +66246,14 @@ "614fb3a1e9d775f07bf1d5b9a846ca48729f7805", "testharness" ], + "mozilla/duplicated_scroll_ids.html": [ + "3a10b783d1f411d7afdb3132b8eb43532c39c683", + "reftest" + ], + "mozilla/duplicated_scroll_ids_ref.html": [ + "b2627acdddf71cf3b2a91d99f6db456397f71032", + "support" + ], "mozilla/element_attribute.html": [ "35625bedf3b323f8538174058c755cbccd25157c", "testharness" diff --git a/tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids.html b/tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids.html new file mode 100644 index 00000000000..a0ac8e578dd --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids.html @@ -0,0 +1,13 @@ + + + + + + Ensure that content which produces duplicate scroll ids does not panic + + +
+
+ + + diff --git a/tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids_ref.html b/tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids_ref.html new file mode 100644 index 00000000000..6783d72a662 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids_ref.html @@ -0,0 +1,11 @@ + + + + + Ensure that content which produces duplicate scroll ids does not panic + + +
+ + +