Account for border spacing correctly when repeating header groups

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

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010, 2013 Apple Inc.
  *               All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
@@ skipping 1242 matching lines @@
   LayoutUnit pageLogicalHeight = pageLogicalHeightForOffset(logicalOffset);
   if (!pageLogicalHeight)
     return 0;
   // If the row is too tall for the page don't insert a strut.
   LayoutUnit rowLogicalHeight = row->logicalHeight();
   if (rowLogicalHeight > pageLogicalHeight)
     return 0;
 
   LayoutUnit remainingLogicalHeight = pageRemainingLogicalHeightForOffset(
       logicalOffset, LayoutBlock::AssociateWithLatterPage);
-  LayoutUnit offsetForBorderSpacing =
-      pageLogicalHeight - (remainingLogicalHeight + table()->vBorderSpacing());
-  // Border spacing from the previous row has pushed this row just past the top
-  // of the page, so we must reposition it to the top of the page and avoid any
-  // repeating header.
-  if (offsetForBorderSpacing < 0)
-    return offsetForBorderSpacing.toInt();
-
   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;
   }
@@ skipping 702 matching lines @@
   return logicalHeight;
 }
 
 void LayoutTableSection::adjustRowForPagination(LayoutTableRow& rowObject,
                                                 SubtreeLayoutScope& layouter) {
   LayoutState& state = *view()->layoutState();
   rowObject.setPaginationStrut(LayoutUnit());
   rowObject.setLogicalHeight(LayoutUnit(logicalHeightForRow(rowObject)));
   int paginationStrut =
       paginationStrutForRow(&rowObject, rowObject.logicalTop());
+  bool rowIsAtTopOfColumn = false;
+  LayoutUnit offsetFromTopOfPage;
   if (!paginationStrut) {
-    bool rowIsAtTopOfColumn =
-        state.heightOffsetForTableHeaders() &&
-        pageLogicalHeightForOffset(rowObject.logicalTop()) ==
-            pageRemainingLogicalHeightForOffset(rowObject.logicalTop(),
-                                                AssociateWithLatterPage);
+    if (state.heightOffsetForTableHeaders()) {

[mstensho (USE GERRIT), 2016/10/24 08:39:35]

Yeah, we used to check for this before your changes too, but is it right?
Shouldn't we always do this, regardless of having repeated headers or not?

We cannot do this right now though, because that will increase the test coverage
of this code, and we'll crash in some existing tests because we sometimes call
pageRemainingLogicalHeightForOffset() when page height is 0.

Should fix this in a separate CL.

+      offsetFromTopOfPage =
+          pageLogicalHeightForOffset(rowObject.logicalTop()) -
+          pageRemainingLogicalHeightForOffset(rowObject.logicalTop(),

[mstensho (USE GERRIT), 2016/10/24 08:39:35]

It's not allowed to call pageRemainingLogicalHeightForOffset() if page height is
0 (e.g. in the initial layout pass for multicol with auto column height).

Should fix this in a separate CL.

+                                              AssociateWithLatterPage);
+      rowIsAtTopOfColumn = !offsetFromTopOfPage ||
+                           offsetFromTopOfPage <= table()->vBorderSpacing();
+    }
+
     if (!rowIsAtTopOfColumn)
       return;
   }
   // We need to push this row to the next fragmentainer. If there are repeated
   // table headers, we need to make room for those at the top of the next
   // fragmentainer, above this row. Otherwise, this row will just go at the top
   // of the next fragmentainer.
 
   // If there isn't room for at least one content row on a page with a
   // header group, then we won't repeat the header on each page.
   LayoutTableSection* header = table()->header();
   if (!rowObject.rowIndex() && header &&
       table()->sectionAbove(this) == header &&
       header->getPaginationBreakability() != AllowAnyBreaks) {
     state.setHeightOffsetForTableHeaders(state.heightOffsetForTableHeaders() -
                                          header->logicalHeight());
   }
   // If we have a header group we will paint it at the top of each page,
   // move the rows down to accomodate it.
+  if (rowIsAtTopOfColumn && offsetFromTopOfPage)
+    paginationStrut -= offsetFromTopOfPage.toInt();
+

[mstensho (USE GERRIT), 2016/10/24 08:39:35]

Better delete this blank line, so that it's a bit more obvious that the comment
above also applies to the code below.

   paginationStrut += state.heightOffsetForTableHeaders().toInt();
   rowObject.setPaginationStrut(LayoutUnit(paginationStrut));
 
   // We have inserted a pagination strut before the row. Adjust the logical top
   // and re-lay out. We no longer want to break inside the row, but rather
   // *before* it. From the previous layout pass, there are most likely
   // pagination struts inside some cell in this row that we need to get rid of.
   rowObject.setLogicalTop(rowObject.logicalTop() + paginationStrut);
   layouter.setChildNeedsLayout(&rowObject);
   rowObject.layoutIfNeeded();
@@ skipping 39 matching lines @@
   // the header in all columns.
   // Note that this is in flow thread coordinates, not visual coordinates. The
   // enclosing LayoutFlowThread will convert to visual coordinates.
   if (table()->header() == this && isRepeatingHeaderGroup())
     rect.setHeight(table()->logicalHeight());
   return LayoutTableBoxComponent::mapToVisualRectInAncestorSpace(ancestor, rect,
                                                                  flags);
 }
 
 }  // namespace blink