Review request #0: 6863566 (Java should support the freedesktop.org startup notification specification)

[Damjan Jovanovic]

On Wed, Jul 29, 2009 at 2:10 PM, Alan Bateman<Alan.Bateman at sun.com> wrote:
> Anthony Petrov wrote:
>>
>> Hello,
>>
>> Please review the fix contributed by Damjan Jovanovic:
>>
>> RFE: https://bugs.openjdk.java.net/show_bug.cgi?id=100094
>>
>> webrev: http://cr.openjdk.java.net/~anthony/7-24-startupNotify-6863566.0/
>>
>> Since the patch includes changes to the src/solaris/bin/java_md.c, I'm
>> CC'ing Kumar and Core Libs alias to review the changes in that file.
>>
>>
>> Damjan, have you by the way tested the fix with a GUI Java application
>> that does not display a top-level window, but rather creates a tray icon
>> only? Does the notification get correctly removed from the task bar in that
>> case?
>>
>> --
>> best regards,
>> Anthony
>
> I think Kumar is on vacation at the moment.
>
> Out of curiosity, are the launcher changes really needed? I assume this
> startup notification protocol is only interesting to applications with a
> user interface and maybe it would be okay to just grab/unset the environment
> variable when the base window becomes visible. Is the concern that the
> environment variable will leak into sub-processes created before the window
> becomes visible? Also, I wonder about applications that launch the VM via
> the JNI invocation API. This would require documenting the system property
> for this to work.

Yes, subprocesses shouldn't inherit the environment variable, and the
specification also says it should be unset.

I thought about unsetting it later, but then there's a battle against
the environment variable caching in
jdk/src/solaris/classes/java/lang/ProcessEnvironment.java.

Any suggestions?

> In passing, I see removeStartupNotification reads the system property. I
> don't know the call stack here but are all the caller frames for methods on
> the boot class path? Just wondering about when there is a security manager
> and if it needs to be in doPrivileged block.

You're probably right, I see XToolkit defines a method that does
system property lookups in a doPrivileged block.

> -Alan.
>
>
>
>
>

Thank you
Damjan