Reviewers: eae
CL:
https://codereview.chromium.org/2382043003/Description:
Use ceil() when integerizing pagination struts before table rows.
Subpixel rendering is not supported in table parts, so everything needs to be
integers. However, instead of rounding the pagination strut down to the nearest
integer, round it up. This way we at least make sure that we manage to push all
the content over to the designated fragmentainer, rather than leaving one tiny
strip behind in the previous fragmentainer. There'll still be off-by-one
errors, but at least all the content is in the right fragmentainer.
Updated some tests to not use subpixel multicol heights, since what they
required cannot really be satisfied without adding full subpixel support to
tables.
Also added a new test that *does* use subpixel multicol height. This test
merely makes sure that nothing is left behind in the previous fragmentainer at
breaks, without worrying about the exact top position of the objects.
This problem was discovered while working on bug 487026, which is about
reducing the amount of forced re-layouts that we do for fragmentation, and it
turns out that table layout in general, and perhaps strut calculation there in
particular, tends to need more layout passes it explicitly asks for (so it
depends on other parts of the system dealing out layout passes for free). Added
body { overflow:hidden; } declarations to some tests, to reduce the number of
layout passes you get for free, i.e. make the tests more evil.
BUG=487026
Affected files (+48, -4 lines):
M third_party/WebKit/LayoutTests/fragmentation/cells-dont-fit-on-page-paginated.html
M third_party/WebKit/LayoutTests/fragmentation/multi-line-cells-paginated.html
M third_party/WebKit/LayoutTests/fragmentation/single-line-cells-paginated.html
A third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer.html
A third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer-expected.txt
M third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
Index: third_party/WebKit/LayoutTests/fragmentation/cells-dont-fit-on-page-paginated.html
diff --git a/third_party/WebKit/LayoutTests/fragmentation/cells-dont-fit-on-page-paginated.html b/third_party/WebKit/LayoutTests/fragmentation/cells-dont-fit-on-page-paginated.html
index 84034832e6891ca07617070178cfad1afc8337a6..578d5f3a1eb98c527c53232415a0002772770e75 100644
--- a/third_party/WebKit/LayoutTests/fragmentation/cells-dont-fit-on-page-paginated.html
+++ b/third_party/WebKit/LayoutTests/fragmentation/cells-dont-fit-on-page-paginated.html
@@ -1,12 +1,13 @@
<!DOCTYPE html>
<style>
+body { overflow: hidden; } /* Avoid triggering a second layout pass, as that might hide bugs. */
table { border-spacing:0; line-height:2em; }
tr { break-inside:avoid; }
td { padding:0; background:red; }
td > div { width:1em; background:green; }
</style>
<p>There should be a green square and no red below.</p>
-<div style="columns:2; column-fill:auto; column-gap:0; width:4em; height:5.9em;">
+<div style="columns:2; column-fill:auto; column-gap:0; width:4em; height:5.5em;">
<table>
<tr>
<td><div><br><br></div></td>
Index: third_party/WebKit/LayoutTests/fragmentation/multi-line-cells-paginated.html
diff --git a/third_party/WebKit/LayoutTests/fragmentation/multi-line-cells-paginated.html b/third_party/WebKit/LayoutTests/fragmentation/multi-line-cells-paginated.html
index 29f1b5cff47939c354c8532ea46778d9d85e096c..4273404dcc4ff6c3b90485e18333b43589a24a28 100644
--- a/third_party/WebKit/LayoutTests/fragmentation/multi-line-cells-paginated.html
+++ b/third_party/WebKit/LayoutTests/fragmentation/multi-line-cells-paginated.html
@@ -1,12 +1,13 @@
<!DOCTYPE html>
<style>
+body { overflow: hidden; } /* Avoid triggering a second layout pass, as that might hide bugs. */
table { border-spacing:0; line-height:1.5em; }
tr { break-inside:avoid; }
td { padding:0; background:red; }
td > div { width:2em; background:green; }
</style>
<p>
crbug.com/99124: A multi-line cell doesn't straddle a page-break when break-inside:avoid is specified on the row. There should be a green square and no red below.</p>
-<div style="columns:2; column-fill:auto; column-gap:0; width:4em; height:7.9em;">
+<div style="columns:2; column-fill:auto; column-gap:0; width:4em; height:8.5em;">
<table>
<tr>
<td><div><br><br></div></td>
Index: third_party/WebKit/LayoutTests/fragmentation/single-line-cells-paginated.html
diff --git a/third_party/WebKit/LayoutTests/fragmentation/single-line-cells-paginated.html b/third_party/WebKit/LayoutTests/fragmentation/single-line-cells-paginated.html
index 18f2b173a7c9850da4e180b6c418585151a05066..a7c67b221c12ef6b1ae7c5e479227ef3b12ac325 100644
--- a/third_party/WebKit/LayoutTests/fragmentation/single-line-cells-paginated.html
+++ b/third_party/WebKit/LayoutTests/fragmentation/single-line-cells-paginated.html
@@ -1,12 +1,13 @@
<!DOCTYPE html>
<style>
+body { overflow: hidden; } /* Avoid triggering a second layout pass, as that might hide bugs. */
table { border-spacing:0; line-height:1em; }
tr { break-inside:avoid; }
td { padding:0; background:red; }
td > div { width:1em; background:green; }
</style>
<p>
crbug.com/99124: A single-line cell doesn't straddle a page-break when break-inside:avoid is specified on the row. There should be a green square and no red below.</p>
-<div style="columns:2; column-fill:auto; column-gap:0; width:2em; height:3.9em;">
+<div style="columns:2; column-fill:auto; column-gap:0; width:2em; height:3.8em;">
<table>
<tr>
<td><div><br></div></td>
Index: third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer-expected.txt
diff --git a/third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer-expected.txt b/third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer-expected.txt
new file mode 100644
index 0000000000000000000000000000000000000000..a1e1361164e6fbdaaf0840ad7669f8931e92f2da
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer-expected.txt
@@ -0,0 +1,3 @@
+There should be six blue squares below. They should ideally be perfectly vertically aligned, but since Blink doesn't support subpixel rendering inside tables, off-by-one is acceptable. But there should be no blue lines below the squares.
+
+PASS
Index: third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer.html
diff --git a/third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer.html b/third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer.html
new file mode 100644
index 0000000000000000000000000000000000000000..f3aacc21c0fb1696bf569fda61d18c713af65310
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fragmentation/table-in-subpixel-fragmentainer.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<style>
+body { overflow:hidden; } /* Avoid triggering a second layout pass, as that might hide bugs. */
+.table { display:table; width:100%; }
+.row { display:table-row; break-inside:avoid; }
+.row > div { width:100%; height:60px; background:blue; }
+</style>
+<p>There should be six blue squares below. They should ideally be
+ perfectly vertically aligned, but since Blink doesn't support
+ subpixel rendering inside tables, off-by-one is acceptable. But
+ there should be no blue lines below the squares.</p>
+<div id="multicol" style="position:relative; width:410px; columns:6; column-fill:auto; column-gap:10px; height:88.7px;">
+ <div class="table">
+ <div class="row">
+ <div data-offset-x="0"></div>
+ </div>
+ <div class="row">
+ <div data-offset-x="70"></div>
+ </div>
+ <div class="row">
+ <div data-offset-x="140"></div>
+ </div>
+ <div class="row">
+ <div data-offset-x="210"></div>
+ </div>
+ <div class="row">
+ <div data-offset-x="280"></div>
+ </div>
+ <div class="row">
+ <div data-offset-x="350"></div>
+ </div>
+ </div>
+</div>
+
+<script src="../resources/check-layout.js"></script>
+<script>
+ checkLayout("#multicol");
+</script>
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 bbe80054f0e4c599e4eb96399e5e697c07f28487..e4d2481ddf1c5af5d145f34d604ab9fadb179fd5 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
@@ -1158,7 +1158,7 @@ int LayoutTableSection::paginationStrutForRow(LayoutTableRow* row, LayoutUnit lo
// completely. No point in leaving a page completely blank.
return 0;
}
- return paginationStrut.toInt();
+ return paginationStrut.ceil();
}
void LayoutTableSection::computeOverflowFromCells()