8231692: Test Infrastructure: enhance KeyEventFirer to inject keyEven…

…ts into scene

Reviewed-by: aghaisas

modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/KeyEventFirer.java

@@ -27,19 +27,60 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
+
+import javafx.event.Event;
+import javafx.event.EventTarget;
 import javafx.event.EventType;
+import javafx.scene.Scene;
 import javafx.scene.input.KeyCode;
 import javafx.scene.input.KeyEvent;
-import javafx.event.Event;
-import javafx.event.EventTarget;
 
 
 public class KeyEventFirer {
 
     private final EventTarget target;
-
+    private final Scene scene;
+
+    /**
+     * Instantiates a KeyEventFirer on the given event target. KeyEvents are
+     * fired directly onto the target.
+     *
+     * <p>
+     * Beware: using this constructor on an <code>EventTarget</code> of type <code>Node</code>
+     * which is not focusOwner may lead
+     * to false greens (see https://bugs.openjdk.java.net/browse/JDK-8231692).
+     *
+     * @param target the target to fire keyEvents onto, must not be null.
+     * @throws NullPointerException if target is null.
+     */
     public KeyEventFirer(EventTarget target) {
+        this(Objects.requireNonNull(target), null);
+    }
+
+    /**
+     * Instantiates a KeyEventFirer for the given target and scene.
+     * Any one of those can be null, but not both. A null/not null scene decides
+     * about the delivering path of events. If null, events are delivered
+     * via <code>EventUtils.fire(target, keyEvent)</code>, otherwise via
+     * <code>scene.processKeyEvent(keyEvent)</code>.
+     * <p>
+     * Note that in the latter case, the target doesn't matter - the scene
+     * delivers keyEvents to its focusOwner. Calling code is responsible to
+     * setup focus state as required.
+     *
+     * @param target eventTarget used to create the event for and fire events onto
+     *    if there's no scene
+     * @param scene to use for delivering events to its focusOwner if not null
+     *
+     * @throws NullPointerException if both target and scene are null
+     */
+    public KeyEventFirer(EventTarget target, Scene scene) {
         this.target = target;
+        this.scene = scene;
+        if (target == null && scene == null) {
+            throw new NullPointerException("both target and scene are null");
+        }
     }
 
     public void doUpArrowPress(KeyModifier... modifiers) {
@@ -66,9 +107,21 @@ public void doKeyTyped(KeyCode keyCode, KeyModifier... modifiers) {
         fireEvents(createEvent(keyCode, KeyEvent.KEY_TYPED, modifiers));
     }
 
+    /**
+     * Dispatches the given events. The process depends on the state of
+     * this firer. If the scene is null, the events are delivered via
+     * Event.fireEvent(target,..), otherwise they are delivered via
+     * scene.processKeyEvent.
+     *
+     * @param events the events to dispatch.
+     */
     private void fireEvents(KeyEvent... events) {
         for (KeyEvent evt : events) {
-            Event.fireEvent(target, evt);
+            if (scene != null) {
+                scene.processKeyEvent(evt);
+            } else {
+                Event.fireEvent(target, evt);
+            }
         }
     }
 

modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/KeyEventFirerTest.java

@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test.com.sun.javafx.scene.control.infrastructure;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static javafx.scene.input.KeyCode.*;
+import static javafx.scene.input.KeyEvent.*;
+import static org.junit.Assert.*;
+
+import javafx.scene.Node;
+import javafx.scene.Scene;
+import javafx.scene.control.Button;
+import javafx.scene.control.TextField;
+import javafx.scene.input.KeyEvent;
+import javafx.scene.layout.Pane;
+import javafx.scene.layout.VBox;
+import javafx.stage.Stage;
+
+/**
+ * Test of enhanced KeyEventFirer.
+ */
+public class KeyEventFirerTest {
+
+    private TextField textField;
+    private Button button;
+    private Pane root;
+    private Stage stage;
+    private Scene scene;
+
+    /**
+     * Test that keyEvent is delivered to focused control and nowhere else.
+     */
+    @Test
+    public void testFireViaScene() {
+        showAndFocus(button);
+        List<KeyEvent> buttonEvents = new ArrayList<>();
+        button.addEventHandler(KEY_PRESSED, buttonEvents::add);
+        List<KeyEvent> textFieldEvents = new ArrayList<>();
+        textField.addEventHandler(KEY_PRESSED, textFieldEvents::add);
+        KeyEventFirer firer = new KeyEventFirer(textField, scene);
+        firer.doKeyPress(A);
+        assertEquals("button must have received the key", 1, buttonEvents.size());
+        assertEquals("textField must not have received the key", 0, textFieldEvents.size());
+    }
+
+    /**
+     * Test that keyEvent is delivered to focused control and nowhere else.
+     * Here we test that the target is not required.
+     */
+    @Test
+    public void testFireViaSceneNullTarget() {
+        showAndFocus(button);
+        List<KeyEvent> buttonEvents = new ArrayList<>();
+        button.addEventHandler(KEY_PRESSED, buttonEvents::add);
+        List<KeyEvent> textFieldEvents = new ArrayList<>();
+        textField.addEventHandler(KEY_PRESSED, textFieldEvents::add);
+        KeyEventFirer firer = new KeyEventFirer(null, scene);
+        firer.doKeyPress(A);
+        assertEquals("button must have received the key", 1, buttonEvents.size());
+        assertEquals("textField must not have received the key", 0, textFieldEvents.size());
+    }
+
+    /**
+     * This simulates a false positive: even though not focused, the textField handlers
+     * are notified when firing directly. That's possible, but typically not what we want to test!
+     */
+    @Test
+    public void testFireTargetFalseGreen() {
+        showAndFocus(button);
+        List<KeyEvent> buttonEvents = new ArrayList<>();
+        button.addEventHandler(KEY_PRESSED, buttonEvents::add);
+        List<KeyEvent> textFieldEvents = new ArrayList<>();
+        textField.addEventHandler(KEY_PRESSED, textFieldEvents::add);
+        // firing on a node that is not focusOwner
+        KeyEventFirer incorrectFirer = new KeyEventFirer(textField);
+        incorrectFirer.doKeyPress(A);
+        int falseTextFieldNotification = textFieldEvents.size();
+        int falseButtonNotification = buttonEvents.size();
+        assertEquals("false green - textField must have received the key", 1, textFieldEvents.size());
+        assertEquals("false green - button must not have received the key", 0, buttonEvents.size());
+        textFieldEvents.clear();
+        buttonEvents.clear();
+        // firing on the scene makes a difference
+        KeyEventFirer correctFirer = new KeyEventFirer(null, scene);
+        correctFirer.doKeyPress(A);
+        assertEquals(falseTextFieldNotification - 1, textFieldEvents.size());
+        assertEquals(falseButtonNotification + 1, buttonEvents.size());
+    }
+
+    @Test (expected= NullPointerException.class)
+    public void testTwoParamConstructorNPE() {
+        new KeyEventFirer(null, null);
+    }
+
+    @Test (expected= NullPointerException.class)
+    public void testSingleParamConstructorNPE() {
+        new KeyEventFirer(null);
+    }
+
+    /**
+     * Need all: stage.show, stage.requestFocus and control.requestFocus to
+     * have consistent focused state on control (that is focusOwner and isFocused)
+     */
+    @Test
+    public void testUIState() {
+        assertEquals(List.of(button, textField), root.getChildren());
+        stage.show();
+        stage.requestFocus();
+        button.requestFocus();
+        assertEquals(button, scene.getFocusOwner());
+        assertTrue(button.isFocused());
+    }
+
+    private void showAndFocus(Node focused) {
+        stage.show();
+        stage.requestFocus();
+        if (focused != null) {
+            focused.requestFocus();
+            assertTrue(focused.isFocused());
+            assertSame(focused, scene.getFocusOwner());
+        }
+    }
+
+    @Before
+    public void setup() {
+        root = new VBox();
+        scene = new Scene(root);
+        stage = new Stage();
+        stage.setScene(scene);
+        button = new Button("I'm a button");
+        textField = new TextField("some text");
+        root.getChildren().addAll(button, textField);
+    }
+
+    @After
+    public void cleanup() {
+        if (stage != null) {
+            stage.hide();
+        }
+    }
+
+}