Respect break-inside:avoid on table rows

third_party/WebKit/Source/core/layout/LayoutTableSection.cpp

Index: third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
index 89b925af6f41a1551f1d609cd14bd23c85fada16..b53446563ff05d0da1e701562a8f0a0d34b64719 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
@@ -965,6 +965,7 @@ void LayoutTableSection::layoutRows()
     for (unsigned r = 0; r < totalRows; r++) {
         // Set the row's x/y position and width/height.
         LayoutTableRow* rowLayoutObject = m_grid[r].rowLayoutObject;
+        int paginationStrut = 0;
         if (rowLayoutObject) {
             rowLayoutObject->setLocation(LayoutPoint(0, m_rowPos[r]));
             rowLayoutObject->setLogicalWidth(logicalWidth());
@@ -972,6 +973,11 @@ void LayoutTableSection::layoutRows()
             rowLayoutObject->updateLayerTransformAfterLayout();
             rowLayoutObject->clearAllOverflows();
             rowLayoutObject->addVisualEffectOverflow();
+            if (view()->layoutState()->isPaginated()) {
+                paginationStrut = adjustForUnsplittableRow(rowLayoutObject, LayoutUnit(m_rowPos[r]));
+                for (unsigned rowIndex = r; paginationStrut && rowIndex <= totalRows; rowIndex++)

[mstensho (USE GERRIT), 2016/03/17 14:04:03]

Kind of sad that we loop through the remainder of the table section here, since
we're eventually going to do that anyway with the outer loop. Could we somehow
accumulate the total strut instead and apply them to each row as we go through
them?

+                    m_rowPos[rowIndex] += paginationStrut;
+            }
         }
 
         int rowHeightIncreaseForPagination = 0;
@@ -1051,7 +1057,7 @@ void LayoutTableSection::layoutRows()
             cell->layoutIfNeeded();
 
             // FIXME: Make pagination work with vertical tables.
-            if (view()->layoutState()->pageLogicalHeight() && cell->logicalHeight() != rHeight) {
+            if (!paginationStrut && view()->layoutState()->isPaginated() && cell->logicalHeight() != rHeight) {

[mstensho (USE GERRIT), 2016/03/17 14:04:03]

Why isPaginated()? I think pageLogicalHeight() was the right check. You want to
skip all this if page height is 0 (which typically means that we're in the
initial balancing pass of multicol, and we don't want to insert struts or breaks
in that layout pass, since we have absolutely no idea how tall the columns are
going to be).

                 // FIXME: Pagination might have made us change size. For now just shrink or grow the cell to fit without doing a relayout.
                 // We'll also do a basic increase of the row height to accommodate the cell if it's bigger, but this isn't quite right
                 // either. It's at least stable though and won't result in an infinite # of relayouts that may never stabilize.
@@ -1095,6 +1101,29 @@ void LayoutTableSection::layoutRows()
     computeOverflowFromCells(totalRows, nEffCols);
 }
 
+int LayoutTableSection::adjustForUnsplittableRow(LayoutTableRow* row, LayoutUnit logicalOffset) const

[mstensho (USE GERRIT), 2016/03/17 14:04:03]

This looks like a LayoutBlockFlow::adjustForUnsplittableChild() for table
sections. But it returns the strut, not the (potentially new) offset. I find
this confusing. I suggest that you either change it to return the offset, or
rename the method (and keep returning the strut). Judging from the usage,
renaming it seems like the best choice.

+{
+    if (row->getPaginationBreakability() == AllowAnyBreaks)
+        return 0;
+    LayoutUnit rowLogicalHeight = row->logicalHeight();
+    LayoutUnit pageLogicalHeight = pageLogicalHeightForOffset(logicalOffset);
+    // If the row is too tall for the page don't insert a strut.
+    if (rowLogicalHeight > pageLogicalHeight)
+        return 0;
+    if (!pageLogicalHeight)

[mstensho (USE GERRIT), 2016/03/17 14:04:03]

Might want to move this check before the rowLogicalHeight > pageLogicalHeight
one. Not that it matters with the current code, but comparing something with a
zero page height is meaningless and confusing.

+        return 0;
+    LayoutUnit remainingLogicalHeight = pageRemainingLogicalHeightForOffset(logicalOffset, LayoutBlock::AssociateWithLatterPage);
+    if (remainingLogicalHeight >= rowLogicalHeight)
+        return 0; // It fits fine where it is. No need to break.
+    LayoutUnit paginationStrut = calculatePaginationStrutToFitContent(logicalOffset, remainingLogicalHeight, rowLogicalHeight);
+    if (paginationStrut == remainingLogicalHeight && remainingLogicalHeight == pageLogicalHeight) {
+        // Don't break if we were at the top of a page, and we failed to fit the content
+        // completely. No point in leaving a page completely blank.
+        return 0;
+    }
+    return paginationStrut;
+}
+
 void LayoutTableSection::computeOverflowFromCells()
 {
     unsigned totalRows = m_grid.size();