Commit 7a0374be authored by Richard Townsend's avatar Richard Townsend Committed by Commit Bot

Revert "Revert "[css-tables] Force layout when colgroups are removed or added""

Fixes an uninitialised variable caught by MSAN.

This reverts commit 66a74d2f.

Change-Id: I1e896476fe201fb05072970220bab5c3b20588a2
Reviewed-on: https://chromium-review.googlesource.com/1233746
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarDavid Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592938}
parent 093ee96f
<!doctype html>
<script src='/resources/testharness.js'></script>
<script src='/resources/testharnessreport.js'></script>
<link rel="author" title="Richard Townsend" href="Richard.Townsend@arm.com" />
<link rel="help" href="https://drafts.csswg.org/css-tables-3/#computing-column-measures" />
<style type="text/css">
td {
padding: 0;
}
div {
/* Bug does not happen when the table's containing block is less
than (100+200+300)=600px, so make sure sure that it's larger. */
width: 750px;
}
</style>
<div>
<h1>Width Distribution</h1>
<h4>"Computing column measures"</h4>
<p>This is testing that the table's auto columns are correctly recalculated after removing and adding a <code>col</code> element.
<a href="https://www.w3.org/TR/CSS2/tables.html#auto-table-layout">Spec Text</a></p>
<hr/>
<table id="one" style="border: 1px solid orange">
<colgroup>
<col style="width: 100px" />
<col style="width: 200px" />
<col style="width: 300px;" id="hideable" />
</colgroup>
<tr>
<td>100</td>
<td>200</td>
<td>300</td>
</tr>
</table>
<table id="two" style="border: 1px solid orange">
<colgroup>
<col style="width: 100px; display: none;" id="displayable" />
<col style="width: 200px;" />
<col style="width: auto;" />
</colgroup>
<tr>
<td>100</td>
<td>200</td>
<td>300</td>
</tr>
</table>
<table id="ref" style="border: 1px solid orange">
<colgroup>
<col style="width: 100px;" />
<col style="width: 200px;" />
<col style="width: auto;" />
</colgroup>
<tr>
<td>100</td>
<td>200</td>
<td>300</td>
</tr>
</table>
</div>
<script>
test(function() {
var one = document.getElementById('one');
var two = document.getElementById('two');
var ref = document.getElementById('ref');
// Force initial layout.
assert_greater_than(one.offsetWidth, ref.offsetWidth);
// Display two's colgroup and hide one's.
document.getElementById('displayable').style.display = 'table-column';
document.getElementById('hideable').style.display = 'none';
// Both tables should now match the reference.
assert_equals(one.offsetWidth, ref.offsetWidth);
assert_equals(two.offsetWidth, ref.offsetWidth);
}, "Table recalculations should match the reference");
</script>
...@@ -66,6 +66,7 @@ LayoutTable::LayoutTable(Element* element) ...@@ -66,6 +66,7 @@ LayoutTable::LayoutTable(Element* element)
has_col_elements_(false), has_col_elements_(false),
needs_section_recalc_(false), needs_section_recalc_(false),
column_logical_width_changed_(false), column_logical_width_changed_(false),
column_structure_changed_(false),
column_layout_objects_valid_(false), column_layout_objects_valid_(false),
no_cell_colspan_at_least_(0), no_cell_colspan_at_least_(0),
h_spacing_(0), h_spacing_(0),
...@@ -254,16 +255,21 @@ void LayoutTable::InvalidateCachedColumns() { ...@@ -254,16 +255,21 @@ void LayoutTable::InvalidateCachedColumns() {
column_layout_objects_.resize(0); column_layout_objects_.resize(0);
} }
void LayoutTable::AddColumn(const LayoutTableCol*) { void LayoutTable::ColumnStructureChanged() {
column_structure_changed_ = true;
InvalidateCachedColumns(); InvalidateCachedColumns();
// We don't really need to recompute our sections, but we do need to update
// our column count, whether we have a column, and possibly the logical width
// distribution too.
SetNeedsSectionRecalc();
}
void LayoutTable::AddColumn(const LayoutTableCol*) {
ColumnStructureChanged();
} }
void LayoutTable::RemoveColumn(const LayoutTableCol*) { void LayoutTable::RemoveColumn(const LayoutTableCol*) {
InvalidateCachedColumns(); ColumnStructureChanged();
// We don't really need to recompute our sections, but we need to update our
// column count and whether we have a column. Currently, we only have one
// size-fit-all flag but we may have to consider splitting it.
SetNeedsSectionRecalc();
} }
bool LayoutTable::IsLogicalWidthAuto() const { bool LayoutTable::IsLogicalWidthAuto() const {
...@@ -1250,11 +1256,16 @@ void LayoutTable::RecalcSections() const { ...@@ -1250,11 +1256,16 @@ void LayoutTable::RecalcSections() const {
child = child->NextSibling()) { child = child->NextSibling()) {
if (child->IsTableSection()) { if (child->IsTableSection()) {
LayoutTableSection* section = ToLayoutTableSection(child); LayoutTableSection* section = ToLayoutTableSection(child);
if (column_structure_changed_) {
section->MarkAllCellsWidthsDirtyAndOrNeedsLayout(
LayoutTable::kMarkDirtyAndNeedsLayout);
}
unsigned section_cols = section->NumEffectiveColumns(); unsigned section_cols = section->NumEffectiveColumns();
if (section_cols > max_cols) if (section_cols > max_cols)
max_cols = section_cols; max_cols = section_cols;
} }
} }
column_structure_changed_ = false;
effective_columns_.resize(max_cols); effective_columns_.resize(max_cols);
effective_column_positions_.resize(max_cols + 1); effective_column_positions_.resize(max_cols + 1);
......
...@@ -430,6 +430,7 @@ class CORE_EXPORT LayoutTable final : public LayoutBlock { ...@@ -430,6 +430,7 @@ class CORE_EXPORT LayoutTable final : public LayoutBlock {
void EnsureIsReadyForPaintInvalidation() override; void EnsureIsReadyForPaintInvalidation() override;
void InvalidatePaint(const PaintInvalidatorContext&) const override; void InvalidatePaint(const PaintInvalidatorContext&) const override;
bool PaintedOutputOfObjectHasNoEffectRegardlessOfSize() const override; bool PaintedOutputOfObjectHasNoEffectRegardlessOfSize() const override;
void ColumnStructureChanged();
private: private:
bool IsOfType(LayoutObjectType type) const override { bool IsOfType(LayoutObjectType type) const override {
...@@ -561,6 +562,12 @@ class CORE_EXPORT LayoutTable final : public LayoutBlock { ...@@ -561,6 +562,12 @@ class CORE_EXPORT LayoutTable final : public LayoutBlock {
mutable bool needs_section_recalc_ : 1; mutable bool needs_section_recalc_ : 1;
bool column_logical_width_changed_ : 1; bool column_logical_width_changed_ : 1;
// This flag indicates whether any columns (with or without fixed widths) have
// been added or removed since the last layout. If they have, then the true
// size of the cell contents needs to be determined with a full layout before
// the layout cache is updated. The layout cache can be invalid when layout is
// valid (e.g. if the table is being painted for the first time).
mutable bool column_structure_changed_ : 1;
mutable bool column_layout_objects_valid_ : 1; mutable bool column_layout_objects_valid_ : 1;
mutable unsigned no_cell_colspan_at_least_; mutable unsigned no_cell_colspan_at_least_;
unsigned CalcNoCellColspanAtLeast() const { unsigned CalcNoCellColspanAtLeast() const {
......
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