<i18n dev> RFR: 8345143: Remove uses of SecurityManager in the java.desktop module

Alexander Zvegintsev azvegint at openjdk.org
Fri Nov 29 01:48:52 UTC 2024


On Thu, 28 Nov 2024 00:36:35 GMT, Phil Race <prr at openjdk.org> wrote:

> Remove SecurityManager related code in the desktop module that is not covered by other PRs

--- a/test/jdk/lib/client/ExtendedRobot.java
+++ b/test/jdk/lib/client/ExtendedRobot.java
@@ -57,11 +57,6 @@ public class ExtendedRobot extends Robot {
 
     private final int syncDelay = DEFAULT_SYNC_DELAY;
 
-    //TODO: uncomment three lines below after moving functionality to java.awt.Robot
-    //{
-    //    syncDelay = AccessController.doPrivileged(new GetIntegerAction("java.awt.robotdelay", DEFAULT_SYNC_DELAY));
-    //}
-
     /**
      * Constructs an ExtendedRobot object in the coordinate system of the primary screen.
      *

src/java.desktop/macosx/classes/com/apple/eio/FileManager.java line 144:

> 142:     public static void setFileType(String filename, int type) throws IOException {
> 143:         _setFileType(filename, type);
> 144:         }

indentation over the entire file is very unstable.

src/java.desktop/share/classes/java/awt/Font.java line 896:

> 894:      * font bytes.
> 895:      */
> 896:     private static boolean hasTempPermission() {

It looks like the `createFont(int fontFormat, InputStream fontStream)` and `createFonts(InputStream fonts)` now have a bit of dead code that can probably be removed(along with this method).


Like


public static Font[] createFonts(InputStream fontStream)
        throws FontFormatException, IOException {

    final int fontFormat = Font.TRUETYPE_FONT;
    return createFont0(fontFormat, fontStream, true, null);
}

src/java.desktop/share/classes/java/awt/Window.java line 2188:

> 2186:             Window window = ref.get();
> 2187:             if (window != null) {
> 2188:                 window.setAlwaysOnTop(alwaysOnTop);

Line 3026, comment can be removed:


if(aot) {
    setAlwaysOnTop(aot); // since 1.5; subject to permission check
}

src/java.desktop/share/classes/javax/swing/JTable.java line 5587:

> 5585:                     type = String.class;
> 5586:                 }
> 5587:                 SwingUtilities2.checkAccess(type.getModifiers());

--- a/src/java.desktop/share/classes/javax/swing/JTable.java
+++ b/src/java.desktop/share/classes/javax/swing/JTable.java
@@ -6364,9 +6364,6 @@ public boolean print(PrintMode printMode,
             }
         }
 
-        // Get a PrinterJob.
-        // Do this before anything with side-effects since it may throw a
-        // security exception - in which case we don't want to do anything else.
         final PrinterJob job = PrinterJob.getPrinterJob();
 
         if (isEditing()) {

src/java.desktop/share/classes/javax/swing/Popup.java line 247:

> 245:             // Try to set "always-on-top" for the popup window.
> 246:             // Applets usually don't have sufficient permissions to do it.
> 247:             // In this case simply ignore the exception.

Suggestion:

src/java.desktop/share/classes/javax/swing/TimerQueue.java line 181:

> 179:                     // Allow run other threads on systems without kernel threads
> 180:                     timer.getLock().newCondition().awaitNanos(1);
> 181:                     timer.getLock().unlock();

It might be a good idea to keep the unlock in the finally block, even if there is no obvious candidate to throw an exception.


https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/util/concurrent/locks/Lock.html

> With this increased flexibility comes additional responsibility. The absence of block-structured locking removes the automatic release of locks that occurs with synchronized methods and statements. In most cases, the following idiom should be used:
>
>  <pre>
>  Lock l = ...;
>  l.lock();
>  try {
>    // access the resource protected by this lock
>  } finally {
>    l.unlock();
>  }
> </pre>

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicComboBoxRenderer.java line 71:

> 69:     }
> 70: 
> 71:     @SuppressWarnings("removal")

Suggestion:

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicComboBoxRenderer.java line 74:

> 72:     private static Border getNoFocusBorder() {
> 73:         if (System.getSecurityManager() != null) {
> 74:             return SAFE_NO_FOCUS_BORDER;

`SAFE_NO_FOCUS_BORDER` can be removed in 3 files: 


./src/java.desktop/share/classes/javax/swing/DefaultListCellRenderer.java:85:    private static final Border SAFE_NO_FOCUS_BORDER = new EmptyBorder(1, 1, 1, 1);
./src/java.desktop/share/classes/javax/swing/plaf/basic/BasicComboBoxRenderer.java:60:    private static final Border SAFE_NO_FOCUS_BORDER = new EmptyBorder(1, 1, 1, 1);
./src/java.desktop/share/classes/javax/swing/table/DefaultTableCellRenderer.java:96:    private static final Border SAFE_NO_FOCUS_BORDER = new EmptyBorder(1, 1, 1, 1);

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicLabelUI.java line 474:

> 472:             AppContext appContext = AppContext.getAppContext();
> 473:             BasicLabelUI safeBasicLabelUI =
> 474:                     (BasicLabelUI) appContext.get(BASIC_LABEL_UI_KEY);

Declaration of the `BASIC_LABEL_UI_KEY` can be  removed

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalLabelUI.java line 75:

> 73:             AppContext appContext = AppContext.getAppContext();
> 74:             MetalLabelUI safeMetalLabelUI =
> 75:                     (MetalLabelUI) appContext.get(METAL_LABEL_UI_KEY);

`METAL_LABEL_UI_KEY` is not used anymore

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalSliderUI.java line 136:

> 134:     private static Icon getHorizThumbIcon() {
> 135:         if (System.getSecurityManager() != null) {
> 136:             return SAFE_HORIZ_THUMB_ICON;

`SAFE_HORIZ_THUMB_ICON` and `SAFE_VERT_THUMB_ICON` is no longer used

src/java.desktop/share/classes/sun/awt/OSInfo.java line 104:

> 102:     }
> 103: 
> 104:     public static WindowsVersion getWindowsVersion() {

--- a/src/java.desktop/share/classes/sun/awt/OSInfo.java
+++ b/src/java.desktop/share/classes/sun/awt/OSInfo.java
@@ -64,7 +64,7 @@ and so the method getWindowsVersion() will return the constant for known OS.
     private static final Map<String, WindowsVersion> windowsVersionMap = new HashMap<String, OSInfo.WindowsVersion>();
 
     // Cache the OSType for getOSType()
-    private static final OSType CURRENT_OSTYPE = getOSTypeImpl();  // No DoPriv needed
+    private static final OSType CURRENT_OSTYPE = getOSTypeImpl();
 
 
     static {

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

PR Review: https://git.openjdk.org/jdk/pull/22424#pullrequestreview-2468791195
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862664776
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862687940
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862691626
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862811167
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862801871
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862817529
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862695795
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862772241
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862774213
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862795969
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862770911
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1862806386


More information about the i18n-dev mailing list