On Sat, 8 May 2021 00:19:21 GMT, Alexander Zuev <kiz...@openjdk.org> wrote:

>> Fix updated after first round of review.
>
> Alexander Zuev has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Small fixes
>    Added testing for MultiResolutionImage
>  - Merge remote-tracking branch 'origin/master' into JDK-8182043
>  - Update src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp
>    
>    Select one icon at a time.
>    
>    Co-authored-by: Alexey Ivanov 
> <70774172+aivanov-...@users.noreply.github.com>
>  - Small fixes
>    Remived size parameter from makeIcon
>    Removed useVGAColors usage as irrelevant since it is ignored on all
>    supported platforms now
>  - 8182043: Access to Windows Large Icons
>    Fix updated based on the previous review.

Changes requested by aivanov (Reviewer).

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1098:

> 1096:                                         imageCache = getLargeIcon ? 
> smallSystemImages : largeSystemImages;
> 1097:                                     }
> 1098:                                     newIcon2 = 
> imageCache.get(Integer.valueOf(index));

Probably, explicit boxing is unnecessary.

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1192:

> 1190:      */
> 1191:     static Image getShell32Icon(int iconID, int size) {
> 1192:         long hIcon = getIconResource("shell32.dll", iconID, size, size);

It's outside the scope for this code review but still, should `getIconResource` 
free the loaded library? With each call, the library gets loaded but it's never 
freed and thus it's never unloaded even if it's not needed any more.

src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1196:

> 1194:             Image icon = makeIcon(hIcon);
> 1195:             disposeIcon(hIcon);
> 1196:             return icon;

Shall it not be wrapped into multi-resolution image if the `size` is different 
from the actual size of the icon?
Previously, it was inside `makeIcon`.

src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp line 929:

> 927:     }
> 928:     HRESULT hres;
> 929:     IExtractIconW* pIcon;

`pIcon->Release()` must be called before returning as done in `extractIcon`.

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

PR: https://git.openjdk.java.net/jdk/pull/2875

Reply via email to