Auto merge of #18506 - mrobinson:position-sticky-table, r=emilio

Fix issues with the combination of position:sticky and tables

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #18441 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18506)
<!-- Reviewable:end -->

components/layout/block.rs

@@ -30,7 +30,7 @@
 use app_units::{Au, MAX_AU};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use display_list_builder::{DisplayListBuildState, EstablishContainingBlock};
+use display_list_builder::{DisplayListBuildState, StackingContextCollectionFlags};
 use display_list_builder::StackingContextCollectionState;
 use euclid::{Point2D, Rect, SideOffsets2D, Size2D};
 use floats::{ClearType, FloatKind, Floats, PlacementInfo};
@@ -2152,7 +2152,7 @@ impl Flow for BlockFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.collect_stacking_contexts_for_block(state, EstablishContainingBlock::Yes);
+        self.collect_stacking_contexts_for_block(state, StackingContextCollectionFlags::empty());
     }
 
     fn build_display_list(&mut self, state: &mut DisplayListBuildState) {

components/layout/display_list_builder.rs

@@ -103,11 +103,10 @@ fn convert_repeat_mode(from: RepeatKeyword) -> RepeatMode {
     }
 }
 
-fn establishes_containing_block_for_absolute(can_establish_containing_block: EstablishContainingBlock,
+fn establishes_containing_block_for_absolute(flags: StackingContextCollectionFlags,
                                              positioning: position::T)
                                              -> bool {
-    can_establish_containing_block == EstablishContainingBlock::Yes &&
+    !flags.contains(NEVER_CREATES_CONTAINING_BLOCK) && position::T::static_ != positioning
-    position::T::static_ != positioning
 }
 
 trait RgbColor {
@@ -2346,16 +2345,19 @@ impl FragmentDisplayListBuilding for Fragment {
     }
 }
 
-#[derive(Clone, Copy, PartialEq)]
+bitflags! {
-pub enum EstablishContainingBlock {
+    pub flags StackingContextCollectionFlags: u8 {
-    Yes,
+        /// This flow never establishes a containing block.
-    No,
+        const NEVER_CREATES_CONTAINING_BLOCK = 0x01,
+        /// This flow never creates a ClipScrollNode.
+        const NEVER_CREATES_CLIP_SCROLL_NODE = 0x02,
+    }
 }
 
 pub trait BlockFlowDisplayListBuilding {
     fn collect_stacking_contexts_for_block(&mut self,
                                            state: &mut StackingContextCollectionState,
-                                           can_establish_containing_block: EstablishContainingBlock);
+                                           flags: StackingContextCollectionFlags);
 
     fn transform_clip_to_coordinate_space(&mut self,
                                           state: &mut StackingContextCollectionState,
@@ -2364,7 +2366,7 @@ pub trait BlockFlowDisplayListBuilding {
                                 state: &mut StackingContextCollectionState,
                                 preserved_state: &mut SavedStackingContextCollectionState,
                                 stacking_context_type: BlockStackingContextType,
-                                can_establish_containing_block: EstablishContainingBlock)
+                                flags: StackingContextCollectionFlags)
                                 -> ClipAndScrollInfo;
     fn setup_clip_scroll_node_for_position(&mut self,
                                            state: &mut StackingContextCollectionState,
@@ -2521,7 +2523,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
 
     fn collect_stacking_contexts_for_block(&mut self,
                                            state: &mut StackingContextCollectionState,
-                                           can_establish_containing_block: EstablishContainingBlock) {
+                                           flags: StackingContextCollectionFlags) {
         let mut preserved_state = SavedStackingContextCollectionState::new(state);
 
         let block_stacking_context_type = self.block_stacking_context_type();
@@ -2544,10 +2546,9 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
             self.setup_clipping_for_block(state,
                                           &mut preserved_state,
                                           block_stacking_context_type,
-                                          can_establish_containing_block);
+                                          flags);
 
-        if establishes_containing_block_for_absolute(can_establish_containing_block,
+        if establishes_containing_block_for_absolute(flags, self.positioning()) {
-                                                     self.positioning()) {
             state.containing_block_clip_and_scroll_info = state.current_clip_and_scroll_info;
         }
 
@@ -2574,7 +2575,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                                 state: &mut StackingContextCollectionState,
                                 preserved_state: &mut SavedStackingContextCollectionState,
                                 stacking_context_type: BlockStackingContextType,
-                                can_establish_containing_block: EstablishContainingBlock)
+                                flags: StackingContextCollectionFlags)
                                 -> ClipAndScrollInfo {
         // If this block is absolutely positioned, we should be clipped and positioned by
         // the scroll root of our nearest ancestor that establishes a containing block.
@@ -2602,14 +2603,17 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
             self.transform_clip_to_coordinate_space(state, preserved_state);
         }
 
-        self.setup_clip_scroll_node_for_position(state, &stacking_relative_border_box);
+        if !flags.contains(NEVER_CREATES_CLIP_SCROLL_NODE) {
-        self.setup_clip_scroll_node_for_overflow(state, &stacking_relative_border_box);
+            self.setup_clip_scroll_node_for_position(state, &stacking_relative_border_box);
-        self.setup_clip_scroll_node_for_css_clip(state, preserved_state, &stacking_relative_border_box);
+            self.setup_clip_scroll_node_for_overflow(state, &stacking_relative_border_box);
+            self.setup_clip_scroll_node_for_css_clip(state, preserved_state,
+                                                     &stacking_relative_border_box);
+        }
         self.base.clip = state.clip_stack.last().cloned().unwrap_or_else(max_rect);
 
         // We keep track of our position so that any stickily positioned elements can
         // properly determine the extent of their movement relative to scrolling containers.
-        if can_establish_containing_block == EstablishContainingBlock::Yes {
+        if !flags.contains(NEVER_CREATES_CONTAINING_BLOCK) {
             let border_box = if self.fragment.establishes_stacking_context() {
                 stacking_relative_border_box
             } else {
@@ -2678,6 +2682,9 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
 
         let new_clip_scroll_node_id = ClipId::new(self.fragment.unique_id(IdType::OverflowClip),
                                                   state.pipeline_id.to_webrender());
+        if state.has_clip_scroll_node(new_clip_scroll_node_id) {
+             return;
+        }
         let parent_id = self.clip_and_scroll_info(state.pipeline_id).scroll_node_id;
         state.add_clip_scroll_node(
             ClipScrollNode {
@@ -2928,7 +2935,7 @@ impl InlineFlowDisplayListBuilding for InlineFlow {
 
         for fragment in self.fragments.fragments.iter_mut() {
             let previous_cb_clip_scroll_info = state.containing_block_clip_and_scroll_info;
-            if establishes_containing_block_for_absolute(EstablishContainingBlock::Yes,
+            if establishes_containing_block_for_absolute(StackingContextCollectionFlags::empty(),
                                                          fragment.style.get_box().position) {
                 state.containing_block_clip_and_scroll_info = state.current_clip_and_scroll_info;
             }

components/layout/table.rs

@@ -11,7 +11,7 @@ use block::{BlockFlow, CandidateBSizeIterator, ISizeAndMarginsComputer};
 use block::{ISizeConstraintInput, ISizeConstraintSolution};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use display_list_builder::{DisplayListBuildState, EstablishContainingBlock};
+use display_list_builder::{DisplayListBuildState, StackingContextCollectionFlags};
 use display_list_builder::StackingContextCollectionState;
 use euclid::Point2D;
 use flow;
@@ -500,7 +500,8 @@ impl Flow for TableFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.block_flow.collect_stacking_contexts_for_block(state, EstablishContainingBlock::Yes);
+        self.block_flow.collect_stacking_contexts_for_block(state,
+                                                            StackingContextCollectionFlags::empty());
     }
 
     fn repair_style(&mut self, new_style: &::ServoArc<ComputedValues>) {

components/layout/table_caption.rs

@@ -10,7 +10,7 @@ use app_units::Au;
 use block::BlockFlow;
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, DisplayListBuildState};
-use display_list_builder::{EstablishContainingBlock, StackingContextCollectionState};
+use display_list_builder::{StackingContextCollectionFlags, StackingContextCollectionState};
 use euclid::Point2D;
 use flow::{Flow, FlowClass, OpaqueFlow};
 use fragment::{Fragment, FragmentBorderBoxIterator, Overflow};
@@ -81,7 +81,8 @@ impl Flow for TableCaptionFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.block_flow.collect_stacking_contexts_for_block(state, EstablishContainingBlock::No);
+        self.block_flow.collect_stacking_contexts_for_block(state,
+                                                            StackingContextCollectionFlags::empty());
     }
 
     fn repair_style(&mut self, new_style: &::ServoArc<ComputedValues>) {

components/layout/table_cell.rs

@@ -10,7 +10,7 @@ use app_units::Au;
 use block::{BlockFlow, ISizeAndMarginsComputer, MarginsMayCollapseFlag};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use display_list_builder::{DisplayListBuildState, EstablishContainingBlock};
+use display_list_builder::{DisplayListBuildState, StackingContextCollectionFlags};
 use display_list_builder::StackingContextCollectionState;
 use euclid::{Point2D, Rect, SideOffsets2D, Size2D};
 use flow::{self, Flow, FlowClass, IS_ABSOLUTELY_POSITIONED, OpaqueFlow};
@@ -263,7 +263,8 @@ impl Flow for TableCellFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.block_flow.collect_stacking_contexts_for_block(state, EstablishContainingBlock::No);
+        self.block_flow.collect_stacking_contexts_for_block(state,
+                                                            StackingContextCollectionFlags::empty());
     }
 
     fn repair_style(&mut self, new_style: &::ServoArc<ComputedValues>) {

components/layout/table_row.rs

@@ -10,7 +10,7 @@ use app_units::Au;
 use block::{BlockFlow, ISizeAndMarginsComputer};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use display_list_builder::{DisplayListBuildState, EstablishContainingBlock};
+use display_list_builder::{DisplayListBuildState, StackingContextCollectionFlags};
 use display_list_builder::StackingContextCollectionState;
 use euclid::Point2D;
 use flow::{self, EarlyAbsolutePositionInfo, Flow, FlowClass, ImmutableFlowUtils, OpaqueFlow};
@@ -477,7 +477,8 @@ impl Flow for TableRowFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.block_flow.collect_stacking_contexts_for_block(state, EstablishContainingBlock::No);
+        self.block_flow.collect_stacking_contexts_for_block(state,
+                                                            StackingContextCollectionFlags::empty());
     }
 
     fn repair_style(&mut self, new_style: &::ServoArc<ComputedValues>) {

components/layout/table_rowgroup.rs

@@ -10,7 +10,7 @@ use app_units::Au;
 use block::{BlockFlow, ISizeAndMarginsComputer};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, DisplayListBuildState};
-use display_list_builder::{EstablishContainingBlock, StackingContextCollectionState};
+use display_list_builder::{NEVER_CREATES_CONTAINING_BLOCK, StackingContextCollectionState};
 use euclid::Point2D;
 use flow::{Flow, FlowClass, OpaqueFlow};
 use fragment::{Fragment, FragmentBorderBoxIterator, Overflow};
@@ -180,7 +180,7 @@ impl Flow for TableRowGroupFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.block_flow.collect_stacking_contexts_for_block(state, EstablishContainingBlock::No);
+        self.block_flow.collect_stacking_contexts_for_block(state, NEVER_CREATES_CONTAINING_BLOCK);
     }
 
     fn repair_style(&mut self, new_style: &::ServoArc<ComputedValues>) {

components/layout/table_wrapper.rs

@@ -18,7 +18,8 @@ use block::{AbsoluteNonReplaced, BlockFlow, FloatNonReplaced, ISizeAndMarginsCom
 use block::{ISizeConstraintSolution, MarginsMayCollapseFlag};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, DisplayListBuildState};
-use display_list_builder::{EstablishContainingBlock, StackingContextCollectionState};
+use display_list_builder::{NEVER_CREATES_CLIP_SCROLL_NODE, NEVER_CREATES_CONTAINING_BLOCK};
+use display_list_builder::StackingContextCollectionState;
 use euclid::Point2D;
 use floats::FloatKind;
 use flow::{Flow, FlowClass, ImmutableFlowUtils, INLINE_POSITION_IS_STATIC, OpaqueFlow};
@@ -458,7 +459,8 @@ impl Flow for TableWrapperFlow {
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
-        self.block_flow.collect_stacking_contexts_for_block(state, EstablishContainingBlock::No);
+        self.block_flow.collect_stacking_contexts_for_block(
+            state, NEVER_CREATES_CONTAINING_BLOCK | NEVER_CREATES_CLIP_SCROLL_NODE);
     }
 
     fn repair_style(&mut self, new_style: &::ServoArc<ComputedValues>) {

tests/wpt/mozilla/meta/MANIFEST.json

@@ -5921,6 +5921,18 @@
      {}
     ]
    ],
+   "css/table_with_position_sticky.html": [
+    [
+     "/_mozilla/css/table_with_position_sticky.html",
+     [
+      [
+       "/_mozilla/css/table_with_position_sticky_ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/text_align_complex_a.html": [
     [
      "/_mozilla/css/text_align_complex_a.html",
@@ -10379,6 +10391,11 @@
      {}
     ]
    ],
+   "css/table_with_position_sticky_ref.html": [
+    [
+     {}
+    ]
+   ],
    "css/test.jpeg": [
     [
      {}
@@ -26612,6 +26629,14 @@
    "0dc40b30f1d518d7c148535d9230a95417aadd56",
    "support"
   ],
+  "css/table_with_position_sticky.html": [
+   "30c4f54f51c8d35614d52d3702e58de8589acd2f",
+   "reftest"
+  ],
+  "css/table_with_position_sticky_ref.html": [
+   "da028fde51bd53b7c7a4314e998162ab10d8d359",
+   "support"
+  ],
   "css/test.jpeg": [
    "b7329039658f91906338b4ec599633f966de1b46",
    "support"

tests/wpt/mozilla/tests/css/table_with_position_sticky.html

@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<title>Tables with position:sticky elements should not cause a panic</title>
+<link rel="match" href="table_with_position_sticky_ref.html" />
+<link rel="help" href="https://www.w3.org/TR/css-position-3/#sticky-pos" />
+<meta name="assert" content="This test checks that tables with position:sticky dont' cause a panic" />
+<div style="width: 100px; height: 100px; background: green"></div>
+<table style="position: sticky; top: 0;">

tests/wpt/mozilla/tests/css/table_with_position_sticky_ref.html

@@ -0,0 +1,3 @@
+<!DOCTYPE html>
+<title>Reference for Tables with position:sticky elements should not cause a panic</title>
+<div style="width: 100px; height: 100px; background: green"></div>