Distinguish between row (fragmentainer group) height and column (fragmentainer) height.

Distinguish between row (fragmentainer group) height and column (fragmentainer) height.

TLDR; The spec [1] says that a fragmentainer height must always be 1px or
greater, to ensure progress. If we just do this, we'll avoid a lot of trouble
with limitations in the data types that we use.

While *column* heights will now be clamped to >= 1px, we still allow the height
of a *row* (fragmentainer group) to be less than 1px. We don't want the row to
take up more space than it should in its container.
E.g. <div style="columns:2; height:0.5px;"><div style="height:1px";></div></div>
will give a row height of 0.5px, as specified. The column height, on the other
hand, should be clamped up to 1px.

And here, for the nastiness that this CL aims to fix:
<div style="columns:2; height:0.25px;">
    <div style="height:10000000px;"></div>
</div>
The content to fragment is 10 million pixels tall, and the column height has
been specified as 0.25px. Internally in our code, heights are stored as
LayoutUnit, which is a fixed-point unit with 6 bits reserved for decimals. On
the other side of the decimal point we have room for 32-6 bits = 26 bits, which
is what we have for a signed integer. That's 25 bits for the absolute value.
That's just over 30 million. LayoutUnit uses saturated arithmetic so there'll
never be any integer overflow or underflow, but there may be other ill effects.
Like in this case, if we actually allow a column height of less than 1px (i.e.
0.25px), when the engine for example wants to figure out the *actual* column
count (column count was *specified* as 2, but there's no way we're going to be
able to fit a 10 millions pixels tall thing in two columns when the column
height is 0.25px, so the actual count will be way higher), we take the flow
thread portion (10000000px) and divide by the column height (0.25px). If we
divide something by something (positive) less than 1, we of course end up with
a quotient larger than the dividend. While the dividend may be small enough to
fit unclamped in a LayoutUnit (10000000px fits just fine), the quotient
(40000000) may not. So, while the actual column count really is 40 million (if
we allow columns to be shorter than 1px), the engine will clamp the 40 million
to fit inside a LayoutUnit. That's 33554431. This is the root of the problem,
and this incorrect column count value may in turn lead to other bad things,
even negative column heights in subsequent rows (and good luck calculating a
used column count off that!). It would probably be possible to cope with this,
if we only take extra care everywhere, when dealing with close-to-insane
numbers.

Or we can just do what the spec says, and clamp column heights to >= 1px.

[1] https://drafts.csswg.org/css-break/#breaking-rules

BUG=712626, 715208
Review-Url: https://codereview.chromium.org/2874933005
Cr-Commit-Position: refs/heads/master@{#471330}
Committed: https://chromium.googlesource.com/chromium/src/+/458588591a96346f4f2fc8c71a2e76a70f3eec5c

[mstensho (USE GERRIT), 2017-05-12 09:04:40]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 09:04:56]

Dry run: 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-05-12 10:20:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 10:20:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mstensho (USE GERRIT), 2017-05-12 10:58:05]

Description was changed from

==========
Distinguish between row (fragmentainer group) height and column (fragmentainer)
height.

TLDR; The spec [1] says that a fragmentainer height must always be 1px or
greater, to ensure progress. If we just do this, we'll avoid a lot of trouble
with limitations in the data types that we use.

The height of a row (fragmentainer group) may still be less than 1px, though.
We don't want the row to take up more space than it should in its container.
E.g. <div style="columns:2; height:0.5px;"><div style="height:1px";></div></div>
will give a row height of 0.5px, as specified. The column height, on the other
hand, should be clamped up to 1px.

And here, for the nastiness that this CL aims to fix:
<div style="columns:2; height:0.25px;">
    <div style="height:10000000px;"></div>
</div>
The content to fragment is 10 million pixels tall, and the column height has
been specified as 0.25px. Internally in our code, heights are stored as
LayoutUnit, which is a fixed-point unit with 6 bits reserved for decimals. On
the other side of the decimal point we have room for 32-6 bits = 26 bits, which
is what we have for a signed integer. That's 25 bits for the absolute value.
That's just over 30 million. LayoutUnit uses saturated arithmetic so there'll
never be any integer overflow or underflow, but there may be other ill effects.
Like in this case, if we actually allow a column height of less than 1px (i.e.
0.25px), when the engine for example wants to figure out the *actual* column
count (column count was *specified* as 2, but there's no way we're going to be
able to fit a 10 millions pixels tall thing in two columns when the column
height is 0.25px, so the actual count will be way higher), we take the flow
thread portion (10000000px) and divide by the column height (0.25px). If we
divide something by something (positive) less than 1, we of course end up with
a quotient larger than the dividend. While the dividend may be small enough to
fit unclamped in a LayoutUnit (10000000px fits just fine), the quotient
(40000000) may not. So, while the actual column count really is 40 million (if
we allow columns to be shorter than 1px), the engine will clamp the 40 million
to fit inside a LayoutUnit. That's 33554431. This is the root of the problem,
and this incorrect column count value may in turn lead to other bad things,
even negative column heights in subsequent rows (and good luck calculating a
used column count off that!). It would probably be possible to cope with this,
if we only take extra care everywhere, when dealing with close-to-insane
numbers.

Or we can just do what the spec says, and clamp column heights to >= 1px.

[1] https://drafts.csswg.org/css-break/#breaking-rules

BUG=712626,715208
==========

to

==========
Distinguish between row (fragmentainer group) height and column (fragmentainer)
height.

TLDR; The spec [1] says that a fragmentainer height must always be 1px or
greater, to ensure progress. If we just do this, we'll avoid a lot of trouble
with limitations in the data types that we use.

While *column* heights will now be clamped to >= 1px, we still allow the height
of a *row* (fragmentainer group) to be less than 1px. We don't want the row to
take up more space than it should in its container.
E.g. <div style="columns:2; height:0.5px;"><div style="height:1px";></div></div>
will give a row height of 0.5px, as specified. The column height, on the other
hand, should be clamped up to 1px.

And here, for the nastiness that this CL aims to fix:
<div style="columns:2; height:0.25px;">
    <div style="height:10000000px;"></div>
</div>
The content to fragment is 10 million pixels tall, and the column height has
been specified as 0.25px. Internally in our code, heights are stored as
LayoutUnit, which is a fixed-point unit with 6 bits reserved for decimals. On
the other side of the decimal point we have room for 32-6 bits = 26 bits, which
is what we have for a signed integer. That's 25 bits for the absolute value.
That's just over 30 million. LayoutUnit uses saturated arithmetic so there'll
never be any integer overflow or underflow, but there may be other ill effects.
Like in this case, if we actually allow a column height of less than 1px (i.e.
0.25px), when the engine for example wants to figure out the *actual* column
count (column count was *specified* as 2, but there's no way we're going to be
able to fit a 10 millions pixels tall thing in two columns when the column
height is 0.25px, so the actual count will be way higher), we take the flow
thread portion (10000000px) and divide by the column height (0.25px). If we
divide something by something (positive) less than 1, we of course end up with
a quotient larger than the dividend. While the dividend may be small enough to
fit unclamped in a LayoutUnit (10000000px fits just fine), the quotient
(40000000) may not. So, while the actual column count really is 40 million (if
we allow columns to be shorter than 1px), the engine will clamp the 40 million
to fit inside a LayoutUnit. That's 33554431. This is the root of the problem,
and this incorrect column count value may in turn lead to other bad things,
even negative column heights in subsequent rows (and good luck calculating a
used column count off that!). It would probably be possible to cope with this,
if we only take extra care everywhere, when dealing with close-to-insane
numbers.

Or we can just do what the spec says, and clamp column heights to >= 1px.

[1] https://drafts.csswg.org/css-break/#breaking-rules

BUG=712626,715208
==========

[mstensho (USE GERRIT), 2017-05-12 11:00:30]

mstensho@opera.com changed reviewers:
+ eae@chromium.org

[mstensho (USE GERRIT), 2017-05-12 14:10:05]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 14:10:27]

Dry run: 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-05-12 15:46:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 15:46:06]

Dry run: This issue passed the CQ dry run.

[eae, 2017-05-12 16:31:21]

LGTM

https://codereview.chromium.org/2874933005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h
(right):

https://codereview.chromium.org/2874933005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h:52: //
group. The spec says that this value must always be >= 1px, to ensure
These comments are great! Thank you.

[eae, 2017-05-12 16:31:25]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2017-05-12 16:32:08]

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-05-12 16:39:12]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494606685307430, "parent_rev":
"8f64c57f0ae067e658a1d903fbbf5cbdb44e135e", "commit_rev":
"458588591a96346f4f2fc8c71a2e76a70f3eec5c"}

[commit-bot: I haz the power, 2017-05-12 16:39:27]

Description was changed from

==========
Distinguish between row (fragmentainer group) height and column (fragmentainer)
height.

TLDR; The spec [1] says that a fragmentainer height must always be 1px or
greater, to ensure progress. If we just do this, we'll avoid a lot of trouble
with limitations in the data types that we use.

While *column* heights will now be clamped to >= 1px, we still allow the height
of a *row* (fragmentainer group) to be less than 1px. We don't want the row to
take up more space than it should in its container.
E.g. <div style="columns:2; height:0.5px;"><div style="height:1px";></div></div>
will give a row height of 0.5px, as specified. The column height, on the other
hand, should be clamped up to 1px.

And here, for the nastiness that this CL aims to fix:
<div style="columns:2; height:0.25px;">
    <div style="height:10000000px;"></div>
</div>
The content to fragment is 10 million pixels tall, and the column height has
been specified as 0.25px. Internally in our code, heights are stored as
LayoutUnit, which is a fixed-point unit with 6 bits reserved for decimals. On
the other side of the decimal point we have room for 32-6 bits = 26 bits, which
is what we have for a signed integer. That's 25 bits for the absolute value.
That's just over 30 million. LayoutUnit uses saturated arithmetic so there'll
never be any integer overflow or underflow, but there may be other ill effects.
Like in this case, if we actually allow a column height of less than 1px (i.e.
0.25px), when the engine for example wants to figure out the *actual* column
count (column count was *specified* as 2, but there's no way we're going to be
able to fit a 10 millions pixels tall thing in two columns when the column
height is 0.25px, so the actual count will be way higher), we take the flow
thread portion (10000000px) and divide by the column height (0.25px). If we
divide something by something (positive) less than 1, we of course end up with
a quotient larger than the dividend. While the dividend may be small enough to
fit unclamped in a LayoutUnit (10000000px fits just fine), the quotient
(40000000) may not. So, while the actual column count really is 40 million (if
we allow columns to be shorter than 1px), the engine will clamp the 40 million
to fit inside a LayoutUnit. That's 33554431. This is the root of the problem,
and this incorrect column count value may in turn lead to other bad things,
even negative column heights in subsequent rows (and good luck calculating a
used column count off that!). It would probably be possible to cope with this,
if we only take extra care everywhere, when dealing with close-to-insane
numbers.

Or we can just do what the spec says, and clamp column heights to >= 1px.

[1] https://drafts.csswg.org/css-break/#breaking-rules

BUG=712626,715208
==========

to

==========
Distinguish between row (fragmentainer group) height and column (fragmentainer)
height.

TLDR; The spec [1] says that a fragmentainer height must always be 1px or
greater, to ensure progress. If we just do this, we'll avoid a lot of trouble
with limitations in the data types that we use.

While *column* heights will now be clamped to >= 1px, we still allow the height
of a *row* (fragmentainer group) to be less than 1px. We don't want the row to
take up more space than it should in its container.
E.g. <div style="columns:2; height:0.5px;"><div style="height:1px";></div></div>
will give a row height of 0.5px, as specified. The column height, on the other
hand, should be clamped up to 1px.

And here, for the nastiness that this CL aims to fix:
<div style="columns:2; height:0.25px;">
    <div style="height:10000000px;"></div>
</div>
The content to fragment is 10 million pixels tall, and the column height has
been specified as 0.25px. Internally in our code, heights are stored as
LayoutUnit, which is a fixed-point unit with 6 bits reserved for decimals. On
the other side of the decimal point we have room for 32-6 bits = 26 bits, which
is what we have for a signed integer. That's 25 bits for the absolute value.
That's just over 30 million. LayoutUnit uses saturated arithmetic so there'll
never be any integer overflow or underflow, but there may be other ill effects.
Like in this case, if we actually allow a column height of less than 1px (i.e.
0.25px), when the engine for example wants to figure out the *actual* column
count (column count was *specified* as 2, but there's no way we're going to be
able to fit a 10 millions pixels tall thing in two columns when the column
height is 0.25px, so the actual count will be way higher), we take the flow
thread portion (10000000px) and divide by the column height (0.25px). If we
divide something by something (positive) less than 1, we of course end up with
a quotient larger than the dividend. While the dividend may be small enough to
fit unclamped in a LayoutUnit (10000000px fits just fine), the quotient
(40000000) may not. So, while the actual column count really is 40 million (if
we allow columns to be shorter than 1px), the engine will clamp the 40 million
to fit inside a LayoutUnit. That's 33554431. This is the root of the problem,
and this incorrect column count value may in turn lead to other bad things,
even negative column heights in subsequent rows (and good luck calculating a
used column count off that!). It would probably be possible to cope with this,
if we only take extra care everywhere, when dealing with close-to-insane
numbers.

Or we can just do what the spec says, and clamp column heights to >= 1px.

[1] https://drafts.csswg.org/css-break/#breaking-rules

BUG=712626,715208

Review-Url: https://codereview.chromium.org/2874933005
Cr-Commit-Position: refs/heads/master@{#471330}
Committed:
https://chromium.googlesource.com/chromium/src/+/458588591a96346f4f2fc8c71a2e...
==========

[commit-bot: I haz the power, 2017-05-12 16:39:28]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/458588591a96346f4f2fc8c71a2e...