[Rev 03] RFR: 8200224: Multiple press event when JFXPanel gains focus

[Kevin Rushforth]

On Tue, 26 Nov 2019 09:23:58 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

> The pull request has been updated with a complete new set of changes (possibly due to a rebase).
> 
> ----------------
> 
> Commits:
>  - 44774dfb: JDK-8200224
>  - d1309fb6: JDK-8200224
>  - 53a66b8f: JDK-8200224
>  - 0be3cb5b: JDK-8200224
>  - 03b59288: Merge branch 'master' of https://github.com/openjdk/jfx into JDK-8200224
>  - 5cd96a56: JDK-8200224
>  - 85c87628: JDK-8200224
>  - 314e423c: JDK-8200224
>  - 725e5fef: JDK-8200224
> 
> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>  Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.03
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>   Stats: 272 lines in 3 files changed: 147 ins; 120 del; 5 mod
>   Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25

To follow-up on my previous comment here are the requested changes:

1. Restore ` tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java`, which was removed by your last commit, so that it doesn't show up in the PR.
2. In the new test, add a dummy `JFXPanel` to the `JPanel` so that the test `JFXPanel` is the second one added (see my inline comments). This will allow the test to catch the error on Windows by failing without your fix.
3. A few other inline comments on the new test.

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
> 3:  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

That should be `2019`

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 35:

> 34: 
> 35: 
> 36: import javax.swing.JFrame;

You only need one blank line here.

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 94:

> 93: 
> 94:     class TestFXPanel extends JFXPanel {
> 95:         protected void processMouseEventPublic(MouseEvent e) {

This class can be `static`

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 108:

> 107: 
> 108:         SwingUtilities.invokeLater(() -> {
> 109:             TestFXPanel fxPnl = new TestFXPanel();

Add the following here:
            JFXPanel dummyFXPanel = new JFXPanel();
            dummyFXPanel.setPreferredSize(new Dimension(100, 100));

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 112:

> 111:             JFrame jframe = new JFrame();
> 112:             JPanel jpanel = new JPanel();
> 113:             jpanel.add(fxPnl);

Add the following here:
            jpanel.add(dummyFXPanel);

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 118:

> 117: 
> 118:             Platform.runLater(() -> {
> 119:                 Group grp = new Group();

Add the following here:
                Scene dummyScene = new Scene(new Group());
                dummyFXPanel.setScene(dummyScene);

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 138:

> 137: 
> 138:         if(!firstPressedEventLatch.await(5000, TimeUnit.MILLISECONDS)) {
> 139:             throw new Exception();

This `if` test can be written more succinctly as:

    assertTrue(firstPressedEventLatch.await(5000, TimeUnit.MILLISECONDS));

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 72:

> 71:     @BeforeClass
> 72:     public static void doSetupOnce() {
> 73:         // Start the Application

If you add `throws Exception` then you don't need the try/catch around the `launchLatch.await`.

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelTest.java line 119:

> 118:             Platform.runLater(() -> {
> 119:                 Group grp = new Group();
> 120:                 Scene scene = new Scene(new Group());

This variable is unused.

----------------

Changes requested by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/25