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

@@ -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)",

components/compositing/compositor.rs

@@ -1171,19 +1171,15 @@ impl<Window: WindowMethods> IOCompositor<Window> {
     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())
+            scroll_states_per_pipeline
-                                     .or_insert(vec![])
+                .entry(scroll_layer_state.id.pipeline_id())
-                                     .push(scroll_state);
+                .or_insert(vec![])
+                .push(scroll_state);
         }
 
         for (pipeline_id, scroll_states) in scroll_states_per_pipeline {

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::{BorderRadius, BorderWidths, BoxShadowClipMode, ColorF, ExtendMode};
-use webrender_api::{FilterOp, GradientStop, ImageBorder, ImageKey, ImageRendering, LayoutPoint};
+use webrender_api::{ExternalScrollId, FilterOp, GradientStop, ImageBorder, ImageKey};
-use webrender_api::{LayoutRect, LayoutSize, LayoutVector2D, LineStyle, LocalClip, MixBlendMode};
+use webrender_api::{ImageRendering, LayoutPoint, LayoutRect, LayoutSize, LayoutVector2D};
-use webrender_api::{NormalBorder, ScrollPolicy, ScrollSensitivity, StickyOffsetBounds};
+use webrender_api::{LineStyle, LocalClip, MixBlendMode, NormalBorder, ScrollPolicy};
-use webrender_api::TransformStyle;
+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<ClipId>,
-
     /// 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<ClipId, Vector2D<f32>>;
+pub type ScrollOffsetMap = HashMap<ExternalScrollId, Vector2D<f32>>;
 
 
 pub trait SimpleMatrixDetection {

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
+/// A special scroll root is a scroll root that is created for generated content.
-/// of an element with `overflow: scroll`; (b) generated content; (c) both (a) and (b).
+static NEXT_SPECIAL_SCROLL_ROOT_ID: AtomicUsize = ATOMIC_USIZE_INIT;
-static NEXT_SPECIAL_STACKING_CONTEXT_ID: AtomicUsize = ATOMIC_USIZE_INIT;
 
-/// If none of the bits outside this mask are set, the stacking context is a special stacking
+/// If none of the bits outside this mask are set, the scroll root is a special scroll root.
-/// context.
-///
 /// 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) &
+    ((NEXT_SPECIAL_SCROLL_ROOT_ID.fetch_add(1, Ordering::SeqCst) + 1) << 2) &
-        SPECIAL_STACKING_CONTEXT_ID_MASK
+        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<usize> {
+pub fn node_id_from_scroll_id(id: usize) -> Option<usize> {
-    if (id & !SPECIAL_STACKING_CONTEXT_ID_MASK) != 0 {
+    if (id & !SPECIAL_SCROLL_ROOT_ID_MASK) != 0 {
         return Some((id & !3) as usize);
     }
     None

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::{self, BorderSide, BoxShadowClipMode, ClipMode, ColorF, ComplexClipRegion};
-use webrender_api::{ComplexClipRegion, FilterOp, ImageBorder, ImageRendering, LayoutRect};
+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.

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);
             },
         }

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<i32>,
 
-    /// A queued response for the scroll root id for a given node.
+    /// A queued response for the scroll id for a given node.
-    pub scroll_root_id_response: Option<ClipId>,
+    pub scroll_id_response: Option<ExternalScrollId>,
 
     /// A queued response for the scroll {top, left, width, height} of a node in pixels.
     pub scroll_area_response: Rect<i32>,
@@ -131,10 +131,10 @@ impl LayoutRPC for LayoutRPCImpl {
         }
     }
 
-    fn node_scroll_root_id(&self) -> NodeScrollRootIdResponse {
+    fn node_scroll_id(&self) -> NodeScrollIdResponse {
-        NodeScrollRootIdResponse(self.0.lock()
+        NodeScrollIdResponse(self.0.lock()
-                                       .unwrap().scroll_root_id_response
+                             .unwrap().scroll_id_response
-                                       .expect("scroll_root_id is not correctly fetched"))
+                             .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<N: LayoutNode>(requested_node: N, layout_ro
     iterator.client_rect
 }
 
-pub fn process_node_scroll_root_id_request<N: LayoutNode>(id: PipelineId,
+pub fn process_node_scroll_id_request<N: LayoutNode>(
-                                                          requested_node: N)
+    id: PipelineId,
-                                                          -> ClipId {
+    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)

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(_) => {
+                    ReflowGoal::NodeScrollIdQuery(_) => {
-                        rw_data.scroll_root_id_response = None;
+                        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,
+                rw_data.scroll_id_response = Some(process_node_scroll_id_request(self.id, node));
-                                                                                           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 {
+        for new_state in &new_scroll_states {
-            let offset = new_scroll_state.scroll_offset;
+            let offset = new_state.scroll_offset;
-            layout_scroll_states.insert(new_scroll_state.scroll_root_id, 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() {
+            } else if let Some(node_id) = node_id_from_scroll_id(new_state.scroll_id.0 as usize) {
-                if let Some(node_id) = node_id_from_clip_id(id as usize) {
+                script_scroll_states.push((UntrustedNodeAddress::from_id(node_id), offset))
-                    script_scroll_states.push((UntrustedNodeAddress::from_id(node_id), offset))
-                }
             }
         }
         let _ = self.script_chan

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);

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())
     }

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::<GlobalScope>();
-            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());
         }

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,
+    pub fn scroll_node(
-                       node: &Node,
+        &self,
-                       x_: f64,
+        node: &Node,
-                       y_: f64,
+        x_: f64,
-                       behavior: ScrollBehavior) {
+        y_: f64,
-        if !self.reflow(ReflowGoal::NodeScrollRootIdQuery(node.to_trusted_node_address()),
+        behavior: ScrollBehavior
-                        ReflowReason::Query) {
+    ) {
+        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",

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<PseudoElement>, 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::NodeScrollIdQuery(_) | ReflowGoal::ResolvedStyleQuery(..) |
-            ReflowGoal::ResolvedStyleQuery(..) |
             ReflowGoal::OffsetParentQuery(_) => false,
             ReflowGoal::NodesFromPointQuery(..) | ReflowGoal::Full |
             ReflowGoal::TickAnimations => true,

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`
+    /// Requests the scroll id of this node. Used by APIs such as `scrollTop`
-    fn node_scroll_root_id(&self) -> NodeScrollRootIdResponse;
+    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<Point2D<overflow_x::computed_value::T>>);
 
-pub struct NodeScrollRootIdResponse(pub ClipId);
+pub struct NodeScrollIdResponse(pub ExternalScrollId);
 
 pub struct ResolvedStyleResponse(pub String);
 

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())
     }
 }
 

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<f32>,
 }

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"

tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <link rel="match" href="duplicated_scroll_ids_ref.html">
+        <title>Ensure that content which produces duplicate scroll ids does not panic</title>
+    </head>
+    <body>
+        <div style="width: 100px; height: 100px; background: green"></div>
+        <details open style="overflow: auto">
+    </body>
+</html>
+

tests/wpt/mozilla/tests/mozilla/duplicated_scroll_ids_ref.html

@@ -0,0 +1,11 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Ensure that content which produces duplicate scroll ids does not panic</title>
+    </head>
+    <body>
+        <div style="width: 100px; height: 100px; background: green"></div>
+    </body>
+</html>
+