Skip to content

GitLab

T
tangled
Commit d09047bc authored by Joy Yu's avatar Joy Yu Committed by Commit Bot
Browse files
Options

Fix Heap-use-after-free Bug 

Add a new member variable LayoutTableCell::is_spanning_collapsed_row_ so that
there is no need to access LayoutTableSection::RowHasVisibilityCollapse
in LayoutTableCell. This avoids using the memory that has already been
freed.

Bug: 750016
Change-Id: I1838a775f3b45315b2dee3e15942af0dff0c5955
Reviewed-on: https://chromium-review.googlesource.com/594935Reviewed-by: default avatarMorten Stenshorne <mstensho@opera.com>
Commit-Queue: Joy Yu <joysyu@google.com>
Cr-Commit-Position: refs/heads/master@{#491139}
parent bd513d56
Hide whitespace changes
Inline Side-by-side
Showing with 52 additions and 12 deletions
third_party/WebKit/LayoutTests/fast/table/table-cell-should-clip-overflow-crash.html 0 → 100644
View file @ d09047bc
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<table>
<tbody>
<tr>
<td id="rowspanned" rowspan="2"></td>
</tr>
<tr id="removeMe"></tr>
</tbody>
</table>
<script>
test(() => {
var rowspanned = document.getElementById("rowspanned");
var removeMe = document.getElementById("removeMe");
// First trigger a layout pass, to build and lay out what we already have.
document.body.offsetTop;
// Then remove the second table row. This will trigger SetNeedsCellRecalc()
// on the table section. Many operations on the table will now be dangerous
// (because there might be dangling pointers), until we have recalculated
// the data structures (by calling RecalcCells()). Unless someone explicitly
// demands this to be done, this recalculation won't happen until the next
// layout pass.
removeMe.parentNode.removeChild(removeMe);
// Before we actually get around to updating the data structures in the
// table, change some arbitrary style on the table cell to trigger
// StyleDidChange() on the cell, which will call ShouldClipOverflow().
rowspanned.style.color = "blue";
}, "No crash or assertion failure. crbug.com/750016");
</script>
\ No newline at end of file
third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
View file @ d09047bc
...@@ -456,17 +456,7 @@ void LayoutTableCell::ComputeOverflow(LayoutUnit old_client_after_edge, ...@@ -456,17 +456,7 @@ void LayoutTableCell::ComputeOverflow(LayoutUnit old_client_after_edge,
} }
bool LayoutTableCell::ShouldClipOverflow() const { bool LayoutTableCell::ShouldClipOverflow() const {
if (LayoutBox::ShouldClipOverflow()) return IsSpanningCollapsedRow() || LayoutBox::ShouldClipOverflow();
return true;
unsigned row_span = RowSpan();
if (row_span == 1)
return false;
unsigned row_index = RowIndex();
for (unsigned r = row_index; r < row_index + row_span; r++) {
if (Section()->RowHasVisibilityCollapse(r))
return true;
}
return false;
} }
LayoutUnit LayoutTableCell::CellBaselinePosition() const { LayoutUnit LayoutTableCell::CellBaselinePosition() const {
......
third_party/WebKit/Source/core/layout/LayoutTableCell.h
View file @ d09047bc
...@@ -37,7 +37,7 @@ ...@@ -37,7 +37,7 @@
namespace blink { namespace blink {
#define BITS_OF_ABSOLUTE_COLUMN_INDEX 28 #define BITS_OF_ABSOLUTE_COLUMN_INDEX 27
static const unsigned kUnsetColumnIndex = static const unsigned kUnsetColumnIndex =
(1u << BITS_OF_ABSOLUTE_COLUMN_INDEX) - 1; (1u << BITS_OF_ABSOLUTE_COLUMN_INDEX) - 1;
static const unsigned kMaxColumnIndex = kUnsetColumnIndex - 1; static const unsigned kMaxColumnIndex = kUnsetColumnIndex - 1;
...@@ -329,6 +329,12 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow { ...@@ -329,6 +329,12 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
RowIndex() + RowSpan() == other->RowIndex() + other->RowSpan(); RowIndex() + RowSpan() == other->RowIndex() + other->RowSpan();
} }
void SetIsSpanningCollapsedRow(bool spanningCollapsedRow) {
is_spanning_collapsed_row_ = spanningCollapsedRow;
}
bool IsSpanningCollapsedRow() const { return is_spanning_collapsed_row_; }
protected: protected:
void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override; void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override;
void ComputePreferredLogicalWidths() override; void ComputePreferredLogicalWidths() override;
...@@ -476,6 +482,7 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow { ...@@ -476,6 +482,7 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
unsigned cell_width_changed_ : 1; unsigned cell_width_changed_ : 1;
unsigned has_col_span_ : 1; unsigned has_col_span_ : 1;
unsigned has_row_span_ : 1; unsigned has_row_span_ : 1;
unsigned is_spanning_collapsed_row_ : 1;
// This is set when collapsed_border_values_ needs recalculation. // This is set when collapsed_border_values_ needs recalculation.
mutable unsigned collapsed_border_values_valid_ : 1; mutable unsigned collapsed_border_values_valid_ : 1;
......
third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
View file @ d09047bc
...@@ -888,6 +888,16 @@ int LayoutTableSection::CalcRowLogicalHeight() { ...@@ -888,6 +888,16 @@ int LayoutTableSection::CalcRowLogicalHeight() {
std::max(index_of_first_stretchable_row, row_index_below_cell); std::max(index_of_first_stretchable_row, row_index_below_cell);
} else if (cell->RowSpan() > 1) { } else if (cell->RowSpan() > 1) {
DCHECK(!row_span_cells.Contains(cell)); DCHECK(!row_span_cells.Contains(cell));
cell->SetIsSpanningCollapsedRow(false);
unsigned end_row = cell->RowSpan() + r;
for (unsigned spanning = r; spanning < end_row; spanning++) {
if (RowHasVisibilityCollapse(spanning)) {
cell->SetIsSpanningCollapsedRow(true);
break;
}
}
row_span_cells.push_back(cell); row_span_cells.push_back(cell);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment