<AWT Dev> [7u4] Review request for 7147435: closed/java/awt/Toolkit/Headless/WrappedToolkitTest/WrappedToolkitTest.sh failed since 7u4b11

[Artem Ananiev]

Hi, Anthony,

On 2/28/2012 2:47 AM, Anthony Petrov wrote:
> Hi Artem,
>
> I believe you've clicked the wrong link. My webrev at

yes, it really sounds like I looked into another webrev...

> http://cr.openjdk.java.net/~anthony/7u4-2-headlessTestFailed-7147435.0/
>
> contains changes to the awt_LoadLibrary.c file only. I have provided a
> link to the webrev for 7124511 as a reference only, and put it in the
> very end of my message to avoid confusion. Please follow the link in the
> beginning of the message to review the fix. Thanks.

Here are some comments from my side:

1. On X11, we don't have any other toolkits than XToolkit, so I don't 
see any point in checking the AWT_TOOLKIT var on X11.

2. On Mac, CToolkit is not explicitly set in awt_LoadLibrary.c - I 
realize it's used by default (inherited from java_props_md.c), but it 
would be fine to leave a comment in the code.

3. AFAIK, there is no need to check global refs for NULL: 
DeleteGlobalRef() is ready to handle null refs.

Thanks,

Artem

> --
> best regards,
> Anthony
>
> On 2/28/2012 2:01 AM, Artem Ananiev wrote:
>> Hi, Anthony,
>>
>> I have an impression your webrev is prepared against an outdated
>> version of code. For example, the change in java_props_md.c:431 is
>> already in the workspace...
>>
>> Other comments:
>>
>> 1. HeadlessGraphicsEnvironment is in the sun.java2d package, not sun.awt
>>
>> 2. Changes to awt_LoadLibrary.c look fine as all the defaults are set
>> in java_props.c
>>
>> 3. GraphicsEnvironment.java also looks fine.
>>
>> Thanks,
>>
>> Artem
>>
>> On 2/27/2012 6:30 AM, Anthony Petrov wrote:
>>> Hello,
>>>
>>> Please review a fix for
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7147435 at:
>>>
>>> http://cr.openjdk.java.net/~anthony/7u4-2-headlessTestFailed-7147435.0/
>>>
>>> This bug is a regression of 7124511 fixed for the JDK Mac Port [1]. With
>>> that fix the code setting the awt.toolkit and java.awt.graphicsenv
>>> system properties has been removed from JNI_OnLoad() of libawt.so.
>>> Actually, the test WrappedToolkitTest.sh relies on the ability to
>>> override the default toolkit by means of setting the AWT_TOOLKIT
>>> environment variable, and because of the removal the test has failed.
>>>
>>> With the fix for 7147435 I'm restoring the removed parts. In order to
>>> not break the fix for 7124511, I'm setting the system properties only if
>>> the XToolkit has been requested explicitly.
>>>
>>> [1] http://cr.openjdk.java.net/~anthony/x-5-forceHeadless.0/
>>>
>>> --
>>> best regards,
>>> Anthony