• Type: Bug
• Component: javafx
• Sub-Component: swing
• Affected Version: 8u144

• Priority: P3
• Status: Resolved
• Resolution: Fixed

• Submitted: 2017-08-01
• Updated: 2024-10-18
• Resolved: 2017-11-09

JDK 10	JDK 8
10Fixed	8u162Fixed

Approved to push to 8u-dev for 8u182. 18-01-2018 I just noticed one other (minor) issue with the test files. Since they were copied from the FX 10 port, you should preserve the copyright start year from the copied files (so "2017, 2018," rather than just "2018,"). +1 pending the copyright year change (no need for a new webrev) 18-01-2018 Thanks [~kcr]. I have incorporated the changes suggested for unit tests in JavaFx side. Request you to have a look. Please find updated webrev (for 8u-backport) at - http://cr.openjdk.java.net/~dkumar/8185634/webrev.01/ . 18-01-2018 Comments for 8u backport: The FX half of the implementation changes look fine. I tested it on Linux and it works as expected. There are two problems with the unit tests: 1. The package name used in each of the three test files doesn't match the location in the directory tree. [This one must be fixed] 2. When used with the default boot JDK of 8u40, the unit tests will fail. This may not be a problem we care about, since they are robot tests and won't get run without explicitly enabling robot tests. [Feel free to ignore this point] 17-01-2018 Webrev for review (8u-backport) - http://cr.openjdk.java.net/~dkumar/8185634/webrev.00/ This patch also includes the changes of JDK-8191269 . Unit test results are fine on mac and windows. There are failures related to this fix on linux (with JDK 10 as well) and JBS for the same is JDK-8194986 . 12-01-2018 I discovered a bug in the unit test, so I will file that shortly. The fix for the testbug will also need to be backported to JDK 8. Test bug: JDK-8191269 16-11-2017 [~dkumar] JDK-8191269 has been pushed to FX 10-dev. When you backport this bug, you should also backport JDK-8191269 (or else fold the patch from that bug into the backported test). 16-11-2017 http://hg.openjdk.java.net/openjfx/10-dev/rt/rev/942008c95e91 09-11-2017 http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/06/ Aligned with latest tip, hence LWLightweightFramePeer import added to CPlatformWindow.java Tests mover to test.robot package Removed wildcard imports 09-11-2017 All looks good now. +1 08-11-2017 I'll review / run the FX tests in more detail, soon, but I noticed two quick issues: 1. All robot-based tests should be in the test.robot.** package hierarchy, so I recommend to put them in a new test.robot.javafx.embed.swing package. 2. Can you replace the wild-card imports with explicit imports? 07-11-2017 Test added: http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/05/ 07-11-2017 One thing to note is that we need a test case for this, perhaps a manual test in tests/manual, as was done for the Print dialogs. 10-10-2017 Looks good now. +1 10-10-2017 Updated version of getRawHandle looks good. I see that you don't pass the FX handle to Swing if javafx.embed.singleThread == true so the fix will not be active in that mode. Is there a reason for that? It seems that you should be able to pass the window handle to Swing, even if you don't want to enable the WINDOW_HIDDEN event handler, although you will still need a way to set it to null when the window is closed. Minor nit: can you add a blank line after the static import and before the '/**' class comment in SwingNode.java? The rest of the FX fix looks fine to me. I have tested on Linux, Windows, and Mac. 10-10-2017 http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/04/ Basically the initial idea was to remove postponed close for singleThread case (since we do not have to synchronize between FX and EDT threads). But it seems that I just got overworked with this issue, sorry for the mess with singleThread. 10-10-2017 Thanks for catching this, please find the new webrev with updated getRawHandle implementation in GtkWindow http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/03/ 10-10-2017 I took a quick look and if you replace the overridden implementation of getRawHandle in GtkWIndow.java with this: @Override public long getRawHandle() { return _getNativeWindowImpl(super.getRawHandle()); } you won't get the exception; the above seems more correct anyway unless I am missing something. 09-10-2017 If I modify the test program slightly to delay the adding of the Swing content then I can make the exception happen every time with the FX patch applied (either with or without the JDK patch). When I run the modified test program on a patched JDK + FX the fix sometimes works and sometimes fails after I get the exception. This is on Linux. 09-10-2017 I tried this with the FX changes (on a JDK without the changes), and I got this exception one time on Linux -- maybe timing related: Exception in thread "AWT-EventQueue-0" java.lang.IllegalStateException: This operation is permitted on the event thread only; currentThread = AWT-EventQueue-0 at javafx.graphics/com.sun.glass.ui.Application.checkEventThread(Application.java:443) at javafx.graphics/com.sun.glass.ui.Window.getNativeWindow(Window.java:360) at javafx.graphics/com.sun.glass.ui.gtk.GtkWindow.getNativeWindow(GtkWindow.java:194) at javafx.graphics/com.sun.glass.ui.gtk.GtkWindow.getRawHandle(GtkWindow.java:234) at javafx.graphics/com.sun.javafx.tk.quantum.WindowStage.getRawHandle(WindowStage.java:923) at javafx.swing/javafx.embed.swing.SwingNode.notifyNativeHandle(SwingNode.java:251) at javafx.swing/javafx.embed.swing.SwingNode.setContentImpl(SwingNode.java:392) at javafx.swing/javafx.embed.swing.SwingNode.lambda$setContent$5(SwingNode.java:273) at javafx.swing/javafx.embed.swing.SwingFXUtils.runOnEDT(SwingFXUtils.java:315) at javafx.swing/javafx.embed.swing.SwingNode.setContent(SwingNode.java:273) at SampleFx$1.run(SampleFx.java:42) at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:313) at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:764) at java.desktop/java.awt.EventQueue.access$500(EventQueue.java:97) at java.desktop/java.awt.EventQueue$3.run(EventQueue.java:717) at java.desktop/java.awt.EventQueue$3.run(EventQueue.java:711) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:89) at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:734) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:199) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90) 09-10-2017 http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/02/ Now we are postponing actual window closing, it happens only after we ensure that native window pointer is valid on Swing side. 09-10-2017 Please review the updated fix with reflection incorporated http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/01/ New issue created JDK-8187803 as JDK counterpart of this issue 22-09-2017 Two comments (Phil pointed out both of them in his email) 1. We will need a separate bug ID for the FX changes. You can link one as "blocks" the other. 2. In order to avoid the need for synchronized changes, which would also require a dependent issue to upgrade the FX build to gradle 4.1, I recommend you use reflection to call the new SwingAccessor method. It should catch the exception and continue if the method is not found (possibly with a warning print message). That way, the fixes can go in in either order; both will, of course, be needed for the fix to be in effect. 21-09-2017 review request: http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/00/ This fix passing native window handle of JavaFX stage to JDK side, and substitutes JDialog's parent with JavaFX stage 21-09-2017 The main idea is to pass native window handle of JavaFX stage to JDK, then we should make this window a parent of JDK dialog. Found a solution for Windows, working on Linux and Mac. 13-09-2017 This is conceptually similar to what was done to fix print dialogs (although that was done by passing a property with the ID of the FX peer). See JDK-8088395. 13-09-2017 Alex, please evaluate this issue 31-08-2017