Avoid overflow when checking if a transform is integer translation.

Avoid overflow when checking if a transform is integer translation.

If the floating point number in the matrix can not be represented as
an integer then we should return false so that the calling code does
not try to cast it as one.

For large enough integers this may return an incorrect result still, as
it may return true if the floating point representation has a
fractional part if that floating point representation also is used for
an integer converted to float. But at that point we can not tell which
would be closer to the original intent - using the integer-casted
number or the floating point one, so we leave the result as before.

R=ajuma@chromium.org
BUG=675435
Committed: https://crrev.com/602038a90b8b56b4a96a619962f41f03d42b3c46
Cr-Commit-Position: refs/heads/master@{#439584}

[danakj, 2016-12-19 16:45:47]

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

[commit-bot: I haz the power, 2016-12-19 16:46:04]

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

[danakj, 2016-12-19 16:46:27]

These tests would pass before too normally, but they would fail on UBSAN bots.

[ajuma, 2016-12-19 18:01:25]

lgtm

https://codereview.chromium.org/2585263003/diff/1/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/2585263003/diff/1/ui/gfx/transform.cc#newcode255
ui/gfx/transform.cc:255: static_cast<int>(t[2]) == t[2];
Maybe update the comment in the .h file to mention this can have false negatives
for large values?

[danakj, 2016-12-19 18:06:22]

https://codereview.chromium.org/2585263003/diff/1/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/2585263003/diff/1/ui/gfx/transform.cc#newcode255
ui/gfx/transform.cc:255: static_cast<int>(t[2]) == t[2];
On 2016/12/19 18:01:25, ajuma wrote:
> Maybe update the comment in the .h file to mention this can have false
negatives
> for large values?

It could actually have false positives too but I convinced myself that its fine
cuz we can't tell if the float or the int is more precise at that point. But ya
I will write something better in the header, thanks.

[danakj, 2016-12-19 18:24:36]

Done

[danakj, 2016-12-19 18:24:55]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-12-19 18:24:55]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2585263003/#ps20001
(title: "integer-transform")

[commit-bot: I haz the power, 2016-12-19 18:25:19]

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, 2016-12-19 22:06:49]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482171895059300,
"parent_rev": "378fa4c2a6e1ce13f958aa42497205bf9ff5c48b", "commit_rev":
"095486cb3221d5d5e045bf68327126bfbfa5c09f"}

[commit-bot: I haz the power, 2016-12-19 22:07:19]

Description was changed from

==========
Avoid overflow when checking if a transform is integer translation.

If the floating point number in the matrix can not be represented as
an integer then we should return false so that the calling code does
not try to cast it as one.

For large enough integers this may return an incorrect result still, as
it may return true if the floating point representation has a
fractional part if that floating point representation also is used for
an integer converted to float. But at that point we can not tell which
would be closer to the original intent - using the integer-casted
number or the floating point one, so we leave the result as before.

R=ajuma@chromium.org
BUG=675435
==========

to

==========
Avoid overflow when checking if a transform is integer translation.

If the floating point number in the matrix can not be represented as
an integer then we should return false so that the calling code does
not try to cast it as one.

For large enough integers this may return an incorrect result still, as
it may return true if the floating point representation has a
fractional part if that floating point representation also is used for
an integer converted to float. But at that point we can not tell which
would be closer to the original intent - using the integer-casted
number or the floating point one, so we leave the result as before.

R=ajuma@chromium.org
BUG=675435

Review-Url: https://codereview.chromium.org/2585263003
==========

[commit-bot: I haz the power, 2016-12-19 22:07:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-19 22:10:08]

Description was changed from

==========
Avoid overflow when checking if a transform is integer translation.

If the floating point number in the matrix can not be represented as
an integer then we should return false so that the calling code does
not try to cast it as one.

For large enough integers this may return an incorrect result still, as
it may return true if the floating point representation has a
fractional part if that floating point representation also is used for
an integer converted to float. But at that point we can not tell which
would be closer to the original intent - using the integer-casted
number or the floating point one, so we leave the result as before.

R=ajuma@chromium.org
BUG=675435

Review-Url: https://codereview.chromium.org/2585263003
==========

to

==========
Avoid overflow when checking if a transform is integer translation.

If the floating point number in the matrix can not be represented as
an integer then we should return false so that the calling code does
not try to cast it as one.

For large enough integers this may return an incorrect result still, as
it may return true if the floating point representation has a
fractional part if that floating point representation also is used for
an integer converted to float. But at that point we can not tell which
would be closer to the original intent - using the integer-casted
number or the floating point one, so we leave the result as before.

R=ajuma@chromium.org
BUG=675435

Committed: https://crrev.com/602038a90b8b56b4a96a619962f41f03d42b3c46
Cr-Commit-Position: refs/heads/master@{#439584}
==========

[commit-bot: I haz the power, 2016-12-19 22:10:09]

Patchset 2 (id:??) landed as
https://crrev.com/602038a90b8b56b4a96a619962f41f03d42b3c46
Cr-Commit-Position: refs/heads/master@{#439584}