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

Sat Nov 30 04:40:44 UTC 2024

On Thu, 28 Nov 2024 20:30:11 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:

>> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8345143
>
> 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.

Noted : out of scope. If you think it important I can file a follow-bug for some one to address.

> 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);
> }

Yes, I have a separate bug to clean up all of this https://bugs.openjdk.org/browse/JDK-8344146

> 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
> }

so remove comment ..

> 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()) {

removing comment

> 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>

ok

> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicComboBoxRenderer.java line 71:
> 
>> 69:     }
>> 70: 
>> 71:     @SuppressWarnings("removal")
> 
> Suggestion:

ok

> 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);

ok

> 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

ok

> 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

ok

> 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

ok

> 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 {

removing comment

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864054298
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864055229
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864055747
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864056130
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864066305
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864057745
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864058236
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864058393
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864058516
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864058783
PR Review Comment: https://git.openjdk.org/jdk/pull/22424#discussion_r1864059005