6603771: Nimbus L&F: Ctrl+F7 keybinding for Jinternal Frame throws a NPE.

[prsadhuk]

In Swingset2 JInternalFrame demo, focusing on any internalframes and pressing Ctrl+F7 and then any of up/down/left/arrow key to move the internal frames results in NPE in NimbusL&F.
It is because JComponent.processKeyBinding() calls SwingUtilities.notifyAction which calls BasicDesktopPaneUI.actionPerformed where it gets Desktop.minOnScreenInsets property to get the Insets and then uses insets.top/bottom/left/right value to move the internalframe but this property is not defined for Nimbus so deferencing Insets object results in NPE.

Fix is added to create a 0 insets incase some L&F do not define Desktop.minOnScreenInsets property.
No regression test is added as it can be easily checked by SwingSet2 JInternalFrame demo.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-6603771: Nimbus L&F: Ctrl+F7 keybinding for Jinternal Frame throws a NPE.

Reviewers

• Abhishek Kumar (@kumarabhi006 - Author) ⚠️ Review applies to 051b325c
• Sergey Bylokhov (@mrserb - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/11605/head:pull/11605
$ git checkout pull/11605

Update a local copy of the PR:
$ git checkout pull/11605
$ git pull https://git.openjdk.org/jdk pull/11605/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11605

View PR using the GUI difftool:
$ git pr show -t 11605

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/11605.diff

[bridgekeeper]

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@prsadhuk The following label will be automatically applied to this pull request:

• client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (c7fe6a4a)
• 02: Full - Incremental (8368f130)
• 01: Full - Incremental (a4927f57)
• 00: Full (051b325c)

[kumarabhi006]

Verified in SwingSet2 demo.

[mrserb]

It would be good to add a test case to make sure this bug will not happen someday again.

[prsadhuk]

It would be good to add a test case to make sure this bug will not happen someday again.

I tried but it seems AWT EventQueue is gobbling up the NPE and not throwing it to application which is why even though NPE is seen but test does not fail...

/*
 * Copyright (c) 2022, 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.
 *
 * 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.
 */
/*
 * @test
 * @key headful
 * @bug 6603771
 * @summary  Verifies Nimbus L&F: Ctrl+F7 keybinding for Jinternal Frame throws a NPE
 * @run main JInternalFrameTest
 */
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

public class JInternalFrameTest {
    static JFrame jFrame;
    static JInternalFrame iFrame;

     private static void setLookAndFeel(UIManager.LookAndFeelInfo laf) {
        try {
            UIManager.setLookAndFeel(laf.getClassName());
//	    UIManager.setLookAndFeel("javax.swing.plaf.nimbus.NimbusLookAndFeel");
        } catch (UnsupportedLookAndFeelException ignored) {
            System.out.println("Unsupported L&F: " + laf.getClassName());
        } catch (ClassNotFoundException | InstantiationException
                | IllegalAccessException e) {
            throw new RuntimeException(e);
        }
    }

    public static void main(String[] args) throws Exception {
        Robot robot = new Robot();
        for (UIManager.LookAndFeelInfo laf :
                 UIManager.getInstalledLookAndFeels()) {
            System.out.println("Testing L&F: " + laf.getClassName());
            SwingUtilities.invokeAndWait(() -> setLookAndFeel(laf));
            try {
                SwingUtilities.invokeAndWait(() -> createUI());
                robot.waitForIdle();
                robot.delay(1000);

                Point pt = iFrame.getLocationOnScreen();
                Rectangle dim = iFrame.getBounds();
                robot.mouseMove(pt.x + dim.width/3, pt.y);
                robot.waitForIdle();
                robot.mousePress(InputEvent.BUTTON1_DOWN_MASK);
                robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
                robot.waitForIdle();
                robot.delay(1000);
                robot.keyPress(KeyEvent.VK_CONTROL);
                robot.keyPress(KeyEvent.VK_F7);
                robot.keyRelease(KeyEvent.VK_F7);
                robot.keyRelease(KeyEvent.VK_CONTROL);
                robot.waitForIdle();
                robot.delay(1000);
                robot.keyPress(KeyEvent.VK_UP);
                robot.keyRelease(KeyEvent.VK_UP);
                robot.waitForIdle();
                robot.delay(1000);
            } finally {
                if (jFrame != null) {
                    SwingUtilities.invokeAndWait(() -> jFrame.dispose());
                }
            }
        }
    }
   
    private static void createUI() {	
     	jFrame = new JFrame();

	JDesktopPane desktopPane = new JDesktopPane();

        iFrame =  new JInternalFrame("Test");

	iFrame.setTitle("InternalFrame");
	iFrame.setClosable(true);
	iFrame.setMaximizable(true);
	iFrame.setIconifiable(true);
	iFrame.setResizable(true);
	iFrame.setLocation(50, 50);
        iFrame.setSize(200, 200);
        iFrame.setVisible(true);

	desktopPane.add(iFrame);

	JPanel panel = new JPanel();
	panel.setLayout(new BorderLayout());
	panel.add(desktopPane, BorderLayout.CENTER);

	jFrame.add(panel, BorderLayout.CENTER);
        jFrame.setSize(400, 400);
	jFrame.setLocationRelativeTo(null);
        jFrame.setVisible(true);
	
    }
}    

[mrserb]

You can install the UncaughtExceptionHandler on EDT to catch that exception:

    public static void main(String[] args) throws Exception {
        EventQueue.invokeAndWait(
                () -> Thread.currentThread().setUncaughtExceptionHandler(
                        (t, e) -> System.out.println("Error?")));

[mrserb]

Looks like there are many "Whitespace error" reported by jcheck in the test.

[openjdk]

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

6603771: Nimbus L&F: Ctrl+F7 keybinding for Jinternal Frame throws a NPE.

Reviewed-by: abhiscxk, serb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 196 new commits pushed to the master branch:

• b17c524: 8298059: Linked stack watermarks don't support nesting
• 98fa48c: 8298093: improve cleanup and error handling of awt_parseColorModel in awt_parseImage.c
• 5f63f7a: 8298726: (fs) Change PollingWatchService to record last modified time as FileTime rather than milliseconds
• 3ae7187: 8298498: sun/net/www/http/KeepAliveCache/B8291637.java fails with "Server exception terminating: java.net.SocketException: Socket closed"
• b9074fa: 8298249: Excessive memory allocation in CipherInputStream AEAD decryption
• 10bc86c: Merge
• 80cadd4: 8298785: gc/TestFullGCCount.java fails with "invalid boolean value: null' for expression vm.opt.ExplicitGCInvokesConcurrent'"
• ebc4710: 8298277: Replace "session" with "scope" for FFM access
• d1085d1: 8281946: VM.native_memory should report size of shareable memory
• 3ef3824: 8298794: Remove JVM_ACC_PROMOTED_FLAGS breaks minimal build
• ... and 186 more: https://git.openjdk.org/jdk/compare/b49fd920b6690a8b828c85e45c10e5c4c54d2022...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[turbanoff]

Suggested change

	iFrame = new JInternalFrame("Test");
	iFrame = new JInternalFrame("Test");

[bridgekeeper]

@prsadhuk This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[prsadhuk]

/integrate

[openjdk]

Going to push as commit 7465de4.
Since your change was applied there have been 689 commits pushed to the master branch:

• fbe5ab0: 8300830: Remove redundant assertion in src/hotspot/share/runtime/javaCalls.cpp
• 1339461: 8300981: Build failure on 32-bit platforms after JDK-8281213
• 81d523d: Merge
• 60b8a98: 8300591: @SuppressWarnings option "lossy-conversions" missing from jdk.compiler module javadoc
• a3ed7e9: 8300623: Lambda deserialization regression involving Enum method reference
• cf46004: 8300272: Improve readability of the test JarWithOneNonDisabledDigestAlg
• b678e70: 8300942: JDK-8299684 breaks x86 build
• 3be5758: 8300769: Remove G1CollectionSet::_inc_bytes_used_before
• 859ca75: 8300862: Remove some G1 collection set remembered set debugging code
• 048705c: 8300910: Remove metaprogramming/integralConstant.hpp
• ... and 679 more: https://git.openjdk.org/jdk/compare/b49fd920b6690a8b828c85e45c10e5c4c54d2022...master

Your commit was automatically rebased without conflicts.

[openjdk]

@prsadhuk Pushed as commit 7465de4.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.