Auto merge of #20034 - Manishearth:table-backgrounds, r=mbrubeck

Handle table cell backgrounds during display list generation for <table>

Fixes #19788

<!-- 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/20034)
<!-- Reviewable:end -->

components/layout/block.rs

@@ -38,6 +38,7 @@ use flow::{BaseFlow, EarlyAbsolutePositionInfo, Flow, FlowClass, ForceNonfloated
 use flow::{ImmutableFlowUtils, LateAbsolutePositionInfo, OpaqueFlow, FragmentationContext, FlowFlags};
 use flow_list::FlowList;
 use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, Overflow, FragmentFlags};
+use gfx::display_list::DisplayListSection;
 use gfx_traits::print_tree::PrintTree;
 use incremental::RelayoutMode;
 use layout_debug;
@@ -1776,7 +1777,7 @@ impl BlockFlow {
         }
     }
 
-    pub fn has_scrolling_overflow(&mut self) -> bool {
+    pub fn has_scrolling_overflow(&self) -> bool {
         self.flags.contains(BlockFlowFlags::HAS_SCROLLING_OVERFLOW)
     }
 
@@ -1794,6 +1795,22 @@ impl BlockFlow {
         as_margins.to_physical(writing_mode)
     }
 
+    pub fn background_border_section(&self) -> DisplayListSection {
+        if self.base.flags.is_float() {
+            DisplayListSection::BackgroundAndBorders
+        } else if self.base
+            .flags
+            .contains(FlowFlags::IS_ABSOLUTELY_POSITIONED)
+        {
+            if self.fragment.establishes_stacking_context() {
+                DisplayListSection::BackgroundAndBorders
+            } else {
+                DisplayListSection::BlockBackgroundsAndBorders
+            }
+        } else {
+            DisplayListSection::BlockBackgroundsAndBorders
+        }
+    }
 }
 
 impl Flow for BlockFlow {

components/layout/display_list/builder.rs

@@ -60,7 +60,7 @@ use style::computed_values::overflow_x::T as StyleOverflow;
 use style::computed_values::pointer_events::T as PointerEvents;
 use style::computed_values::position::T as StylePosition;
 use style::computed_values::visibility::T as Visibility;
-use style::logical_geometry::{LogicalMargin, LogicalPoint, LogicalRect, LogicalSize, WritingMode};
+use style::logical_geometry::{LogicalMargin, LogicalPoint, LogicalRect};
 use style::properties::ComputedValues;
 use style::properties::style_structs;
 use style::servo::restyle_damage::ServoRestyleDamage;
@@ -531,6 +531,18 @@ pub trait FragmentDisplayListBuilding {
         absolute_bounds: &Rect<Au>,
     );
 
+    /// Same as build_display_list_for_background_if_applicable, but lets you
+    /// override the actual background used
+    fn build_display_list_for_background_if_applicable_with_background(
+        &self,
+        state: &mut DisplayListBuildState,
+        style: &ComputedValues,
+        background: &style_structs::Background,
+        background_color: RGBA,
+        display_list_section: DisplayListSection,
+        absolute_bounds: &Rect<Au>,
+    );
+
     /// Determines where to place an element background image or gradient.
     ///
     /// Photos have their resolution as intrinsic size while gradients have
@@ -639,17 +651,23 @@ pub trait FragmentDisplayListBuilding {
     /// * `state`: The display building state, including the display list currently
     ///   under construction and other metadata useful for constructing it.
     /// * `dirty`: The dirty rectangle in the coordinate system of the owning flow.
-    /// * `stacking_relative_flow_origin`: Position of the origin of the owning flow with respect
-    ///   to its nearest ancestor stacking context.
-    /// * `relative_containing_block_size`: The size of the containing block that
-    ///   `position: relative` makes use of.
     /// * `clip`: The region to clip the display items to.
     fn build_display_list(
         &mut self,
         state: &mut DisplayListBuildState,
-        stacking_relative_flow_origin: &Vector2D<Au>,
-        relative_containing_block_size: &LogicalSize<Au>,
-        relative_containing_block_mode: WritingMode,
+        stacking_relative_border_box: Rect<Au>,
+        border_painting_mode: BorderPaintingMode,
+        display_list_section: DisplayListSection,
+        clip: &Rect<Au>,
+    );
+
+    /// build_display_list, but don't update the restyle damage
+    ///
+    /// Must be paired with a self.restyle_damage.remove(REPAINT) somewhere
+    fn build_display_list_no_damage(
+        &self,
+        state: &mut DisplayListBuildState,
+        stacking_relative_border_box: Rect<Au>,
         border_painting_mode: BorderPaintingMode,
         display_list_section: DisplayListSection,
         clip: &Rect<Au>,
@@ -689,7 +707,7 @@ pub trait FragmentDisplayListBuilding {
 
     /// A helper method that `build_display_list` calls to create per-fragment-type display items.
     fn build_fragment_type_specific_display_items(
-        &mut self,
+        &self,
         state: &mut DisplayListBuildState,
         stacking_relative_border_box: &Rect<Au>,
         clip: &Rect<Au>,
@@ -929,13 +947,28 @@ impl FragmentDisplayListBuilding for Fragment {
         style: &ComputedValues,
         display_list_section: DisplayListSection,
         absolute_bounds: &Rect<Au>,
+    ) {
+        let background = style.get_background();
+        let background_color = style.resolve_color(background.background_color);
+        // XXXManishearth the below method should ideally use an iterator over
+        // backgrounds
+        self.build_display_list_for_background_if_applicable_with_background(
+            state, style, background, background_color, display_list_section, absolute_bounds)
+    }
+
+    fn build_display_list_for_background_if_applicable_with_background(
+        &self,
+        state: &mut DisplayListBuildState,
+        style: &ComputedValues,
+        background: &style_structs::Background,
+        background_color: RGBA,
+        display_list_section: DisplayListSection,
+        absolute_bounds: &Rect<Au>,
     ) {
         // FIXME: This causes a lot of background colors to be displayed when they are clearly not
         // needed. We could use display list optimization to clean this up, but it still seems
         // inefficient. What we really want is something like "nearest ancestor element that
         // doesn't have a fragment".
-        let background = style.get_background();
-        let background_color = style.resolve_color(background.background_color);
 
         // 'background-clip' determines the area within which the background is painted.
         // http://dev.w3.org/csswg/css-backgrounds-3/#the-background-clip
@@ -1694,31 +1727,31 @@ impl FragmentDisplayListBuilding for Fragment {
     fn build_display_list(
         &mut self,
         state: &mut DisplayListBuildState,
-        stacking_relative_flow_origin: &Vector2D<Au>,
-        relative_containing_block_size: &LogicalSize<Au>,
-        relative_containing_block_mode: WritingMode,
+        stacking_relative_border_box: Rect<Au>,
         border_painting_mode: BorderPaintingMode,
         display_list_section: DisplayListSection,
         clip: &Rect<Au>,
     ) {
         self.restyle_damage.remove(ServoRestyleDamage::REPAINT);
+        self.build_display_list_no_damage(state, stacking_relative_border_box,
+                                border_painting_mode, display_list_section, clip)
+    }
+
+    fn build_display_list_no_damage(
+        &self,
+        state: &mut DisplayListBuildState,
+        stacking_relative_border_box: Rect<Au>,
+        border_painting_mode: BorderPaintingMode,
+        display_list_section: DisplayListSection,
+        clip: &Rect<Au>,
+    ) {
         if self.style().get_inheritedbox().visibility != Visibility::Visible {
             return;
         }
 
-        // Compute the fragment position relative to the parent stacking context. If the fragment
-        // itself establishes a stacking context, then the origin of its position will be (0, 0)
-        // for the purposes of this computation.
-        let stacking_relative_border_box = self.stacking_relative_border_box(
-            stacking_relative_flow_origin,
-            relative_containing_block_size,
-            relative_containing_block_mode,
-            CoordinateSystem::Own,
-        );
-
         debug!(
-            "Fragment::build_display_list at rel={:?}, abs={:?}, flow origin={:?}: {:?}",
-            self.border_box, stacking_relative_border_box, stacking_relative_flow_origin, self
+            "Fragment::build_display_list at rel={:?}, abs={:?}: {:?}",
+            self.border_box, stacking_relative_border_box, self
         );
 
         // Check the clip rect. If there's nothing to render at all, don't even construct display
@@ -1830,7 +1863,7 @@ impl FragmentDisplayListBuilding for Fragment {
     }
 
     fn build_fragment_type_specific_display_items(
-        &mut self,
+        &self,
         state: &mut DisplayListBuildState,
         stacking_relative_border_box: &Rect<Au>,
         clip: &Rect<Au>,
@@ -1954,7 +1987,7 @@ impl FragmentDisplayListBuilding for Fragment {
                     state.add_display_item(item);
                 }
             },
-            SpecificFragmentInfo::Image(ref mut image_fragment) => {
+            SpecificFragmentInfo::Image(ref image_fragment) => {
                 // Place the image into the display list.
                 if let Some(ref image) = image_fragment.image {
                     let base = state.create_base_display_item(
@@ -2320,6 +2353,16 @@ pub trait BlockFlowDisplayListBuilding {
         state: &mut DisplayListBuildState,
         border_painting_mode: BorderPaintingMode,
     );
+    fn build_display_list_for_block_no_damage(
+        &self,
+        state: &mut DisplayListBuildState,
+        border_painting_mode: BorderPaintingMode,
+    );
+    fn build_display_list_for_background_if_applicable_with_background(
+        &self,
+        state: &mut DisplayListBuildState,
+        background: &style_structs::Background,
+        background_color: RGBA);
 
     fn block_stacking_context_type(
         &self,
@@ -2858,38 +2901,20 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         self.base.collect_stacking_contexts_for_children(state);
     }
 
-    fn build_display_list_for_block(
-        &mut self,
+    fn build_display_list_for_block_no_damage(
+        &self,
         state: &mut DisplayListBuildState,
         border_painting_mode: BorderPaintingMode,
     ) {
-        let background_border_section = if self.base.flags.is_float() {
-            DisplayListSection::BackgroundAndBorders
-        } else if self.base
-            .flags
-            .contains(FlowFlags::IS_ABSOLUTELY_POSITIONED)
-        {
-            if self.fragment.establishes_stacking_context() {
-                DisplayListSection::BackgroundAndBorders
-            } else {
-                DisplayListSection::BlockBackgroundsAndBorders
-            }
-        } else {
-            DisplayListSection::BlockBackgroundsAndBorders
-        };
+        let background_border_section = self.background_border_section();
 
         state.processing_scrolling_overflow_element = self.has_scrolling_overflow();
-
+        let stacking_relative_border_box =
+            self.base.stacking_relative_border_box_for_display_list(&self.fragment);
         // Add the box that starts the block context.
-        self.fragment.build_display_list(
+        self.fragment.build_display_list_no_damage(
             state,
-            &self.base.stacking_relative_position,
-            &self.base
-                .early_absolute_position_info
-                .relative_containing_block_size,
-            self.base
-                .early_absolute_position_info
-                .relative_containing_block_mode,
+            stacking_relative_border_box,
             border_painting_mode,
             background_border_section,
             &self.base.clip,
@@ -2901,6 +2926,31 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         state.processing_scrolling_overflow_element = false;
     }
 
+    fn build_display_list_for_block(
+        &mut self,
+        state: &mut DisplayListBuildState,
+        border_painting_mode: BorderPaintingMode,
+    ) {
+        self.fragment.restyle_damage.remove(ServoRestyleDamage::REPAINT);
+        self.build_display_list_for_block_no_damage(state, border_painting_mode);
+    }
+
+    fn build_display_list_for_background_if_applicable_with_background(
+        &self,
+        state: &mut DisplayListBuildState,
+        background: &style_structs::Background,
+        background_color: RGBA) {
+        let stacking_relative_border_box =
+            self.base.stacking_relative_border_box_for_display_list(&self.fragment);
+        let background_border_section = self.background_border_section();
+
+        self.fragment.build_display_list_for_background_if_applicable_with_background(
+            state, self.fragment.style(), background, background_color,
+            background_border_section, &stacking_relative_border_box
+        )
+
+    }
+
     #[inline]
     fn block_stacking_context_type(
         &self,
@@ -2992,15 +3042,11 @@ impl InlineFlowDisplayListBuilding for InlineFlow {
         index: usize,
     ) {
         let fragment = self.fragments.fragments.get_mut(index).unwrap();
+        let stacking_relative_border_box =
+            self.base.stacking_relative_border_box_for_display_list(fragment);
         fragment.build_display_list(
             state,
-            &self.base.stacking_relative_position,
-            &self.base
-                .early_absolute_position_info
-                .relative_containing_block_size,
-            self.base
-                .early_absolute_position_info
-                .relative_containing_block_mode,
+            stacking_relative_border_box,
             BorderPaintingMode::Separate,
             DisplayListSection::Content,
             &self.base.clip,
@@ -3052,17 +3098,11 @@ impl ListItemFlowDisplayListBuilding for ListItemFlow {
     fn build_display_list_for_list_item(&mut self, state: &mut DisplayListBuildState) {
         // Draw the marker, if applicable.
         for marker in &mut self.marker_fragments {
+            let stacking_relative_border_box =
+                self.block_flow.base.stacking_relative_border_box_for_display_list(marker);
             marker.build_display_list(
                 state,
-                &self.block_flow.base.stacking_relative_position,
-                &self.block_flow
-                    .base
-                    .early_absolute_position_info
-                    .relative_containing_block_size,
-                self.block_flow
-                    .base
-                    .early_absolute_position_info
-                    .relative_containing_block_mode,
+                stacking_relative_border_box,
                 BorderPaintingMode::Separate,
                 DisplayListSection::Content,
                 &self.block_flow.base.clip,

components/layout/flow.rs

@@ -171,6 +171,12 @@ pub trait Flow: HasBaseFlow + fmt::Debug + Sync + Send + 'static {
         panic!("called as_mut_table_colgroup() on a non-tablecolgroup flow")
     }
 
+    /// If this is a table colgroup flow, returns the underlying object. Fails
+    /// otherwise.
+    fn as_table_colgroup(&self) -> &TableColGroupFlow {
+        panic!("called as_table_colgroup() on a non-tablecolgroup flow")
+    }
+
     /// If this is a table rowgroup flow, returns the underlying object, borrowed mutably. Fails
     /// otherwise.
     fn as_mut_table_rowgroup(&mut self) -> &mut TableRowGroupFlow {
@@ -1149,6 +1155,19 @@ impl BaseFlow {
         self.speculated_float_placement_out.left > Au(0) ||
             self.speculated_float_placement_out.right > Au(0)
     }
+
+
+    /// Compute the fragment position relative to the parent stacking context. If the fragment
+    /// itself establishes a stacking context, then the origin of its position will be (0, 0)
+    /// for the purposes of this computation.
+    pub fn stacking_relative_border_box_for_display_list(&self, fragment: &Fragment) -> Rect<Au> {
+        fragment.stacking_relative_border_box(
+            &self.stacking_relative_position,
+            &self.early_absolute_position_info.relative_containing_block_size,
+            self.early_absolute_position_info.relative_containing_block_mode,
+            CoordinateSystem::Own,
+        )
+    }
 }
 
 impl<'a> ImmutableFlowUtils for &'a Flow {

components/layout/fragment.rs

@@ -1408,6 +1408,16 @@ impl Fragment {
         }
     }
 
+    /// If this is a Column fragment, get the col span
+    ///
+    /// Panics for non-column fragments
+    pub fn column_span(&self) -> u32 {
+        match self.specific {
+            SpecificFragmentInfo::TableColumn(col_fragment) => max(col_fragment.span, 1),
+            _ => panic!("non-table-column fragment inside table column?!"),
+        }
+    }
+
     /// Returns true if this element can be split. This is true for text fragments, unless
     /// `white-space: pre` or `white-space: nowrap` is set.
     pub fn can_split(&self) -> bool {

components/layout/table.rs

@@ -14,20 +14,21 @@ use display_list::{BlockFlowDisplayListBuilding, BorderPaintingMode};
 use display_list::{DisplayListBuildState, StackingContextCollectionFlags, StackingContextCollectionState};
 use euclid::Point2D;
 use flow::{BaseFlow, EarlyAbsolutePositionInfo, Flow, FlowClass, ImmutableFlowUtils, GetBaseFlow, OpaqueFlow};
-use flow_list::MutFlowListIterator;
+use flow_list::{FlowListIterator, MutFlowListIterator};
 use fragment::{Fragment, FragmentBorderBoxIterator, Overflow};
 use gfx_traits::print_tree::PrintTree;
 use layout_debug;
 use model::{IntrinsicISizes, IntrinsicISizesContribution, MaybeAuto};
-use std::cmp;
-use std::fmt;
+use std::{cmp, fmt};
 use style::computed_values::{border_collapse, border_spacing, table_layout};
 use style::context::SharedStyleContext;
 use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
+use style::properties::style_structs::Background;
 use style::servo::restyle_damage::ServoRestyleDamage;
 use style::values::CSSFloat;
 use style::values::computed::LengthOrPercentageOrAuto;
+use table_cell::TableCellFlow;
 use table_row::{self, CellIntrinsicInlineSize, CollapsedBorder, CollapsedBorderProvenance};
 use table_row::TableRowFlow;
 use table_wrapper::TableLayout;
@@ -203,6 +204,37 @@ impl TableFlow {
         }
         self.spacing().horizontal() * (num_columns as i32 + 1)
     }
+
+    fn column_styles(&self) -> Vec<ColumnStyle> {
+        let mut styles = vec![];
+        for group in self.block_flow.base.child_iter()
+                       .filter(|kid| kid.is_table_colgroup()) {
+            // XXXManishearth these as_foo methods should return options
+            // so that we can filter_map
+            let group = group.as_table_colgroup();
+            let colgroup_style = group.fragment.as_ref()
+                                      .map(|f| f.style());
+
+            // The colgroup's span attribute is only relevant when
+            // it has no children
+            // https://html.spec.whatwg.org/multipage/#forming-a-table
+            if group.cols.is_empty() {
+                let span = group.fragment.as_ref()
+                                .map(|f| f.column_span()).unwrap_or(1);
+                styles.push(ColumnStyle { span, colgroup_style, col_style: None });
+            } else {
+                for col in &group.cols {
+                    // XXXManishearth Arc-cloning colgroup_style is suboptimal
+                    styles.push(ColumnStyle {
+                        span: col.column_span(),
+                        colgroup_style: colgroup_style,
+                        col_style: Some(col.style()),
+                    })
+                }
+            }
+        }
+        styles
+    }
 }
 
 impl Flow for TableFlow {
@@ -497,6 +529,11 @@ impl Flow for TableFlow {
         };
 
         self.block_flow.build_display_list_for_block(state, border_painting_mode);
+
+        let iter = TableCellStyleIterator::new(&self);
+        for mut style in iter {
+            style.build_display_list(state)
+        }
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {
@@ -529,6 +566,20 @@ impl Flow for TableFlow {
     }
 }
 
+#[derive(Debug)]
+// XXXManishearth We might be able to avoid the Arc<T>s if
+// the table is structured such that the columns always come
+// first in the flow tree, at which point we can
+// reuse the iterator that we use for colgroups
+// for rows (and have no borrowing issues between
+// holding on to both ColumnStyle<'table> and
+// the rows)
+struct ColumnStyle<'table> {
+    span: u32,
+    colgroup_style: Option<&'table ComputedValues>,
+    col_style: Option<&'table ComputedValues>,
+}
+
 impl fmt::Debug for TableFlow {
     /// Outputs a debugging string describing this table flow.
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -825,40 +876,43 @@ enum NextBlockCollapsedBorders<'a> {
     FromTable(CollapsedBorder),
 }
 
-/// Iterator over all the rows of a table
-struct TableRowIterator<'a> {
-    kids: MutFlowListIterator<'a>,
-    grandkids: Option<MutFlowListIterator<'a>>,
+/// Iterator over all the rows of a table, which also
+/// provides the Fragment for rowgroups if any
+struct TableRowAndGroupIterator<'a> {
+    kids: FlowListIterator<'a>,
+    group: Option<(&'a Fragment, FlowListIterator<'a>)>
 }
 
-impl<'a> TableRowIterator<'a> {
-    fn new(base: &'a mut BaseFlow) -> Self {
-        TableRowIterator {
-            kids: base.child_iter_mut(),
-            grandkids: None,
+impl<'a> TableRowAndGroupIterator<'a> {
+    fn new(base: &'a BaseFlow) -> Self {
+        TableRowAndGroupIterator {
+            kids: base.child_iter(),
+            group: None,
         }
     }
 }
 
-impl<'a> Iterator for TableRowIterator<'a> {
-    type Item = &'a mut TableRowFlow;
+impl<'a> Iterator for TableRowAndGroupIterator<'a> {
+    type Item = (Option<&'a Fragment>, &'a TableRowFlow);
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
         // If we're inside a rowgroup, iterate through the rowgroup's children.
-        if let Some(ref mut grandkids) = self.grandkids {
-            if let Some(grandkid) = grandkids.next() {
-                return Some(grandkid.as_mut_table_row())
+        if let Some(ref mut group) = self.group {
+            if let Some(grandkid) = group.1.next() {
+                return Some((Some(group.0), grandkid.as_table_row()))
             }
         }
         // Otherwise, iterate through the table's children.
-        self.grandkids = None;
+        self.group = None;
         match self.kids.next() {
             Some(kid) => {
                 if kid.is_table_rowgroup() {
-                    self.grandkids = Some(kid.mut_base().child_iter_mut());
+                    let mut rowgroup = kid.as_table_rowgroup();
+                    let iter = rowgroup.block_flow.base.child_iter();
+                    self.group = Some((&rowgroup.block_flow.fragment, iter));
                     self.next()
                 } else if kid.is_table_row() {
-                    Some(kid.as_mut_table_row())
+                    Some((None, kid.as_table_row()))
                 } else {
                     self.next() // Skip children that are not rows or rowgroups
                 }
@@ -867,3 +921,271 @@ impl<'a> Iterator for TableRowIterator<'a> {
         }
     }
 }
+
+/// Iterator over all the rows of a table, which also
+/// provides the Fragment for rowgroups if any
+struct MutTableRowAndGroupIterator<'a> {
+    kids: MutFlowListIterator<'a>,
+    group: Option<(&'a Fragment, MutFlowListIterator<'a>)>
+}
+
+impl<'a> MutTableRowAndGroupIterator<'a> {
+    fn new(base: &'a mut BaseFlow) -> Self {
+        MutTableRowAndGroupIterator {
+            kids: base.child_iter_mut(),
+            group: None,
+        }
+    }
+}
+
+impl<'a> Iterator for MutTableRowAndGroupIterator<'a> {
+    type Item = (Option<&'a Fragment>, &'a mut TableRowFlow);
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        // If we're inside a rowgroup, iterate through the rowgroup's children.
+        if let Some(ref mut group) = self.group {
+            if let Some(grandkid) = group.1.next() {
+                return Some((Some(group.0), grandkid.as_mut_table_row()))
+            }
+        }
+        // Otherwise, iterate through the table's children.
+        self.group = None;
+        match self.kids.next() {
+            Some(kid) => {
+                if kid.is_table_rowgroup() {
+                    let mut rowgroup = kid.as_mut_table_rowgroup();
+                    let iter = rowgroup.block_flow.base.child_iter_mut();
+                    self.group = Some((&rowgroup.block_flow.fragment, iter));
+                    self.next()
+                } else if kid.is_table_row() {
+                    Some((None, kid.as_mut_table_row()))
+                } else {
+                    self.next() // Skip children that are not rows or rowgroups
+                }
+            }
+            None => None
+        }
+    }
+}
+
+/// Iterator over all the rows of a table
+struct TableRowIterator<'a>(MutTableRowAndGroupIterator<'a>);
+
+impl<'a> TableRowIterator<'a> {
+    fn new(base: &'a mut BaseFlow) -> Self {
+        TableRowIterator(MutTableRowAndGroupIterator::new(base))
+    }
+}
+
+impl<'a> Iterator for TableRowIterator<'a> {
+    type Item = &'a mut TableRowFlow;
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(|n| n.1)
+    }
+}
+
+/// An iterator over table cells, yielding all relevant style objects
+/// for each cell
+///
+/// Used for correctly handling table layers from
+/// https://drafts.csswg.org/css2/tables.html#table-layers
+struct TableCellStyleIterator<'table> {
+    column_styles: Vec<ColumnStyle<'table>>,
+    row_iterator: TableRowAndGroupIterator<'table>,
+    row_info: Option<TableCellStyleIteratorRowInfo<'table>>,
+    column_index: TableCellColumnIndexData,
+
+}
+
+struct TableCellStyleIteratorRowInfo<'table> {
+    row: &'table TableRowFlow,
+    rowgroup: Option<&'table Fragment>,
+    cell_iterator: FlowListIterator<'table>,
+}
+
+impl<'table> TableCellStyleIterator<'table> {
+    fn new(table: &'table TableFlow) -> Self {
+        let column_styles = table.column_styles();
+        let mut row_iterator = TableRowAndGroupIterator::new(&table.block_flow.base);
+        let row_info = if let Some((group, row)) = row_iterator.next() {
+            Some(TableCellStyleIteratorRowInfo {
+                row: &row,
+                rowgroup: group,
+                cell_iterator: row.block_flow.base.child_iter()
+            })
+        } else {
+            None
+        };
+        TableCellStyleIterator {
+            column_styles, row_iterator, row_info,
+            column_index: Default::default(),
+        }
+    }
+}
+
+struct TableCellStyleInfo<'table> {
+    cell: &'table TableCellFlow,
+    colgroup_style: Option<&'table ComputedValues>,
+    col_style: Option<&'table ComputedValues>,
+    rowgroup_style: Option<&'table ComputedValues>,
+    row_style: &'table ComputedValues,
+}
+
+struct TableCellColumnIndexData {
+    /// Which column this is in the table
+    pub absolute: u32,
+    /// The index of the current column in column_styles
+    /// (i.e. which <col> element it is)
+    pub relative: u32,
+    /// In case of multispan <col>s, where we are in the
+    /// span of the current <col> element
+    pub relative_offset: u32,
+}
+
+impl Default for TableCellColumnIndexData {
+    fn default() -> Self {
+        TableCellColumnIndexData {
+            absolute: 0,
+            relative: 0,
+            relative_offset: 0,
+        }
+    }
+}
+
+impl TableCellColumnIndexData {
+    /// Moves forward by `amount` columns, updating the various indices used
+    ///
+    /// This totally ignores rowspan -- if colspan and rowspan clash,
+    /// they just overlap, so we ignore it.
+    fn advance(&mut self, amount: u32, column_styles: &[ColumnStyle]) {
+        self.absolute += amount;
+        self.relative_offset += amount;
+        if let Some(mut current_col) =
+            column_styles.get(self.relative as usize) {
+            while self.relative_offset >= current_col.span {
+                // move to the next column
+                self.relative += 1;
+                self.relative_offset -= current_col.span;
+                if let Some(column_style) =
+                    column_styles.get(self.relative as usize) {
+                    current_col = column_style;
+                } else {
+                    // we ran out of column_styles,
+                    // so we don't need to update the indices
+                    break;
+                }
+            }
+        }
+    }
+}
+
+impl<'table> Iterator for TableCellStyleIterator<'table> {
+    type Item = TableCellStyleInfo<'table>;
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        // FIXME We do this awkward .take() followed by shoving it back in
+        // because without NLL the row_info borrow lasts too long
+        if let Some(mut row_info) = self.row_info.take() {
+            if let Some(rowspan) = row_info.row.incoming_rowspan.get(self.column_index.absolute as usize) {
+                // we are not allowed to use this column as a starting point. Try the next one.
+                if *rowspan > 1 {
+                    self.column_index.advance(1, &self.column_styles);
+                    // put row_info back in
+                    self.row_info = Some(row_info);
+                    // try again
+                    return self.next();
+                }
+            }
+            if let Some(cell) = row_info.cell_iterator.next() {
+                let rowgroup_style = row_info.rowgroup.map(|r| r.style());
+                let row_style = row_info.row.block_flow.fragment.style();
+                let cell = cell.as_table_cell();
+                let (col_style, colgroup_style) = if let Some(column_style) =
+                        self.column_styles.get(self.column_index.relative as usize) {
+                    let styles = (column_style.col_style.clone(), column_style.colgroup_style.clone());
+                    self.column_index.advance(cell.column_span, &self.column_styles);
+
+                    styles
+                } else {
+                    (None, None)
+                };
+                // put row_info back in
+                self.row_info = Some(row_info);
+                return Some(TableCellStyleInfo {
+                    cell,
+                    colgroup_style,
+                    col_style,
+                    rowgroup_style,
+                    row_style,
+                })
+            } else {
+                // next row
+                if let Some((group, row)) = self.row_iterator.next() {
+                    self.row_info = Some(TableCellStyleIteratorRowInfo {
+                        row: &row,
+                        rowgroup: group,
+                        cell_iterator: row.block_flow.base.child_iter()
+                    });
+                    self.column_index = Default::default();
+                    self.next()
+                } else {
+                    // out of rows
+                    // row_info stays None
+                    None
+                }
+            }
+        } else {
+            // empty table
+            None
+        }
+    }
+}
+
+impl<'table> TableCellStyleInfo<'table> {
+    fn build_display_list(&self, mut state: &mut DisplayListBuildState) {
+        use style::computed_values::visibility::T as Visibility;
+
+        if !self.cell.visible || self.cell.block_flow.fragment.style()
+                                     .get_inheritedbox().visibility != Visibility::Visible {
+            return
+        }
+        let border_painting_mode = match self.cell.block_flow
+                                             .fragment
+                                             .style
+                                             .get_inheritedtable()
+                                             .border_collapse {
+            border_collapse::T::Separate => BorderPaintingMode::Separate,
+            border_collapse::T::Collapse => BorderPaintingMode::Collapse(&self.cell.collapsed_borders),
+        };
+        {
+            let cell_flow = &self.cell.block_flow;
+            let initial = ComputedValues::initial_values();
+
+            let build_dl = |sty: &ComputedValues, state: &mut &mut DisplayListBuildState| {
+                let background = sty.get_background();
+                // Don't redraw backgrounds that we've already drawn
+                if background as *const Background == initial.get_background() as *const _ {
+                    return;
+                }
+                let background_color = sty.resolve_color(background.background_color);
+                cell_flow.build_display_list_for_background_if_applicable_with_background(
+                    state, background, background_color
+                );
+            };
+
+            if let Some(ref sty) = self.colgroup_style {
+                build_dl(&sty, &mut state);
+            }
+            if let Some(ref sty) = self.col_style {
+                build_dl(&sty, &mut state);
+            }
+            if let Some(ref sty) = self.rowgroup_style {
+                build_dl(sty, &mut state);
+            }
+            build_dl(self.row_style, &mut state);
+        }
+        // the restyle damage will be set in TableCellFlow::build_display_list()
+        self.cell.block_flow.build_display_list_for_block_no_damage(state, border_painting_mode)
+    }
+}

components/layout/table_cell.rs

@@ -9,9 +9,8 @@
 use app_units::Au;
 use block::{BlockFlow, ISizeAndMarginsComputer, MarginsMayCollapseFlag};
 use context::LayoutContext;
-use display_list::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use display_list::{DisplayListBuildState, StackingContextCollectionFlags};
-use display_list::StackingContextCollectionState;
+use display_list::{BlockFlowDisplayListBuilding, DisplayListBuildState};
+use display_list::{StackingContextCollectionFlags, StackingContextCollectionState};
 use euclid::{Point2D, Rect, SideOffsets2D, Size2D};
 use flow::{Flow, FlowClass, FlowFlags, GetBaseFlow, OpaqueFlow};
 use fragment::{Fragment, FragmentBorderBoxIterator, Overflow};
@@ -20,7 +19,6 @@ use layout_debug;
 use model::MaybeAuto;
 use script_layout_interface::wrapper_traits::ThreadSafeLayoutNode;
 use std::fmt;
-use style::computed_values::border_collapse::T as BorderCollapse;
 use style::logical_geometry::{LogicalMargin, LogicalRect, LogicalSize, WritingMode};
 use style::properties::ComputedValues;
 use style::values::computed::Color;
@@ -248,21 +246,14 @@ impl Flow for TableCellFlow {
         self.block_flow.update_late_computed_block_position_if_necessary(block_position)
     }
 
-    fn build_display_list(&mut self, state: &mut DisplayListBuildState) {
-        if !self.visible {
-            return
-        }
+    fn build_display_list(&mut self, _: &mut DisplayListBuildState) {
+        use style::servo::restyle_damage::ServoRestyleDamage;
+        // This is handled by TableCellStyleInfo::build_display_list()
+        // when the containing table builds its display list
 
-        let border_painting_mode = match self.block_flow
-                                             .fragment
-                                             .style
-                                             .get_inheritedtable()
-                                             .border_collapse {
-            BorderCollapse::Separate => BorderPaintingMode::Separate,
-            BorderCollapse::Collapse => BorderPaintingMode::Collapse(&self.collapsed_borders),
-        };
-
-        self.block_flow.build_display_list_for_block(state, border_painting_mode)
+        // we skip setting the damage in TableCellStyleInfo::build_display_list()
+        // because we only have immutable access
+        self.block_flow.fragment.restyle_damage.remove(ServoRestyleDamage::REPAINT);
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {

components/layout/table_colgroup.rs

@@ -11,9 +11,8 @@ use context::LayoutContext;
 use display_list::{DisplayListBuildState, StackingContextCollectionState};
 use euclid::Point2D;
 use flow::{BaseFlow, Flow, FlowClass, ForceNonfloatedFlag, OpaqueFlow};
-use fragment::{Fragment, FragmentBorderBoxIterator, Overflow, SpecificFragmentInfo};
+use fragment::{Fragment, FragmentBorderBoxIterator, Overflow};
 use layout_debug;
-use std::cmp::max;
 use std::fmt;
 use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
@@ -63,6 +62,10 @@ impl Flow for TableColGroupFlow {
         self
     }
 
+    fn as_table_colgroup(&self) -> &TableColGroupFlow {
+        self
+    }
+
     fn bubble_inline_sizes(&mut self) {
         let _scope = layout_debug_scope!("table_colgroup::bubble_inline_sizes {:x}",
                                             self.base.debug_id());
@@ -70,11 +73,7 @@ impl Flow for TableColGroupFlow {
         for fragment in &self.cols {
             // Retrieve the specified value from the appropriate CSS property.
             let inline_size = fragment.style().content_inline_size();
-            let span = match fragment.specific {
-                SpecificFragmentInfo::TableColumn(col_fragment) => max(col_fragment.span, 1),
-                _ => panic!("non-table-column fragment inside table column?!"),
-            };
-            for _ in 0..span {
+            for _ in 0..fragment.column_span() {
                 self.inline_sizes.push(inline_size)
             }
         }

components/layout/table_row.rs

@@ -9,9 +9,8 @@
 use app_units::Au;
 use block::{BlockFlow, ISizeAndMarginsComputer};
 use context::LayoutContext;
-use display_list::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use display_list::{DisplayListBuildState, StackingContextCollectionFlags};
-use display_list::StackingContextCollectionState;
+use display_list::{BlockFlowDisplayListBuilding, DisplayListBuildState};
+use display_list::{StackingContextCollectionFlags, StackingContextCollectionState};
 use euclid::Point2D;
 use flow::{EarlyAbsolutePositionInfo, Flow, FlowClass, ImmutableFlowUtils, GetBaseFlow, OpaqueFlow};
 use flow_list::MutFlowListIterator;
@@ -467,17 +466,12 @@ impl Flow for TableRowFlow {
         self.block_flow.update_late_computed_block_position_if_necessary(block_position)
     }
 
-    fn build_display_list(&mut self, state: &mut DisplayListBuildState) {
-        let border_painting_mode = match self.block_flow
-                                             .fragment
-                                             .style
-                                             .get_inheritedtable()
-                                             .border_collapse {
-            BorderCollapse::Separate => BorderPaintingMode::Separate,
-            BorderCollapse::Collapse => BorderPaintingMode::Hidden,
-        };
-
-        self.block_flow.build_display_list_for_block(state, border_painting_mode);
+    fn build_display_list(&mut self, _: &mut DisplayListBuildState) {
+        use style::servo::restyle_damage::ServoRestyleDamage;
+        // handled in TableCellStyleInfo::build_display_list
+        // we skip setting the damage in TableCellStyleInfo::build_display_list()
+        // because we only have immutable access
+        self.block_flow.fragment.restyle_damage.remove(ServoRestyleDamage::REPAINT);
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {

components/layout/table_rowgroup.rs

@@ -176,9 +176,12 @@ impl Flow for TableRowGroupFlow {
         self.block_flow.update_late_computed_block_position_if_necessary(block_position)
     }
 
-    fn build_display_list(&mut self, state: &mut DisplayListBuildState) {
-        debug!("build_display_list_table_rowgroup: same process as block flow");
-        self.block_flow.build_display_list(state);
+    fn build_display_list(&mut self, _: &mut DisplayListBuildState) {
+        use style::servo::restyle_damage::ServoRestyleDamage;
+
+        // we skip setting the damage in TableCellStyleInfo::build_display_list()
+        // because we only have immutable access
+        self.block_flow.fragment.restyle_damage.remove(ServoRestyleDamage::REPAINT);
     }
 
     fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) {

tests/wpt/metadata/css/CSS2/backgrounds/background-attachment-applies-to-001.xht.ini

@@ -0,0 +1,2 @@
+[background-attachment-applies-to-001.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-attachment-applies-to-002.xht.ini

@@ -0,0 +1,2 @@
+[background-attachment-applies-to-002.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-attachment-applies-to-003.xht.ini

@@ -0,0 +1,2 @@
+[background-attachment-applies-to-003.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-attachment-applies-to-004.xht.ini

@@ -0,0 +1,2 @@
+[background-attachment-applies-to-004.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-image-applies-to-001.xht.ini

@@ -0,0 +1,2 @@
+[background-image-applies-to-001.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-image-applies-to-002.xht.ini

@@ -0,0 +1,2 @@
+[background-image-applies-to-002.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-image-applies-to-003.xht.ini

@@ -0,0 +1,2 @@
+[background-image-applies-to-003.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-image-applies-to-004.xht.ini

@@ -0,0 +1,2 @@
+[background-image-applies-to-004.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-position-applies-to-001.xht.ini

@@ -0,0 +1,2 @@
+[background-position-applies-to-001.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-position-applies-to-002.xht.ini

@@ -0,0 +1,2 @@
+[background-position-applies-to-002.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-position-applies-to-003.xht.ini

@@ -0,0 +1,2 @@
+[background-position-applies-to-003.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-position-applies-to-004.xht.ini

@@ -0,0 +1,2 @@
+[background-position-applies-to-004.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-repeat-applies-to-001.xht.ini

@@ -0,0 +1,2 @@
+[background-repeat-applies-to-001.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-repeat-applies-to-002.xht.ini

@@ -0,0 +1,2 @@
+[background-repeat-applies-to-002.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-repeat-applies-to-003.xht.ini

@@ -0,0 +1,2 @@
+[background-repeat-applies-to-003.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/backgrounds/background-repeat-applies-to-004.xht.ini

@@ -0,0 +1,2 @@
+[background-repeat-applies-to-004.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/bidi-text/direction-applies-to-001.xht.ini

@@ -0,0 +1,2 @@
+[direction-applies-to-001.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/bidi-text/direction-applies-to-002.xht.ini

@@ -0,0 +1,2 @@
+[direction-applies-to-002.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/bidi-text/direction-applies-to-003.xht.ini

@@ -0,0 +1,2 @@
+[direction-applies-to-003.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/bidi-text/direction-applies-to-004.xht.ini

@@ -0,0 +1,2 @@
+[direction-applies-to-004.xht]
+  expected: FAIL

tests/wpt/metadata/css/CSS2/box-display/display-012.xht.ini

@@ -1,3 +1,4 @@
 [display-012.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "mac": FAIL # Fonts on OSX make this gain an additional pixel of background (also fails on Firefox)

tests/wpt/metadata/css/CSS2/selectors/default-attribute-selector-004.xht.ini

@@ -1,3 +0,0 @@
-[default-attribute-selector-004.xht]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata/css/css-backgrounds/background-attachment-local/attachment-local-clipping-color-4.html.ini

@@ -1,3 +0,0 @@
-[attachment-local-clipping-color-4.html]
-  type: reftest
-  expected: FAIL