From 0b866b2bf1db0f111ffe0adfa07ecb6e1054448c Mon Sep 17 00:00:00 2001 From: nathan Date: Thu, 28 Sep 2017 23:51:54 +0200 Subject: [PATCH] SystemTray menu item callbacks now occur on their own dispatch thread (instead of being on whatever OS's event dispatch thread), in order to provide consistent actions across all platforms. --- .../systemTray/ui/awt/AwtMenuItem.java | 31 +++++++------ .../ui/awt/AwtMenuItemCheckbox.java | 22 ++++++---- .../systemTray/ui/gtk/GtkMenuItem.java | 43 +++++++++++++------ .../ui/gtk/GtkMenuItemCheckbox.java | 26 ++++++----- .../systemTray/ui/swing/SwingMenuItem.java | 29 +++++++------ .../ui/swing/SwingMenuItemCheckbox.java | 22 ++++++---- .../systemTray/util/EventDispatch.java | 24 +++++++++++ 7 files changed, 134 insertions(+), 63 deletions(-) create mode 100644 src/dorkbox/systemTray/util/EventDispatch.java diff --git a/src/dorkbox/systemTray/ui/awt/AwtMenuItem.java b/src/dorkbox/systemTray/ui/awt/AwtMenuItem.java index 384a8ebf..5ca7523a 100644 --- a/src/dorkbox/systemTray/ui/awt/AwtMenuItem.java +++ b/src/dorkbox/systemTray/ui/awt/AwtMenuItem.java @@ -22,6 +22,7 @@ import java.awt.event.ActionListener; import dorkbox.systemTray.MenuItem; import dorkbox.systemTray.SystemTray; import dorkbox.systemTray.peer.MenuItemPeer; +import dorkbox.systemTray.util.EventDispatch; import dorkbox.util.SwingUtil; class AwtMenuItem implements MenuItemPeer { @@ -75,28 +76,32 @@ class AwtMenuItem implements MenuItemPeer { _native.removeActionListener(callback); } - if (menuItem.getCallback() != null) { + callback = menuItem.getCallback(); // can be set to null + + if (callback != null) { callback = new ActionListener() { + final ActionListener cb = menuItem.getCallback(); + @Override public void actionPerformed(ActionEvent e) { - // we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms) - ActionListener cb = menuItem.getCallback(); - if (cb != null) { - try { - cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); - } catch (Throwable throwable) { - SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); - } - } + // we want it to run on our own with our own action event info (so it is consistent across all platforms) + EventDispatch.runLater(new Runnable() { + @Override + public + void run() { + try { + cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); + } catch (Throwable throwable) { + SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + } + } + }); } }; _native.addActionListener(callback); } - else { - callback = null; - } } @Override diff --git a/src/dorkbox/systemTray/ui/awt/AwtMenuItemCheckbox.java b/src/dorkbox/systemTray/ui/awt/AwtMenuItemCheckbox.java index fac468c6..86c96d4e 100644 --- a/src/dorkbox/systemTray/ui/awt/AwtMenuItemCheckbox.java +++ b/src/dorkbox/systemTray/ui/awt/AwtMenuItemCheckbox.java @@ -22,6 +22,7 @@ import java.awt.event.ActionListener; import dorkbox.systemTray.Checkbox; import dorkbox.systemTray.SystemTray; import dorkbox.systemTray.peer.CheckboxPeer; +import dorkbox.systemTray.util.EventDispatch; import dorkbox.util.SwingUtil; class AwtMenuItemCheckbox implements CheckboxPeer { @@ -75,21 +76,26 @@ class AwtMenuItemCheckbox implements CheckboxPeer { if (callback != null) { callback = new ActionListener() { + final ActionListener cb = menuItem.getCallback(); + @Override public void actionPerformed(ActionEvent e) { // this will run on the EDT, since we are calling it from the EDT menuItem.setChecked(!isChecked); - // we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms) - ActionListener cb = menuItem.getCallback(); - if (cb != null) { - try { - cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); - } catch (Throwable throwable) { - SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + // we want it to run on our own with our own action event info (so it is consistent across all platforms) + EventDispatch.runLater(new Runnable() { + @Override + public + void run() { + try { + cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); + } catch (Throwable throwable) { + SystemTray.logger.error("Error calling menu checkbox entry {} click event.", menuItem.getText(), throwable); + } } - } + }); } }; diff --git a/src/dorkbox/systemTray/ui/gtk/GtkMenuItem.java b/src/dorkbox/systemTray/ui/gtk/GtkMenuItem.java index 5b4d74ca..ae5cc116 100644 --- a/src/dorkbox/systemTray/ui/gtk/GtkMenuItem.java +++ b/src/dorkbox/systemTray/ui/gtk/GtkMenuItem.java @@ -17,6 +17,7 @@ package dorkbox.systemTray.ui.gtk; import static dorkbox.util.jna.linux.Gtk.Gtk2; +import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import com.sun.jna.Pointer; @@ -24,6 +25,7 @@ import com.sun.jna.Pointer; import dorkbox.systemTray.MenuItem; import dorkbox.systemTray.SystemTray; import dorkbox.systemTray.peer.MenuItemPeer; +import dorkbox.systemTray.util.EventDispatch; import dorkbox.util.jna.linux.GCallback; import dorkbox.util.jna.linux.Gobject; import dorkbox.util.jna.linux.GtkEventDispatch; @@ -32,7 +34,7 @@ class GtkMenuItem extends GtkBaseMenuItem implements MenuItemPeer, GCallback { private final GtkMenu parent; // these have to be volatile, because they can be changed from any thread - private volatile MenuItem menuItemForActionCallback; + private volatile ActionListener callback; private volatile Pointer image; // The mnemonic will ONLY show-up once a menu entry is selected. IT WILL NOT show up before then! @@ -56,15 +58,9 @@ class GtkMenuItem extends GtkBaseMenuItem implements MenuItemPeer, GCallback { @Override public int callback(final Pointer instance, final Pointer data) { - if (menuItemForActionCallback != null) { - final ActionListener cb = menuItemForActionCallback.getCallback(); - if (cb != null) { - try { - GtkEventDispatch.proxyClick(menuItemForActionCallback, cb); - } catch (Exception e) { - SystemTray.logger.error("Error calling menu entry {} click event.", menuItemForActionCallback.getText(), e); - } - } + ActionListener callback = this.callback; + if (callback != null) { + GtkEventDispatch.proxyClick(callback); } return Gtk2.TRUE; @@ -153,10 +149,34 @@ class GtkMenuItem extends GtkBaseMenuItem implements MenuItemPeer, GCallback { }); } + @SuppressWarnings("Duplicates") @Override public void setCallback(final MenuItem menuItem) { - this.menuItemForActionCallback = menuItem; + callback = menuItem.getCallback(); // can be set to null + + if (callback != null) { + callback = new ActionListener() { + final ActionListener cb = menuItem.getCallback(); + + @Override + public + void actionPerformed(ActionEvent e) { + // we want it to run on our own with our own action event info (so it is consistent across all platforms) + EventDispatch.runLater(new Runnable() { + @Override + public + void run() { + try { + cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); + } catch (Throwable throwable) { + SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + } + } + }); + } + }; + } } @Override @@ -193,7 +213,6 @@ class GtkMenuItem extends GtkBaseMenuItem implements MenuItemPeer, GCallback { GtkMenuItem.super.remove(); - menuItemForActionCallback = null; if (image != null) { Gtk2.gtk_container_remove(_native, image); // will automatically get destroyed if no other references to it image = null; diff --git a/src/dorkbox/systemTray/ui/gtk/GtkMenuItemCheckbox.java b/src/dorkbox/systemTray/ui/gtk/GtkMenuItemCheckbox.java index 31011b6a..8dfcef92 100644 --- a/src/dorkbox/systemTray/ui/gtk/GtkMenuItemCheckbox.java +++ b/src/dorkbox/systemTray/ui/gtk/GtkMenuItemCheckbox.java @@ -27,6 +27,7 @@ import com.sun.jna.Pointer; import dorkbox.systemTray.Checkbox; import dorkbox.systemTray.SystemTray; import dorkbox.systemTray.peer.CheckboxPeer; +import dorkbox.systemTray.util.EventDispatch; import dorkbox.systemTray.util.HeavyCheckMark; import dorkbox.systemTray.util.ImageResizeUtil; import dorkbox.util.OSUtil; @@ -133,9 +134,9 @@ class GtkMenuItemCheckbox extends GtkBaseMenuItem implements CheckboxPeer, GCall @Override public int callback(final Pointer instance, final Pointer data) { + ActionListener callback = this.callback; if (callback != null) { - // this will redispatch to our created callback via `setCallback` - GtkEventDispatch.proxyClick(null, callback); + GtkEventDispatch.proxyClick(callback); } return Gtk2.TRUE; @@ -206,21 +207,26 @@ class GtkMenuItemCheckbox extends GtkBaseMenuItem implements CheckboxPeer, GCall if (callback != null) { callback = new ActionListener() { + final ActionListener cb = menuItem.getCallback(); + @Override public void actionPerformed(ActionEvent e) { // this will run on the EDT, since we are calling it from the EDT. This can ALSO recursively call the callback menuItem.setChecked(!isChecked); - // we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms) - ActionListener cb = menuItem.getCallback(); - if (cb != null) { - try { - cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); - } catch (Throwable throwable) { - SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + // we want it to run on our own with our own action event info (so it is consistent across all platforms) + EventDispatch.runLater(new Runnable() { + @Override + public + void run() { + try { + cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); + } catch (Throwable throwable) { + SystemTray.logger.error("Error calling menu checkbox entry {} click event.", menuItem.getText(), throwable); + } } - } + }); } }; } diff --git a/src/dorkbox/systemTray/ui/swing/SwingMenuItem.java b/src/dorkbox/systemTray/ui/swing/SwingMenuItem.java index 1e771f57..555b3ad3 100644 --- a/src/dorkbox/systemTray/ui/swing/SwingMenuItem.java +++ b/src/dorkbox/systemTray/ui/swing/SwingMenuItem.java @@ -27,6 +27,7 @@ import dorkbox.systemTray.Entry; import dorkbox.systemTray.MenuItem; import dorkbox.systemTray.SystemTray; import dorkbox.systemTray.peer.MenuItemPeer; +import dorkbox.systemTray.util.EventDispatch; import dorkbox.systemTray.util.ImageResizeUtil; import dorkbox.util.SwingUtil; @@ -126,28 +127,32 @@ class SwingMenuItem implements MenuItemPeer { _native.removeActionListener(callback); } - if (menuItem.getCallback() != null) { + callback = menuItem.getCallback(); // can be set to null + + if (callback != null) { callback = new ActionListener() { + final ActionListener cb = menuItem.getCallback(); + @Override public void actionPerformed(ActionEvent e) { - // we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms) - ActionListener cb = menuItem.getCallback(); - if (cb != null) { - try { - cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); - } catch (Throwable throwable) { - SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + // we want it to run on our own with our own action event info (so it is consistent across all platforms) + EventDispatch.runLater(new Runnable() { + @Override + public + void run() { + try { + cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); + } catch (Throwable throwable) { + SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + } } - } + }); } }; _native.addActionListener(callback); } - else { - callback = null; - } } @Override diff --git a/src/dorkbox/systemTray/ui/swing/SwingMenuItemCheckbox.java b/src/dorkbox/systemTray/ui/swing/SwingMenuItemCheckbox.java index c2dd0687..a72e5901 100644 --- a/src/dorkbox/systemTray/ui/swing/SwingMenuItemCheckbox.java +++ b/src/dorkbox/systemTray/ui/swing/SwingMenuItemCheckbox.java @@ -25,6 +25,7 @@ import dorkbox.systemTray.Checkbox; import dorkbox.systemTray.Entry; import dorkbox.systemTray.SystemTray; import dorkbox.systemTray.peer.CheckboxPeer; +import dorkbox.systemTray.util.EventDispatch; import dorkbox.systemTray.util.HeavyCheckMark; import dorkbox.util.FontUtil; import dorkbox.util.SwingUtil; @@ -106,21 +107,26 @@ class SwingMenuItemCheckbox extends SwingMenuItem implements CheckboxPeer { if (callback != null) { callback = new ActionListener() { + final ActionListener cb = menuItem.getCallback(); + @Override public void actionPerformed(ActionEvent e) { // this will run on the EDT, since we are calling it from the EDT menuItem.setChecked(!isChecked); - // we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms) - ActionListener cb = menuItem.getCallback(); - if (cb != null) { - try { - cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); - } catch (Throwable throwable) { - SystemTray.logger.error("Error calling menu entry {} click event.", menuItem.getText(), throwable); + // we want it to run on our own with our own action event info (so it is consistent across all platforms) + EventDispatch.runLater(new Runnable() { + @Override + public + void run() { + try { + cb.actionPerformed(new ActionEvent(menuItem, ActionEvent.ACTION_PERFORMED, "")); + } catch (Throwable throwable) { + SystemTray.logger.error("Error calling menu checkbox entry {} click event.", menuItem.getText(), throwable); + } } - } + }); } }; diff --git a/src/dorkbox/systemTray/util/EventDispatch.java b/src/dorkbox/systemTray/util/EventDispatch.java new file mode 100644 index 00000000..b0b5f2ba --- /dev/null +++ b/src/dorkbox/systemTray/util/EventDispatch.java @@ -0,0 +1,24 @@ +package dorkbox.systemTray.util; + +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; + +import dorkbox.util.DaemonThreadFactory; + +/** + * Adds events to a single thread event dispatch, so that regardless of OS, all event callbacks happen on the same thread -- which is NOT + * the GTK/AWT/SWING event dispatch thread. There can be odd peculariaties across on GTK with how AWT/SWING react with the GTK Event + * Dispatch Thread. + */ +public +class EventDispatch { + private static final Executor eventDispatchExecutor = Executors.newSingleThreadExecutor(new DaemonThreadFactory("SystemTray")); + + /** + * Schedule an event to occur sometime in the future. + */ + public static + void runLater(Runnable runnable) { + eventDispatchExecutor.execute(runnable); + } +}