Fix to LayoutManager size calculations during transform. (#12138)

* Fix to LayoutManager size calculations during transform.

- ComputedStyle is slower but more reliable than using
getBoundingClientRect, which does not work as expected if a transform
has been applied to the element or one of its parents. This is a problem
e.g. with PopupView, where getBoundingClientRect will return too small
size (or even zero size) for all the popup contents while the opening
animation is active. ComputedStyle ignores the transform and returns the
expected value.
- The presence of the element in DOM must be checked before the size is
requested from ComputedStyle, if the element has disappeared from DOM
without a warning and calculation is attempted anyway, the browser gets
stuck.
- Possibility to configure LayoutManager to use the less reliable
calculations for applications where the slight performance difference is
more important than layout issues within elements that have transform
animations.
- Manual test, problem isn't reproducible by TestBench.

Fixes: #11187

client/src/main/java/com/vaadin/client/LayoutManager.java

@@ -69,6 +69,7 @@ public class LayoutManager {
         }
     };
     private boolean everythingNeedsMeasure = false;
+    private boolean thoroughSizeCheck = true;
 
     /**
      * Sets the application connection this instance is connected to. Called
@@ -85,6 +86,24 @@ public class LayoutManager {
         this.connection = connection;
     }
 
+    /**
+     * Set whether the measuring should use a thorough size check that evaluates
+     * the presence of the element and uses calculated size, or default to a
+     * slightly faster check that can result in incorrect size information if
+     * the check is triggered while a transform animation is ongoing. This can
+     * happen e.g. when a PopupView is opened.
+     * <p>
+     * By default, the thorough size check is enabled.
+     *
+     * @param thoroughSizeCheck
+     *            {@code true} if thorough size check enabled, {@code false} if
+     *            not
+     * @since
+     */
+    public void setThoroughSizeChck(boolean thoroughSizeCheck) {
+        this.thoroughSizeCheck = thoroughSizeCheck;
+    }
+
     /**
      * Returns the application connection for this layout manager.
      *
@@ -822,7 +841,8 @@ public class LayoutManager {
 
     private MeasureResult measuredAndUpdate(Element element,
             MeasuredSize measuredSize) {
-        MeasureResult measureResult = measuredSize.measure(element);
+        MeasureResult measureResult = measuredSize.measure(element,
+                thoroughSizeCheck);
         if (measureResult.isChanged()) {
             notifyListenersAndDepdendents(element,
                     measureResult.isWidthChanged(),

client/src/main/java/com/vaadin/client/MeasuredSize.java

@@ -18,6 +18,7 @@ package com.vaadin.client;
 import java.util.logging.Logger;
 
 import com.google.gwt.core.client.JsArrayString;
+import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 
 public class MeasuredSize {
@@ -186,7 +187,44 @@ public class MeasuredSize {
         return paddings[3];
     }
 
+    /**
+     * Measures paddings, margins, border, height, and weight of the given
+     * element and stores the results within this {@link MeasuredSize} object.
+     * The measurements are unreliable if the element or any of its parents are
+     * in the middle of a transform animation.
+     *
+     * @param element
+     *            element to be measured
+     * @return data object for whether the width or height of the given element
+     *         has changed since previous measure
+     * @see {@link #measure(Element, boolean)}
+     */
     public MeasureResult measure(Element element) {
+        return measure(element, false);
+    }
+
+    /**
+     * Measures paddings, margins, border, height, and weight of the given
+     * element and stores the results within this {@link MeasuredSize} object.
+     *
+     * @param element
+     *            element to be measured
+     * @param thoroughSizeCheck
+     *            {@code true} if the measuring should use the more reliable
+     *            size check that requires ensuring that the element is still
+     *            present in the DOM tree, {@code false} for the slightly faster
+     *            check that will give incorrect size information if this method
+     *            is called while the element or any of its parents are in the
+     *            middle of a transform animation.
+     * @return data object for whether the width or height of the given element
+     *         has changed since previous measure
+     */
+    public MeasureResult measure(Element element, boolean thoroughSizeCheck) {
+        if (thoroughSizeCheck
+                && !Document.get().getBody().isOrHasChild(element)) {
+            return new MeasureResult(false, false);
+        }
+
         Profiler.enter("MeasuredSize.measure");
         boolean heightChanged = false;
         boolean widthChanged = false;
@@ -239,7 +277,15 @@ public class MeasuredSize {
         Profiler.leave("Measure borders");
 
         Profiler.enter("Measure height");
-        double requiredHeight = WidgetUtil.getRequiredHeightDouble(element);
+        double requiredHeight;
+        if (thoroughSizeCheck) {
+            requiredHeight = computedStyle.getHeightIncludingBorderPadding();
+            if (Double.isNaN(requiredHeight)) {
+                requiredHeight = 0;
+            }
+        } else {
+            requiredHeight = WidgetUtil.getRequiredHeightDouble(element);
+        }
         double outerHeight = requiredHeight + sumHeights(margins);
         double oldHeight = height;
         if (setOuterHeight(outerHeight)) {
@@ -249,7 +295,15 @@ public class MeasuredSize {
         Profiler.leave("Measure height");
 
         Profiler.enter("Measure width");
-        double requiredWidth = WidgetUtil.getRequiredWidthDouble(element);
+        double requiredWidth;
+        if (thoroughSizeCheck) {
+            requiredWidth = computedStyle.getWidthIncludingBorderPadding();
+            if (Double.isNaN(requiredWidth)) {
+                requiredWidth = 0;
+            }
+        } else {
+            requiredWidth = WidgetUtil.getRequiredWidthDouble(element);
+        }
         double outerWidth = requiredWidth + sumWidths(margins);
         double oldWidth = width;
         if (setOuterWidth(outerWidth)) {

client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java

@@ -15,6 +15,16 @@
  */
 package com.vaadin.client.ui.ui;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.logging.Logger;
+
 import com.google.gwt.core.client.Scheduler;
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
@@ -91,17 +101,8 @@ import com.vaadin.shared.ui.ui.UIServerRpc;
 import com.vaadin.shared.ui.ui.UIState;
 import com.vaadin.shared.util.SharedUtil;
 import com.vaadin.ui.UI;
-import elemental.client.Browser;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.logging.Logger;
+import elemental.client.Browser;
 
 @Connect(value = UI.class, loadStyle = LoadStyle.EAGER)
 public class UIConnector extends AbstractSingleComponentContainerConnector
@@ -924,6 +925,11 @@ public class UIConnector extends AbstractSingleComponentContainerConnector
         getRpcProxy(DebugWindowServerRpc.class).showServerDesign(connector);
     }
 
+    @OnStateChange("thoroughSizeCheck")
+    void onThoroughSizeChckChange() {
+        getLayoutManager().setThoroughSizeChck(getState().thoroughSizeCheck);
+    }
+
     @OnStateChange("theme")
     void onThemeChange() {
         final String oldTheme = activeTheme;

server/src/main/java/com/vaadin/ui/UI.java

@@ -2021,6 +2021,40 @@ public abstract class UI extends AbstractSingleComponentContainer
         getRpcProxy(PageClientRpc.class).initializeMobileHtml5DndPolyfill();
     }
 
+    /**
+     * Returns whether LayoutManager uses thorough size check that evaluates the
+     * presence of the element and uses calculated size, or defaults to a
+     * slightly faster check that can result in incorrect size information if
+     * the check is triggered while a transform animation is ongoing. This can
+     * happen e.g. when a PopupView is opened.
+     * <p>
+     * By default, the thorough size check is enabled.
+     *
+     * @return {@code true} if thorough size check enabled, {@code false} if not
+     * @since
+     */
+    public boolean isUsingThoroughSizeCheck() {
+        return getState(false).thoroughSizeCheck;
+    }
+
+    /**
+     * Set whether LayoutManager should use thorough size check that evaluates
+     * the presence of the element and uses calculated size, or default to a
+     * slightly faster check that can result in incorrect size information if
+     * the check is triggered while a transform animation is ongoing. This can
+     * happen e.g. when a PopupView is opened.
+     * <p>
+     * By default, the thorough size check is enabled.
+     *
+     * @param thoroughSizeCheck
+     *            {@code true} if thorough size check enabled, {@code false} if
+     *            not
+     * @since
+     */
+    public void setUsingThoroughSizeCheck(boolean thoroughSizeCheck) {
+        getState().thoroughSizeCheck = thoroughSizeCheck;
+    }
+
     /**
      * Event which is fired when the ordering of the windows is updated.
      * <p>

shared/src/main/java/com/vaadin/shared/ui/ui/UIState.java

@@ -85,13 +85,22 @@ public class UIState extends AbstractSingleComponentContainerState {
         // Default is 1 for legacy reasons
         tabIndex = 1;
     }
-
     /**
      * Enable Mobile HTML5 DnD support.
      *
      * @since 8.1
      */
     public boolean enableMobileHTML5DnD = false;
+    /**
+     * Should the more thorough size check be in use in LayoutManager
+     * calculations. If this value is changed to {@code false}, the size
+     * calculations can result in incorrect values if they are triggered while a
+     * transform animation is ongoing. This can happen e.g. when a PopupView is
+     * opened.
+     *
+     * @since
+     */
+    public boolean thoroughSizeCheck = true;
 
     public static class LoadingIndicatorConfigurationState
             implements Serializable {

uitest/src/main/java/com/vaadin/tests/components/popupview/PopupViewContentWithExpandRatio.java

@@ -0,0 +1,65 @@
+package com.vaadin.tests.components.popupview;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.util.LoremIpsum;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.JavaScript;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.PopupView;
+import com.vaadin.ui.VerticalLayout;
+
+public class PopupViewContentWithExpandRatio extends AbstractTestUI {
+    private PopupView popup;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        popup = new PopupView("Open popup", createPopupContent());
+        popup.setHideOnMouseOut(false);
+        popup.setPopupVisible(false);
+        addComponent(popup);
+    }
+
+    private VerticalLayout createPopupContent() {
+        Label label = new Label(
+                "Placeholder content that should take up most of the available space");
+        label.setValue(LoremIpsum.get(56));
+        label.setSizeFull();
+        label.setId("label");
+
+        Button refreshBtn = new Button("Force layout", e -> {
+            JavaScript.eval("vaadin.forceLayout()");
+        });
+        refreshBtn.setId("refresh");
+
+        Button submitBtn = new Button("Close popup");
+        submitBtn.addClickListener(clickEvent -> {
+            popup.setPopupVisible(false);
+        });
+        submitBtn.setId("close");
+
+        VerticalLayout content = new VerticalLayout();
+        content.setHeight("300px");
+        content.setSpacing(true);
+        content.setMargin(true);
+
+        content.addComponent(label);
+        content.addComponent(refreshBtn);
+        content.addComponent(submitBtn);
+        content.setExpandRatio(label, 2.0f);
+        return content;
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11187;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Expand ratio shouldn't cause contents to overflow "
+                + "from popup view. The popup should be opened at least "
+                + "20 times without SuperDevMode or TestBench or other "
+                + "configurations that might slow down the processing.";
+    }
+}