From 9dce2937d73e455bca9d5162b61b48d14eb3f132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pekka=20Hyv=C3=B6nen?= Date: Mon, 4 Jul 2016 16:00:53 +0300 Subject: [PATCH] Bind nodes deferredly (#1119) Closes #1051, Fixes #1034 --- .../vaadin/client/hummingbird/StateTree.java | 28 ++++++++++++- .../hummingbird/TreeChangeProcessor.java | 41 +++++++++++-------- .../client/hummingbird/binding/Binder.java | 3 ++ .../ElementTemplateBindingStrategy.java | 7 +++- .../template/GwtTemplateBinderTest.java | 5 +++ .../client/hummingbird/StateTreeTest.java | 9 ++++ 6 files changed, 74 insertions(+), 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 index d1ca01f81a1..456272785fa 100644 --- 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 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 + * true if the tree is being updated, + * false 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 true if being updated, false 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 index 7d308f26693..6f53f59a49d 100644 --- 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,7 +38,7 @@ 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 @@ -46,26 +46,35 @@ private TreeChangeProcessor() { * 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 index 851f234a0d3..d27e8386c8e 100644 --- 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/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/template/ElementTemplateBindingStrategy.java b/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/template/ElementTemplateBindingStrategy.java index 46ce7c05536..b3b7ad7c4f0 100644 --- a/hummingbird-client/src/main/java/com/vaadin/client/hummingbird/template/ElementTemplateBindingStrategy.java +++ b/hummingbird-client/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/hummingbird-client/src/test-gwt/java/com/vaadin/client/hummingbird/template/GwtTemplateBinderTest.java b/hummingbird-client/src/test-gwt/java/com/vaadin/client/hummingbird/template/GwtTemplateBinderTest.java index 79a2d3fe719..b393093b7f9 100644 --- a/hummingbird-client/src/test-gwt/java/com/vaadin/client/hummingbird/template/GwtTemplateBinderTest.java +++ b/hummingbird-client/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 index 2376ea0f19c..c4584493e13 100644 --- 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); + } + }