<AWT Dev> RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Phil Race]

On Mon, 1 Feb 2021 22:17:38 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8260616: Removing remaining JNF dependencies in the java.desktop module
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/CTextPipe.m line 611:
> 
>> 609:         const jchar *unichars = (*env)->GetStringChars(env, str, NULL);
>> 610:         if (unichars == NULL) {
>> 611:             return;
> 
> Do not we need to throw an exception here? Otherwise, GetStringChars error will be ignored?

Look a few lines further up at my reply 3 days ago Gerard about this.

> src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 967:
> 
>> 965: static NSNumber* JavaNumberToNSNumber(JNIEnv *env, jobject jnumber) {
>> 966:     if (jnumber == NULL) {
>> 967:         return nil;
> 
> Based on its usage it is probably should be zero on NULL number?

Not an unreasonable idea and I considered it but :
- It is never called with NULL. There is always a null check
- The JNF equivalent returns nil on NULL

BTW two of the functions in which the code appears : accessibilityMinValueAttribute and accessibilityMaxValueAttribute  (SFAIC) aren't used anywhere.

> src/java.desktop/macosx/native/libosxapp/JNIUtilities.m line 30:
> 
>> 28: NSString* JavaStringToNSString(JNIEnv *env, jstring jstr) {
>> 29:     if (jstr == NULL) {
>> 30:         return NULL;
> 
> In other methods, the nil is used

Not understanding what you mean ? This is the same behavior as the function being replaced.

> src/java.desktop/macosx/native/libosxapp/ThreadUtilities.m line 53:
> 
>> 51: @implementation ThreadUtilities
>> 52: 
>> 53: + (void)initialize {
> 
> I think we need to check how this new modes will work when the AWT is embedded inside SWT and FX.

We are just specifying an additional run mode for JDK internal use.
It means that when we are saying to process only events for that mode, then only those will be processed.
And it is used only for nested event loops.
Nothing eternal should be assuming AWT uses JNF at all, never mind in a particular way.

There is a special case for FX but I don't see a problem.

If you look at Java_sun_lwawt_macosx_LWCToolkit_doAWTRunLoopImpl
there's a variable "inAWT".
         isRunning = [[NSRunLoop currentRunLoop] runMode:(inAWT ? [JNFRunLoop javaRunLoopMode] : NSDefaultRunLoopMode) ... ]];

This is specified as
        inAWT = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
            @Override
            public Boolean run() {
                return !Boolean.parseBoolean(System.getProperty("javafx.embed.singleThread", "false"));
            }
        });
    }

So generally FX doesn't care, and is ignorant of this new mode.
Unless you set that property to true, in which case AWT use the  NSDefaultRunLoopMode and so again FX is unaffected.
Nothing in the FX sources goes anywhere near JNF .. or has a reference to the special mode.

Bottom line I don't see it being affected. 

I'll check with Kevin but also Gerard had a lot to do with the creation of the current FX toolkit.

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

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