[css-grid] Clamp the number of auto-repeat tracks

[css-grid] Clamp the number of auto-repeat tracks

The computation of auto-repeat tracks might issue a result larger than the
total number of tracks limit. We need to clamp the grid also in those cases.

BUG=681381
Review-Url: https://codereview.chromium.org/2657863005
Cr-Commit-Position: refs/heads/master@{#446646}
Committed: https://chromium.googlesource.com/chromium/src/+/946f89ab4dfffec7f9d13bac423ca5a0df949bb6

[svillar, 2017-01-26 16:09:10]

svillar@igalia.com changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org, jfernandez@igalia.com,
rego@igalia.com

[eae, 2017-01-26 17:07:23]

LGTM

https://codereview.chromium.org/2657863005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/2657863005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:2048: (kGridMaxTracks -
trackSizes.size()) / autoRepeatTrackListLength;
I know we early return if autoRepeatTrackListLength is zero but that is over
three hundred lines away and might change. Would you mind adding a
DCHECK(autoRepeatTrackListLength) here?

[svillar, 2017-01-26 18:10:39]

On 2017/01/26 17:07:23, eae wrote:
> LGTM
> 
>
https://codereview.chromium.org/2657863005/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):
> 
>
https://codereview.chromium.org/2657863005/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutGrid.cpp:2048: (kGridMaxTracks -
> trackSizes.size()) / autoRepeatTrackListLength;
> I know we early return if autoRepeatTrackListLength is zero but that is over
> three hundred lines away and might change. Would you mind adding a
> DCHECK(autoRepeatTrackListLength) here?

Sure thing, sir.

Thanks for the review!

[eae, 2017-01-26 18:12:34]

Thanks!

(still LGTM)

[Manuel Rego, 2017-01-27 09:05:58]

https://codereview.chromium.org/2657863005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/2657863005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:2045: // Clamp the number
of repetitions so we don't end up with too many tracks
Nit: Missing dot after comment.

[svillar, 2017-01-27 09:39:31]

The CQ bit was checked by svillar@igalia.com

[svillar, 2017-01-27 09:39:31]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2657863005/#ps60001
(title: "Patch for landing")

[commit-bot: I haz the power, 2017-01-27 09:39:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 11:10:53]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485509971755940,
"parent_rev": "6572bfc539d50b280938e5e371f7c002c4fd6c84", "commit_rev":
"946f89ab4dfffec7f9d13bac423ca5a0df949bb6"}

[commit-bot: I haz the power, 2017-01-27 11:11:28]

Description was changed from

==========
[css-grid] Clamp the number of auto-repeat tracks

The computation of auto-repeat tracks might issue a result larger than the
total number of tracks limit. We need to clamp the grid also in those cases.

BUG=681381
==========

to

==========
[css-grid] Clamp the number of auto-repeat tracks

The computation of auto-repeat tracks might issue a result larger than the
total number of tracks limit. We need to clamp the grid also in those cases.

BUG=681381

Review-Url: https://codereview.chromium.org/2657863005
Cr-Commit-Position: refs/heads/master@{#446646}
Committed:
https://chromium.googlesource.com/chromium/src/+/946f89ab4dfffec7f9d13bac423c...
==========

[commit-bot: I haz the power, 2017-01-27 11:11:29]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/946f89ab4dfffec7f9d13bac423c...