Bug 1386130 - Actually wait for up to 30 frames to test scroll. r?dholbert draft
authorBrendan Dahl <brendan.dahl@gmail.com>
Wed, 04 Oct 2017 14:40:12 -0700
changeset 675159 a2328010608534fa89bbdf1b528461302719d48b
parent 674983 ee1c41cf306df0043a8e68af042f133acf2ef94e
child 734530 37d18c40349af9ed6953c06f9ccff56bfe3cb1ba
push id83054
push userbmo:bdahl@mozilla.com
push dateWed, 04 Oct 2017 21:52:01 +0000
reviewersdholbert
bugs1386130
milestone58.0a1
Bug 1386130 - Actually wait for up to 30 frames to test scroll. r?dholbert The frame count test was inverted (should be '>' not '<'), causing the test to never actually wait up to 30 frames to test the scroll position. The test will also now explicitly fail if the frame count is reached to make finding this issue easier in the future.
layout/base/tests/test_scroll_snapping_scrollbars.html
--- a/layout/base/tests/test_scroll_snapping_scrollbars.html
+++ b/layout/base/tests/test_scroll_snapping_scrollbars.html
@@ -258,19 +258,23 @@ function doTest() {
 
   stopFrameCount = 0;
   waitForScrollStart();
 }
 
 function waitForScrollStart() {
   // Wait for up to 30 frames for scrolling to start
   var testCase = testCases[step];
-  if (testCase.startScroll.y != sc.scrollTop
-      || testCase.startScroll.x != sc.scrollLeft
-      || ++stopFrameCount < 30) {
+  if (++stopFrameCount > 30) {
+    ok(false,
+      "Step " + step + ": Frame count limit reached without scrolling." +
+      "(" + testCase.description + ")");
+    window.requestAnimationFrame(doMouseUp);
+  } else if (testCase.startScroll.y != sc.scrollTop
+             || testCase.startScroll.x != sc.scrollLeft) {
     window.requestAnimationFrame(doMouseUp);
   } else {
     window.requestAnimationFrame(waitForScrollStart);
   }
 }
 
 function doMouseUp() {
   var testCase = testCases[step];