Rewrite overhanging float arithmetic to work near numeric limits

Rewrite overhanging float arithmetic to work near numeric limits

This patch fixes a bug where floats near LayoutUnit::max() would
incorrectly get added to two LayoutObjects which would lead to
duplicate display items when the floats were painted by both parents.

Examples sometimes make these easier to review:
  logicalTop = LayoutUnit::max()
  prevLogicalTop = (LayoutUnit::max() - 4)
  prevLowestFloatLogicalBottom = 8

Previous logic:
  if (prevLowestFloatLogicalBottom > logicalTop - prevLogicalTop) { ...

New logic:
  if (logicalTop < prevLowestFloatLogicalBottom + prevLogicalTop) { ...

BUG=604331
Committed: https://crrev.com/6c961fe1112914b9d63d6551e31b96c415dfb83f
Cr-Commit-Position: refs/heads/master@{#389679}

[pdr., 2016-04-25 04:20:06]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-25 04:20:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1916693002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1916693002/1

[pdr., 2016-04-25 04:21:23]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org, robhogan@chromium.org

[pdr., 2016-04-25 05:17:49]

Description was changed from

==========
Rewrite overhanging float arithmetic to not crash near numeric limits

This patch fixes a bug where floats near LayoutUnit::max() would
incorrectly get added to two LayoutObjects which would lead to
duplicate display items when the floats were painted by both parents.

Examples sometimes make these easier to review:
  logicalTop = LayoutUnit::max()
  prevLogicalTop = (LayoutUnit::max() - 4)
  prevLowestFloatLogicalBottom = 8

Previous logic:
  if (prevLowestFloatLogicalBottom > logicalTop - prevLogicalTop) { ...

New logic:
  if (logicalTop < prevLowestFloatLogicalBottom + prevLogicalTop) { ...

BUG=604331
==========

to

==========
Rewrite overhanging float arithmetic to work near numeric limits

This patch fixes a bug where floats near LayoutUnit::max() would
incorrectly get added to two LayoutObjects which would lead to
duplicate display items when the floats were painted by both parents.

Examples sometimes make these easier to review:
  logicalTop = LayoutUnit::max()
  prevLogicalTop = (LayoutUnit::max() - 4)
  prevLowestFloatLogicalBottom = 8

Previous logic:
  if (prevLowestFloatLogicalBottom > logicalTop - prevLogicalTop) { ...

New logic:
  if (logicalTop < prevLowestFloatLogicalBottom + prevLogicalTop) { ...

BUG=604331
==========

[commit-bot: I haz the power, 2016-04-25 05:43:52]

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

[commit-bot: I haz the power, 2016-04-25 05:43:53]

Dry run: This issue passed the CQ dry run.

[eae, 2016-04-25 06:24:34]

eae@chromium.org changed reviewers:
+ eae@chromium.org

[eae, 2016-04-25 06:24:34]

This is great! Could you add a test for negative numbers though? Ideally one
with a value near LayoutUnit::min().

[pdr., 2016-04-26 03:09:24]

On 2016/04/25 at 06:24:34, eae wrote:
> This is great! Could you add a test for negative numbers though? Ideally one
with a value near LayoutUnit::min().

This is a good idea and I hadn't thought of it. I spent some time trying to make
a test and wasn't able to though, because prevLowestFloatLogicalBottom is always
positive (in manual testing, and in all of our layout tests). I think it's
probably best to let clusterfuzz run wild with the new testcase we've added here
and see if anything pops up.

Looking forward a bit, it's surprising this bug made it all the way to paint. We
could add assertions that a floating LayoutObject is only in at most one other
LayoutObject's m_floatingObjects.

[eae, 2016-04-26 03:12:14]

Sounds like a good plan.

LGTM

[pdr., 2016-04-26 03:12:37]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-04-26 03:13:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1916693002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1916693002/1

[commit-bot: I haz the power, 2016-04-26 03:17:52]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-26 03:20:01]

Description was changed from

==========
Rewrite overhanging float arithmetic to work near numeric limits

This patch fixes a bug where floats near LayoutUnit::max() would
incorrectly get added to two LayoutObjects which would lead to
duplicate display items when the floats were painted by both parents.

Examples sometimes make these easier to review:
  logicalTop = LayoutUnit::max()
  prevLogicalTop = (LayoutUnit::max() - 4)
  prevLowestFloatLogicalBottom = 8

Previous logic:
  if (prevLowestFloatLogicalBottom > logicalTop - prevLogicalTop) { ...

New logic:
  if (logicalTop < prevLowestFloatLogicalBottom + prevLogicalTop) { ...

BUG=604331
==========

to

==========
Rewrite overhanging float arithmetic to work near numeric limits

This patch fixes a bug where floats near LayoutUnit::max() would
incorrectly get added to two LayoutObjects which would lead to
duplicate display items when the floats were painted by both parents.

Examples sometimes make these easier to review:
  logicalTop = LayoutUnit::max()
  prevLogicalTop = (LayoutUnit::max() - 4)
  prevLowestFloatLogicalBottom = 8

Previous logic:
  if (prevLowestFloatLogicalBottom > logicalTop - prevLogicalTop) { ...

New logic:
  if (logicalTop < prevLowestFloatLogicalBottom + prevLogicalTop) { ...

BUG=604331

Committed: https://crrev.com/6c961fe1112914b9d63d6551e31b96c415dfb83f
Cr-Commit-Position: refs/heads/master@{#389679}
==========

[commit-bot: I haz the power, 2016-04-26 03:20:02]

Patchset 1 (id:??) landed as
https://crrev.com/6c961fe1112914b9d63d6551e31b96c415dfb83f
Cr-Commit-Position: refs/heads/master@{#389679}