RFR: JDK-8318854: [macos14] Running any AWT app prints Secure coding warning [v2]

[Sergey Bylokhov]

On Mon, 20 Nov 2023 23:51:09 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> @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 with defaultMenuBar case 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.
>
> So in  either case -- with or without the delegate being overridden -- the defaultMenuBar is returning `nil`. This makes sense, since in the case of a JavaFX app, JavaFX has already initialized the `NSApplication` and set the system menu bar.
> 
> I do recommend leaving the `sharedDelegate` null checks that you added as part of this PR, since that way we short-circuit any other logic (e.g., in this specific case you avoid setting `isDisabled = NO` which seems more accurate), and you aren't relying on Objective C to treat this case as a no-op.
> 
> You might even consider making the `sharedDelegate` null check (and bailing out) earlier, but maybe there is a good reason to leave the check where it is.

>The reason has to why we don't observe NPE or crash when sharedDelegate is null is

But why it is null? The OSXAPP_SetApplicationDelegate is called when the LWCToolkit is initialized in non-headless [mode](https://github.com/openjdk/jdk/blob/9a6ca233c7e91ffa2ce9451568b3be88ccd04504/src/java.desktop/macosx/native/libawt_lwawt/awt/LWCToolkit.m#L355) and that code does not depend on "NSApplicationAWT". Does it mean that the toolkit is not initiated? Then how we can call peer-menu-related code? if that code path is not executed probably we should change that?

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

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