Support rowspan=0; always fixup overflowing rowspans beforehand

This makes rowspan=0 work by storing notes on which group of rows we're
in for tables containing both rowgroups and rows, and using that to
respan cells with overlarge or zero rowspans.

This also gets rid of the largest_leftover_incoming_size business,
because now spans will always be correct and we'll never have any left
over.

components/layout/table.rs

@@ -30,7 +30,7 @@ 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_row::{TableRowFlow, TableRowSizeData};
 use table_wrapper::TableLayout;
 
 #[allow(unsafe_code)]
@@ -785,50 +785,52 @@ impl TableLikeFlow for BlockFlow {
         }
 
         if self.base.restyle_damage.contains(ServoRestyleDamage::REFLOW) {
-            // (size, cumulative_border_spacing)
-            let mut sizes = vec![(Au(0), Au(0))];
+            let mut sizes = vec![Default::default()];
             // The amount of border spacing up to and including this row,
             // but not including the spacing beneath it
-            let mut cumulative_border = Au(0);
+            let mut cumulative_border_spacing = Au(0);
             let mut incoming_rowspan_data = vec![];
+            let mut rowgroup_id = 0;
+            let mut first = true;
 
             // First pass: Compute block-direction border spacings
             // XXXManishearth this can be done in tandem with the second pass,
             // provided we never hit any rowspan cases
-            for kid in self.base.child_iter_mut()
-                           .filter(|k| k.is_table_row())
-                           .skip(1) {
-                cumulative_border +=
-                    border_spacing_for_row(&self.fragment, kid.as_table_row(),
-                                           block_direction_spacing);
-                // we haven't calculated sizes yet
-                sizes.push((Au(0), cumulative_border));
+            for kid in self.base.child_iter_mut() {
+                if kid.is_table_row() {
+                    // skip the first row, it is accounted for
+                    if first {
+                        first = false;
+                        continue;
+                    }
+                    cumulative_border_spacing +=
+                        border_spacing_for_row(&self.fragment, kid.as_table_row(),
+                                               block_direction_spacing);
+                    sizes.push(TableRowSizeData {
+                        // we haven't calculated sizes yet
+                        size: Au(0),
+                        cumulative_border_spacing,
+                        rowgroup_id
+                    });
+                } else if kid.is_table_rowgroup() && !first {
+                    rowgroup_id += 1;
+                }
             }
 
             // Second pass: Compute row block sizes
             // [expensive: iterates over cells]
             let mut i = 0;
-            let mut overflow = Au(0);
             for kid in self.base.child_iter_mut() {
                 if kid.is_table_row() {
-                    let (size, oflo) = kid.as_mut_table_row()
+                    let size = kid.as_mut_table_row()
                         .compute_block_size_table_row_base(layout_context,
                                                            &mut incoming_rowspan_data,
                                                            &sizes,
                                                            i);
-                    sizes[i].0 = size;
-                    overflow = oflo;
+                    sizes[i].size = size;
                     i += 1;
-                // new rowgroups stop rowspans
-                } else if kid.is_table_rowgroup() {
-                    if i > 0 {
-                        sizes[i - 1].0 = cmp::max(sizes[i - 1].0, overflow);
-                    }
                 }
             }
-            if i > 0 {
-                sizes[i - 1].0 = cmp::max(sizes[i - 1].0, overflow);
-            }
 
 
             // Our current border-box position.

components/layout/table_row.rs

@@ -19,7 +19,7 @@ use gfx_traits::print_tree::PrintTree;
 use layout_debug;
 use model::MaybeAuto;
 use serde::{Serialize, Serializer};
-use std::cmp::{max, min};
+use std::cmp::max;
 use std::fmt;
 use std::iter::{Enumerate, IntoIterator, Peekable};
 use style::computed_values::border_collapse::T as BorderCollapse;
@@ -110,33 +110,27 @@ impl TableRowFlow {
     ///
     /// TODO(pcwalton): This doesn't handle floats and positioned elements right.
     ///
-    /// Returns the block size, as well as the size this should be if this is the last row
+    /// Returns the block size
     pub fn compute_block_size_table_row_base<'a>(&'a mut self, layout_context: &LayoutContext,
                                                  incoming_rowspan_data: &mut Vec<Au>,
-                                                 border_info: &[(Au, Au)], // (_, cumulative_border_size)
-                                                 row_index: usize) -> (Au, Au) {
+                                                 border_info: &[TableRowSizeData],
+                                                 row_index: usize) -> Au {
         fn include_sizes_from_previous_rows(col: &mut usize,
                                             incoming_rowspan: &[u32],
                                             incoming_rowspan_data: &mut Vec<Au>,
-                                            max_block_size: &mut Au,
-                                            largest_leftover_incoming_size: &mut Au) {
+                                            max_block_size: &mut Au) {
             while let Some(span) = incoming_rowspan.get(*col) {
-                if *span <= 1 {
+                if *span == 1 {
                     break;
                 }
                 let incoming = incoming_rowspan_data[*col];
                 *max_block_size = max(*max_block_size, incoming);
-                if *span > 2 {
-                    *largest_leftover_incoming_size = max(*largest_leftover_incoming_size,
-                                                          incoming * (*span - 1) as i32)
-                }
                 *col += 1;
             }
         }
         // Per CSS 2.1 § 17.5.3, find max_y = max(computed `block-size`, minimum block-size of
         // all cells).
         let mut max_block_size = Au(0);
-        let mut largest_leftover_incoming_size = Au(0);
         let thread_id = self.block_flow.base.thread_id;
         let content_box = self.block_flow.base.position
             - self.block_flow.fragment.border_padding
@@ -145,15 +139,14 @@ impl TableRowFlow {
         let mut col = 0;
         for kid in self.block_flow.base.child_iter_mut() {
             include_sizes_from_previous_rows(&mut col, &self.incoming_rowspan,
-                                             incoming_rowspan_data, &mut max_block_size,
-                                             &mut largest_leftover_incoming_size);
+                                             incoming_rowspan_data, &mut max_block_size);
             kid.place_float_if_applicable();
             debug_assert!(!kid.base().flags.is_float(), "table cells should never float");
             kid.assign_block_size_for_inorder_child_if_necessary(layout_context,
                                                                  thread_id,
                                                                  content_box);
 
-            let row_span;
+            let mut row_span;
             let column_span;
             let cell_total;
             {
@@ -166,12 +159,11 @@ impl TableRowFlow {
             child_node.position.start.b = Au(0);
             let mut cell_block_size_pressure = max(cell_total, child_node.position.size.block);
 
-            if row_span > 1 {
+            if row_span != 1 {
                 if incoming_rowspan_data.len() <= col {
                     incoming_rowspan_data.resize(col + 1, Au(0));
                 }
-                let border_sizes_spanned = get_spanned_border_size(border_info, row_index, row_span);
-                let pressure_copy = cell_block_size_pressure;
+                let border_sizes_spanned = get_spanned_border_size(border_info, row_index, &mut row_span);
 
                 cell_block_size_pressure -= border_sizes_spanned;
 
@@ -180,18 +172,12 @@ impl TableRowFlow {
                 // require an extra slow-path loop, sadly.
                 cell_block_size_pressure /= row_span as i32;
                 incoming_rowspan_data[col] = cell_block_size_pressure;
-
-                // If this ends up being the last row, it needs to cover
-                // *all* this space
-                largest_leftover_incoming_size = max(largest_leftover_incoming_size,
-                                                     pressure_copy);
             }
 
             max_block_size = max(max_block_size, cell_block_size_pressure);
             col += column_span;
         }
-        include_sizes_from_previous_rows(&mut col, &self.incoming_rowspan, incoming_rowspan_data, &mut max_block_size,
-                                         &mut largest_leftover_incoming_size);
+        include_sizes_from_previous_rows(&mut col, &self.incoming_rowspan, incoming_rowspan_data, &mut max_block_size);
 
         let mut block_size = max_block_size;
         // TODO: Percentage block-size
@@ -203,20 +189,21 @@ impl TableRowFlow {
             MaybeAuto::Auto => block_size,
             MaybeAuto::Specified(value) => max(value, block_size),
         };
-        (block_size, largest_leftover_incoming_size)
+        block_size
     }
 
-    pub fn assign_block_size_to_self_and_children(&mut self, sizes: &[(Au, Au)], index: usize) {
+    pub fn assign_block_size_to_self_and_children(&mut self, sizes: &[TableRowSizeData], index: usize) {
         // Assign the block-size of kid fragments, which is the same value as own block-size.
-        let block_size = sizes[index].0;
+        let block_size = sizes[index].size;
         for kid in self.block_flow.base.child_iter_mut() {
             let child_table_cell = kid.as_mut_table_cell();
-            let block_size = if child_table_cell.row_span > 1 {
-                let row_sizes = sizes[index..].iter()
-                                              .take(child_table_cell.row_span as usize)
-                                              .fold(Au(0), |accum, size| accum + size.0);
+            let block_size = if child_table_cell.row_span != 1 {
+                let mut row_span = child_table_cell.row_span;
                 let border_sizes_spanned =
-                    get_spanned_border_size(sizes, index, child_table_cell.row_span);
+                    get_spanned_border_size(sizes, index, &mut row_span);
+                let row_sizes = sizes[index..].iter()
+                                              .take(row_span as usize)
+                                              .fold(Au(0), |accum, r| accum + r.size);
                 row_sizes + border_sizes_spanned
             } else {
                 block_size
@@ -279,12 +266,45 @@ impl TableRowFlow {
     }
 }
 
+#[derive(Debug, Default)]
+pub struct TableRowSizeData {
+    /// The block-size of the row.
+    pub size: Au,
+    /// Border spacing up to this row (not including spacing below the row)
+    pub cumulative_border_spacing: Au,
+    /// The "segment" of the table it is in. Tables containing
+    /// both row groups and rows have the bare rows grouped in
+    /// segments separated by row groups. It's helpful to look
+    /// at these as if they are rowgroups themselves.
+    ///
+    /// This is enough information for us to be able to check whether we
+    /// are in a case where we are overflowing a rowgroup with rowspan,
+    /// however calculating the amount of overflow requires lookahead.
+    pub rowgroup_id: u32,
+}
+
 /// Given an array of (_, cumulative_border_size), the index of the
 /// current row, and the >1 row_span of the cell, calculate the amount of
-/// border-spacing spanned by the row
-fn get_spanned_border_size(sizes: &[(Au, Au)], row_index: usize, row_span: u32) -> Au {
-    let last_row_idx = min(row_index + row_span as usize - 1, sizes.len() - 1);
-    sizes[last_row_idx].1 - sizes[row_index].1
+/// border-spacing spanned by the row. In case the rowspan was larger
+/// than required, this will fix it up.
+fn get_spanned_border_size(sizes: &[TableRowSizeData], row_index: usize, row_span: &mut u32) -> Au {
+    // A zero rowspan is functionally equivalent to rowspan=infinity
+    if *row_span == 0 || row_index + *row_span as usize > sizes.len() {
+        *row_span = (sizes.len() - row_index) as u32;
+    }
+    let mut last_row_idx = row_index + *row_span as usize - 1;
+    // This is a slow path and should be rare -- this should only get triggered
+    // when you use `rowspan=0` or an overlarge rowspan in a table with
+    // mixed rows + rowgroups
+    if sizes[last_row_idx].rowgroup_id != sizes[row_index].rowgroup_id {
+        // XXXManishearth this loop can be avoided by also storing
+        // a "last_rowgroup_at" index so we can leapfrog back quickly
+        *row_span = sizes[row_index..last_row_idx + 1].iter()
+                        .position(|s| s.rowgroup_id != sizes[row_index].rowgroup_id)
+                        .unwrap() as u32;
+        last_row_idx = row_index + *row_span as usize - 1;
+    }
+    sizes[last_row_idx].cumulative_border_spacing - sizes[row_index].cumulative_border_spacing
 }
 
 impl Flow for TableRowFlow {