<Swing Dev> RFR: 8259869: [macOS] Remove desktop module dependencies on JNF Reference APIs

[Phil Race]

On Wed, 20 Jan 2021 06:02:55 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> This removes desktop module usage of the JNF JNI reference convenience APIs
>> These are simply a direct conversion
>> JNFNewGlobalRef 
>> JNFDeleteGlobalRef 
>> JNFNewWeakGlobalRef 
>> JNFDeleteWeakGlobalRef 
>> 
>> These two
>> JNFJObjectWrapper 
>> JNFWeakJObjectWrapper 
>> exist to allow clean up of the refs when a Cocoa wrapper object is released.
>> However in all cases there are more direct ways to clean it up and in at least one usage
>> the existing code directly releases it with the comment that this is more efficient.
>> 
>> All our automated regression and JCK tests pass with this change.
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1547:
> 
>> 1545:         AWTWindow *awtWindow = [AWTWindow getTopmostWindowUnderMouse];
>> 1546:         if (awtWindow != nil) {
>> 1547:             topmostWindowUnderMouse = awtWindow.javaPlatformWindow;
> 
> I wonder what type of "reference" we should return here, I do not remember how the "NewWeakGlobalRef" behave when returned to java.

It is no different than other cases. Java will get a new strong ref to the object
and expect it to be of the return type of this native method.

> src/java.desktop/macosx/native/libawt_lwawt/awt/CDragSourceContextPeer.m line 58:
> 
>> 56:     jobject gTriggerEvent = (*env)->NewGlobalRef(env, jtrigger);
>> 57:     jlongArray gFormats = (*env)->NewGlobalRef(env, jformats);
>> 58:     jobject gFormatMap = (*env)->NewGlobalRef(env, jformatmap);
> 
> All above should be checked for NULL since OOM may occur, but it looks like it does not throw OOM?
> https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#NewGlobalRef

It is the case that per even the latest spec. none of these throw any exception.
https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#newglobalref
So I think the existing code doesn't do anything in the event of a NULL return.
And if you want to check for NULL here and return NULL from the native method there's semantic implications that require the caller never pass NULL for any of these.
It is not illegal to pass NULL to NewGlobalRef.
Investigating and confirming that is beyond the scope of this change.
Or we make the code a bit more complex here and check that we get back non-null for a non-null arg. But once again nothing new here.

> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.h line 55:
> 
>> 53: @property (nonatomic, retain) NSWindow *nsWindow;
>> 54: 
>> 55: @property (nonatomic) jobject javaPlatformWindow;
> 
> I think it will be useful to have a comment here that this value is a weak reference and should be copied to the local ref before usage.

I suppose I can add a comment ... but it is already done in the couple of dozen usages so any one adding a new usage who misses that will likely miss the comment too.

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

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