Inconsistent section grid could lead to CrashOnOverflow
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Dec 2017 04:22:47 +0000 (04:22 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Dec 2017 04:22:47 +0000 (04:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180850
<rdar://problem/34064811>

Reviewed by Simon Fraser.

Source/WebCore:

Each RenderTableSection maintains a grid of rows and columns. The number of columns in this grid equals the
maximum number of columns in the entire table (taking spans and multiple sections into account).
Since the maximum number of columns might change while re-computing the sections, we need to
adjust them accordingly at the end (otherwise it could lead to inconsistent grids where rows have different number of columns).

Test: fast/table/table-row-oveflow-crash.html

* rendering/RenderTable.cpp:
(WebCore::RenderTable::recalcSections const):
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::removeRedundantColumns):
* rendering/RenderTableSection.h:

LayoutTests:

* fast/table/table-row-oveflow-crash-expected.txt: Added.
* fast/table/table-row-oveflow-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225960 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/table/table-row-oveflow-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/table/table-row-oveflow-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTableSection.cpp
Source/WebCore/rendering/RenderTableSection.h

index 2ad8938..63c88cc 100644 (file)
@@ -1,3 +1,14 @@
+2017-12-14  Zalan Bujtas  <zalan@apple.com>
+
+        Inconsistent section grid could lead to CrashOnOverflow
+        https://bugs.webkit.org/show_bug.cgi?id=180850
+        <rdar://problem/34064811>
+
+        Reviewed by Simon Fraser.
+
+        * fast/table/table-row-oveflow-crash-expected.txt: Added.
+        * fast/table/table-row-oveflow-crash.html: Added.
+
 2017-12-14  Chris Dumez  <cdumez@apple.com>
 
         [iOS] Many serviceworker tests are flaky timeouts on iOS bots
diff --git a/LayoutTests/fast/table/table-row-oveflow-crash-expected.txt b/LayoutTests/fast/table/table-row-oveflow-crash-expected.txt
new file mode 100644 (file)
index 0000000..095f9e2
--- /dev/null
@@ -0,0 +1,4 @@
+PASS if no crash.
+5
+2
+43
diff --git a/LayoutTests/fast/table/table-row-oveflow-crash.html b/LayoutTests/fast/table/table-row-oveflow-crash.html
new file mode 100644 (file)
index 0000000..291ca08
--- /dev/null
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+PASS if no crash.
+<table>
+  <tbody>
+    <tr id="tr_first_table"></tr>
+  </tbody>
+  <tbody>
+   <tr>
+     <th>2</th>
+     <th id="th_first_table">3</th>
+  </tr>
+  </tbody>
+</table>
+<br>
+<table>
+  <th id="th_second_table">4</th>
+  <th rowspan="6" id="th_withh_rowspan">5</th>
+</table>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+  
+document.body.offsetHeight;
+th_second_table.appendChild(th_first_table);
+document.body.offsetHeight;
+tr_first_table.appendChild(th_withh_rowspan);
+document.body.offsetHeight;
+</script>
+</body>
+</html>
index 60c93f3..147c1bb 100644 (file)
@@ -1,3 +1,24 @@
+2017-12-14  Zalan Bujtas  <zalan@apple.com>
+
+        Inconsistent section grid could lead to CrashOnOverflow
+        https://bugs.webkit.org/show_bug.cgi?id=180850
+        <rdar://problem/34064811>
+
+        Reviewed by Simon Fraser.
+
+        Each RenderTableSection maintains a grid of rows and columns. The number of columns in this grid equals the
+        maximum number of columns in the entire table (taking spans and multiple sections into account).
+        Since the maximum number of columns might change while re-computing the sections, we need to
+        adjust them accordingly at the end (otherwise it could lead to inconsistent grids where rows have different number of columns).
+
+        Test: fast/table/table-row-oveflow-crash.html
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::recalcSections const):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::removeRedundantColumns):
+        * rendering/RenderTableSection.h:
+
 2017-12-14  David Kilzer  <ddkilzer@apple.com>
 
         Enable -Wstrict-prototypes for WebKit
index b23e162..5bda411 100644 (file)
@@ -1111,6 +1111,10 @@ void RenderTable::recalcSections() const
     m_columns.resize(maxCols);
     m_columnPos.resize(maxCols + 1);
 
+    // Now that we know the number of maximum number of columns, let's shrink the sections grids if needed.
+    for (auto& section : childrenOfType<RenderTableSection>(const_cast<RenderTable&>(*this)))
+        section.removeRedundantColumns();
+
     ASSERT(selfNeedsLayout());
 
     m_needsSectionRecalc = false;
index d71d473..810b032 100644 (file)
@@ -1385,6 +1385,16 @@ void RenderTableSection::recalcCells()
     setNeedsLayout();
 }
 
+void RenderTableSection::removeRedundantColumns()
+{
+    auto maximumNumberOfColumns = table()->numEffCols();
+    for (auto& rowItem : m_grid) {
+        if (rowItem.row.size() <= maximumNumberOfColumns)
+            continue;
+        rowItem.row.resize(maximumNumberOfColumns);
+    }
+}
+
 // FIXME: This function could be made O(1) in certain cases (like for the non-most-constrainive cells' case).
 void RenderTableSection::rowLogicalHeightChanged(unsigned rowIndex)
 {
index 25ceb08..1ce473e 100644 (file)
@@ -127,6 +127,7 @@ public:
     unsigned numColumns() const;
     void recalcCells();
     void recalcCellsIfNeeded();
+    void removeRedundantColumns();
 
     bool needsCellRecalc() const { return m_needsCellRecalc; }
     void setNeedsCellRecalc();