From 629c9f98ea1ff1428a7af44791483c9d290aa1b7 Mon Sep 17 00:00:00 2001 From: Mathieu Baudier Date: Sun, 24 Sep 2023 07:15:45 +0200 Subject: [PATCH] Make JavaScript parts initialisation more robust and predictable --- .../org/argeo/app/geo/swt/SwtJsMapPart.java | 20 +++-- .../argeo/app/swt/chart/AbstractJsChart.java | 4 + .../argeo/app/swt/chart/SwtJsBarChart.java | 10 +-- .../argeo/app/swt/js/SwtBrowserJsPart.java | 76 +++++++++++++++---- 4 files changed, 82 insertions(+), 28 deletions(-) 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 5e90d39..8b557cb 100644 --- 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 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 f114621..8cae325 100644 --- 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 332d0eb..36d80dc 100644 --- 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 fac099f..f479f96 100644 --- 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 pageLoaded = new CompletableFuture<>(); + private final CompletableFuture 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> 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 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 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 evaluate(String js, Object... args) { - CompletableFuture 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 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 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; } -- 2.30.2