Fixes exception when scrolled down and removing header/footer row (#15411)

This is somewhat bad patch for something that should be done with a some kind
of lazy/finally functionality, where these kinds of operations are made JIT, 
instead of eagerly whenever called.

Change-Id: I9121c3715e9eeccff0f768c7b0f0904ee9cdc3a5

3 files changed, 117 insertions, 17 deletions

diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index 641a8d9adb..715b811e80 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -753,8 +753,7 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
          * that the sizes of the scroll handles appear correct in the browser
          */
         public void recalculateScrollbarsForVirtualViewport() {
-            double scrollContentHeight = body
-                    .calculateEstimatedTotalRowHeight();
+            double scrollContentHeight = body.calculateTotalRowHeight();
             double scrollContentWidth = columnConfiguration.calculateRowWidth();
 
             double tableWrapperHeight = heightOfEscalator;
@@ -1495,12 +1494,9 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
         abstract protected void recalculateSectionHeight();
 
         /**
-         * Returns the estimated height of all rows in the row container.
-         * <p>
-         * The estimate is promised to be correct as long as there are no rows
-         * with calculated heights.
+         * Returns the height of all rows in the row container.
          */
-        protected double calculateEstimatedTotalRowHeight() {
+        protected double calculateTotalRowHeight() {
             return getDefaultRowHeight() * getRowCount();
         }
 
@@ -2087,9 +2083,23 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
 
         @Override
         public void removeRows(int index, int numberOfRows) {
+
+            /*
+             * While the rows in a static section are removed, the scrollbar is
+             * temporarily shrunk and then re-expanded. This leads to the fact
+             * that the scroll position is scooted up a bit. This means that we
+             * need to reset the position here.
+             * 
+             * If Escalator, at some point, gets a JIT evaluation functionality,
+             * this re-setting is a strong candidate for removal.
+             */
+            double oldScrollPos = verticalScrollbar.getScrollPos();
+
             super.removeRows(index, numberOfRows);
             recalculateElementSizes();
             applyHeightByRows();
+
+            verticalScrollbar.setScrollPos(oldScrollPos);
         }
 
         @Override
@@ -2120,10 +2130,21 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
         protected void recalculateSectionHeight() {
             Profiler.enter("Escalator.AbstractStaticRowContainer.recalculateSectionHeight");
 
-            double newHeight = calculateEstimatedTotalRowHeight();
+            double newHeight = calculateTotalRowHeight();
             if (newHeight != heightOfSection) {
                 heightOfSection = newHeight;
                 sectionHeightCalculated();
+
+                /*
+                 * We need to update the scrollbar dimension at this point. If
+                 * we are scrolled too far down and the static section shrinks,
+                 * the body will try to render rows that don't exist during
+                 * body.verifyEscalatorCount. This is because the logical row
+                 * indices are calculated from the scrollbar position.
+                 */
+                verticalScrollbar.setOffsetSize(heightOfEscalator
+                        - header.heightOfSection - footer.heightOfSection);
+
                 body.verifyEscalatorCount();
             }
 
@@ -2648,12 +2669,13 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
                 throw new IllegalArgumentException(
                         "Visual target must not be greater than the number of escalator rows");
             } else if (logicalTargetIndex + visualSourceRange.length() > getRowCount()) {
-                final int logicalEndIndex = logicalTargetIndex
-                        + visualSourceRange.length() - 1;
-                throw new IllegalArgumentException(
-                        "Logical target leads to rows outside of the data range ("
-                                + logicalTargetIndex + ".." + logicalEndIndex
-                                + ")");
+                Range logicalTargetRange = Range.withLength(logicalTargetIndex,
+                        visualSourceRange.length());
+                Range availableRange = Range.withLength(0, getRowCount());
+                throw new IllegalArgumentException("Logical target leads "
+                        + "to rows outside of the data range ("
+                        + logicalTargetRange + " goes beyond " + availableRange
+                        + ")");
             }
 
             /*
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorScrollTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorScrollTest.java
index 91527504a5..41ad370b98 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorScrollTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorScrollTest.java
@@ -17,14 +17,22 @@ package com.vaadin.tests.components.grid.basicfeatures.escalator;
 
 import static org.junit.Assert.assertEquals;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebElement;
 
 import com.vaadin.tests.components.grid.basicfeatures.EscalatorBasicClientFeaturesTest;
 
+@SuppressWarnings("all")
 public class EscalatorScrollTest extends EscalatorBasicClientFeaturesTest {
 
+    @Before
+    public void setUp() {
+        openTestURL();
+        populate();
+    }
+
     /**
      * Before the fix, removing and adding rows and also scrolling would put the
      * scroll state in an internally inconsistent state. The scrollbar would've
@@ -35,9 +43,6 @@ public class EscalatorScrollTest extends EscalatorBasicClientFeaturesTest {
      */
     @Test
     public void testScrollRaceCondition() {
-        openTestURL();
-        populate();
-
         scrollVerticallyTo(40);
         String originalStyle = getTBodyStyle();
         selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, REMOVE_ALL_INSERT_SCROLL);
@@ -46,6 +51,43 @@ public class EscalatorScrollTest extends EscalatorBasicClientFeaturesTest {
         assertEquals(originalStyle, getTBodyStyle());
     }
 
+    @Test
+    public void scrollToBottomAndRemoveHeader() throws Exception {
+        scrollVerticallyTo(999999); // to bottom
+
+        /*
+         * apparently the scroll event isn't fired by the time the next assert
+         * would've been done.
+         */
+        Thread.sleep(50);
+
+        assertEquals("Unexpected last row cell before header removal",
+                "Row 99: 0,99", getBodyCell(-1, 0).getText());
+        selectMenuPath(COLUMNS_AND_ROWS, HEADER_ROWS,
+                REMOVE_ONE_ROW_FROM_BEGINNING);
+        assertEquals("Unexpected last row cell after header removal",
+                "Row 99: 0,99", getBodyCell(-1, 0).getText());
+
+    }
+
+    @Test
+    public void scrollToBottomAndRemoveFooter() throws Exception {
+        scrollVerticallyTo(999999); // to bottom
+
+        /*
+         * apparently the scroll event isn't fired by the time the next assert
+         * would've been done.
+         */
+        Thread.sleep(50);
+
+        assertEquals("Unexpected last row cell before footer removal",
+                "Row 99: 0,99", getBodyCell(-1, 0).getText());
+        selectMenuPath(COLUMNS_AND_ROWS, FOOTER_ROWS,
+                REMOVE_ONE_ROW_FROM_BEGINNING);
+        assertEquals("Unexpected last row cell after footer removal",
+                "Row 99: 0,99", getBodyCell(-1, 0).getText());
+    }
+
     private String getTBodyStyle() {
         WebElement tbody = getEscalator().findElement(By.tagName("tbody"));
         return tbody.getAttribute("style");
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
index aafff7953c..761f32bc9a 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
@@ -419,6 +419,34 @@ public class EscalatorBasicClientFeaturesWidget extends
                         insertColumns(0, 10);
                     }
                 }, menupath);
+
+        createSizeMenu();
+    }
+
+    private void createSizeMenu() {
+        String[] menupath = new String[] { "General", "Size" };
+
+        addSizeMenuItem(null, "height", menupath);
+        addSizeMenuItem("200px", "height", menupath);
+        addSizeMenuItem("400px", "height", menupath);
+        addSizeMenuItem(null, "width", menupath);
+        addSizeMenuItem("200px", "width", menupath);
+        addSizeMenuItem("400px", "width", menupath);
+    }
+
+    private void addSizeMenuItem(final String size, final String direction,
+            String[] menupath) {
+        final String title = (size != null ? size : "undefined");
+        addMenuCommand(title + " " + direction, new ScheduledCommand() {
+            @Override
+            public void execute() {
+                if (direction.equals("height")) {
+                    escalator.setHeight(size);
+                } else {
+                    escalator.setWidth(size);
+                }
+            }
+        }, menupath);
     }
 
     private void createColumnMenu() {
@@ -574,6 +602,14 @@ public class EscalatorBasicClientFeaturesWidget extends
                 removeRows(container, offset, number);
             }
         }, menupath);
+        addMenuCommand("Remove all rows", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                if (container.getRowCount() > 0) {
+                    removeRows(container, 0, container.getRowCount());
+                }
+            }
+        }, menupath);
     }
 
     private void insertRows(final RowContainer container, int offset, int number) {