Allow MainSync deviation of 1px in AndroidScrollIntegrationTest

android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java

Index: android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
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 6a1d9ac6916b1f5a4968fe5e7141cbb110fd76d1..4c5d5dfec4440916601dc1bea7f83113adb72770 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());
     }
 }