Re: RFR: 8331746: Create a test to verify that the cmm id is not ignored [v3]
> The new test to cover the https://bugs.openjdk.org/browse/JDK-8326661 and > verify that the cmm id of the icc profile is properly reported. Before > JDK-8321489 we always report 'lcms' as a cmm id. Sergey Bylokhov 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 four additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8331746 - Update CustomCMMID.java - Update CustomCMMID.java - 8331746: Create a test to verify that the cmm id is not ignored - Changes: - all: https://git.openjdk.org/jdk/pull/19110/files - new: https://git.openjdk.org/jdk/pull/19110/files/6bfb2cb8..c6109b3d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19110=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19110=01-02 Stats: 24165 lines in 537 files changed: 13082 ins; 7446 del; 3637 mod Patch: https://git.openjdk.org/jdk/pull/19110.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19110/head:pull/19110 PR: https://git.openjdk.org/jdk/pull/19110
RFR: 8329667: [macos] Issue with JTree related fix for JDK-8317771
Caching children and selected children of the thee on the native level; Caching all children of a specific parent in CAccessibility to enhance recursive children selection algorithm; Removing fix for JDK-8317771 as no longer needed; - Commit messages: - 8329667: [macos] Issue with JTree related fix for JDK-8317771 Changes: https://git.openjdk.org/jdk/pull/19255/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19255=00 Issue: https://bugs.openjdk.org/browse/JDK-8329667 Stats: 151 lines in 4 files changed: 80 ins; 62 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/19255.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19255/head:pull/19255 PR: https://git.openjdk.org/jdk/pull/19255
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v3]
> In GTK LAF, the menu mnemonics are always displayed which is different from > the native behavior. In native application **(tested with gedit**), the menu > mnemonics toggle on press of `ALT` key. Menu mnemonics are hidden initially > and then toggles between show/hide on `ALT` press. > Proposed fix is to handle the `ALT` key press for GTK LAF and mimic the > native behavior. Fix is similar to the `ALT` key processing in Windows LAF. > Automated test case is added to verify the fix and tested in Ubuntu and > Oracle linux. > > CI testing is green and link attached in JBS. Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision: Alt press property added for GTK L - Changes: - all: https://git.openjdk.org/jdk/pull/18992/files - new: https://git.openjdk.org/jdk/pull/18992/files/fa18d524..d097dcdd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18992=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18992=01-02 Stats: 18 lines in 3 files changed: 10 ins; 8 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18992.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18992/head:pull/18992 PR: https://git.openjdk.org/jdk/pull/18992
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v2]
On Thu, 9 May 2024 17:47:03 GMT, Phil Race wrote:
>> Abhishek Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Var moved to local scope
>
> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthLookAndFeel.java
> line 668:
>
>> 666: KeyboardFocusManager.getCurrentKeyboardFocusManager().
>> 667: addPropertyChangeListener(_handler);
>> 668: if (UIManager.getLookAndFeel().getName().contains("GTK")) {
>
> This doesn't seem like an ideal way of doing this.
> I don't see any precedent for this approach in the Swing implementation.
>
> We need some method that is more about the properties of a L rather than
> keying off name.
>
> Please look around at LookAndFeel and UIDefaults (etc) for a place you could
> store a property that tells you what to do.
> Or perhaps you this "altProcessor" would be the value you retrieve and if it
> is non-null you install it.
> Or perhaps this should be pushed down to the GTK L
>
> I don't know what the right answer is, I just don't think this approach is it.
@prrace Added the property to handle ALT key press specific to GTK L Based
on the property value the event handler for ALT key press is installed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1602065156
Re: RFR: 8321428: Deprecate for removal the package java.beans.beancontext [v5]
> the beancontext package was added (by me) in JDK 1.2 to provide JavaBeans(tm) > with a containment and services hierarchy. > > based upon concepts from OpenDoc, which was a popular component model at the > time, the API pre-dated the addition of language features such as > annotations, and the invention and adoption of programming design patterns > such as "Inversion of Control" and/or "Dependency Injection". > > This API (package) has not evolved with either the language or current design > trends, as such it is, at best, anachronistic, and is probably an > anti-pattern that should be avoided. > > This package is therefore deprecated **FOR REMOVAL IN AT FUTURE RELEASE** Larry Cable has updated the pull request incrementally with one additional commit since the last revision: removed trailing whitespace reported by jcheck - Changes: - all: https://git.openjdk.org/jdk/pull/18569/files - new: https://git.openjdk.org/jdk/pull/18569/files/86680a2c..255b17d0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18569=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18569=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18569.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18569/head:pull/18569 PR: https://git.openjdk.org/jdk/pull/18569
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting > the use of JNI in the following ways: > > * `System::load` and `System::loadLibrary` are now restricted methods > * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods > * binding a JNI `native` method declaration to a native implementation is now > considered a restricted operation > > This PR slightly changes the way in which the JDK deals with restricted > methods, even for FFM API calls. In Java 22, the single > `--enable-native-access` was used both to specify a set of modules for which > native access should be allowed *and* to specify whether illegal native > access (that is, native access occurring from a module not specified by > `--enable-native-access`) should be treated as an error or a warning. More > specifically, an error is only issued if the `--enable-native-access flag` is > used at least once. > > Here, a new flag is introduced, namely > `illegal-native-access=allow/warn/deny`, which is used to specify what should > happen when access to a restricted method and/or functionality is found > outside the set of modules specified with `--enable-native-access`. The > default policy is `warn`, but users can select `allow` to suppress the > warnings, or `deny` to cause `IllegalCallerException` to be thrown. This > aligns the treatment of restricted methods with other mechanisms, such as > `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. > > Some changes were required in the package-info javadoc for > `java.lang.foreign`, to reflect the changes in the command line flags > described above. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.org/jdk/pull/19213/files - new: https://git.openjdk.org/jdk/pull/19213/files/daf729f4..1c45e5d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19213=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19213=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19213.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19213/head:pull/19213 PR: https://git.openjdk.org/jdk/pull/19213
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v5]
On Wed, 15 May 2024 10:40:34 GMT, Maurizio Cimadamore
wrote:
>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting
>> the use of JNI in the following ways:
>>
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is
>> now considered a restricted operation
>>
>> This PR slightly changes the way in which the JDK deals with restricted
>> methods, even for FFM API calls. In Java 22, the single
>> `--enable-native-access` was used both to specify a set of modules for which
>> native access should be allowed *and* to specify whether illegal native
>> access (that is, native access occurring from a module not specified by
>> `--enable-native-access`) should be treated as an error or a warning. More
>> specifically, an error is only issued if the `--enable-native-access flag`
>> is used at least once.
>>
>> Here, a new flag is introduced, namely
>> `illegal-native-access=allow/warn/deny`, which is used to specify what
>> should happen when access to a restricted method and/or functionality is
>> found outside the set of modules specified with `--enable-native-access`.
>> The default policy is `warn`, but users can select `allow` to suppress the
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This
>> aligns the treatment of restricted methods with other mechanisms, such as
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>>
>> Some changes were required in the package-info javadoc for
>> `java.lang.foreign`, to reflect the changes in the command line flags
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Refine warning text for JNI method binding
src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 871:
> 869: return IllegalNativeAccess.WARN;
> 870: } else {
> 871: fail("Value specified to --illegal-access not recognized:"
Typo in the message, should be --illegal-native-access.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601898238
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote:
>> WIP
>>
>> This changeset contains hsdis for Windows/gcc Port. It supports both the
>> binutils and capstone backends, though the LLVM backend is left out due to
>> compatibility issues encountered during the build. Currently, which gcc
>> distributions are supported is still to be clarified, as several, ranging
>> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason,
>> the build system hack in place at the moment to compile the binutils backend
>> on Windows is still left in place, for now.
>
> Julian Waters has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains two commits:
>
> - Add check for compiler in configure
> - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
Hi Julian, sorry for not getting back to you.
The problem from my PoV is that this is a very large and intrusive patch in the
build of the actual product, for a claimed problem in the tangential hsdis
library. I have not understood a pressing business need to get a Windows/gcc
port for hsdis, which means I can't really prioritize trying to understand this
patch.
As you know, the build system does not currently really separate between "the
OS is Windows" and "the toolchain is Microsoft". I understand that you want to
fix that, and in theory I support it. However, you must also realize that
making a complete fix of this requires a lot of work, for something that there
is no clearly defined need. (After all, cl.exe works fine to create the build,
has no apparent issues, and is as far as an "official" compiler for Windows as
you can get.) That makes it fall squarely in the WIBNIs bucked ("wouldn't it be
nice if...").
If you can fix just the hsdis build without changing anything outside the hsdis
Makefiles, that would be a different story. Such a change would be limited in
scope, easy to say it will not affect the product proper, and be easier to
review.
-
PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112546029
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Add check for compiler in configure > - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis Magnus? Erik? You guys there? :( - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112465392
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]
On Wed, 15 May 2024 10:34:01 GMT, Maurizio Cimadamore wrote: > I don't fully agree that this option is not module related (which is why I > gave it that name). The very definition of illegal native access is related > to native access occurring from a module that is outside a specific set. So I > think it's 50/50 as to whether this option is module-related or not. Of > course I can fix the code if there's something clearly better. It maps here to a jdk.module.* property so I suppose it's okay. The functions were introduced to create jdk.module.* properties where the values were a set module names or a module path. Maybe these functions should be renamed at some point (not here) as they are more widely useful now. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601421535
Re: RFR: 8331746: Create a test to verify that the cmm id is not ignored [v2]
On Tue, 14 May 2024 19:12:25 GMT, Sergey Bylokhov wrote: >> The new test to cover the https://bugs.openjdk.org/browse/JDK-8326661 and >> verify that the cmm id of the icc profile is properly reported. Before >> JDK-8321489 we always report 'lcms' as a cmm id. > > Sergey Bylokhov has updated the pull request incrementally with one > additional commit since the last revision: > > Update CustomCMMID.java test/jdk/java/awt/color/ICC_Profile/CustomCMMID.java line 60: > 58: byte[] header = p.getData(ICC_Profile.icSigHead); > 59: byte[] id = new byte[4]; > 60: System.arraycopy(header, ICC_Profile.icHdrCmmId, id, 0, 4); Does it make sense to use a constant `ID_LENGTH` instead of 4? The calls to arraycopy could use `JAVA_ID.length` and `id.length` correspondingly. - PR Review Comment: https://git.openjdk.org/jdk/pull/19110#discussion_r1601392726
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v5]
> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting > the use of JNI in the following ways: > > * `System::load` and `System::loadLibrary` are now restricted methods > * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods > * binding a JNI `native` method declaration to a native implementation is now > considered a restricted operation > > This PR slightly changes the way in which the JDK deals with restricted > methods, even for FFM API calls. In Java 22, the single > `--enable-native-access` was used both to specify a set of modules for which > native access should be allowed *and* to specify whether illegal native > access (that is, native access occurring from a module not specified by > `--enable-native-access`) should be treated as an error or a warning. More > specifically, an error is only issued if the `--enable-native-access flag` is > used at least once. > > Here, a new flag is introduced, namely > `illegal-native-access=allow/warn/deny`, which is used to specify what should > happen when access to a restricted method and/or functionality is found > outside the set of modules specified with `--enable-native-access`. The > default policy is `warn`, but users can select `allow` to suppress the > warnings, or `deny` to cause `IllegalCallerException` to be thrown. This > aligns the treatment of restricted methods with other mechanisms, such as > `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. > > Some changes were required in the package-info javadoc for > `java.lang.foreign`, to reflect the changes in the command line flags > described above. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Refine warning text for JNI method binding - Changes: - all: https://git.openjdk.org/jdk/pull/19213/files - new: https://git.openjdk.org/jdk/pull/19213/files/0d21bf99..daf729f4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19213=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19213=03-04 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19213.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19213/head:pull/19213 PR: https://git.openjdk.org/jdk/pull/19213
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]
On Wed, 15 May 2024 06:15:35 GMT, Alan Bateman wrote: >> So my recollection/understanding is that we use this mechanism to convert >> module-related `--` flags passed to the VM into system properties that the >> Java code can then read, but we set them up such that you are not allowed to >> specify them directly via `-D`. Is the question here whether this new >> property should be in the `jdk.module` namespace? > > That's my recollection too. The usage here isn' related to modules which > makes me wonder if this function should be renamed (not by this PR of course) > of if we should be using PropertyList_unique_add (with AddProperty, > WriteableProperty, InternalProperty) instead. There will be further GNU style > options coming that will likely need to map to an internal system property in > the same way. I don't fully agree that this option is not module related (which is why I gave it that name). The very definition of illegal native access is related to native access occurring from a module that is outside a specific set. So I think it's 50/50 as to whether this option is module-related or not. Of course I can fix the code if there's something clearly better. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601386336
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v4]
On Wed, 15 May 2024 07:55:27 GMT, ExE Boss wrote: > Note that this line is still not entirely correct, as for code like: You are correct - the message is however consistent with what written in JEP 472. I'll discuss with @pron - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601335120
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v4]
On Tue, 14 May 2024 18:10:28 GMT, Maurizio Cimadamore
wrote:
>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting
>> the use of JNI in the following ways:
>>
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is
>> now considered a restricted operation
>>
>> This PR slightly changes the way in which the JDK deals with restricted
>> methods, even for FFM API calls. In Java 22, the single
>> `--enable-native-access` was used both to specify a set of modules for which
>> native access should be allowed *and* to specify whether illegal native
>> access (that is, native access occurring from a module not specified by
>> `--enable-native-access`) should be treated as an error or a warning. More
>> specifically, an error is only issued if the `--enable-native-access flag`
>> is used at least once.
>>
>> Here, a new flag is introduced, namely
>> `illegal-native-access=allow/warn/deny`, which is used to specify what
>> should happen when access to a restricted method and/or functionality is
>> found outside the set of modules specified with `--enable-native-access`.
>> The default policy is `warn`, but users can select `allow` to suppress the
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This
>> aligns the treatment of restricted methods with other mechanisms, such as
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>>
>> Some changes were required in the package-info javadoc for
>> `java.lang.foreign`, to reflect the changes in the command line flags
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Address review comments
>Improve warning for JNI methods, similar to what's described in JEP 472
>Beef up tests
> - Address review comments
src/java.base/share/classes/java/lang/Module.java line 334:
> 332: System.err.printf("""
> 333: WARNING: A native method in %s has been bound
> 334: WARNING: %s has been called by %s in %s
Note that this line is still not entirely correct, as for code like:
// in module a:
package a;
import b.Foo;
public class Foo {
public static void main(String... args) {
System.load("JNI library implementing Java_b_Bar_nativeMethod");
Bar.nativeMethod();
}
}
// in module b:
package b;
public class Bar {
public static native void nativeMethod();
}
It’ll show `Bar` as the caller of `Bar::nativeMethod()`, even though the caller
is `Foo` in this case, which is why I initially suggested just omitting the
caller from **JNI** linkage warnings.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601140578
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]
On Wed, 15 May 2024 00:54:43 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/arguments.cpp line 2271:
>>
>>> 2269: } else if (match_option(option, "--illegal-native-access=",
>>> )) {
>>> 2270: if (!create_module_property("jdk.module.illegal.native.access",
>>> tail, InternalProperty)) {
>>> 2271: return JNI_ENOMEM;
>>
>> I think it would be helpful to get guidance on if this is the right way to
>> add this system property, only because this one not a "module property". The
>> configuration (WriteableProperty + InternalProperty) look right.
>
> So my recollection/understanding is that we use this mechanism to convert
> module-related `--` flags passed to the VM into system properties that the
> Java code can then read, but we set them up such that you are not allowed to
> specify them directly via `-D`. Is the question here whether this new
> property should be in the `jdk.module` namespace?
That's my recollection too. The usage here isn' related to modules which makes
me wonder if this function should be renamed (not by this PR of course) of if
we should be using PropertyList_unique_add (with AddProperty,
WriteableProperty, InternalProperty) instead. There will be further GNU style
options coming that will likely need to map to an internal system property in
the same way.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601002132
