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

Review URL: https://codereview.chromium.org/1374373002

Cr-Commit-Position: refs/heads/master@{#353937}

1 files changed, 67 insertions, 17 deletions

diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
index 6a1d9ac..4c5d5df 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
@@ -26,6 +26,8 @@ import java.util.Locale;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Integration tests for synchronous scrolling.
@@ -216,13 +218,45 @@ public class AndroidScrollIntegrationTest extends AwTestBase {
 
     private void assertScrollOnMainSync(final ScrollTestContainerView testContainerView,
             final int scrollXPix, final int scrollYPix) {
+        final AtomicInteger scrolledXPix = new AtomicInteger();
+        final AtomicInteger scrolledYPix = new AtomicInteger();
         getInstrumentation().runOnMainSync(new Runnable() {
             @Override
             public void run() {
-                assertEquals(scrollXPix, testContainerView.getScrollX());
-                assertEquals(scrollYPix, testContainerView.getScrollY());
+                scrolledXPix.set(testContainerView.getScrollX());
+                scrolledYPix.set(testContainerView.getScrollY());
             }
         });
+        // Actual scrolling is done using this formula:
+        // floor (scroll_offset_dip * max_offset) / max_scroll_offset_dip
+        // where max_offset is calculated using a ceil operation.
+        // This combination of ceiling and flooring can lead to a deviation from the test
+        // calculation, which simply uses the more direct:
+        // floor (scroll_offset_dip * dip_scale)
+        //
+        // While the math used in the functional code is correct (See crbug.com/261239), it can't
+        // be verified down to the pixel in this test which doesn't have all internal values.
+        // In non-rational cases, this can lead to a deviation of up to one pixel when using
+        // the floor directly. To accomodate this scenario, the test allows a -1 px deviation.
+        //
+        // For example, imagine the following valid values:
+        // scroll_offset_dip = 132
+        // max_offset = 532
+        // max_scroll_offset_dip = 399
+        // dip_scale = 1.33125
+        //
+        // The functional code will return
+        // floor (132 * 532 / 399) = 176
+        // The test code will return
+        // floor (132 * 1.33125) = 175
+        //
+        // For more information, see crbug.com/537343
+        assertTrue("Actual and expected x-scroll offsets do not match. Expected " + scrollXPix
+                + ", actual " + scrolledXPix.get(),
+                scrollXPix == scrolledXPix.get() || scrollXPix == scrolledXPix.get() - 1);
+        assertTrue("Actual and expected y-scroll offsets do not match. Expected " + scrollYPix
+                + ", actual " + scrolledYPix.get(),
+                scrollYPix == scrolledYPix.get() || scrollYPix == scrolledYPix.get() - 1);
     }
 
     private void assertScrollInJs(final AwContents awContents,
@@ -790,35 +824,51 @@ public class AndroidScrollIntegrationTest extends AwTestBase {
 
         loadTestPageAndWaitForFirstFrame(testContainerView, contentsClient, null, "");
 
+        // Containers to execute asserts on the test thread
+        final AtomicBoolean canZoomIn = new AtomicBoolean(false);
+        final AtomicReference<Float> atomicOldScale = new AtomicReference<Float>();
+        final AtomicReference<Float> atomicNewScale = new AtomicReference<Float>();
+        final AtomicInteger atomicOldScrollRange = new AtomicInteger();
+        final AtomicInteger atomicNewScrollRange = new AtomicInteger();
+        final AtomicInteger atomicContentHeight = new AtomicInteger();
+        final AtomicInteger atomicOldContentHeightApproximation = new AtomicInteger();
+        final AtomicInteger atomicNewContentHeightApproximation = new AtomicInteger();
         getInstrumentation().runOnMainSync(new Runnable() {
             @Override
             public void run() {
-                assertTrue(awContents.canZoomIn());
+                canZoomIn.set(awContents.canZoomIn());
 
                 int oldScrollRange =
                         awContents.computeVerticalScrollRange() - testContainerView.getHeight();
                 float oldScale = awContents.getScale();
-                int oldContentHeightApproximation =
-                        (int) Math.ceil(awContents.computeVerticalScrollRange() / oldScale);
+                atomicOldContentHeightApproximation.set(
+                        (int) Math.ceil(awContents.computeVerticalScrollRange() / oldScale));
 
                 awContents.zoomIn();
 
                 int newScrollRange =
                         awContents.computeVerticalScrollRange() - testContainerView.getHeight();
                 float newScale = awContents.getScale();
-                int newContentHeightApproximation =
-                        (int) Math.ceil(awContents.computeVerticalScrollRange() / newScale);
-
-                assertTrue(String.format(Locale.ENGLISH,
-                        "Scale range should increase after zoom (%f) > (%f)",
-                        newScale, oldScale), newScale > oldScale);
-                assertTrue(String.format(Locale.ENGLISH,
-                        "Scroll range should increase after zoom (%d) > (%d)",
-                        newScrollRange, oldScrollRange), newScrollRange > oldScrollRange);
-                assertEquals(awContents.getContentHeightCss(), oldContentHeightApproximation);
-                assertEquals(awContents.getContentHeightCss(), newContentHeightApproximation);
+                atomicNewContentHeightApproximation.set(
+                        (int) Math.ceil(awContents.computeVerticalScrollRange() / newScale));
+
+                atomicOldScale.set(oldScale);
+                atomicNewScale.set(newScale);
+                atomicOldScrollRange.set(oldScrollRange);
+                atomicNewScrollRange.set(newScrollRange);
+                atomicContentHeight.set(awContents.getContentHeightCss());
             }
         });
-
+        assertTrue(canZoomIn.get());
+        assertTrue(String.format(Locale.ENGLISH,
+                "Scale range should increase after zoom (%f) > (%f)",
+                atomicNewScale.get(), atomicOldScale.get()),
+                atomicNewScale.get() > atomicOldScale.get());
+        assertTrue(String.format(Locale.ENGLISH,
+                "Scroll range should increase after zoom (%d) > (%d)",
+                atomicNewScrollRange.get(), atomicOldScrollRange.get()),
+                atomicNewScrollRange.get() > atomicOldScrollRange.get());
+        assertEquals(atomicContentHeight.get(), atomicOldContentHeightApproximation.get());
+        assertEquals(atomicContentHeight.get(), atomicNewContentHeightApproximation.get());
     }
 }