Title: [143331] trunk
- Revision
- 143331
- Author
- jchaffr...@webkit.org
- Date
- 2013-02-19 07:36:16 -0800 (Tue, 19 Feb 2013)
Log Message
RenderGrid::computedUsedBreadthOfGridTracks can read past m_grid's size
https://bugs.webkit.org/show_bug.cgi?id=110126
Reviewed by Ojan Vafai.
Source/WebCore:
The issue comes from how we store the column information inside m_grid.
Because m_grid is a Vector of rows, we could lose the column information
if we had no row, no grid item but some columns defined in CSS. As the
logic would assume that our row / column size would be greater than what
the style defines explicitely, we would access past our Vector's boundary.
The fix is to ensure that we have at least a row so that we can store the
column information in every case. This fix is overly broad as it also forces
the grid to have one column, which shouldn't be an issue.
Test: fast/css-grid-layout/grid-element-empty-row-column.html
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::maximumIndexInDirection):
Forced this function to return at least one as the maximum index so that
m_grid has at least one row / column.
(WebCore::RenderGrid::placeItemsOnGrid):
Added a ASSERT that m_grid is bigger than the explicit grid-rows / grid-columns.
Also changed an existing ASSERT to use gridWasPopulated for consistency and changed
the code not to call gridRowCount as it would ASSERT (we are in the middle of populating
the grid).
* rendering/RenderGrid.h:
(WebCore::RenderGrid::gridWasPopulated):
Added this helper function.
(WebCore::RenderGrid::gridColumnCount):
Replaced a now unneeded branch with an ASSERT. As placeItemsOnGrid should be called
prior to read m_grid, this change should be fine.
(WebCore::RenderGrid::gridRowCount):
Added an ASSERT.
LayoutTests:
* fast/css-grid-layout/grid-element-empty-row-column-expected.txt: Added.
* fast/css-grid-layout/grid-element-empty-row-column.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (143330 => 143331)
--- trunk/LayoutTests/ChangeLog 2013-02-19 15:19:02 UTC (rev 143330)
+++ trunk/LayoutTests/ChangeLog 2013-02-19 15:36:16 UTC (rev 143331)
@@ -1,3 +1,13 @@
+2013-02-19 Julien Chaffraix <jchaffr...@webkit.org>
+
+ RenderGrid::computedUsedBreadthOfGridTracks can read past m_grid's size
+ https://bugs.webkit.org/show_bug.cgi?id=110126
+
+ Reviewed by Ojan Vafai.
+
+ * fast/css-grid-layout/grid-element-empty-row-column-expected.txt: Added.
+ * fast/css-grid-layout/grid-element-empty-row-column.html: Added.
+
2013-02-19 Zan Dobersek <zdober...@igalia.com>
Unreviewed GTK gardening.
Added: trunk/LayoutTests/fast/css-grid-layout/grid-element-empty-row-column-expected.txt (0 => 143331)
--- trunk/LayoutTests/fast/css-grid-layout/grid-element-empty-row-column-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-element-empty-row-column-expected.txt 2013-02-19 15:36:16 UTC (rev 143331)
@@ -0,0 +1,4 @@
+This test checks that a grid element with row(s) (resp. column(s)) but no column (resp. row) is properly laid out.
+
+PASS
+PASS
Added: trunk/LayoutTests/fast/css-grid-layout/grid-element-empty-row-column.html (0 => 143331)
--- trunk/LayoutTests/fast/css-grid-layout/grid-element-empty-row-column.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-element-empty-row-column.html 2013-02-19 15:36:16 UTC (rev 143331)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<script>
+ testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
+</script>
+<link href="" rel=stylesheet>
+<style>
+.gridNoRow {
+ -webkit-grid-columns: 50px;
+ /* Make the grid shrink-to-fit. */
+ position: absolute;
+}
+.gridNoColumn {
+ -webkit-grid-rows: 50px 80px;
+ /* Make the grid shrink-to-fit. */
+ position: absolute;
+}
+</style>
+<script src=""
+<body _onload_="checkLayout('.grid');">
+<p>This test checks that a grid element with row(s) (resp. column(s)) but no column (resp. row) is properly laid out.</p>
+<div class="grid gridNoRow" data-expected-width="50" data-expected-height="0"></div>
+<div class="grid gridNoColumn" data-expected-width="0" data-expected-height="130"></div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (143330 => 143331)
--- trunk/Source/WebCore/ChangeLog 2013-02-19 15:19:02 UTC (rev 143330)
+++ trunk/Source/WebCore/ChangeLog 2013-02-19 15:36:16 UTC (rev 143331)
@@ -1,3 +1,44 @@
+2013-02-19 Julien Chaffraix <jchaffr...@webkit.org>
+
+ RenderGrid::computedUsedBreadthOfGridTracks can read past m_grid's size
+ https://bugs.webkit.org/show_bug.cgi?id=110126
+
+ Reviewed by Ojan Vafai.
+
+ The issue comes from how we store the column information inside m_grid.
+ Because m_grid is a Vector of rows, we could lose the column information
+ if we had no row, no grid item but some columns defined in CSS. As the
+ logic would assume that our row / column size would be greater than what
+ the style defines explicitely, we would access past our Vector's boundary.
+
+ The fix is to ensure that we have at least a row so that we can store the
+ column information in every case. This fix is overly broad as it also forces
+ the grid to have one column, which shouldn't be an issue.
+
+ Test: fast/css-grid-layout/grid-element-empty-row-column.html
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::maximumIndexInDirection):
+ Forced this function to return at least one as the maximum index so that
+ m_grid has at least one row / column.
+
+ (WebCore::RenderGrid::placeItemsOnGrid):
+ Added a ASSERT that m_grid is bigger than the explicit grid-rows / grid-columns.
+ Also changed an existing ASSERT to use gridWasPopulated for consistency and changed
+ the code not to call gridRowCount as it would ASSERT (we are in the middle of populating
+ the grid).
+
+ * rendering/RenderGrid.h:
+ (WebCore::RenderGrid::gridWasPopulated):
+ Added this helper function.
+
+ (WebCore::RenderGrid::gridColumnCount):
+ Replaced a now unneeded branch with an ASSERT. As placeItemsOnGrid should be called
+ prior to read m_grid, this change should be fine.
+
+ (WebCore::RenderGrid::gridRowCount):
+ Added an ASSERT.
+
2013-02-19 Sergio Villar Senin <svil...@igalia.com>
[Soup] Use synchronous calls to close completely processed streams
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (143330 => 143331)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2013-02-19 15:19:02 UTC (rev 143330)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2013-02-19 15:36:16 UTC (rev 143331)
@@ -313,7 +313,7 @@
{
const Vector<GridTrackSize>& trackStyles = (direction == ForColumns) ? style()->gridColumns() : style()->gridRows();
- size_t maximumIndex = trackStyles.size();
+ size_t maximumIndex = std::max<size_t>(1, trackStyles.size());
for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
const GridPosition& position = (direction == ForColumns) ? child->style()->gridItemColumn() : child->style()->gridItemRow();
@@ -478,12 +478,12 @@
void RenderGrid::placeItemsOnGrid()
{
- ASSERT(m_grid.isEmpty());
+ ASSERT(!gridWasPopulated());
ASSERT(m_gridItemCoordinate.isEmpty());
m_grid.grow(maximumIndexInDirection(ForRows));
size_t maximumColumnIndex = maximumIndexInDirection(ForColumns);
- for (size_t i = 0; i < gridRowCount(); ++i)
+ for (size_t i = 0; i < m_grid.size(); ++i)
m_grid[i].grow(maximumColumnIndex);
for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
@@ -491,6 +491,9 @@
size_t rowTrack = resolveGridPositionFromStyle(child->style()->gridItemRow());
insertItemIntoGrid(child, rowTrack, columnTrack);
}
+
+ ASSERT(gridRowCount() >= style()->gridRows().size());
+ ASSERT(gridColumnCount() >= style()->gridColumns().size());
}
void RenderGrid::clearGrid()
Modified: trunk/Source/WebCore/rendering/RenderGrid.h (143330 => 143331)
--- trunk/Source/WebCore/rendering/RenderGrid.h 2013-02-19 15:19:02 UTC (rev 143330)
+++ trunk/Source/WebCore/rendering/RenderGrid.h 2013-02-19 15:36:16 UTC (rev 143331)
@@ -98,10 +98,19 @@
#ifndef NDEBUG
bool tracksAreWiderThanMinTrackBreadth(TrackSizingDirection, const Vector<GridTrack>&);
+ bool gridWasPopulated() const { return !m_grid.isEmpty() && !m_grid[0].isEmpty(); }
#endif
- size_t gridColumnCount() const { return m_grid.isEmpty() ? 0 : m_grid[0].size(); }
- size_t gridRowCount() const { return m_grid.size(); }
+ size_t gridColumnCount() const
+ {
+ ASSERT(gridWasPopulated());
+ return m_grid[0].size();
+ }
+ size_t gridRowCount() const
+ {
+ ASSERT(gridWasPopulated());
+ return m_grid.size();
+ }
Vector<Vector<Vector<RenderBox*, 1> > > m_grid;
HashMap<const RenderBox*, GridCoordinate> m_gridItemCoordinate;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
