RFR: 8289770: Remove Windows version macro from ShellFolder2.cpp

Alexey Ivanov aivanov at openjdk.org
Tue Apr 16 11:30:00 UTC 2024


On Tue, 16 Apr 2024 06:58:19 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> This clean-up PR removes unused Windows version macro from `ShellFolder2.cpp`.
>> 
>> `IS_WINVISTA` was not used at all.
>> 
>> `IS_WINXP` guarded support for icons with alpha channel. It is now safe to assume Java runs on a Windows version later than Windows XP. Java launchers specify 6.0 as the minimum OS version which corresponds to Windows Vista.
>
> src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp line 131:
> 
>> 129: #ifndef IS_WINVISTA
>> 130: #define IS_WINVISTA (!(::GetVersion() & 0x80000000) && LOBYTE(LOWORD(::GetVersion())) >= 6)
>> 131: #endif
> 
> there are exactly the same macro in awt.h which are mostly unused except "IS_WIN8"

There's a separate bug for removing the macro from `awt.h`:

[JDK-8289772](https://bugs.openjdk.org/browse/JDK-8289772): Remove obsolete Windows version macros: IS_WIN2000, IS_WINXP, IS_WINVISTA

Why am I taking `ShellFolder2.cpp` separately? The comment in the `ShellFolder2.cpp` file suggest these are copies from `awt.h`. This makes `ShellFolder2.cpp` isolated from any other usage. Removing the macro cleans up a single file, easy to do, easy to review.

Removing the macro from `awt.h` is not as simple, some macro are used in the code. For example, `IS_WIN8` is used in `awt_Toolkit.cpp` to determine whether touch keyboard handling is enabled.

https://github.com/openjdk/jdk/blob/58911ccc2c48b4b5dd2ebc9d2a44dcf3737eca50/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp#L669

Then `IS_WINVISTA` is used in the code which determines whether DWM is available; `IS_WINXP` is used in many places to determine menu bar colors, to handle subclassing default Windows controls. Removing the macro will make the code cleaner but it requires modifying the code. Eventually, the task may be broken into smaller pieces.

The usage of `IS_WIN8` could be replaced with [`IsWindows8OrGreater`](https://learn.microsoft.com/en-us/windows/win32/api/VersionHelpers/nf-versionhelpers-iswindows8orgreater) which is part of [Version Helper functions](https://learn.microsoft.com/en-us/windows/win32/sysinfo/version-helper-apis) and is the recommended method to determine the version of the Windows OS.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18736#discussion_r1567192769


More information about the client-libs-dev mailing list