From a26522411ea6e477bf64fd9428715233441d0551 Mon Sep 17 00:00:00 2001 From: nathan Date: Sun, 16 Oct 2016 15:58:02 +0200 Subject: [PATCH] Code cleanup, GTK.dispatchAndWait refactor, fixed issue with hiding the tray icon in MacOSx. --- src/dorkbox/systemTray/SystemTray.java | 14 ++++-- src/dorkbox/systemTray/jna/linux/Gtk.java | 39 ++++++++++++++- src/dorkbox/systemTray/nativeUI/GtkMenu.java | 45 +++-------------- .../nativeUI/_AppIndicatorNativeTray.java | 4 +- src/dorkbox/systemTray/nativeUI/_AwtTray.java | 49 +++++++++++++++++-- .../nativeUI/_GtkStatusIconNativeTray.java | 4 +- .../systemTray/swingUI/_AppIndicatorTray.java | 9 ++-- .../swingUI/_GtkStatusIconTray.java | 6 ++- .../systemTray/swingUI/_SwingTray.java | 6 +-- 9 files changed, 117 insertions(+), 59 deletions(-) diff --git a/src/dorkbox/systemTray/SystemTray.java b/src/dorkbox/systemTray/SystemTray.java index c5c447cb..144c8a6d 100644 --- a/src/dorkbox/systemTray/SystemTray.java +++ b/src/dorkbox/systemTray/SystemTray.java @@ -672,12 +672,13 @@ class SystemTray implements Menu { } /** - * Shuts-down the SystemTray, by removing the menus + tray icon. + * Shuts-down the SystemTray, by removing the menus + tray icon. After calling this method, you MUST call `get()` or `getNative()` + * again to obtain a new reference to the SystemTray. */ public void shutdown() { + // this will call "dispatchAndWait()" behind the scenes, so it is thread-safe final Menu menu = systemTrayMenu; - if (menu instanceof _AppIndicatorTray) { ((_AppIndicatorTray) menu).shutdown(); } @@ -693,7 +694,7 @@ class SystemTray implements Menu { else if (menu instanceof _AwtTray) { ((_AwtTray) menu).shutdown(); } - else { + else if (menu instanceof _SwingTray) { ((_SwingTray) menu).shutdown(); } systemTrayMenu = null; @@ -721,9 +722,12 @@ class SystemTray implements Menu { else if (menu instanceof _AwtTray) { return ((_AwtTray) menu).getStatus(); } - else { + else if (menu instanceof _SwingTray) { return ((_SwingTray) menu).getStatus(); } + else { + return ""; + } } /** @@ -750,7 +754,7 @@ class SystemTray implements Menu { else if (menu instanceof _AwtTray) { ((_AwtTray) menu).setStatus(statusText); } - else { + else if (menu instanceof _SwingTray) { ((_SwingTray) menu).setStatus(statusText); } } diff --git a/src/dorkbox/systemTray/jna/linux/Gtk.java b/src/dorkbox/systemTray/jna/linux/Gtk.java index 0961fafb..d4a92c87 100644 --- a/src/dorkbox/systemTray/jna/linux/Gtk.java +++ b/src/dorkbox/systemTray/jna/linux/Gtk.java @@ -57,6 +57,8 @@ class Gtk { // there is ONLY a single thread EVER setting this value!! private static volatile boolean isDispatch = false; + private static final int TIMEOUT = 2; + // objdump -T /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0 | grep gtk // objdump -T /usr/lib/x86_64-linux-gnu/libgtk-3.so.0 | grep gtk @@ -363,9 +365,44 @@ class Gtk { } } + public static + void dispatchAndWait(final Runnable runnable) { + + final CountDownLatch countDownLatch = new CountDownLatch(1); + + Gtk.dispatch(new Runnable() { + @Override + public + void run() { + try { + runnable.run(); + } finally { + countDownLatch.countDown(); + } + } + }); + + // this is slightly different than how swing does it. We have a timeout here so that we can make sure that updates on the GUI + // thread occur in REASONABLE time-frames, and alert the user if not. + try { + if (!countDownLatch.await(TIMEOUT, TimeUnit.SECONDS)) { + if (SystemTray.DEBUG) { + SystemTray.logger.error("Event dispatch queue took longer than " + TIMEOUT + " seconds to complete. Please adjust " + + "`SystemTray.TIMEOUT` to a value which better suites your environment."); + } else { + throw new RuntimeException("Event dispatch queue took longer than " + TIMEOUT + " seconds to complete. Please adjust " + + "`SystemTray.TIMEOUT` to a value which better suites your environment."); + } + } + } catch (InterruptedException e) { + SystemTray.logger.error("Error waiting for dispatch to complete.", new Exception()); + } + + } + public static void shutdownGui() { - dispatch(new Runnable() { + dispatchAndWait(new Runnable() { @Override public void run() { diff --git a/src/dorkbox/systemTray/nativeUI/GtkMenu.java b/src/dorkbox/systemTray/nativeUI/GtkMenu.java index 82f08856..74702a2e 100644 --- a/src/dorkbox/systemTray/nativeUI/GtkMenu.java +++ b/src/dorkbox/systemTray/nativeUI/GtkMenu.java @@ -19,8 +19,6 @@ package dorkbox.systemTray.nativeUI; import java.io.File; import java.util.ArrayList; import java.util.Iterator; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import com.sun.jna.Pointer; @@ -34,8 +32,6 @@ import dorkbox.systemTray.jna.linux.Gtk; import dorkbox.systemTray.util.MenuBase; class GtkMenu extends MenuBase implements NativeUI { - static int TIMEOUT = 2; - // menu entry that this menu is attached to. Will be NULL when it's the system tray private final GtkEntryItem menuEntry; @@ -79,35 +75,7 @@ class GtkMenu extends MenuBase implements NativeUI { */ protected void dispatchAndWait(final Runnable runnable) { - final CountDownLatch countDownLatch = new CountDownLatch(1); - - Gtk.dispatch(new Runnable() { - @Override - public - void run() { - try { - runnable.run(); - } finally { - countDownLatch.countDown(); - } - } - }); - - // this is slightly different than how swing does it. We have a timeout here so that we can make sure that updates on the GUI - // thread occur in REASONABLE time-frames, and alert the user if not. - try { - if (!countDownLatch.await(TIMEOUT, TimeUnit.SECONDS)) { - if (SystemTray.DEBUG) { - SystemTray.logger.error("Event dispatch queue took longer than " + TIMEOUT + " seconds to complete. Please adjust " + - "`SystemTray.TIMEOUT` to a value which better suites your environment."); - } else { - throw new RuntimeException("Event dispatch queue took longer than " + TIMEOUT + " seconds to complete. Please adjust " + - "`SystemTray.TIMEOUT` to a value which better suites your environment."); - } - } - } catch (InterruptedException e) { - SystemTray.logger.error("Error waiting for dispatch to complete.", new Exception()); - } + Gtk.dispatchAndWait(runnable); } public @@ -117,10 +85,11 @@ class GtkMenu extends MenuBase implements NativeUI { public void run() { obliterateMenu(); - - Gtk.shutdownGui(); } }); + + // does not need to be called on the dispatch (it does that) + Gtk.shutdownGui(); } // public here so that Swing/Gtk/AppIndicator can access this @@ -314,7 +283,7 @@ class GtkMenu extends MenuBase implements NativeUI { // will also get: gsignal.c:2516: signal 'child-added' is invalid for instance '0x7f1df8244080' of type 'GtkMenu' Gtk.gtk_menu_shell_append(this._native, entry._native); Gobject.g_object_ref_sink(entry._native); // undoes "floating" - Gtk.gtk_widget_show_all(entry._native); + Gtk.gtk_widget_show_all(entry._native); // necessary to guarantee widget is visible } else if (menuEntry__ instanceof GtkMenu) { GtkMenu subMenu = (GtkMenu) menuEntry__; @@ -322,7 +291,7 @@ class GtkMenu extends MenuBase implements NativeUI { // will also get: gsignal.c:2516: signal 'child-added' is invalid for instance '0x7f1df8244080' of type 'GtkMenu' Gtk.gtk_menu_shell_append(this._native, subMenu.menuEntry._native); Gobject.g_object_ref_sink(subMenu.menuEntry._native); // undoes "floating" - Gtk.gtk_widget_show_all(subMenu.menuEntry._native); + Gtk.gtk_widget_show_all(subMenu.menuEntry._native); // necessary to guarantee widget is visible if (subMenu.getParent() != GtkMenu.this) { // we don't want to "createMenu" on our sub-menu that is assigned to us directly, as they are already doing it @@ -332,7 +301,7 @@ class GtkMenu extends MenuBase implements NativeUI { } onMenuAdded(_native); - Gtk.gtk_widget_show_all(_native); + Gtk.gtk_widget_show_all(_native); // necessary to guarantee widget is visible (doesn't always show_all for all children) } } diff --git a/src/dorkbox/systemTray/nativeUI/_AppIndicatorNativeTray.java b/src/dorkbox/systemTray/nativeUI/_AppIndicatorNativeTray.java index 62272052..3105c89f 100644 --- a/src/dorkbox/systemTray/nativeUI/_AppIndicatorNativeTray.java +++ b/src/dorkbox/systemTray/nativeUI/_AppIndicatorNativeTray.java @@ -157,8 +157,6 @@ class _AppIndicatorNativeTray extends GtkMenu { @Override public final void setEnabled(final boolean setEnabled) { - visible = !setEnabled; - Gtk.dispatch(new Runnable() { @Override public @@ -166,9 +164,11 @@ class _AppIndicatorNativeTray extends GtkMenu { if (visible && !setEnabled) { // STATUS_PASSIVE hides the indicator AppIndicator.app_indicator_set_status(appIndicator, AppIndicator.STATUS_PASSIVE); + visible = false; } else if (!visible && setEnabled) { AppIndicator.app_indicator_set_status(appIndicator, AppIndicator.STATUS_ACTIVE); + visible = true; } } }); diff --git a/src/dorkbox/systemTray/nativeUI/_AwtTray.java b/src/dorkbox/systemTray/nativeUI/_AwtTray.java index 6ec2f364..cae15220 100644 --- a/src/dorkbox/systemTray/nativeUI/_AwtTray.java +++ b/src/dorkbox/systemTray/nativeUI/_AwtTray.java @@ -24,8 +24,11 @@ import java.io.File; import javax.swing.ImageIcon; +import dorkbox.util.OS; + /** - * Class for handling all system tray interaction, via AWT. + * Class for handling all system tray interaction, via AWT. Pretty much EXCLUSIVELY for on MacOS, because that is the only time this + * looks good * * It doesn't work well on linux. See bugs: * http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6267936 @@ -41,6 +44,9 @@ class _AwtTray extends AwtMenu { // is the system tray visible or not. private volatile boolean visible = true; + private final Object keepAliveLock = new Object[0]; + private Thread keepAliveThread; + // Called in the EDT public _AwtTray(final dorkbox.systemTray.SystemTray systemTray) { @@ -56,7 +62,7 @@ class _AwtTray extends AwtMenu { public void shutdown() { - dispatch(new Runnable() { + dispatchAndWait(new Runnable() { @Override public void run() { @@ -103,7 +109,42 @@ class _AwtTray extends AwtMenu { @SuppressWarnings("Duplicates") public void setEnabled(final boolean setEnabled) { - visible = !setEnabled; + if (OS.isMacOsX()) { + if (keepAliveThread != null) { + synchronized (keepAliveLock) { + keepAliveLock.notifyAll(); + } + } + keepAliveThread = null; + + if (visible && !setEnabled) { + // THIS WILL NOT keep the app running, so we use a "keep-alive" thread so this behavior is THE SAME across + // all platforms. This was only noticed on MacOS (where the app would quit after calling setEnabled(false); + keepAliveThread = new Thread(new Runnable() { + @Override + public + void run() { + synchronized (keepAliveLock) { + keepAliveLock.notifyAll(); + + try { + keepAliveLock.wait(); + } catch (InterruptedException ignored) { + } + } + } + }, "KeepAliveThread"); + keepAliveThread.start(); + } + + synchronized (keepAliveLock) { + try { + keepAliveLock.wait(); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + } dispatch(new Runnable() { @Override @@ -111,10 +152,12 @@ class _AwtTray extends AwtMenu { void run() { if (visible && !setEnabled) { tray.remove(trayIcon); + visible = false; } else if (!visible && setEnabled) { try { tray.add(trayIcon); + visible = true; } catch (AWTException e) { dorkbox.systemTray.SystemTray.logger.error("Error adding the icon back to the tray"); } diff --git a/src/dorkbox/systemTray/nativeUI/_GtkStatusIconNativeTray.java b/src/dorkbox/systemTray/nativeUI/_GtkStatusIconNativeTray.java index d9ef1a66..9ca45747 100644 --- a/src/dorkbox/systemTray/nativeUI/_GtkStatusIconNativeTray.java +++ b/src/dorkbox/systemTray/nativeUI/_GtkStatusIconNativeTray.java @@ -169,16 +169,16 @@ class _GtkStatusIconNativeTray extends GtkMenu { @Override public final void setEnabled(final boolean setEnabled) { - visible = !setEnabled; - Gtk.dispatch(new Runnable() { @Override public void run() { if (visible && !setEnabled) { Gtk.gtk_status_icon_set_visible(trayIcon, setEnabled); + visible = false; } else if (!visible && setEnabled) { Gtk.gtk_status_icon_set_visible(trayIcon, setEnabled); + visible = true; } } }); diff --git a/src/dorkbox/systemTray/swingUI/_AppIndicatorTray.java b/src/dorkbox/systemTray/swingUI/_AppIndicatorTray.java index ea816c4d..00539487 100644 --- a/src/dorkbox/systemTray/swingUI/_AppIndicatorTray.java +++ b/src/dorkbox/systemTray/swingUI/_AppIndicatorTray.java @@ -213,6 +213,7 @@ class _AppIndicatorTray extends SwingMenu { } }); + // does not need to be called on the dispatch (it does that) Gtk.shutdownGui(); // uses EDT @@ -230,7 +231,7 @@ class _AppIndicatorTray extends SwingMenu { @Override public final void setImage_(final File imageFile) { - dispatch(new Runnable() { + Gtk.dispatch(new Runnable() { @Override public void run() { @@ -247,6 +248,8 @@ class _AppIndicatorTray extends SwingMenu { } }); + + // needs to be on EDT dispatch(new Runnable() { @Override public @@ -259,8 +262,6 @@ class _AppIndicatorTray extends SwingMenu { @Override public final void setEnabled(final boolean setEnabled) { - visible = !setEnabled; - Gtk.dispatch(new Runnable() { @Override public @@ -268,9 +269,11 @@ class _AppIndicatorTray extends SwingMenu { if (visible && !setEnabled) { // STATUS_PASSIVE hides the indicator AppIndicator.app_indicator_set_status(appIndicator, AppIndicator.STATUS_PASSIVE); + visible = false; } else if (!visible && setEnabled) { AppIndicator.app_indicator_set_status(appIndicator, AppIndicator.STATUS_ACTIVE); + visible = true; } } }); diff --git a/src/dorkbox/systemTray/swingUI/_GtkStatusIconTray.java b/src/dorkbox/systemTray/swingUI/_GtkStatusIconTray.java index 3056fca7..5f95fc51 100644 --- a/src/dorkbox/systemTray/swingUI/_GtkStatusIconTray.java +++ b/src/dorkbox/systemTray/swingUI/_GtkStatusIconTray.java @@ -162,6 +162,7 @@ class _GtkStatusIconTray extends SwingMenu { } }); + // does not need to be called on the dispatch (it does that) Gtk.shutdownGui(); // uses EDT @@ -185,6 +186,7 @@ class _GtkStatusIconTray extends SwingMenu { } }); + // needs to be on EDT dispatch(new Runnable() { @Override public @@ -196,16 +198,16 @@ class _GtkStatusIconTray extends SwingMenu { public void setEnabled(final boolean setEnabled) { - visible = !setEnabled; - Gtk.dispatch(new Runnable() { @Override public void run() { if (visible && !setEnabled) { Gtk.gtk_status_icon_set_visible(trayIcon, setEnabled); + visible = false; } else if (!visible && setEnabled) { Gtk.gtk_status_icon_set_visible(trayIcon, setEnabled); + visible = true; } } }); diff --git a/src/dorkbox/systemTray/swingUI/_SwingTray.java b/src/dorkbox/systemTray/swingUI/_SwingTray.java index ac32f229..3a5113e0 100644 --- a/src/dorkbox/systemTray/swingUI/_SwingTray.java +++ b/src/dorkbox/systemTray/swingUI/_SwingTray.java @@ -118,20 +118,20 @@ class _SwingTray extends SwingMenu { @SuppressWarnings("Duplicates") public void setEnabled(final boolean setEnabled) { - visible = !setEnabled; - dispatch(new Runnable() { @Override public void run() { if (visible && !setEnabled) { tray.remove(trayIcon); + visible = false; } else if (!visible && setEnabled) { try { tray.add(trayIcon); + visible = true; } catch (AWTException e) { - dorkbox.systemTray.SystemTray.logger.error("Error adding the icon back to the tray"); + dorkbox.systemTray.SystemTray.logger.error("Error adding the icon back to the tray", e); } } }