<AWT Dev> [8] Review request for 7123476: DesktopOpenTests:When enter the file path and click the open button, it crash

[Anthony Petrov]

Hi Denis,

src/solaris/native/sun/xawt/gnome_interface.c
>    2  * Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved.

I think this file has just been created in 2012 :)

>  30     fprintf(stderr, "gnome_load\n");

Please remove all the debugging output, or put it under the #ifdef 
INTERNAL_BUILD.

>   70         fprintf(stderr, "can not find symble gnome_url_show\n");

s/symble/symbol/

src/solaris/native/sun/awt/gtk2_interface.c
>  447      return TRUE;

I think it also makes sense to check fp_gtk_show_uri for NULL before 
returning TRUE here.

The rest of the fix looks fine to me. Thank you!

--
best regards,
Anthony

On 3/27/2012 5:03 PM, Denis S. Fokin wrote:
> Hi Anthony,
> 
> here is a new version of the fix.
> 
> http://cr.openjdk.java.net/~denis/7123476/webrev.03/
> 
> I took into account you suggestions. Now the implementation loads gtk 
> API if it exists on the library path. If it does not exists we try to 
> load gnome API. If it is not successful we do not support the 
> functionality.
> 
> I introduced a couple of files to keep gnome interface separately like 
> we do with gtk. I expect that we remove them as soon as all our 
> supported OS configurations will be have installed the proper GTK 
> library by default.
> 
> As for the synchronization section, I do not see how to use the 
> fp_gdk_threads_* functions with gnome API so I put the critical under 
> gtk-specific if-clause section.
> 
> Thank you,
>    Denis.
> 
> On 2/1/2012 9:03 PM, Anthony Petrov wrote:
>> Hi Denis,
>>
>> The gtk_show_uri() is available since GTK 2.14. Did you verify if all
>> platforms supposed to be supported by JDK 8 have this version of GTK
>> libraries installed by default? I'm mostly concerned about Solaris
>> systems, as well as corporate Linux desktops. If this is not the case,
>> then perhaps using this function should be conditional, and with the old
>> GTK library we should fall back to using the old API. You may notice
>> that, for example, for the file chooser we have an explicit check for
>> GTK 2.8.0 and use the new
>> gtk_file_chooser_set_do_overwrite_confirmation() API only when it's
>> available.
>>
>> I like that we move all the GTK-related utility code to the
>> gtk2_interface files. A few comments:
>>
>> 1. Please use the TRUE and FALSE constants instead of 1 and 0 as a
>> return value for gtk2_show_uri_load().
>>
>> 2. Should the fprintf() call be #ifdef'ed for INTERNAL_BUILD's only?
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 2/1/2012 7:39 PM, Denis S. Fokin wrote:
>>> Hi AWT team,
>>>
>>> Please review a fix for the CR 7123476 at
>>>
>>> http://cr.openjdk.java.net/~denis/7123476/webrev.01
>>>
>>> CR URL:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123476
>>>
>>> The Gnome API is deprecated so we need to migrate on GTK function. See
>>> the next thread.
>>>
>>> http://mail.gnome.org/archives/gnome-devel-list/2009-January/msg00004.html 
>>>
>>>
>>>
>>> Thank you,
>>> Denis.
>