Allow for 1px rounding errors in scroll tests

Allow for 1px rounding errors in scroll tests

Dependent on device screen sizes, scrolling can lead to
1px deviations from anticipated scroll values on Android
devices.
This change alters touch_action_browsertest to allow those
deviations.

BUG=647719
Committed: https://crrev.com/91e62e1806ecaf761a64619b51bb48b8a0d1784d
Cr-Commit-Position: refs/heads/master@{#419282}

[kraush, 2016-09-16 17:53:18]

kraush@amazon.com changed reviewers:
+ aelias@chromium.org

[kraush, 2016-09-16 17:53:18]

Hi aelias,

Can you take a look at this test fix for devices with unusual DIP scales?

Thanks! :)
Holger

[aelias_OOO_until_Jul13, 2016-09-16 18:20:21]

Could you instead find a way to reformulate the math to get a precisely
predictable result, instead of slackening the test?  E.g. an extra floor() or
ceil() somewhere, but still EXPECT_EQ.

Also, would you be able to say the exact screen size and DPI scale causing the
problem?  (It's OK if you cannot say it if would implicitly leak unannounced
product specs, but if it's nonsensitive it might be useful to know.)

[kraush, 2016-09-16 19:33:46]

On 2016/09/16 18:20:21, aelias wrote:
> Could you instead find a way to reformulate the math to get a precisely
> predictable result, instead of slackening the test?  E.g. an extra floor() or
> ceil() somewhere, but still EXPECT_EQ.
> 
> Also, would you be able to say the exact screen size and DPI scale causing the
> problem?  (It's OK if you cannot say it if would implicitly leak unannounced
> product specs, but if it's nonsensitive it might be useful to know.)

Gotcha, thanks, I'll look into whether I can remove the functional rounding
error.
(From what I remember from the other bug linked in this CRBug, it was
multiplying by the DIP scale and dividing through it again, which lead to the
rounding problem)

For reference, one of the devices this is happening on is a Fire HD 7
Using org.chromium.ui.gfx.DeviceDisplayInfo, I get the following numbers:
getDisplayWidth() 800
getDisplayHeight() 1216
getDIPScale() 1.3312500715255737

(Copying this response to the bug report to keep it in a central place)

[aelias_OOO_until_Jul13, 2016-09-16 20:03:21]

Hmm, on second thought, lgtm as is.

Reasoning:
1) this is probably http://crbug.com/414283, which has been difficult to make
happen, so I think you'll have a lot of trouble making progress on the real fix.
2) locking in a specific test expectation to the buggy scroll offset would cause
an unnecessary test failure when http://crbug.com/414283 *is* fixed, so it's
better to keep the check approximate.
3) this test is intended to check whether we scrolled at all.  The exact value
isn't important.

[dtapuska, 2016-09-16 20:07:55]

On 2016/09/16 20:03:21, aelias wrote:
> Hmm, on second thought, lgtm as is.
> 
> Reasoning:
> 1) this is probably http://crbug.com/414283, which has been difficult to make
> happen, so I think you'll have a lot of trouble making progress on the real
fix.
> 2) locking in a specific test expectation to the buggy scroll offset would
cause
> an unnecessary test failure when http://crbug.com/414283 *is* fixed, so it's
> better to keep the check approximate.
> 3) this test is intended to check whether we scrolled at all.  The exact value
> isn't important.

ya lgtm with this for now.

[kraush, 2016-09-16 20:43:22]

The CQ bit was checked by kraush@amazon.com

[commit-bot: I haz the power, 2016-09-16 20:43:40]

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-09-16 21:41:16]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-16 21:43:55]

Description was changed from

==========
Allow for 1px rounding errors in scroll tests

Dependent on device screen sizes, scrolling can lead to
1px deviations from anticipated scroll values on Android
devices.
This change alters touch_action_browsertest to allow those
deviations.

BUG=647719
==========

to

==========
Allow for 1px rounding errors in scroll tests

Dependent on device screen sizes, scrolling can lead to
1px deviations from anticipated scroll values on Android
devices.
This change alters touch_action_browsertest to allow those
deviations.

BUG=647719

Committed: https://crrev.com/91e62e1806ecaf761a64619b51bb48b8a0d1784d
Cr-Commit-Position: refs/heads/master@{#419282}
==========

[commit-bot: I haz the power, 2016-09-16 21:43:56]

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