Defer nested components to paint them as top-level changes (#8304).

Refactoring and changes to paint nested component contents after the
component in which they are nested. The client side can create
uninitialized components immediately but defer configuring them.

src/com/vaadin/terminal/PaintTarget.java

@@ -36,6 +36,31 @@ public interface PaintTarget extends Serializable {
     public void addSection(String sectionTagName, String sectionData)
             throws PaintException;
 
+    /**
+     * Result of starting to paint a Paintable (
+     * {@link PaintTarget#startPaintable(Paintable, String)}).
+     * 
+     * @since 7.0
+     */
+    public enum PaintStatus {
+        /**
+         * Painting started, addVariable() and addAttribute() etc. methods may
+         * be called.
+         */
+        PAINTING,
+        /**
+         * Paintable is cached on the client side and unmodified - only an
+         * indication of that should be painted.
+         */
+        CACHED,
+        /**
+         * A previously unpainted or painted {@link Paintable} has been queued
+         * be created/update later in a separate change in the same set of
+         * changes.
+         */
+        DEFER
+    };
+
     /**
      * Prints element start tag of a paintable section. Starts a paintable
      * section using the given tag. The PaintTarget may implement a caching
@@ -52,21 +77,22 @@ public interface PaintTarget extends Serializable {
      * </p>
      * <p>
      * Each paintable being painted should be closed by a matching
-     * {@link #endPaintable(Paintable)}.
+     * {@link #endPaintable(Paintable)} regardless of the {@link PaintStatus}
+     * returned.
      * </p>
      * 
      * @param paintable
      *            the paintable to start.
      * @param tag
      *            the name of the start tag.
-     * @return <code>true</code> if paintable found in cache, <code>false</code>
-     *         otherwise.
+     * @return {@link PaintStatus} - ready to paint, already cached on the
+     *         client or defer painting to another change
      * @throws PaintException
      *             if the paint operation failed.
      * @see #startTag(String)
      * @since 7.0 (previously using startTag(Paintable, String))
      */
-    public boolean startPaintable(Paintable paintable, String tag)
+    public PaintStatus startPaintable(Paintable paintable, String tag)
             throws PaintException;
 
     /**

src/com/vaadin/terminal/gwt/client/ApplicationConnection.java

@@ -1865,6 +1865,8 @@ public class ApplicationConnection {
      */
     public VPaintableWidget getPaintable(UIDL uidl) {
         final String pid = uidl.getId();
+        // the actual content UIDL may be deferred, but it always contains
+        // enough information to create a paintable instance
         if (!paintableMap.hasPaintable(pid)) {
             // Create and register a new paintable if no old was found
             VPaintableWidget p = widgetSet.createWidget(uidl.getTag(),

src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java

@@ -106,7 +106,8 @@ public abstract class VAbstractPaintableWidget implements VPaintableWidget {
     }
 
     protected static boolean isRealUpdate(UIDL uidl) {
-        return !isCachedUpdate(uidl) && !uidl.getBooleanAttribute("invisible");
+        return !isCachedUpdate(uidl) && !uidl.getBooleanAttribute("invisible")
+                && !uidl.hasAttribute("deferred");
     }
 
     protected static boolean isCachedUpdate(UIDL uidl) {

src/com/vaadin/terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java

@@ -15,25 +15,24 @@ public abstract class CellBasedLayoutPaintable extends
     public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {
         getWidgetForPaintable().client = client;
 
-        // Only non-cached UIDL:s can introduce changes
-        if (isCachedUpdate(uidl)) {
-            return;
+        if (isRealUpdate(uidl)) {
+            /**
+             * Margin and spacing detection depends on classNames and must be
+             * set before setting size. Here just update the details from UIDL
+             * and from overridden setStyleName run actual margin detections.
+             */
+            updateMarginAndSpacingInfo(uidl);
         }
 
-        /**
-         * Margin and spacind detection depends on classNames and must be set
-         * before setting size. Here just update the details from UIDL and from
-         * overridden setStyleName run actual margin detections.
-         */
-        updateMarginAndSpacingInfo(uidl);
-
         /*
          * This call should be made first. Ensure correct implementation, handle
          * size etc.
          */
         super.updateFromUIDL(uidl, client);
 
-        handleDynamicDimensions(uidl);
+        if (isRealUpdate(uidl)) {
+            handleDynamicDimensions(uidl);
+        }
     }
 
     private void handleDynamicDimensions(UIDL uidl) {
@@ -58,18 +57,15 @@ public abstract class CellBasedLayoutPaintable extends
     }
 
     void updateMarginAndSpacingInfo(UIDL uidl) {
-        if (!uidl.hasAttribute("invisible")) {
-            int bitMask = uidl.getIntAttribute("margins");
-            if (getWidgetForPaintable().activeMarginsInfo.getBitMask() != bitMask) {
-                getWidgetForPaintable().activeMarginsInfo = new VMarginInfo(
-                        bitMask);
-                getWidgetForPaintable().marginsNeedsRecalculation = true;
-            }
-            boolean spacing = uidl.getBooleanAttribute("spacing");
-            if (spacing != getWidgetForPaintable().spacingEnabled) {
-                getWidgetForPaintable().marginsNeedsRecalculation = true;
-                getWidgetForPaintable().spacingEnabled = spacing;
-            }
+        int bitMask = uidl.getIntAttribute("margins");
+        if (getWidgetForPaintable().activeMarginsInfo.getBitMask() != bitMask) {
+            getWidgetForPaintable().activeMarginsInfo = new VMarginInfo(bitMask);
+            getWidgetForPaintable().marginsNeedsRecalculation = true;
+        }
+        boolean spacing = uidl.getBooleanAttribute("spacing");
+        if (spacing != getWidgetForPaintable().spacingEnabled) {
+            getWidgetForPaintable().marginsNeedsRecalculation = true;
+            getWidgetForPaintable().spacingEnabled = spacing;
         }
     }
 

src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java

@@ -856,12 +856,13 @@ public abstract class AbstractCommunicationManager implements
                 // rendered already (changes with only cached flag)
                 if (paintTarget.needsToBePainted(p)) {
                     paintTarget.startTag("change");
-                    paintTarget.addAttribute("format", "uidl");
                     final String pid = getPaintableId(p);
                     paintTarget.addAttribute("pid", pid);
 
-                    // TODO this should paint subcomponents as references and
-                    // defer painting their contents to another top-level change
+                    // paints subcomponents as references (via
+                    // JsonPaintTarget.startPaintable()) and defers painting
+                    // their contents to another top-level change (via
+                    // queuePaintable())
                     p.paint(paintTarget);
 
                     paintTarget.endTag("change");

src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java

@@ -11,6 +11,7 @@ import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.io.Serializable;
 import java.io.StringWriter;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -90,6 +91,8 @@ public class JsonPaintTarget implements PaintTarget {
 
     private Collection<Paintable> identifiersCreatedDueRefPaint;
 
+    private Collection<Paintable> deferredPaintables;
+
     private final Collection<Class<? extends Paintable>> usedPaintableTypes = new LinkedList<Class<? extends Paintable>>();
 
     /**
@@ -120,6 +123,8 @@ public class JsonPaintTarget implements PaintTarget {
         openPaintables = new Stack<Paintable>();
         openPaintableTags = new Stack<String>();
 
+        deferredPaintables = new ArrayList<Paintable>();
+
         cacheEnabled = cachingRequired;
     }
 
@@ -677,37 +682,46 @@ public class JsonPaintTarget implements PaintTarget {
     /*
      * (non-Javadoc)
      * 
-     * @see com.vaadin.terminal.PaintTarget#startTag(com.vaadin.terminal
+     * @see com.vaadin.terminal.PaintTarget#startPaintable(com.vaadin.terminal
      * .Paintable, java.lang.String)
      */
-    public boolean startPaintable(Paintable paintable, String tagName)
+    public PaintStatus startPaintable(Paintable paintable, String tagName)
             throws PaintException {
         startTag(tagName, true);
         final boolean isPreviouslyPainted = manager.hasPaintableId(paintable)
                 && (identifiersCreatedDueRefPaint == null || !identifiersCreatedDueRefPaint
-                        .contains(paintable));
+                        .contains(paintable))
+                && !deferredPaintables.contains(paintable);
         final String id = manager.getPaintableId(paintable);
         paintable.addListener(manager);
         addAttribute("id", id);
 
-        // TODO if to queue if already painting a paintable
-        // if (openPaintables.isEmpty()) {
+        // queue for painting later if already painting a paintable
+        boolean topLevelPaintableTag = openPaintables.isEmpty();
+
         openPaintables.push(paintable);
         openPaintableTags.push(tagName);
 
-        paintedComponents.add(paintable);
-        // } else {
-        // // notify manager: add to paint queue instead of painting now
-        // manager.queuePaintable(paintable);
-        // // TODO return suitable value to defer painting and close tag if a
-        // // paintable tag is already open
-        // }
+        if (cacheEnabled && isPreviouslyPainted) {
+            // cached (unmodified) paintable, paint the it now
+            paintedComponents.add(paintable);
+            deferredPaintables.remove(paintable);
+            return PaintStatus.CACHED;
+        } else if (!topLevelPaintableTag) {
+            // notify manager: add to paint queue instead of painting now
+            manager.queuePaintable(paintable);
+            deferredPaintables.add(paintable);
+            return PaintStatus.DEFER;
+        } else {
+            // not a nested paintable, paint the it now
+            paintedComponents.add(paintable);
+            deferredPaintables.remove(paintable);
 
-        if (paintable instanceof CustomLayout) {
-            customLayoutArgumentsOpen = true;
+            if (paintable instanceof CustomLayout) {
+                customLayoutArgumentsOpen = true;
+            }
+            return PaintStatus.PAINTING;
         }
-
-        return cacheEnabled && isPreviouslyPainted;
     }
 
     public void endPaintable(Paintable paintable) throws PaintException {

src/com/vaadin/ui/AbstractComponent.java

@@ -27,6 +27,7 @@ import com.vaadin.event.ShortcutListener;
 import com.vaadin.terminal.ErrorMessage;
 import com.vaadin.terminal.PaintException;
 import com.vaadin.terminal.PaintTarget;
+import com.vaadin.terminal.PaintTarget.PaintStatus;
 import com.vaadin.terminal.Resource;
 import com.vaadin.terminal.Terminal;
 import com.vaadin.terminal.gwt.client.ComponentState;
@@ -753,9 +754,16 @@ public abstract class AbstractComponent implements Component, MethodEventSource
      */
     public void paint(PaintTarget target) throws PaintException {
         final String tag = target.getTag(this);
-        if (!target.startPaintable(this, tag)
-                || repaintRequestListenersNotified) {
-
+        final PaintStatus status = target.startPaintable(this, tag);
+        if (PaintStatus.DEFER == status) {
+            // nothing to do but flag as deferred and close the paintable tag
+            // paint() will be called again later to paint the contents
+            target.addAttribute("deferred", true);
+        } else if (PaintStatus.CACHED == status
+                && !repaintRequestListenersNotified) {
+            // Contents have not changed, only cached presentation can be used
+            target.addAttribute("cached", true);
+        } else {
             // Paint the contents of the component
 
             // Only paint content of visible components.
@@ -809,10 +817,6 @@ public abstract class AbstractComponent implements Component, MethodEventSource
             } else {
                 target.addAttribute("invisible", true);
             }
-        } else {
-
-            // Contents have not changed, only cached presentation can be used
-            target.addAttribute("cached", true);
         }
         target.endPaintable(this);