Allow MainSync deviation of 1px in AndroidScrollIntegrationTest

Allow MainSync deviation of 1px in AndroidScrollIntegrationTest

This change alters AndroidScrollIntegrationTest to allow for a
1px negative deviation when asserting scroll values.
This is necessary due to Math being used in productive code that
is slightly different than the DIP scale verification done in
this test.

It also moves asserts onto the testing thread to get stack traces
in case a test fails.

BUG=537343
Committed: https://crrev.com/79e506a61a58a10da7e826e4acba6f7fe4e94845
Cr-Commit-Position: refs/heads/master@{#353937}

[kraush, 2015-09-29 20:30:38]

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

[kraush, 2015-09-29 20:32:15]

Hi boliu,

Here's a proposed fix for http://crbug.com/537343
Can you please take a look at this change?

Thanks! :)
Holger

[boliu, 2015-09-29 22:23:51]

Sent a try job and let's see if it works on nexus 5 (scale == 3)

I'm not entirely sure this is correct I suppose. Those floors where converted
from round in https://codereview.chromium.org/23533051/, and the author of that
patch (and most of the reviewers) are no longer actively working on chromium.

I guess can you describe the problem more in detail. Eg pick a failing test, and
log all the values being used and/or calculated in
BrowserViewRenderer::SetTotalRootLayerScrollOffset, and see how does that not
match the test. At least in there, it appears to be a ceiling operation: ceil in
max_scroll_offset(), which then ends up in the numerator in
BrowserViewRenderer::SetTotalRootLayerScrollOffset.

[hush (inactive), 2015-09-30 20:09:57]

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

[hush (inactive), 2015-09-30 20:09:57]

I remember having trouble with nexus 6 regard to floating point too. Like these
bugs:
https://code.google.com/p/chromium/issues/detail?id=438379

Can you see if the tests mentioned in this bug pass in Nexus 6? (if you have one
available)

[kraush, 2015-10-01 02:32:55]

@hush:
Unfortunately I do not have a Nexus 6 at hand :(

@boliu:
Good point, thanks! :)
I've updated the bug with some actual values that I get without this change.
There are other devices I had slightly different failures on - I'll add those
numbers as well as soon as I get them.

[kraush, 2015-10-13 23:35:28]

This new patch set keeps all the flooring and ceiling, and instead allows for a
deviation of 1 pixel downwards within assertOnMainSync.
See linked bug for more information.

[boliu, 2015-10-13 23:48:52]

https://codereview.chromium.org/1374373002/diff/20001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(right):

https://codereview.chromium.org/1374373002/diff/20001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:230:
assertTrue("Actual and expected x-scroll offsets do not match. Expected " +
scrollXPix
Giant comment here why this -1 thing is needed, for the next person staring at
this code.

You can quote some numbers, and mention crbug.com/537343 in the comment

[kraush, 2015-10-14 00:01:59]

On 2015/10/13 23:48:52, boliu wrote:
>
https://codereview.chromium.org/1374373002/diff/20001/android_webview/javates...
> File
>
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
> (right):
> 
>
https://codereview.chromium.org/1374373002/diff/20001/android_webview/javates...
>
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:230:
> assertTrue("Actual and expected x-scroll offsets do not match. Expected " +
> scrollXPix
> Giant comment here why this -1 thing is needed, for the next person staring at
> this code.
> 
> You can quote some numbers, and mention crbug.com/537343 in the comment

Makes sense, thanks! :)

I'll add a comment.

[kraush, 2015-10-14 00:26:37]

Added a comment as suggested by boliu.

Also updated the commit message to reflect the new implementation (and the
description accordingly).

[boliu, 2015-10-14 00:36:35]

lgtm

[kraush, 2015-10-14 00:45:32]

The CQ bit was checked by kraush@amazon.com

[commit-bot: I haz the power, 2015-10-14 00:47:19]

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

[commit-bot: I haz the power, 2015-10-14 01:18:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-14 01:20:34]

Patchset 3 (id:??) landed as
https://crrev.com/79e506a61a58a10da7e826e4acba6f7fe4e94845
Cr-Commit-Position: refs/heads/master@{#353937}