Bind nodes deferredly (#1119)

Closes #1051, Fixes #1034
vaadin · Jul 4, 2016 · 9dce293 · 9dce293
1 parent 5509195
commit 9dce293
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 19 deletions.
diff --git a/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/StateTree.java b/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/StateTree.java
@@ -40,6 +40,8 @@ public class StateTree {
 
     private JsMap<Integer, String> nodeFeatureDebugName;
 
+    private boolean updateInProgress;
+
     /**
      * Creates a new instance connected to the given registry.
      *
@@ -51,11 +53,35 @@ public StateTree(Registry registry) {
         registerNode(rootNode);
     }
 
+    /**
+     * Mark this tree as being updated.
+     *
+     * @param updateInProgress
+     *            <code>true</code> if the tree is being updated,
+     *            <code>false</code> if not
+     * @see #isUpdateInProgress()
+     */
+    public void setUpdateInProgress(boolean updateInProgress) {
+        assert this.updateInProgress != updateInProgress : "Inconsistent state tree updating status, expected "
+                + (updateInProgress ? "no " : "") + " updates in progress.";
+        this.updateInProgress = updateInProgress;
+    }
+
+    /**
+     * Returns whether this tree is currently being updated by
+     * {@link TreeChangeProcessor#processChanges(StateTree, JsonArray)}.
+     *
+     * @return <code>true</code> if being updated, <code>false</code> if not
+     */
+    public boolean isUpdateInProgress() {
+        return updateInProgress;
+    }
+
     /**
      * Registers a node with this tree.
      *
      * @param node
-     *            the node to regsiter
+     *            the node to register
      */
     public final void registerNode(StateNode node) {
         assert node != null;

diff --git a/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/TreeChangeProcessor.java b/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/TreeChangeProcessor.java
@@ -38,34 +38,43 @@ private TreeChangeProcessor() {
     }
 
     /**
-     * Update a state tree based on an JSON array of changes.
+     * Update a state tree based on a JSON array of changes.
      *
      * @param tree
      *            the tree to update
      * @param changes
      *            the JSON array of changes
      */
     public static void processChanges(StateTree tree, JsonArray changes) {
-        int length = changes.length();
+        assert !tree
+                .isUpdateInProgress() : "Previous tree change processing has not completed";
+        try {
+            tree.setUpdateInProgress(true);
+            int length = changes.length();
 
-        // Attach all nodes before doing anything else
-        for (int i = 0; i < length; i++) {
-            JsonObject change = changes.getObject(i);
-            if (isAttach(change)) {
-                int nodeId = (int) change.getNumber(JsonConstants.CHANGE_NODE);
-
-                StateNode node = new StateNode(nodeId, tree);
-                tree.registerNode(node);
+            // Attach all nodes before doing anything else
+            for (int i = 0; i < length; i++) {
+                JsonObject change = changes.getObject(i);
+                if (isAttach(change)) {
+                    int nodeId = (int) change
+                            .getNumber(JsonConstants.CHANGE_NODE);
+
+                    StateNode node = new StateNode(nodeId, tree);
+                    tree.registerNode(node);
+                }
             }
-        }
 
-        // Then process all non-attach changes
-        for (int i = 0; i < length; i++) {
-            JsonObject change = changes.getObject(i);
-            if (!isAttach(change)) {
-                processChange(tree, change);
+            // Then process all non-attach changes
+            for (int i = 0; i < length; i++) {
+                JsonObject change = changes.getObject(i);
+                if (!isAttach(change)) {
+                    processChange(tree, change);
+                }
             }
+        } finally {
+            tree.setUpdateInProgress(false);
         }
+
     }
 
     private static boolean isAttach(JsonObject change) {

diff --git a/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/binding/Binder.java b/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/binding/Binder.java
@@ -102,6 +102,9 @@ private Binder() {
      */
     @SuppressWarnings({ "rawtypes", "unchecked" })
     public static void bind(StateNode stateNode, Node domNode) {
+        assert !stateNode.getTree()
+                .isUpdateInProgress() : "Binding state node while processing state tree changes";
+
         BindingStrategy applicable = getApplicableStrategy(stateNode);
         applicable.bind(stateNode, domNode, CONTEXT);
     }

diff --git a/.../src/main/java/com/vaadin/client/hummingbird/template/ElementTemplateBindingStrategy.java b/.../src/main/java/com/vaadin/client/hummingbird/template/ElementTemplateBindingStrategy.java
@@ -24,6 +24,7 @@
 import com.vaadin.client.hummingbird.dom.DomApi;
 import com.vaadin.client.hummingbird.nodefeature.MapProperty;
 import com.vaadin.client.hummingbird.nodefeature.NodeList;
+import com.vaadin.client.hummingbird.reactive.Reactive;
 import com.vaadin.client.hummingbird.util.NativeFunction;
 import com.vaadin.hummingbird.shared.NodeFeatures;
 
@@ -103,8 +104,10 @@ protected void bind(StateNode modelNode, Element element, int templateId,
             /*
              * React in case an override nodes appears later on.
              */
-            EventRemover remover = overrideProperty.addChangeListener(
-                    e -> bindOverrideNode(element, overrideProperty, context));
+            EventRemover remover = overrideProperty
+                    .addChangeListener(e -> Reactive
+                            .addFlushListener(() -> bindOverrideNode(element,
+                                    overrideProperty, context)));
 
             /*
              * Should preferably remove the change listener immediately when the

diff --git a/...lient/src/test-gwt/java/com/vaadin/client/hummingbird/template/GwtTemplateBinderTest.java b/...lient/src/test-gwt/java/com/vaadin/client/hummingbird/template/GwtTemplateBinderTest.java
@@ -406,10 +406,15 @@ public void testBindOverrideNodeAfterCreated() {
 
         Reactive.flush();
 
+        // tests the rare(ish) case resulting in #1050 (fixed by #1051)
+        tree.setUpdateInProgress(true);
+
         StateNode overrideNode = createSimpleOverrideNode(id);
         overrideNode.getMap(NodeFeatures.ELEMENT_PROPERTIES).getProperty("id")
                 .setValue("override");
 
+        tree.setUpdateInProgress(false);
+
         Reactive.flush();
 
         assertEquals("override", element.getId());

diff --git a/hummingbird-client/src/test/java/com/vaadin/client/hummingbird/StateTreeTest.java b/hummingbird-client/src/test/java/com/vaadin/client/hummingbird/StateTreeTest.java
@@ -20,6 +20,8 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import com.vaadin.client.hummingbird.binding.Binder;
+
 import elemental.events.EventRemover;
 
 public class StateTreeTest {
@@ -104,4 +106,11 @@ public void unregisterUnregisteredNode() {
         }
     }
 
+    @Test(expected = AssertionError.class)
+    public void testUpdatingTree_triggeringBinder_causesAssertionError() {
+        tree.registerNode(node);
+        tree.setUpdateInProgress(true);
+        Binder.bind(node, null);
+    }
+
 }