Make JavaScript parts initialisation more robust and predictable

diff --git a/swt/org.argeo.app.geo.swt/src/org/argeo/app/geo/swt/SwtJsMapPart.java b/swt/org.argeo.app.geo.swt/src/org/argeo/app/geo/swt/SwtJsMapPart.java
index 5e90d3991943eea49c4a091cab5b3d78ff3fd7a1..8b557cb26ff30b782d0d2c1f2803916efba1121e 100644 (file)
--- a/swt/org.argeo.app.geo.swt/src/org/argeo/app/geo/swt/SwtJsMapPart.java
+++ b/swt/org.argeo.app.geo.swt/src/org/argeo/app/geo/swt/SwtJsMapPart.java
@@ -1,8 +1,8 @@
 package org.argeo.app.geo.swt;
 
-import java.util.concurrent.CompletionStage;
 import java.util.function.Consumer;
 import java.util.function.Function;
+
 import org.argeo.app.geo.GeoUtils;
 import org.argeo.app.geo.ux.JsImplementation;
 import org.argeo.app.geo.ux.MapPart;
@@ -35,33 +35,37 @@ public class SwtJsMapPart extends SwtBrowserJsPart implements MapPart {
 
        @Override
        public void addPoint(double lng, double lat, String style) {
-               callMapMethod("addPoint(%f, %f, %s)", lng, lat, style == null ? "'default'" : style);
+               executeMapMethod("addPoint(%f, %f, %s)", lng, lat, style == null ? "'default'" : style);
        }
 
        @Override
        public void addUrlLayer(String url, GeoFormat format, String style) {
-               callMapMethod("addUrlLayer('%s', '%s', %s, false)", url, format.name(), style);
+               executeMapMethod("addUrlLayer('%s', '%s', %s, false)", url, format.name(), style);
        }
 
        public void addCssUrlLayer(String url, GeoFormat format, String css) {
                String style = GeoUtils.createSldFromCss("layer", "Layer", css);
-               callMapMethod("addUrlLayer('%s', '%s', '%s', true)", url, format.name(), style);
+               executeMapMethod("addUrlLayer('%s', '%s', '%s', true)", url, format.name(), style);
        }
 
        @Override
        public void setZoom(int zoom) {
-               callMapMethod("setZoom(%d)", zoom);
+               executeMapMethod("setZoom(%d)", zoom);
        }
 
        @Override
        public void setCenter(double lng, double lat) {
-               callMapMethod("setCenter(%f, %f)", lng, lat);
+               executeMapMethod("setCenter(%f, %f)", lng, lat);
        }
 
-       protected CompletionStage<Object> callMapMethod(String methodCall, Object... args) {
+       protected Object callMapMethod(String methodCall, Object... args) {
                return callMethod(getJsMapVar(), methodCall, args);
        }
 
+       protected void executeMapMethod(String methodCall, Object... args) {
+               executeMethod(getJsMapVar(), methodCall, args);
+       }
+
        private String getJsMapVar() {
                return getJsVarName(mapName);
        }
@@ -93,7 +97,7 @@ public class SwtJsMapPart extends SwtBrowserJsPart implements MapPart {
                getReadyStage().thenAccept((ready) -> {
                        String functionName = createJsFunction(mapName + "__on" + suffix, toDo);
                        doExecute(getJsMapVar() + ".callbacks['on" + suffix + "']=" + functionName + ";");
-                       callMethod(mapName, "enable" + suffix + "()");
+                       executeMethod(mapName, "enable" + suffix + "()");
                });
        }
 }

diff --git a/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/AbstractJsChart.java b/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/AbstractJsChart.java
index f1146210c0ff6248f67baee7b7a0fb2373950995..8cae3259d0fa3162ceddd0a1dcad7ef2ccc62d10 100644 (file)
--- a/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/AbstractJsChart.java
+++ b/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/AbstractJsChart.java
@@ -24,4 +24,8 @@ public abstract class AbstractJsChart extends SwtBrowserJsPart {
                return getJsVarName(chartName);
        }
 
+       protected void executeChartMethod(String methodCall, Object... args) {
+               executeMethod(getJsChartVar(), methodCall, args);
+       }
+
 }

diff --git a/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/SwtJsBarChart.java b/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/SwtJsBarChart.java
index 332d0eb3b7898e4b1694dc2244dc09055ddb47fb..36d80dc8f1bfd29952a1b79a7b2288aca85490ee 100644 (file)
--- a/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/SwtJsBarChart.java
+++ b/swt/org.argeo.app.swt/src/org/argeo/app/swt/chart/SwtJsBarChart.java
@@ -19,19 +19,19 @@ public class SwtJsBarChart extends AbstractJsChart {
        }
 
        public void setLabels(String[] labels) {
-               callMethod(getJsChartVar(), "setLabels(%s)", toJsArray(labels));
+               executeChartMethod("setLabels(%s)", toJsArray(labels));
        }
 
        public void addDataset(String label, int[] values) {
-               callMethod(getJsChartVar(), "addDataset('%s', %s)", label, toJsArray(values));
+               executeChartMethod("addDataset('%s', %s)", label, toJsArray(values));
        }
 
        public void setData(String[] labels, String label, int[] values) {
-               callMethod(getJsChartVar(), "setData(%s, '%s', %s)", toJsArray(labels), label, toJsArray(values));
+               executeChartMethod("setData(%s, '%s', %s)", toJsArray(labels), label, toJsArray(values));
        }
 
        public void setDatasets(String[] labels, String[] label, int[][] values) {
-               callMethod(getJsChartVar(), "setDatasets(%s, %s)", toJsArray(labels), toDatasets(label, values));
+               executeChartMethod("setDatasets(%s, %s)", toJsArray(labels), toDatasets(label, values));
        }
 
        protected String toDatasets(String[] label, int[][] values) {
@@ -56,6 +56,6 @@ public class SwtJsBarChart extends AbstractJsChart {
        }
 
        public void clearDatasets() {
-               callMethod(getJsChartVar(), "clearDatasets()");
+               executeChartMethod("clearDatasets()");
        }
 }

diff --git a/swt/org.argeo.app.swt/src/org/argeo/app/swt/js/SwtBrowserJsPart.java b/swt/org.argeo.app.swt/src/org/argeo/app/swt/js/SwtBrowserJsPart.java
index fac099ffe36bfa65bb746509bdd5874d62071815..f479f962d5ecbba6922058202efe6c561e8b4dbe 100644 (file)
--- a/swt/org.argeo.app.swt/src/org/argeo/app/swt/js/SwtBrowserJsPart.java
+++ b/swt/org.argeo.app.swt/src/org/argeo/app/swt/js/SwtBrowserJsPart.java
@@ -1,11 +1,14 @@
 package org.argeo.app.swt.js;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Locale;
 import java.util.StringJoiner;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
 import java.util.function.Function;
+import java.util.function.Supplier;
 
 import org.argeo.api.cms.CmsLog;
 import org.eclipse.swt.SWT;
@@ -17,6 +20,7 @@ import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
 
 /**
  * A part using a {@link Browser} and remote JavaScript components on the client
@@ -28,7 +32,13 @@ public class SwtBrowserJsPart {
        private final static String GLOBAL_THIS_ = "globalThis.";
 
        private final Browser browser;
-       private final CompletableFuture<Boolean> pageLoaded = new CompletableFuture<>();
+       private final CompletableFuture<Boolean> readyStage = new CompletableFuture<>();
+
+       /**
+        * Tasks that were requested before the context was ready. Typically
+        * configuration methods on the part while the user interfaces is being build.
+        */
+       private List<Supplier<Boolean>> preReadyToDos = new ArrayList<>();
 
        public SwtBrowserJsPart(Composite parent, int style, String url) {
                this.browser = new Browser(parent, 0);
@@ -45,10 +55,17 @@ public class SwtBrowserJsPart {
                                try {
                                        init();
                                        loadExtensions();
-                                       pageLoaded.complete(true);
+                                       // execute todos in order
+                                       for (Supplier<Boolean> toDo : preReadyToDos) {
+                                               boolean success = toDo.get();
+                                               if (!success)
+                                                       throw new IllegalStateException("Post-initalisation JavaScript execution failed");
+                                       }
+                                       preReadyToDos.clear();
+                                       readyStage.complete(true);
                                } catch (Exception e) {
                                        log.error("Cannot initialise " + url + " in browser", e);
-                                       pageLoaded.complete(false);
+                                       readyStage.complete(false);
                                }
                        }
 
@@ -62,7 +79,12 @@ public class SwtBrowserJsPart {
         * LIFECYCLE
         */
 
-       /** Called when the page has been loaded. */
+       /**
+        * Called when the page has been loaded, typically in order to initialise
+        * JavaScript objects. One MUST use {@link #doExecute(String, Object...)} in
+        * order to do so, since the context is not yet considered ready and calls to
+        * {@link #evaluate(String, Object...)} will block.
+        */
        protected void init() {
        }
 
@@ -82,7 +104,7 @@ public class SwtBrowserJsPart {
        }
 
        protected CompletionStage<Boolean> getReadyStage() {
-               return pageLoaded.minimalCompletionStage();
+               return readyStage.minimalCompletionStage();
        }
 
        /*
@@ -99,14 +121,30 @@ public class SwtBrowserJsPart {
         * @param args the optional arguments of
         *             {@link String#format(String, Object...)}
         */
-       protected CompletionStage<Object> evaluate(String js, Object... args) {
-               CompletableFuture<Object> res = pageLoaded.thenApply((ready) -> {
-                       if (!ready)
-                               throw new IllegalStateException("Component is not initialised.");
-                       Object result = browser.evaluate(String.format(Locale.ROOT, js, args));
-                       return result;
-               });
-               return res.minimalCompletionStage();
+       protected Object evaluate(String js, Object... args) {
+               assert browser.getDisplay().equals(Display.findDisplay(Thread.currentThread())) : "Not the proper UI thread.";
+               if (!readyStage.isDone())
+                       throw new IllegalStateException("Methods returning a result can only be called after UI initilaisation.");
+               // wait for the context to be ready
+//             boolean ready = readyStage.join();
+//             if (!ready)
+//                     throw new IllegalStateException("Component is not initialised.");
+               Object result = browser.evaluate(String.format(Locale.ROOT, js, args));
+               return result;
+       }
+
+       protected void execute(String js, Object... args) {
+               if (readyStage.isDone()) {
+                       boolean success = browser.execute(String.format(Locale.ROOT, js, args));
+                       if (!success)
+                               throw new RuntimeException("JavaScript execution failed.");
+               } else {
+                       Supplier<Boolean> toDo = () -> {
+                               boolean success = browser.execute(String.format(Locale.ROOT, js, args));
+                               return success;
+                       };
+                       preReadyToDos.add(toDo);
+               }
        }
 
        /** @return the globally usable function name. */
@@ -124,15 +162,23 @@ public class SwtBrowserJsPart {
                return "window." + name;
        }
 
-       /** Directly executes */
+       /**
+        * Directly executes, even if {@link #getReadyStage()} is not completed. Except
+        * in initialisation, {@link #evaluate(String, Object...)} should be used
+        * instead.
+        */
        protected void doExecute(String js, Object... args) {
                browser.execute(String.format(Locale.ROOT, js, args));
        }
 
-       protected CompletionStage<Object> callMethod(String jsObject, String methodCall, Object... args) {
+       protected Object callMethod(String jsObject, String methodCall, Object... args) {
                return evaluate(jsObject + '.' + methodCall, args);
        }
 
+       protected void executeMethod(String jsObject, String methodCall, Object... args) {
+               execute(jsObject + '.' + methodCall, args);
+       }
+
        protected String getJsVarName(String name) {
                return GLOBAL_THIS_ + name;
        }