Fixes Escalator's assertion error regarding scroll position (#13334)

Change-Id: I72dd8c5160022d127692e223c267edf09e73e091

client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java

@@ -47,6 +47,33 @@ abstract class ScrollbarBundle {
         private final ScheduledCommand fireEventCommand = new ScheduledCommand() {
             @Override
             public void execute() {
+
+                /*
+                 * Some kind of native-scroll-event related asynchronous problem
+                 * occurs here (at least on desktops) where the internal
+                 * bookkeeping isn't up to date with the real scroll position.
+                 * The weird thing is, that happens only once, and if you drag
+                 * scrollbar fast enough. After it has failed once, it never
+                 * fails again.
+                 * 
+                 * Theory: the user drags the scrollbar, and this command is
+                 * executed before the browser has a chance to fire a scroll
+                 * event (which normally would correct this situation). This
+                 * would explain why slow scrolling doesn't trigger the problem,
+                 * while fast scrolling does.
+                 * 
+                 * To make absolutely sure that we have the latest scroll
+                 * position, let's update the internal value.
+                 * 
+                 * This might lead to a slight performance hit (on my computer
+                 * it was never more than 3ms on either of Chrome 38 or Firefox
+                 * 31). It also _slightly_ counteracts the purpose of the
+                 * internal bookkeeping. But since getScrollPos is called 3
+                 * times (on one direction) per scroll loop, it's still better
+                 * to have take this small penalty than removing it altogether.
+                 */
+                updateScrollPosFromDom();
+
                 if (!pixelValuesEqual(startScrollPos, getScrollPos())) {
                     getHandlerManager().fireEvent(new ScrollEvent());
                 }
@@ -451,6 +478,10 @@ abstract class ScrollbarBundle {
              */
             internalSetScrollPos(toInt32(scrollPos));
 
+            /*
+             * TODO it looks like this call isn't strictly required, as long as
+             * the updateScrollPosFromDom() is called correctly.
+             */
             scrollEventFirer.scheduleEvent();
         }
     }
@@ -643,6 +674,14 @@ abstract class ScrollbarBundle {
      * the DOM.
      */
     private final void updateScrollPosFromDom() {
+
+        /*
+         * TODO: this method probably shouldn't be called from Escalator's JSNI,
+         * but probably could be handled internally by this listening to its own
+         * element. Would clean up the code quite a bit. Needs further
+         * investigation.
+         */
+
         int newScrollPos = internalGetScrollPos();
         if (!isLocked()) {
             scrollPos = newScrollPos;