Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(920)

Issue 21205004: [CSS Grid Layout] Store m_grid across layout (Closed)

Created:
7 years, 4 months ago by Julien - ping for review
Modified:
7 years, 4 months ago
Reviewers:
ojan
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, esprehn, eseidel
Visibility:
Public.

Description

[CSS Grid Layout] Store m_grid across layout This patch implements the logic to correctly invalidate m_grid whenever needed. This will make it possible to speed up painting across layout as we can use it to select the right sub-grid to be painted. The added tests are the blind spots in our testing (we already have test checking grid items removal / addition, ...). BUG=248151 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155262

Patch Set 1 #

Total comments: 6

Patch Set 2 : Updated change after Ojan's review #

Messages

Total messages: 8 (0 generated)
Julien - ping for review
The Mac failures are unrelated.
7 years, 4 months ago (2013-07-30 22:21:10 UTC) #1
ojan
lgtm https://codereview.chromium.org/21205004/diff/1/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/21205004/diff/1/Source/core/rendering/RenderGrid.cpp#newcode167 Source/core/rendering/RenderGrid.cpp:167: if (gridIsDirty() || !newChild->isBox()) Don't we force the ...
7 years, 4 months ago (2013-07-30 23:18:57 UTC) #2
Julien - ping for review
https://codereview.chromium.org/21205004/diff/1/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/21205004/diff/1/Source/core/rendering/RenderGrid.cpp#newcode167 Source/core/rendering/RenderGrid.cpp:167: if (gridIsDirty() || !newChild->isBox()) On 2013/07/30 23:18:57, ojan wrote: ...
7 years, 4 months ago (2013-07-30 23:57:42 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/21205004/6001
7 years, 4 months ago (2013-07-31 00:04:21 UTC) #4
ojan
https://codereview.chromium.org/21205004/diff/1/Source/core/rendering/RenderGrid.cpp File Source/core/rendering/RenderGrid.cpp (right): https://codereview.chromium.org/21205004/diff/1/Source/core/rendering/RenderGrid.cpp#newcode822 Source/core/rendering/RenderGrid.cpp:822: // could hold onto the memory to avoid massive ...
7 years, 4 months ago (2013-07-31 00:13:18 UTC) #5
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_lint, webkit_python_tests, webkit_tests, webkit_unit_tests, weborigin_unittests, wtf_unittests ...
7 years, 4 months ago (2013-07-31 03:20:10 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/21205004/6001
7 years, 4 months ago (2013-07-31 15:51:47 UTC) #7
commit-bot: I haz the power
7 years, 4 months ago (2013-07-31 19:45:47 UTC) #8
Message was sent while issue was closed.
Change committed as 155262

Powered by Google App Engine
This is Rietveld 408576698