Bug 528414 (swtWaylandLauncher) Part 1.2 : Polishing of SWT GDBus

Not a prereq to launcher patch, but nice to merge.

- Mostly polish. No real functional change to existing functionality.
- Clarified code that was confusing when there was an issue.
- Added documentation with samples.
- Added/tested support for Boolean and Integer to GDBus implementation,
  I initially was planning on using them for ping method, but later
  discovered that I could use DBus's standard Peer.Ping method.
  However, Boolean/Integer handling might come in handy in the future,
  since I developed/tested it, we might as well add it.
- Minor bug-fixes here and there.

Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=528414
Change-Id: I201ce2bfd71afb563d3463f2f8ce980422423a8d
Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>

6 files changed, 147 insertions, 35 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
index d6b860ef94..21535dfb98 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
@@ -3858,6 +3858,18 @@ JNIEXPORT jdouble JNICALL OS_NATIVE(_1g_1variant_1get_1double)
 }
 #endif
 
+#ifndef NO__1g_1variant_1get_1int32
+JNIEXPORT jint JNICALL OS_NATIVE(_1g_1variant_1get_1int32)
+	(JNIEnv *env, jclass that, jintLong arg0)
+{
+	jint rc = 0;
+	OS_NATIVE_ENTER(env, that, _1g_1variant_1get_1int32_FUNC);
+	rc = (jint)g_variant_get_int32((GVariant *)arg0);
+	OS_NATIVE_EXIT(env, that, _1g_1variant_1get_1int32_FUNC);
+	return rc;
+}
+#endif
+
 #ifndef NO__1g_1variant_1get_1string
 JNIEXPORT jintLong JNICALL OS_NATIVE(_1g_1variant_1get_1string)
 	(JNIEnv *env, jclass that, jintLong arg0, jlongArray arg1)
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
index f5bc1cc4c0..2e67575423 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
@@ -388,6 +388,7 @@ char * OS_nativeFunctionNames[] = {
 	"_1g_1variant_1get_1byte",
 	"_1g_1variant_1get_1child_1value",
 	"_1g_1variant_1get_1double",
+	"_1g_1variant_1get_1int32",
 	"_1g_1variant_1get_1string",
 	"_1g_1variant_1get_1type",
 	"_1g_1variant_1get_1type_1string",
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
index 41480bd6d2..61a1ca03e1 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
@@ -398,6 +398,7 @@ typedef enum {
 	_1g_1variant_1get_1byte_FUNC,
 	_1g_1variant_1get_1child_1value_FUNC,
 	_1g_1variant_1get_1double_FUNC,
+	_1g_1variant_1get_1int32_FUNC,
 	_1g_1variant_1get_1string_FUNC,
 	_1g_1variant_1get_1type_FUNC,
 	_1g_1variant_1get_1type_1string_FUNC,
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
index db7a6fce50..126bad80b3 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
@@ -106,11 +106,19 @@ public class OS extends C {
 		}
 	}
 
+	public static final String GDBUS_SYSTEM_PROPERTY = "swt.dbus.init";
+
 	// Bug 519124
 	static {
 		String swt_lib_versions = getEnvironmentalVariable (OS.SWT_LIB_VERSIONS); // Note, this is read in multiple places.
 		if (swt_lib_versions != null && swt_lib_versions.equals("1")) {
-			System.out.println("SWT_LIB_Gtk:"+gtk_major_version()+"."+gtk_minor_version()+"."+gtk_micro_version());
+			System.out.print("SWT_LIB_Gtk:"+gtk_major_version()+"."+gtk_minor_version()+"."+gtk_micro_version());
+			if (System.getProperty(GDBUS_SYSTEM_PROPERTY) != null) {
+				System.out.print(" (DBus enabled)");
+			} else {
+				System.out.print(" (DBus dissabled)");
+			}
+			System.out.print("\n");
 		}
 	}
 
@@ -587,6 +595,8 @@ public class OS extends C {
 	/** @category gdbus */
 	public static final String DBUS_TYPE_STRING_ARRAY = "as";
 	/** @category gdbus */
+	public static final String DBUS_TYPE_INT32 = "i";
+	/** @category gdbus */
 	public static final String DBUS_TYPE_DOUBLE = "d";
 	/** @category gdbus */
 	public static final String DBUS_TYPE_STRUCT = "r"; // Not used by Dbus, but implemented by GDBus.
@@ -608,6 +618,8 @@ public class OS extends C {
 	/** @category gdbus */
 	public static final byte[] G_VARIANT_TYPE_STRING = ascii(DBUS_TYPE_STRING);
 	/** @category gdbus */
+	public static final byte[] G_VARIANT_TYPE_IN32 = ascii(DBUS_TYPE_INT32);
+	/** @category gdbus */
 	public static final byte[] G_VARIANT_TYPE_DOUBLE = ascii(DBUS_TYPE_DOUBLE);
 	/** @category gdbus */
 	public static final byte[] G_VARIANT_TYPE_TUPLE = ascii(DBUS_TYPE_STRUCT);
@@ -16782,6 +16794,23 @@ public static final long /*int*/ g_variant_new_int32 (int intval) {
 	}
 }
 
+
+/**
+ * @param gvariant cast=(GVariant *)
+ * @category gdbus
+ * @return int
+ */
+public static final native int _g_variant_get_int32 (long /*int*/ gvariant);
+/** @category gdbus */
+public static final int g_variant_get_int32 (long /*int*/ gvariant) {
+	lock.lock();
+	try {
+		return _g_variant_get_int32 (gvariant);
+	} finally {
+		lock.unlock();
+	}
+}
+
 /**
  * @param gvariant cast=(GVariant *)
  * @category gdbus
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/internal/GDBus.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/internal/GDBus.java
index 9a85c87327..e1bd5bd8c0 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/internal/GDBus.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/internal/GDBus.java
@@ -37,11 +37,15 @@ import org.eclipse.swt.internal.gtk.*;
  *    Meaning if you don't like org.eclipse.swt, you can add more session names.
  *  - This implementation is only a small subset of GDBus.
  *    E.g not all types are translated and not functionality implemented. Add them as you need them.
+ *  - At time of writing (v 1.4), only handles incoming gdbus calls. But could be easily extended to
+ *    handle outgoing gdbus calls.
  *
  * @category gdbus
  */
 public class GDBus {
 
+	final static String SWT_GDBUS_VERSION_INFO = "SWT_LIB GDbus firing up. Implementation v1.4";
+
 	public static class GDBusMethod {
 		final private String name;
 		final private Function<Object[], Object[]> userFunction;
@@ -51,9 +55,51 @@ public class GDBus {
 		 * Create a method that GDBus will listen to.
 		 *
 		 * @param name of the method. It will be part of 'org.eclipse.swt.NAME'
-		 * @param inputArgs   2D array pair of Strings :  (DBUS_TYPE_*, argument_name). Where argument_name is only so that it's seen by command line by user.
+		 * @param inputArgs   2D array pair of Strings in the format of:  (DBUS_TYPE_*, argument_name).
+		 *                    Where argument_name is only so that it's seen by command line by user.
 		 * @param outputArgs  Same as inputArgs, but for returning values.
 		 * @param userFunction  A Function<Object[],Object[]>, that you would like to run when the user calls the method over gdbus.
+		 *                      Note, input argument(s) are provided as an Object[] array. You need to cast items manually.
+		 *                      Output must always be an Object[] array or null. (E.g Object[] with only 1 element in it).
+		 *
+		 * Here are examples that you can base your methods off of:
+		 *      // Multiple input types and multiple output types.
+		 *      // From command line, it can be called like this:
+		 *      //   gdbus call --session --dest org.eclipse.swt --object-path /org/eclipse/swt --method org.eclipse.swt.typeTest true "world" 1234
+		 *      // The call will return a tuple (struct) like: (true, 'world', 5678)
+ 		 * 		GDBusMethod typeTest = new GDBusMethod(
+		 * 		"typeTest",
+		 * 		new String [][] {{OS.DBUS_TYPE_BOOLEAN, "boolean Test Val"}, {OS.DBUS_TYPE_STRING, "string Test Val"}, {OS.DBUS_TYPE_INT32, "int Test Val"}},
+		 * 		new String [][] {{OS.DBUS_TYPE_BOOLEAN, "boolean Response"}, {OS.DBUS_TYPE_STRING, "string Test Response"}, {OS.DBUS_TYPE_INT32, "int Test Response"}},
+		 * 		(args) -> {
+		 * 			System.out.println(args[0] + " " + args[1] + " " + args[2]); // A
+		 * 			return new Object[] {new Boolean(true), new String("world"), new Integer(5678)} ;
+		 * 		});
+		 *
+		 * 		// Single input and single output. Observe input is an array with one item. Output is an array with one item.
+		 * 		// It can be called from cmd via:
+		 * 		//    gdbus call --session --dest org.eclipse.swt --object-path /org/eclipse/swt --method org.eclipse.swt.singleValueTest 123
+		 * 		// The return is a tuple with one element: (456,)
+		 * 		GDBusMethod singleValTest = new GDBusMethod(
+		 * 		"singleValueTest",
+		 * 		new String[][] {{OS.DBUS_TYPE_INT32, "int val"}},
+		 * 		new String[][] {{OS.DBUS_TYPE_INT32, "int ret"}},
+		 * 		(arg) -> {
+		 * 			System.out.println("Input int: " + arg[0]);
+		 * 			return new Object[] {(Integer) 456};
+		 * 		});
+		 *
+		 *      // A simple method for clients to check if org.eclipse.swt is up and running. (getting/parsing interface xml is too tedious).
+		 *		// Reached via: gdbus call --session --dest org.eclipse.swt --object-path /org/eclipse/swt --method org.eclipse.swt.PingResponse
+		 *		// return is: (true,)    #i.e (b)
+		 * 		new GDBusMethod(
+		 *		"PingResponse",
+		 *		new String [0][0], // No input args.
+		 *		new String [][] {{OS.DBUS_TYPE_BOOLEAN, "Ping boolean response"}}, // Single boolean res
+		 *		(args) -> {
+		 *			return new Object[] {true}; // You should 'g_variant_unref(result)' on client side.
+		 *		})
+		 *
 		 */
 		public GDBusMethod(String name, String [][] inputArgs, String [][] outputArgs, Function<Object[], Object[]> userFunction) {
 			this.name = name;
@@ -142,7 +188,7 @@ public class GDBus {
 
 		String swt_lib_versions = OS.getEnvironmentalVariable (OS.SWT_LIB_VERSIONS); // Note, this is read in multiple places.
 		if (swt_lib_versions != null && swt_lib_versions.equals("1")) {
-			System.out.println("SWT_LIB GDbus firing up. Implementation v1.1");
+			System.out.println(SWT_GDBUS_VERSION_INFO);
 		}
 	}
 
@@ -250,21 +296,23 @@ public class GDBus {
 		long /*int*/ resultGVariant = 0;
 		try {
 			String java_method_name = Converter.cCharPtrToJavaString(method_name, false);
-
 			for (GDBusMethod gdbusMethod : gdbusMethods) {
 				if (gdbusMethod.getName().equals(java_method_name)) {
 					Object [] args = convertGVariantToJava(gvar_parameters);
-					Object [] returnVal = gdbusMethod.getUserFunction().apply(args); // Return value currently not used. Can be added/implemented if required.
-					resultGVariant = convertJavaToGVariant(returnVal);
+					Object [] returnVal = gdbusMethod.getUserFunction().apply(args);
+					if (returnVal == null || (returnVal instanceof Object [])) {
+						resultGVariant = convertJavaToGVariant(returnVal);
+					} else {
+						System.err.println("SWT GDBus error processing user return value: " + returnVal.toString() + ". Return value must be an Object[] or null.");
+					}
 					break;
 				}
 			}
 		} catch (Exception e) {
-			System.err.println("SWT GDBUS ERROR: Error in handling method.");
+			System.err.println("SWT GDBUS ERROR: Error in handling method: \n" + e.getMessage());
 		} finally {
 			// - GDBus method should always return to prevent caller from hanging.
-			// - Note, result must be a tuple. (or null..).
-
+			// - Note, result must be a tuple. (or null (0)..).
 			OS.g_dbus_method_invocation_return_value(invocation, resultGVariant);
 		}
 		return 0; // void return value.
@@ -303,7 +351,16 @@ public class GDBus {
 			return Converter.cCharPtrToJavaString(OS.g_variant_get_string(gVariant, null), false);
 		}
 
-		// <add your primitive types>
+		if (OS.g_variant_is_of_type(gVariant, OS.G_VARIANT_TYPE_BOOLEAN)) {
+			return new Boolean(OS.g_variant_get_boolean(gVariant));
+		}
+
+		if (OS._g_variant_is_of_type(gVariant, OS.G_VARIANT_TYPE_IN32)) {
+			return new Integer(OS.g_variant_get_int32(gVariant));
+		}
+
+		// <You can add more primitive types here>
+
 
 		//2) Handle struct of defined types. (Sort of like arrays, but note, tuples!=array).
 		if (OS.g_variant_is_of_type(gVariant, OS.G_VARIANT_TYPE_TUPLE)) {
@@ -326,7 +383,9 @@ public class GDBus {
 		}
 
 		String typeString = Converter.cCharPtrToJavaString(OS.g_variant_get_type_string(gVariant), false);
-		SWT.error (SWT.ERROR_INVALID_ARGUMENT, new Throwable("SWT GDBus: Unhandled variant type " + typeString ));
+		System.err.println("SWT GDBus: Error. Unhandled variant type (i.e DBus type):  " + typeString +
+				"     You probably need to update  (SWT) GDBus.java:convertGVariantToJavaHelper() to support this type.");
+		SWT.error (SWT.ERROR_INVALID_ARGUMENT);
 		return null;
 	}
 
@@ -347,14 +406,22 @@ public class GDBus {
 			return OS.g_variant_new_string (Converter.javaStringToCString((String) javaObject));
 		}
 
-		// Add your types here.
+		if (javaObject instanceof Boolean) {
+			return OS.g_variant_new_boolean((Boolean) javaObject);
+		}
+
+		if (javaObject instanceof Integer) {
+			return OS.g_variant_new_int32((Integer) javaObject);
+		}
+
+		// <You can add more primitive types here>
+
 
 		// Dangers:
 		// Null values and empty arrays can cause problems.
 		//   - DBus doesn't have notion of 'null'.
 		//   - DBus doesn't support empty arrays.
 		// If needed, see workaround implemented in WebkitGDBus.java
-
 		if (javaObject instanceof Object[]) {
 			Object[] arrayValue = (Object[]) javaObject;
 			int length = arrayValue.length;
@@ -366,8 +433,8 @@ public class GDBus {
 			return OS.g_variant_new_tuple(variants, length);
 		}
 
-		System.err.println("SWT GDbus: Invalid object being returned to caller: " + javaObject.toString());
-		throw new SWTException(SWT.ERROR_INVALID_ARGUMENT, "Given object is not valid: " + javaObject.toString());
+		System.err.println("SWT GDbus: Invalid object being returned to caller: " + javaObject.toString() + "   You probably need to update (SWT) GDBus.java:convertJavaToGVariant()");
+		throw new SWTException(SWT.ERROR_INVALID_ARGUMENT);
 	}
 
 
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
index b3339705e3..1c9e721ae1 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
@@ -1114,27 +1114,29 @@ void createDisplay (DeviceData data) {
 	// of the case where an SWT app could steal the well-known name and make the equinox launcher confused.
 	// We only initiate GDBus if system property is set.
 	//
-	// To force enable this, in a run-configuration, under arguments, append to the "VM arguments" : -Dswt.dbus.init.
+	// To force enable this, in a run-configuration, under arguments, append to the "VM arguments" : -Dswt.dbus.init
 	// For equinox launcher, See eclipseGtk.c:argVM_JAVA
-	if (System.getProperty("swt.dbus.init") != null) {
-		// For example, fileOpenMethod call can be reached via:
-		// gdbus call --session --dest org.eclipse.swt --object-path /org/eclipse/swt --method org.eclipse.swt.FileOpen "['/tmp/hi','/tmp/there']"
-		// In a child eclipse, this will open the files in a new editor.
-		// This is reached by equinox launcher from eclipseGtk.c. Look for "g_dbus_proxy_call_sync"
-		GDBusMethod fileOpenMethod = new GDBusMethod(
-				"FileOpen",
-				new String [][] {{OS.DBUS_TYPE_STRING_ARRAY,"FileNameArray"}}, // 'FileNameArray' is only descriptive name for arg for human use from gdbus cmdline.
-				new String [0][0],
-				(args) -> {
-					String[] fileNames = (String[]) args[0]; // Arg 1 is an arraay of strings.
-					for (int i = 0; i < fileNames.length; i++) {
-						Event event = new Event ();
-						event.text = fileNames[i];
-						sendEvent (SWT.OpenDocument, event);
-					}
-					return null;
-				});
-		GDBusMethod[] methods = {fileOpenMethod};
+	if (System.getProperty(OS.GDBUS_SYSTEM_PROPERTY) != null) {
+		GDBusMethod[] methods = {
+				new GDBusMethod(
+						// FileOpen call can be reached via:
+						// gdbus call --session --dest org.eclipse.swt --object-path /org/eclipse/swt --method org.eclipse.swt.FileOpen "['/tmp/hi','/tmp/there']"
+						// In a child eclipse, this will open the files in a new editor.
+						// This is reached by equinox launcher from eclipseGtk.c. Look for "g_dbus_proxy_call_sync"
+						"FileOpen",
+						new String [][] {{OS.DBUS_TYPE_STRING_ARRAY,"A String array containing file paths for OpenDocument signal"}},
+						new String [0][0],
+						(args) -> {
+							// Dev note: GDBus Arguments are typically packaged into an Object of sort. First step is normally to un-pack them.
+							String[] fileNames = (String[]) args[0]; // Arg 1 is an array of strings.
+							for (int i = 0; i < fileNames.length; i++) {
+								Event event = new Event ();
+								event.text = fileNames[i];
+								sendEvent (SWT.OpenDocument, event);
+							}
+							return null;
+						})
+				};
 		GDBus.init(methods);
 	}
 }