From 62c73e8f32372fc91dba9f75c4349952a1a848ad Mon Sep 17 00:00:00 2001 From: nathan Date: Tue, 4 Oct 2016 15:36:00 +0200 Subject: [PATCH] Fixed gtk entry/sub-menu removal for SWT. code polish --- src/dorkbox/systemTray/Menu.java | 104 +++++++----------- src/dorkbox/systemTray/SystemTray.java | 11 +- src/dorkbox/systemTray/linux/GtkEntry.java | 39 ++++--- src/dorkbox/systemTray/linux/GtkMenu.java | 94 +++++++++------- .../systemTray/linux/GtkTypeSystemTray.java | 2 +- src/dorkbox/systemTray/swing/SwingMenu.java | 14 +-- ...a => SwingSystemTrayMenuWindowsPopup.java} | 6 +- 7 files changed, 132 insertions(+), 138 deletions(-) rename src/dorkbox/systemTray/swing/{SwingSystemTrayMenuPopup.java => SwingSystemTrayMenuWindowsPopup.java} (94%) diff --git a/src/dorkbox/systemTray/Menu.java b/src/dorkbox/systemTray/Menu.java index 0f3a92c0..f2449644 100644 --- a/src/dorkbox/systemTray/Menu.java +++ b/src/dorkbox/systemTray/Menu.java @@ -91,12 +91,6 @@ class Menu { protected abstract Menu addMenu_(final String menuText, final File imagePath); - /* - * Called when this menu is removed from it's parent menu - */ - protected abstract - void removePrivate(); - /* * Necessary to guarantee all updates occur on the dispatch thread */ @@ -380,35 +374,7 @@ class Menu { @Override public void run() { - try { - synchronized (menuEntries) { - for (Iterator iterator = menuEntries.iterator(); iterator.hasNext(); ) { - final MenuEntry entry = iterator.next(); - if (entry == menuEntry) { - iterator.remove(); - - // this will also reset the menu - menuEntry.remove(); - break; - } - } - - // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. - if (!menuEntries.isEmpty()) { - if (menuEntries.get(0) instanceof MenuSpacer) { - remove(menuEntries.get(0)); - } - } - // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. - if (!menuEntries.isEmpty()) { - if (menuEntries.get(menuEntries.size()-1) instanceof MenuSpacer) { - remove(menuEntries.get(menuEntries.size() - 1)); - } - } - } - } catch (Exception e) { - SystemTray.logger.error("Error removing menu entry from list.", e); - } + remove__(menuEntry); } }); } @@ -422,47 +388,53 @@ class Menu { public void remove(final Menu menu) { if (menu == null) { - throw new NullPointerException("No menu entry exists for menuEntry"); + throw new NullPointerException("No sub-menu entry exists for menu"); } dispatchAndWait(new Runnable() { - @SuppressWarnings("Duplicates") @Override public void run() { - try { - synchronized (menuEntries) { - for (Iterator iterator = menuEntries.iterator(); iterator.hasNext(); ) { - final MenuEntry entry = iterator.next(); - if (entry == menu) { - iterator.remove(); - - // this will also reset the menu - menu.removePrivate(); - break; - } - } - - // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. - if (!menuEntries.isEmpty()) { - if (menuEntries.get(0) instanceof MenuSpacer) { - remove(menuEntries.get(0)); - } - } - // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. - if (!menuEntries.isEmpty()) { - if (menuEntries.get(menuEntries.size()-1) instanceof MenuSpacer) { - remove(menuEntries.get(menuEntries.size() - 1)); - } - } - } - } catch (Exception e) { - SystemTray.logger.error("Error removing menu entry from list.", e); - } + remove__(menu); } }); } + protected + void remove__(final Object menuEntry) { + try { + synchronized (menuEntries) { + // null is passed in when a sub-menu is removing itself from us (because they have already called "remove" and have also + // removed themselves from the menuEntries) + if (menuEntry != null) { + for (Iterator iterator = menuEntries.iterator(); iterator.hasNext(); ) { + final MenuEntry entry = iterator.next(); + if (entry == menuEntry) { + iterator.remove(); + entry.remove(); + break; + } + } + } + + // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. + if (!menuEntries.isEmpty()) { + if (menuEntries.get(0) instanceof MenuSpacer) { + remove(menuEntries.get(0)); + } + } + // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. + if (!menuEntries.isEmpty()) { + if (menuEntries.get(menuEntries.size()-1) instanceof MenuSpacer) { + remove(menuEntries.get(menuEntries.size() - 1)); + } + } + } + } catch (Exception e) { + SystemTray.logger.error("Error removing entry from menu.", e); + } + } + /** * This removes a menu entry or sub-menu (via the text label) from the dropdown menu. * diff --git a/src/dorkbox/systemTray/SystemTray.java b/src/dorkbox/systemTray/SystemTray.java index ebb11ecd..bb29cc39 100644 --- a/src/dorkbox/systemTray/SystemTray.java +++ b/src/dorkbox/systemTray/SystemTray.java @@ -104,7 +104,7 @@ class SystemTray extends Menu { *

* This is an advanced feature, and it is recommended to leave at 0. */ - public static int FORCE_TRAY_TYPE = 3; + public static int FORCE_TRAY_TYPE = 0; @Property /** @@ -875,6 +875,14 @@ class SystemTray extends Menu { void removePrivate() { } + /** + * Removes the system tray. This is the same as calling `shutdown()` + */ + public + void remove() { + shutdown(); + } + /** * Gets the menu entry for a specified text @@ -1048,7 +1056,6 @@ class SystemTray extends Menu { systemTrayMenu.remove(menuEntry); } - /** * This removes a menu entry (via the text label) from the dropdown menu. * diff --git a/src/dorkbox/systemTray/linux/GtkEntry.java b/src/dorkbox/systemTray/linux/GtkEntry.java index 8d924692..0e25a5b2 100644 --- a/src/dorkbox/systemTray/linux/GtkEntry.java +++ b/src/dorkbox/systemTray/linux/GtkEntry.java @@ -64,9 +64,19 @@ class GtkEntry implements MenuEntry { abstract void renderText(final String text); + /** + * must always be called in the GTK thread + */ abstract void setImage_(final File imageFile); + /** + * must always be called in the GTK thread + * called when this item is removed. Necessary to cleanup/remove itself + */ + abstract + void removePrivate(); + /** * Enables, or disables the sub-menu entry. */ @@ -155,27 +165,28 @@ class GtkEntry implements MenuEntry { } } - /** - * will always be on the dispatch thread - */ + // a child will always remove itself from the parent. + @Override public final void remove() { - Gtk.gtk_container_remove(parent._native, _native); - Gtk.gtk_menu_shell_deactivate(parent._native, _native); + parent.dispatchAndWait(new Runnable() { + @Override + public + void run() { + Gtk.gtk_container_remove(parent._native, _native); + Gtk.gtk_menu_shell_deactivate(parent._native, _native); - removePrivate(); + removePrivate(); - Gtk.gtk_widget_destroy(_native); + Gtk.gtk_widget_destroy(_native); - // have to rebuild the menu now... - parent.deleteMenu(); - parent.createMenu(); + // have to rebuild the menu now... + parent.deleteMenu(); + parent.createMenu(); + } + }); } - // called when this item is removed. Necessary to cleanup/remove itself - abstract - void removePrivate(); - @Override public final int hashCode() { diff --git a/src/dorkbox/systemTray/linux/GtkMenu.java b/src/dorkbox/systemTray/linux/GtkMenu.java index e4ca7702..23a6d710 100644 --- a/src/dorkbox/systemTray/linux/GtkMenu.java +++ b/src/dorkbox/systemTray/linux/GtkMenu.java @@ -20,6 +20,7 @@ import static dorkbox.systemTray.SystemTray.TIMEOUT; import java.io.File; import java.io.InputStream; import java.net.URL; +import java.util.Iterator; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -44,14 +45,16 @@ class GtkMenu extends Menu implements MenuEntry { private boolean obliterateInProgress = false; // called on dispatch - GtkMenu(final SystemTray systemTray, final GtkMenu parent, final GtkEntryItem menuEntry) { + GtkMenu(final SystemTray systemTray, final GtkMenu parent) { super(systemTray, parent); - if (menuEntry != null) { + if (parent != null) { + this.menuEntry = new GtkEntryItem(parent, null); // by default, no callback on a menu entry means it's DISABLED. we have to undo that, because we don't have a callback for menus - Gtk.gtk_widget_set_sensitive(menuEntry._native, Gtk.TRUE); + menuEntry.setEnabled(true); + } else { + this.menuEntry = null; } - this.menuEntry = menuEntry; } /** @@ -197,25 +200,18 @@ class GtkMenu extends Menu implements MenuEntry { return menuEntry.hasImage(); } + // NO OP. @Override public void setCallback(final SystemTrayMenuAction callback) { - // NO OP. } - - /** - * Called when this sub-menu is removed from it's parent menu - */ + /** + * Called inside the gdk_threads block + */ protected - void removePrivate() { - // delete all of the children - obliterateMenu(); - - // remove the gtk entry item from our parent menu - if (menuEntry != null) { - menuEntry.remove(); - } + void onMenuAdded(final Pointer menu) { + // only needed for AppIndicator } // some GTK libraries DO NOT let us add items AFTER the menu has been attached to the indicator. @@ -224,7 +220,11 @@ class GtkMenu extends Menu implements MenuEntry { * Deletes the menu, and unreferences everything in it. ALSO recreates ONLY the menu object. */ void deleteMenu() { - if (_native != null && !obliterateInProgress) { + if (obliterateInProgress) { + return; + } + + if (_native != null) { // have to remove all other menu entries synchronized (menuEntries) { for (int i = 0; i < menuEntries.size(); i++) { @@ -313,15 +313,11 @@ class GtkMenu extends Menu implements MenuEntry { } /** - * Called inside the gdk_threads block - */ - void onMenuAdded(final Pointer menu) { - // only needed for AppIndicator - } - - /** + * must be called on the dispatch thread + * * Completely obliterates the menu, no possible way to reconstruct it. */ + private void obliterateMenu() { if (_native != null && !obliterateInProgress) { obliterateInProgress = true; @@ -330,13 +326,7 @@ class GtkMenu extends Menu implements MenuEntry { synchronized (menuEntries) { for (int i = 0; i < menuEntries.size(); i++) { MenuEntry menuEntry__ = menuEntries.get(i); - - if (menuEntry__ instanceof GtkEntry) { - ((GtkEntry) menuEntry__).removePrivate(); - } - else if (menuEntry__ instanceof GtkMenu) { - ((GtkMenu) menuEntry__).removePrivate(); - } + menuEntry__.remove(); } menuEntries.clear(); @@ -418,7 +408,7 @@ class GtkMenu extends Menu implements MenuEntry { // To work around this issue, we destroy then recreate the menu every time something is changed. deleteMenu(); - GtkMenu subMenu = new GtkMenu(getSystemTray(), GtkMenu.this, new GtkEntryItem(GtkMenu.this, null)); + GtkMenu subMenu = new GtkMenu(getSystemTray(), GtkMenu.this); subMenu.setText(menuText); subMenu.setImage(imagePath); @@ -440,20 +430,42 @@ class GtkMenu extends Menu implements MenuEntry { return value.get(); } + + // a child will always remove itself from the parent. @Override public void remove() { - GtkMenu parent = (GtkMenu) getParent(); + dispatchAndWait(new Runnable() { + @Override + public + void run() { + GtkMenu parent = (GtkMenu) getParent(); - Gtk.gtk_container_remove(parent._native, _native); - Gtk.gtk_menu_shell_deactivate(parent._native, _native); + // have to remove from the parent.menuEntries first + for (Iterator iterator = parent.menuEntries.iterator(); iterator.hasNext(); ) { + final MenuEntry entry = iterator.next(); + if (entry == GtkMenu.this) { + iterator.remove(); + break; + } + } - removePrivate(); + // cleans up the menu + parent.remove__(null); - Gtk.gtk_widget_destroy(_native); + // delete all of the children of this submenu (must happen before the menuEntry is removed) + obliterateMenu(); - // have to rebuild the menu now... - parent.deleteMenu(); - parent.createMenu(); + // remove the gtk entry item from our parent menu NATIVE components + // NOTE: this will rebuild the parent menu + if (menuEntry != null) { + menuEntry.remove(); + } else { + // have to rebuild the menu now... + parent.deleteMenu(); + parent.createMenu(); + } + } + }); } } diff --git a/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java b/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java index 42b93484..6a9cfaea 100644 --- a/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java +++ b/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java @@ -27,7 +27,7 @@ abstract class GtkTypeSystemTray extends GtkMenu { GtkTypeSystemTray(final SystemTray systemTray) { - super(systemTray, null, null); + super(systemTray, null); } public diff --git a/src/dorkbox/systemTray/swing/SwingMenu.java b/src/dorkbox/systemTray/swing/SwingMenu.java index c5bfadbf..9570d476 100644 --- a/src/dorkbox/systemTray/swing/SwingMenu.java +++ b/src/dorkbox/systemTray/swing/SwingMenu.java @@ -72,7 +72,7 @@ class SwingMenu extends Menu implements MenuEntry { if (OS.isLinux()) { _native = new SwingSystemTrayLinuxMenuPopup(); } else { - _native = new SwingSystemTrayMenuPopup(); + _native = new SwingSystemTrayMenuWindowsPopup(); } } } @@ -321,8 +321,8 @@ class SwingMenu extends Menu implements MenuEntry { public void run() { _native.setVisible(false); - if (_native instanceof SwingSystemTrayMenuPopup) { - ((SwingSystemTrayMenuPopup) _native).close(); + if (_native instanceof SwingSystemTrayMenuWindowsPopup) { + ((SwingSystemTrayMenuWindowsPopup) _native).close(); } else if (_native instanceof SwingSystemTrayLinuxMenuPopup) { ((SwingSystemTrayLinuxMenuPopup) _native).close(); @@ -335,12 +335,4 @@ class SwingMenu extends Menu implements MenuEntry { } }); } - - /* - * Called when this menu is removed from it's parent menu - */ - protected - void removePrivate() { - remove(); - } } diff --git a/src/dorkbox/systemTray/swing/SwingSystemTrayMenuPopup.java b/src/dorkbox/systemTray/swing/SwingSystemTrayMenuWindowsPopup.java similarity index 94% rename from src/dorkbox/systemTray/swing/SwingSystemTrayMenuPopup.java rename to src/dorkbox/systemTray/swing/SwingSystemTrayMenuWindowsPopup.java index 4ad7ab4b..99484110 100644 --- a/src/dorkbox/systemTray/swing/SwingSystemTrayMenuPopup.java +++ b/src/dorkbox/systemTray/swing/SwingSystemTrayMenuWindowsPopup.java @@ -28,13 +28,13 @@ import javax.swing.border.EmptyBorder; * work, so we must implement an "auto-hide" feature that checks if our mouse is still inside a menu every POPUP_HIDE_DELAY seconds */ public -class SwingSystemTrayMenuPopup extends JPopupMenu { +class SwingSystemTrayMenuWindowsPopup extends JPopupMenu { private static final long serialVersionUID = 1L; // NOTE: we can use the "hidden dialog" focus window trick... only on windows and mac private JDialog hiddenDialog; - SwingSystemTrayMenuPopup() { + SwingSystemTrayMenuWindowsPopup() { super(); setFocusable(true); // setBorder(new BorderUIResource.EmptyBorderUIResource(0, 0, 0, 0)); // borderUI resource border type will get changed! @@ -54,7 +54,7 @@ class SwingSystemTrayMenuPopup extends JPopupMenu { this.hiddenDialog.addWindowFocusListener(new WindowFocusListener() { @Override public void windowLostFocus (WindowEvent we ) { - SwingSystemTrayMenuPopup.this.setVisible(false); + SwingSystemTrayMenuWindowsPopup.this.setVisible(false); } @Override public void windowGainedFocus (WindowEvent we) {}