Cleanup, javadoc etc. for shared state serialization (#8304).

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

@@ -17,8 +17,14 @@ public class ComponentState extends SharedState {
     private String height = "";
     private String width = "";
 
-    // TODO more javadoc
-
+    /**
+     * Returns the component height as set by the server.
+     * 
+     * Can be relative (containing the percent sign) or absolute, or empty
+     * string for undefined height.
+     * 
+     * @return component height as defined by the server, not null
+     */
     public String getHeight() {
         if (height == null) {
             return "";
@@ -26,14 +32,37 @@ public class ComponentState extends SharedState {
         return height;
     }
 
+    /**
+     * Sets the height of the component in the server format.
+     * 
+     * Can be relative (containing the percent sign) or absolute, or null or
+     * empty string for undefined height.
+     * 
+     * @param height
+     *            component height
+     */
     public void setHeight(String height) {
         this.height = height;
     }
 
+    /**
+     * Returns true if the component height is undefined, false if defined
+     * (absolute or relative).
+     * 
+     * @return true if component height is undefined
+     */
     public boolean isUndefinedHeight() {
         return "".equals(getHeight());
     }
 
+    /**
+     * Returns the component width as set by the server.
+     * 
+     * Can be relative (containing the percent sign) or absolute, or empty
+     * string for undefined height.
+     * 
+     * @return component width as defined by the server, not null
+     */
     public String getWidth() {
         if (width == null) {
             return "";
@@ -41,20 +70,30 @@ public class ComponentState extends SharedState {
         return width;
     }
 
+    /**
+     * Sets the width of the component in the server format.
+     * 
+     * Can be relative (containing the percent sign) or absolute, or null or
+     * empty string for undefined width.
+     * 
+     * @param width
+     *            component width
+     */
     public void setWidth(String width) {
         this.width = width;
     }
 
+    /**
+     * Returns true if the component width is undefined, false if defined
+     * (absolute or relative).
+     * 
+     * @return true if component width is undefined
+     */
     public boolean isUndefinedWidth() {
         return "".equals(getWidth());
     }
 
-    // TODO constants for the state attributes for now
-    public static final String STATE_STYLE = "style";
-    public static final String STATE_READONLY = "readonly";
-    public static final String STATE_IMMEDIATE = "immediate";
-    public static final String STATE_DISABLED = "disabled";
-    public static final String STATE_CAPTION = "caption";
-    public static final String STATE_DESCRIPTION = "description";
+    // TODO more fields to move here: style, readonly, immediate, disabled,
+    // caption and description
 
 }

src/com/vaadin/terminal/gwt/client/communication/JsonDecoder.java

@@ -82,6 +82,7 @@ public class JsonDecoder {
             // object, class name as type
             VaadinSerializer serializer = serializerMap
                     .getSerializer(variableType);
+            // TODO handle case with no serializer found
             Object object = serializer
                     .deserialize((JSONObject) value, idMapper);
             return object;

src/com/vaadin/terminal/gwt/client/communication/SerializerMap.java

@@ -1,7 +1,34 @@
+/*
+@VaadinApache2LicenseForJavaFiles@
+ */
+
 package com.vaadin.terminal.gwt.client.communication;
 
+import com.vaadin.terminal.gwt.widgetsetutils.SerializerMapGenerator;
+
+/**
+ * Provide a mapping from a type (communicated between the server and the
+ * client) and a {@link VaadinSerializer} instance.
+ * 
+ * An implementation of this class is created at GWT compilation time by
+ * {@link SerializerMapGenerator}, so this interface can be instantiated with
+ * GWT.create().
+ * 
+ * @since 7.0
+ */
 public interface SerializerMap {
 
+    /**
+     * Returns a serializer instance for a given type.
+     * 
+     * @param type
+     *            type communicated on between the server and the client
+     *            (currently fully qualified class name)
+     * @return serializer instance, not null
+     * @throws RuntimeException
+     *             if no serializer is found
+     */
+    // TODO better error handling in javadoc and in generator
     public VaadinSerializer getSerializer(String type);
 
 }

src/com/vaadin/terminal/gwt/client/communication/SharedState.java

@@ -6,7 +6,6 @@ package com.vaadin.terminal.gwt.client.communication;
 
 import java.io.Serializable;
 
-import com.google.gwt.core.client.GWT;
 import com.vaadin.terminal.gwt.client.ui.VAbstractPaintableWidget;
 
 /**
@@ -29,19 +28,14 @@ import com.vaadin.terminal.gwt.client.ui.VAbstractPaintableWidget;
  * necessary (changed or missing on the client side) parts are re-sent to the
  * client, but the client will have access to the whole state.
  * 
- * TODO the rest of the javadoc corresponds to the design that is not yet
- * implemented
- * 
  * A shared state class should be a bean with getters and setters for each
  * field, and should only contain simple data types, or arrays or maps of
  * supported data types.
  * 
- * On the client side, SharedState instances must be created using
- * {@link GWT#create(Class)} to let a generator create custom deserialization
- * support for them. For most widgets,
- * {@link VAbstractPaintableWidget#createSharedState()} method should be
- * overridden to create a shared state instance of the correct type using
- * {@link GWT#create(Class)}.
+ * On the client side, for most widgets,
+ * {@link VAbstractPaintableWidget#createState()} and
+ * {@link VAbstractPaintableWidget#getState()} methods should be overridden to
+ * create and use a shared state instance of the correct type.
  * 
  * Subclasses of a paintable using shared state should also provide a subclass
  * of the shared state class of the parent class to extend the state - a single

src/com/vaadin/terminal/gwt/client/communication/VaadinSerializer.java

@@ -1,10 +1,32 @@
+/*
+@VaadinApache2LicenseForJavaFiles@
+ */
+
 package com.vaadin.terminal.gwt.client.communication;
 
 import com.google.gwt.json.client.JSONObject;
 import com.vaadin.terminal.gwt.client.VPaintableMap;
 
+/**
+ * Serializer that can deserialize custom objects received from the server.
+ * 
+ * Each serializer can handle objects of a single type - see
+ * {@link SerializerMap}.
+ * 
+ * @since 7.0
+ */
 public interface VaadinSerializer {
 
+    /**
+     * Creates and deserializes an object received from the server.
+     * 
+     * @param jsonValue
+     *            JSON map from property name to property value
+     * @param idMapper
+     *            mapper from paintable id to paintable, used to decode
+     *            references to paintables
+     * @return deserialized object
+     */
     // TODO Object -> something
     Object deserialize(JSONObject jsonValue, VPaintableMap idMapper);
 

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

@@ -110,6 +110,17 @@ public abstract class VAbstractPaintableWidget implements VPaintableWidget {
         this.id = id;
     }
 
+    /**
+     * Returns the shared state object for a paintable widget.
+     * 
+     * A new state instance is created using {@link #createState()} if none has
+     * been set by the server.
+     * 
+     * If overriding this method to return a more specific type, also
+     * {@link #createState()} must be overridden.
+     * 
+     * @return current shared state (not null)
+     */
     public ComponentState getState() {
         if (state == null) {
             state = createState();
@@ -118,6 +129,17 @@ public abstract class VAbstractPaintableWidget implements VPaintableWidget {
         return state;
     }
 
+    /**
+     * Creates a new instance of a shared state object for the widget. Normally,
+     * the state instance is created by the server and sent to the client before
+     * being used - this method is used if no shared state has been sent by the
+     * server.
+     * 
+     * When overriding {@link #getState()}, also {@link #createState()} should
+     * be overridden to match it.
+     * 
+     * @return newly created component shared state instance
+     */
     protected ComponentState createState() {
         return GWT.create(ComponentState.class);
     }
@@ -349,6 +371,13 @@ public abstract class VAbstractPaintableWidget implements VPaintableWidget {
         return styleBuf.toString();
     }
 
+    /**
+     * Sets the shared state for the paintable widget.
+     * 
+     * @param new shared state (must be compatible with the return value of
+     *        {@link #getState()} - {@link ComponentState} if
+     *        {@link #getState()} is not overridden
+     */
     public final void setState(SharedState state) {
         this.state = (ComponentState) state;
     }

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

@@ -8,6 +8,7 @@ import com.google.gwt.core.client.GWT;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.terminal.gwt.client.ApplicationConnection;
+import com.vaadin.terminal.gwt.client.ComponentState;
 import com.vaadin.terminal.gwt.client.EventHelper;
 import com.vaadin.terminal.gwt.client.UIDL;
 
@@ -98,4 +99,9 @@ public class VButtonPaintable extends VAbstractPaintableWidget {
     public VButtonState getState() {
         return (VButtonState) super.getState();
     }
+
+    @Override
+    protected ComponentState createState() {
+        return GWT.create(VButtonState.class);
+    }
 }

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

@@ -1,23 +1,63 @@
+/*
+@VaadinApache2LicenseForJavaFiles@
+ */
+
 package com.vaadin.terminal.gwt.client.ui;
 
 import com.vaadin.terminal.gwt.client.ComponentState;
+import com.vaadin.ui.Button;
 
+/**
+ * Shared state for Button and NativeButton.
+ * 
+ * @see ComponentState
+ * 
+ * @since 7.0
+ */
 public class VButtonState extends ComponentState {
     private boolean disableOnClick = false;
     private int clickShortcutKeyCode = 0;
 
+    /**
+     * Checks whether the button should be disabled on the client side on next
+     * click.
+     * 
+     * @return true if the button should be disabled on click
+     */
     public boolean isDisableOnClick() {
         return disableOnClick;
     }
 
+    /**
+     * Sets whether the button should be disabled on the client side on next
+     * click.
+     * 
+     * @param disableOnClick
+     *            true if the button should be disabled on click
+     */
     public void setDisableOnClick(boolean disableOnClick) {
         this.disableOnClick = disableOnClick;
     }
 
+    /**
+     * Returns the key code for activating the button via a keyboard shortcut.
+     * 
+     * See {@link Button#setClickShortcut(int, int...)} for more information.
+     * 
+     * @return key code or 0 for none
+     */
     public int getClickShortcutKeyCode() {
         return clickShortcutKeyCode;
     }
 
+    /**
+     * Sets the key code for activating the button via a keyboard shortcut.
+     * 
+     * See {@link Button#setClickShortcut(int, int...)} for more information.
+     * 
+     * @param clickShortcutKeyCode
+     *            key code or 0 for none
+     */
     public void setClickShortcutKeyCode(int clickShortcutKeyCode) {
         this.clickShortcutKeyCode = clickShortcutKeyCode;
     }

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

@@ -940,20 +940,13 @@ public class VGridLayout extends SimplePanel implements Container {
             if (state != null && !cached) {
                 boolean widthDefined = !state.isUndefinedWidth();
                 boolean heightDefined = !state.isUndefinedHeight();
-                if (heightDefined && state.getHeight().contains("%")) {
-                    relHeight = true;
-                } else {
-                    relHeight = false;
-                }
+
+                relHeight = state.getHeight().contains("%");
+                relWidth = state.getWidth().contains("%");
                 if (widthDefined) {
-                    widthCanAffectHeight = relWidth = state.getWidth()
-                            .contains("%");
-                    if (heightDefined) {
-                        widthCanAffectHeight = false;
-                    }
+                    widthCanAffectHeight = (relWidth && !heightDefined);
                 } else {
                     widthCanAffectHeight = !heightDefined;
-                    relWidth = false;
                 }
             }
         }

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

@@ -7,6 +7,7 @@ import com.google.gwt.core.client.GWT;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.terminal.gwt.client.ApplicationConnection;
+import com.vaadin.terminal.gwt.client.ComponentState;
 import com.vaadin.terminal.gwt.client.EventHelper;
 import com.vaadin.terminal.gwt.client.UIDL;
 
@@ -94,4 +95,9 @@ public class VNativeButtonPaintable extends VAbstractPaintableWidget {
     public VButtonState getState() {
         return (VButtonState) super.getState();
     }
+
+    @Override
+    protected ComponentState createState() {
+        return GWT.create(VButtonState.class);
+    }
 }

src/com/vaadin/terminal/gwt/widgetsetutils/SerializerGenerator.java

@@ -1,3 +1,7 @@
+/*
+@VaadinApache2LicenseForJavaFiles@
+ */
+
 package com.vaadin.terminal.gwt.widgetsetutils;
 
 import java.io.PrintWriter;
@@ -22,6 +26,14 @@ import com.vaadin.terminal.gwt.client.VPaintableMap;
 import com.vaadin.terminal.gwt.client.communication.JsonDecoder;
 import com.vaadin.terminal.gwt.client.communication.VaadinSerializer;
 
+/**
+ * GWT generator for creating serializer classes for custom classes sent from
+ * server to client.
+ * 
+ * Only fields with a correspondingly named setter are deserialized.
+ * 
+ * @since 7.0
+ */
 public class SerializerGenerator extends Generator {
 
     private String packageName;
@@ -50,13 +62,16 @@ public class SerializerGenerator extends Generator {
     }
 
     /**
-     * Generate source code for WidgetMapImpl
+     * Generate source code for a VaadinSerializer implementation.
      * 
      * @param logger
      *            Logger object
      * @param context
      *            Generator context
-     * @param typeName
+     * @param beanTypeName
+     *            bean type for which the serializer is to be generated
+     * @param beanSerializerTypeName
+     *            name of the serializer class to generate
      */
     private void generateClass(TreeLogger logger, GeneratorContext context,
             String beanTypeName, String beanSerializerTypeName) {
@@ -121,6 +136,7 @@ public class SerializerGenerator extends Generator {
             // JSONArray jsonHeight = (JSONArray) jsonValue.get("height");
             sourceWriter.println("JSONArray " + jsonFieldName
                     + " = (JSONArray) jsonValue.get(\"" + fieldName + "\");");
+
             // state.setHeight((String)
             // JsonDecoder.convertValue(jsonFieldValue,idMapper));
 

src/com/vaadin/terminal/gwt/widgetsetutils/SerializerMapGenerator.java

@@ -1,3 +1,7 @@
+/*
+@VaadinApache2LicenseForJavaFiles@
+ */
+
 package com.vaadin.terminal.gwt.widgetsetutils;
 
 import java.io.PrintWriter;
@@ -16,6 +20,13 @@ import com.vaadin.terminal.gwt.client.communication.SerializerMap;
 import com.vaadin.terminal.gwt.client.communication.SharedState;
 import com.vaadin.terminal.gwt.client.communication.VaadinSerializer;
 
+/**
+ * GWT generator that creates a {@link SerializerMap} implementation (mapper
+ * from type string to serializer instance) and serializer classes for all
+ * subclasses of {@link SharedState}.
+ * 
+ * @since 7.0
+ */
 public class SerializerMapGenerator extends Generator {
 
     private String packageName;
@@ -32,9 +43,10 @@ public class SerializerMapGenerator extends Generator {
             JClassType classType = typeOracle.getType(typeName);
             packageName = classType.getPackage().getName();
             className = classType.getSimpleSourceName() + "Impl";
-            // Generate class source code
+            // Generate class source code for SerializerMapImpl
             generateClass(logger, context);
 
+            // Generate serializer classes for each subclass of SharedState
             JClassType serializerType = typeOracle.findType(SharedState.class
                     .getName());
             JClassType[] serializerSubtypes = serializerType.getSubtypes();
@@ -87,6 +99,7 @@ public class SerializerMapGenerator extends Generator {
                 + " getSerializer(String type) {");
         sourceWriter.indent();
 
+        // TODO cache serializer instances in a map
         for (JClassType type : serializerSubtypes) {
             sourceWriter.println("if (type.equals(\""
                     + type.getQualifiedSourceName() + "\")) {");

src/com/vaadin/ui/AbstractComponent.java

@@ -890,7 +890,6 @@ public abstract class AbstractComponent implements Component, MethodEventSource
 
         // TODO for now, this superclass always recreates the state from
         // scratch, whereas subclasses should only modify it
-        // Map<String, Object> state = new HashMap<String, Object>();
 
         if (getHeight() >= 0
                 && (getHeightUnits() != Unit.PERCENTAGE || ComponentSizeValidator
@@ -927,8 +926,6 @@ public abstract class AbstractComponent implements Component, MethodEventSource
         // if (getDescription() != null && getDescription().length() > 0) {
         // state.put(ComponentState.STATE_DESCRIPTION, getDescription());
         // }
-        //
-        // sharedState.setState(state);
 
         return sharedState;
     }