diff --git a/.github/workflows/build-natives.yaml b/.github/workflows/build-natives.yaml index 59fb6581..9e18e26c 100644 --- a/.github/workflows/build-natives.yaml +++ b/.github/workflows/build-natives.yaml @@ -82,6 +82,10 @@ jobs: working-directory: src/native/linux run: bash run_pixmap_test.sh + - name: Primary action controls ItemIsMenu + working-directory: src/native/linux + run: bash run_primary_action_test.sh + - name: JNI strings are standard UTF-8 (issue #441) working-directory: src/native/shared run: bash run_jni_utf8_test.sh diff --git a/.github/workflows/pr-build-check.yml b/.github/workflows/pr-build-check.yml index 3e9dd46d..63c1b17b 100644 --- a/.github/workflows/pr-build-check.yml +++ b/.github/workflows/pr-build-check.yml @@ -76,3 +76,6 @@ jobs: - name: Build run: ./gradlew build --continue + + - name: Linux primary action updates reach the tray host + run: dbus-run-session -- env COMPOSE_TRAY_DBUS_TEST=1 ./gradlew :jvmTest --tests '*LinuxPrimaryActionTest' --rerun-tasks diff --git a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt index d8b6c56f..1ae78baf 100644 --- a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt +++ b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxTrayManager.kt @@ -58,6 +58,22 @@ internal class LinuxTrayManager( // Native handle private var trayHandle: Long = 0L + private val clickCallback = + JniRunnable { + val action = lock.withLock { onLeftClick } ?: return@JniRunnable + try { + val xy = IntArray(2) + native.nativeGetLastClickXY(trayHandle, xy) + TrayClickTracker.recordClick(xy[0], xy[1]) + } catch (_: Throwable) { + } + action() + } + + private fun syncClickCallback() { + native.nativeSetClickCallback(trayHandle, if (onLeftClick != null) clickCallback else null) + } + // Thread / lifecycle private var shutdownHook: Thread? = null private var loopThread: Thread? = null @@ -110,7 +126,9 @@ internal class LinuxTrayManager( tooltipChanged = (tooltip != newTooltip) iconPath = newIconPath tooltip = newTooltip + val clickPresenceChanged = (onLeftClick == null) != (newOnLeftClick == null) onLeftClick = newOnLeftClick + if (clickPresenceChanged && trayHandle != 0L) syncClickCallback() onMenuOpened = newOnMenuOpened if (newMenuItems != null) { menuItems.clear() @@ -164,18 +182,7 @@ internal class LinuxTrayManager( runCatching { native.nativeSetTitle(trayHandle, tooltip) } // Set click callback - native.nativeSetClickCallback( - trayHandle, - JniRunnable { - try { - val xy = IntArray(2) - native.nativeGetLastClickXY(trayHandle, xy) - TrayClickTracker.recordClick(xy[0], xy[1]) - } catch (_: Throwable) { - } - onLeftClick?.invoke() - }, - ) + lock.withLock { syncClickCallback() } // Set menu-opened callback native.nativeSetMenuOpenedCallback( diff --git a/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt new file mode 100644 index 00000000..06504079 --- /dev/null +++ b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/lib/linux/LinuxPrimaryActionTest.kt @@ -0,0 +1,91 @@ +package dev.nucleusframework.composenativetray.lib.linux + +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Run on Linux with busctl installed: + * `dbus-run-session -- env COMPOSE_TRAY_DBUS_TEST=1 ./gradlew :jvmTest --tests '*LinuxPrimaryActionTest'` + */ +class LinuxPrimaryActionTest { + @Test + fun `manager synchronizes primary action presence and replacement`() { + if (System.getenv("COMPOSE_TRAY_DBUS_TEST") != "1") return + val calls = AtomicInteger() + val tray = LinuxTrayManager("primary-action-test", "", "test") + tray.startTray() + try { + awaitProperty("b true") + activate() + assertEquals(0, calls.get()) + + tray.update("", "test", { calls.incrementAndGet() }, null) + awaitProperty("b false") + activate() + awaitCalls(calls, 1) + + tray.update("", "test", { calls.addAndGet(10) }, null) + activate() + awaitCalls(calls, 11) + + tray.update("", "test", null, null) + awaitProperty("b true") + activate() + assertEquals(11, calls.get()) + } finally { + tray.stopTray() + } + + val initialActionTray = + LinuxTrayManager("initial-action-test", "", "test", { calls.incrementAndGet() }) + initialActionTray.startTray() + try { + awaitProperty("b false") + activate() + awaitCalls(calls, 12) + } finally { + initialActionTray.stopTray() + } + } + + private fun awaitCalls( + calls: AtomicInteger, + expected: Int, + ) { + repeat(200) { + if (calls.get() == expected) return + Thread.sleep(10) + } + assertEquals(expected, calls.get()) + } + + private fun awaitProperty(expected: String) { + var output = "" + repeat(200) { + output = busctl("get-property", service, path, iface, "ItemIsMenu") + if (output == expected) return + Thread.sleep(10) + } + assertEquals(expected, output) + } + + private fun activate() { + assertEquals("", busctl("call", service, path, iface, "Activate", "ii", "12", "34")) + // Activate's D-Bus reply can arrive before the queued JVM callback. + Thread.sleep(50) + } + + private fun busctl(vararg args: String): String { + val process = + ProcessBuilder(listOf("busctl", "--user", "--timeout=2") + args).redirectErrorStream(true).start() + assertTrue(process.waitFor(5, TimeUnit.SECONDS), "busctl timed out") + return process.inputStream.bufferedReader().readText().trim() + } + + private val service = "org.kde.StatusNotifierItem-${ProcessHandle.current().pid()}-1" + private val path = "/StatusNotifierItem" + private val iface = "org.kde.StatusNotifierItem" +} diff --git a/src/native/linux/jni_bridge.c b/src/native/linux/jni_bridge.c index 5c813cfb..a9e40aeb 100644 --- a/src/native/linux/jni_bridge.c +++ b/src/native/linux/jni_bridge.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "jni_utf8.h" #include "sni.h" @@ -53,6 +54,7 @@ typedef struct CallbackEntry { } CallbackEntry; static CallbackEntry *g_clickCallback = NULL; +static pthread_mutex_t g_clickCallbackLock = PTHREAD_MUTEX_INITIALIZER; static CallbackEntry *g_rclickCallback = NULL; static CallbackEntry *g_menuCallbacks = NULL; static CallbackEntry *g_menuOpenedCallback = NULL; @@ -132,8 +134,16 @@ static void invokeRunnable(jobject runnable) { static void click_trampoline(int32_t x, int32_t y, void *userdata) { (void)x; (void)y; uintptr_t key = (uintptr_t)userdata; - jobject runnable = findCallback(g_clickCallback, key); - if (runnable) invokeRunnable(runnable); + JNIEnv *env = getJNIEnv(); + if (!env) return; + pthread_mutex_lock(&g_clickCallbackLock); + jobject globalRef = findCallback(g_clickCallback, key); + jobject runnable = globalRef ? (*env)->NewLocalRef(env, globalRef) : NULL; + pthread_mutex_unlock(&g_clickCallbackLock); + if (runnable) { + invokeRunnable(runnable); + (*env)->DeleteLocalRef(env, runnable); + } } static void rclick_trampoline(int32_t x, int32_t y, void *userdata) { @@ -215,7 +225,9 @@ Java_dev_nucleusframework_composenativetray_lib_linux_LinuxNativeBridge_nativeDe /* Clean up all callbacks for this tray */ uintptr_t key = (uintptr_t)tray; + pthread_mutex_lock(&g_clickCallbackLock); storeCallback(&g_clickCallback, key, env, NULL); + pthread_mutex_unlock(&g_clickCallbackLock); storeCallback(&g_rclickCallback, key, env, NULL); storeCallback(&g_menuOpenedCallback, key, env, NULL); /* Menu callbacks are keyed by item id, clear all */ @@ -274,7 +286,9 @@ Java_dev_nucleusframework_composenativetray_lib_linux_LinuxNativeBridge_nativeSe sni_tray *tray = (sni_tray *)(uintptr_t)handle; if (!tray) return; uintptr_t key = (uintptr_t)tray; + pthread_mutex_lock(&g_clickCallbackLock); storeCallback(&g_clickCallback, key, env, callback); + pthread_mutex_unlock(&g_clickCallbackLock); sni_tray_set_click_callback(tray, callback ? click_trampoline : NULL, (void *)key); diff --git a/src/native/linux/run_primary_action_test.sh b/src/native/linux/run_primary_action_test.sh new file mode 100644 index 00000000..7cb64a6e --- /dev/null +++ b/src/native/linux/run_primary_action_test.sh @@ -0,0 +1,12 @@ +#!/bin/bash +# Use a private session bus so the test does not register with the user's desktop. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +BIN="$(mktemp)" +trap 'rm -f "$BIN"' EXIT + +gcc -O2 -g -Wall -Wextra -Werror -Wno-unused-parameter \ + $(pkg-config --cflags libsystemd) \ + "$SCRIPT_DIR/sni.c" "$SCRIPT_DIR/test_sni_primary_action.c" \ + $(pkg-config --libs libsystemd) -lpthread -lm -ldl -o "$BIN" +dbus-run-session -- "$BIN" diff --git a/src/native/linux/sni.c b/src/native/linux/sni.c index edfd7084..8b8c553b 100644 --- a/src/native/linux/sni.c +++ b/src/native/linux/sni.c @@ -246,9 +246,14 @@ static void flush_callbacks(sni_tray *tray) { for (int i = 0; i < tray->pending_count; i++) { pending_cb *c = &tray->pending[i]; switch (c->type) { - case PCB_CLICK: - if (tray->on_click) tray->on_click(c->x, c->y, tray->on_click_data); + case PCB_CLICK: { + bus_lock(tray); + sni_click_cb callback = tray->on_click; + void *userdata = tray->on_click_data; + bus_unlock(tray); + if (callback) callback(c->x, c->y, userdata); break; + } case PCB_RCLICK: if (tray->on_rclick) tray->on_rclick(c->x, c->y, tray->on_rclick_data); break; @@ -510,7 +515,7 @@ static int sni_get_property(sd_bus *bus, const char *path, const char *interface if (strcmp(property, "Menu") == 0) return sd_bus_message_append(reply, "o", tray->current_menu_path); if (strcmp(property, "ItemIsMenu") == 0) - return sd_bus_message_append(reply, "b", 1); + return sd_bus_message_append(reply, "b", tray->on_click == NULL); if (strcmp(property, "IconName") == 0) return sd_bus_message_append(reply, "s", ""); if (strcmp(property, "IconPixmap") == 0) @@ -1285,8 +1290,17 @@ void sni_tray_set_tooltip(sni_tray *tray, const char *tooltip) { void sni_tray_set_click_callback(sni_tray *tray, sni_click_cb cb, void *userdata) { if (!tray) return; + bus_lock(tray); + int changed = (tray->on_click == NULL) != (cb == NULL); tray->on_click = cb; tray->on_click_data = userdata; + if (changed) { + emit_sni_properties_changed(tray, "ItemIsMenu"); + /* Plasma refreshes its cached ItemIsMenu via SNI signals, rather than + * PropertiesChanged. NewTitle triggers GetAll without rebuilding the menu. */ + emit_new_title(tray); + } + bus_unlock(tray); } void sni_tray_set_rclick_callback(sni_tray *tray, sni_click_cb cb, void *userdata) { diff --git a/src/native/linux/sni.h b/src/native/linux/sni.h index adc8a555..8f83fb04 100644 --- a/src/native/linux/sni.h +++ b/src/native/linux/sni.h @@ -67,6 +67,8 @@ void sni_tray_set_tooltip(sni_tray *tray, const char *tooltip); /* ── Click callbacks ───────────────────────────────────────────────── */ +/* A non-NULL primary action sets ItemIsMenu=false; NULL restores menu-only + * activation. Changes are announced to the tray host while running. */ void sni_tray_set_click_callback(sni_tray *tray, sni_click_cb cb, void *userdata); void sni_tray_set_rclick_callback(sni_tray *tray, sni_click_cb cb, void *userdata); void sni_tray_set_menu_callback(sni_tray *tray, sni_menu_item_cb cb, void *userdata); diff --git a/src/native/linux/test_sni_primary_action.c b/src/native/linux/test_sni_primary_action.c new file mode 100644 index 00000000..7f1b380f --- /dev/null +++ b/src/native/linux/test_sni_primary_action.c @@ -0,0 +1,188 @@ +/* Exercise the public SNI protocol on a private session bus. */ +#include "sni.h" +#include +#include +#include +#include +#include +#include +#include + +static const char *path = "/StatusNotifierItem"; +static const char *iface = "org.kde.StatusNotifierItem"; +static atomic_int clicks, right_clicks; +static int changes, last_item_is_menu; +static int refresh_pending, refreshes, cached_item_is_menu; +static char service[128]; + +/* Model Plasma's host cache: refresh GetAll on NewTitle, ignoring + * PropertiesChanged for routing clicks. See StatusNotifierItemSource in: + * https://github.com/KDE/plasma-workspace/blob/v6.7.5/applets/systemtray/statusnotifieritemsource.cpp */ +static void refresh_host_cache(sd_bus *bus) { + sd_bus_message *reply = NULL; + assert(sd_bus_call_method(bus, service, path, "org.freedesktop.DBus.Properties", + "GetAll", NULL, &reply, "s", iface) >= 0); + assert(sd_bus_message_enter_container(reply, 'a', "{sv}") > 0); + int found = 0; + while (sd_bus_message_enter_container(reply, 'e', "sv") > 0) { + const char *property; + assert(sd_bus_message_read(reply, "s", &property) > 0); + if (strcmp(property, "ItemIsMenu") == 0) { + assert(sd_bus_message_enter_container(reply, 'v', "b") > 0); + assert(sd_bus_message_read(reply, "b", &cached_item_is_menu) > 0); + assert(sd_bus_message_exit_container(reply) >= 0); + found = 1; + } else { + assert(sd_bus_message_skip(reply, "v") >= 0); + } + assert(sd_bus_message_exit_container(reply) >= 0); + } + assert(found); + sd_bus_message_unref(reply); +} + +static int new_title(sd_bus_message *message, void *data, sd_bus_error *error) { + (void)message; (void)data; (void)error; + refresh_pending = 1; + refreshes++; + return 0; +} + +static void *run(void *tray) { + assert(sni_tray_run(tray) == 0); + return NULL; +} + +static void click(int32_t x, int32_t y, void *data) { + assert(x == 12 && y == 34); + atomic_fetch_add(&clicks, 1); + /* User callbacks must be able to update the tray without deadlocking. */ + if (data) sni_tray_set_click_callback(data, NULL, NULL); +} + +static void right_click(int32_t x, int32_t y, void *data) { + (void)data; + assert(x == 12 && y == 34); + atomic_fetch_add(&right_clicks, 1); +} + +static int property_changed(sd_bus_message *message, void *data, sd_bus_error *error) { + (void)data; (void)error; + const char *interface, *property; + assert(sd_bus_message_read(message, "s", &interface) > 0); + assert(sd_bus_message_enter_container(message, 'a', "{sv}") > 0); + while (sd_bus_message_enter_container(message, 'e', "sv") > 0) { + assert(sd_bus_message_read(message, "s", &property) > 0); + if (strcmp(interface, iface) == 0 && strcmp(property, "ItemIsMenu") == 0) { + assert(sd_bus_message_enter_container(message, 'v', "b") > 0); + assert(sd_bus_message_read(message, "b", &last_item_is_menu) > 0); + assert(sd_bus_message_exit_container(message) >= 0); + changes++; + } else { + assert(sd_bus_message_skip(message, "v") >= 0); + } + assert(sd_bus_message_exit_container(message) >= 0); + } + return 0; +} + +static void pump(sd_bus *bus, int milliseconds) { + for (int i = 0; i < milliseconds; i++) { + int result; + while ((result = sd_bus_process(bus, NULL)) > 0) {} + assert(result >= 0); + if (refresh_pending) { + refresh_pending = 0; + refresh_host_cache(bus); + } + usleep(1000); + } +} + +static void expect_property(sd_bus *bus, int expected) { + int value = -1, result = -1; + /* Registration happens asynchronously on the tray thread. */ + for (int i = 0; i < 200 && result < 0; i++) { + result = sd_bus_get_property_trivial(bus, service, path, iface, + "ItemIsMenu", NULL, 'b', &value); + if (result < 0) usleep(10000); + } + assert(result >= 0 && value == expected); +} + +static void call(sd_bus *bus, const char *method) { + assert(sd_bus_call_method(bus, service, path, iface, method, NULL, NULL, + "ii", 12, 34) >= 0); + pump(bus, 100); +} + +static void host_click(sd_bus *bus) { + call(bus, cached_item_is_menu ? "ContextMenu" : "Activate"); +} + +int main(void) { + sd_bus *bus = NULL; + sd_bus_slot *slot = NULL; + sd_bus_slot *title_slot = NULL; + assert(sd_bus_open_user(&bus) >= 0); + snprintf(service, sizeof(service), "org.kde.StatusNotifierItem-%d-1", getpid()); + assert(sd_bus_match_signal(bus, &slot, service, path, + "org.freedesktop.DBus.Properties", "PropertiesChanged", + property_changed, NULL) >= 0); + assert(sd_bus_match_signal(bus, &title_slot, service, path, iface, + "NewTitle", new_title, NULL) >= 0); + sni_tray *tray = sni_tray_create(NULL, 0, "primary action test"); + assert(tray); + sni_tray_set_rclick_callback(tray, right_click, NULL); + pthread_t thread; + assert(pthread_create(&thread, NULL, run, tray) == 0); + expect_property(bus, 1); + refresh_host_cache(bus); + call(bus, "Activate"); + assert(atomic_load(&clicks) == 0); + + sni_tray_set_click_callback(tray, click, NULL); + expect_property(bus, 0); + pump(bus, 100); + assert(changes == 1 && last_item_is_menu == 0); + assert(refreshes == 1 && cached_item_is_menu == 0); + host_click(bus); + assert(atomic_load(&clicks) == 1); + call(bus, "ContextMenu"); + assert(atomic_load(&right_clicks) == 1); + + /* Replacing a present callback must not announce a property transition. */ + sni_tray_set_click_callback(tray, click, tray); + pump(bus, 100); + assert(changes == 1 && refreshes == 1); + host_click(bus); + assert(atomic_load(&clicks) == 2); + expect_property(bus, 1); + assert(changes == 2 && last_item_is_menu == 1); + assert(refreshes == 2 && cached_item_is_menu == 1); + call(bus, "Activate"); + assert(atomic_load(&clicks) == 2); + host_click(bus); + assert(atomic_load(&right_clicks) == 2); + + sni_tray_quit(tray); + pthread_join(thread, NULL); + sni_tray_destroy(tray); + + /* A primary action installed before registration starts with ItemIsMenu=false. */ + tray = sni_tray_create(NULL, 0, "initial primary action"); + assert(tray); + sni_tray_set_click_callback(tray, click, NULL); + assert(pthread_create(&thread, NULL, run, tray) == 0); + expect_property(bus, 0); + call(bus, "Activate"); + assert(atomic_load(&clicks) == 3); + sni_tray_quit(tray); + pthread_join(thread, NULL); + sni_tray_destroy(tray); + sd_bus_slot_unref(slot); + sd_bus_slot_unref(title_slot); + sd_bus_unref(bus); + puts("Primary action properties, host cache refresh, activation and context menu passed."); + return 0; +}