On Fri, 10 Nov 2023 22:37:42 GMT, Phil Race <[email protected]> wrote:

>> In the case when AWT is embedded AWT does NOT start an  NSApplication and 
>> does not own it.
>> AWT overwriting FX's delegate, or any one else's is just plain wrong.
>> 
>> We can turn around what you said
>> "To access any native code in JavafX, the app should initialise GlassToolkit 
>> which sets its delegate, if not already set".
>> 
>> But you can't have both of these.
>> 
>> Meaning AWT isn't special here. It has to play by the same rules.
>> 
>> The toolkit that starts the NSApplication is the one that should be setting 
>> the delegate.
>> If you are embedded in someone else's application, it seems most logical 
>> that you defer to their application
>> and if this means some things aren't possible,  so be it.
>
>> Can you please provide details on how the delegate can be null here and 
>> there if it should be set at the startup of any awt application?
> 
> I commented on these in https://bugs.openjdk.org/browse/JDK-8319255 which is 
> now closed as a dup.
> There I wrote (in part) 
> #######
> 
> I am not sure if these are all cases that can provably be in un-reachable 
> code in the "nil" case, which
> means some embedded case. But if we ever do reach them, it seems we'd crash 
> the app.
> And if we need to make the delegate "nil" in the non-subclassed NSApplication 
> case (ie like FX)
> then we are more likely to cause a crash.
> 
> [list of places elided]
> 
> All these call sites need to be examined to understand that, and if necessary 
> a test developed to
> confirm it.
> And even then, it might be prudent to add checks for nil.
> #####
> 
> So the request was to investigate and understand if they were needed, not 
> just add them
> @honkar-jdk can comment on what she did there.

@prrace @kevinrushforth @mrserb 

The reason has to why we don't observe NPE or crash when sharedDelegate is null 
is because in Objective C when a method is called on a nil object it is treated 
as no-op and returns nil instead of NPE, contrary to what we would usually 
expect. Details 
[here](https://stackoverflow.com/questions/2696891/calling-a-method-on-an-uninitialized-object-null-pointer).

For example executing the following line when shared delegate is null and 
accessing defaultMenuBar on null delegate returns null and not NPE.

`[[ApplicationDelegate sharedDelegate] defaultMenuBar];`

Additionally we are not seeing any difference in defaultMenuBar with unpatched 
JDK + unpatched JFX (Non-Nil shared delegate) vs patched JDK + patched JFX (Nil 
shared delegate) at 
[AWTWindow.m](https://github.com/openjdk/jdk/blob/5493f5f92d569d8e94d1f271480f11c48257e896/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m#L844)
 and 
[CMenubar.m](https://github.com/openjdk/jdk/blob/5493f5f92d569d8e94d1f271480f11c48257e896/src/java.desktop/macosx/native/libawt_lwawt/awt/CMenuBar.m#L213)
  because in both cases defaultMenuBar returned is nil. This might be the case 
at other places too and are being checked if they worked differently without 
and with this JDK patch.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16569#discussion_r1399604866

Reply via email to