[8] Review request for 7123476: DesktopOpenTests:When enter the file path and click the open button, it crash (original) (raw)

Denis S. Fokin denis.fokin at oracle.com
Wed Mar 28 10:14:41 PDT 2012

Hi Anthony,

thank you for the review notes.

Actually, I expect that if fp_gtk_show_uri is null we have some kind of dlerror. Anyway I check fp_gtk_show_uri in Java_sun_awt_X11_XDesktopPeer_gnome_1url_1show. So I would not add additional check here. But if you insist I will add the NULL check.

Thank you, Denis.

On 3/28/2012 7:01 PM, Anthony Petrov wrote:

Hi Denis,

src/solaris/native/sun/xawt/gnomeinterface.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, "gnomeload\n"); Please remove all the debugging output, or put it under the #ifdef INTERNALBUILD. 70 fprintf(stderr, "can not find symble gnomeurlshow\n"); s/symble/symbol/ src/solaris/native/sun/awt/gtk2interface.c 447 return TRUE; I think it also makes sense to check fpgtkshowuri 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 fpgdkthreads* 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 gtkshowuri() 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 gtkfilechoosersetdooverwriteconfirmation() API only when it's available. I like that we move all the GTK-related utility code to the gtk2interface files. A few comments: 1. Please use the TRUE and FALSE constants instead of 1 and 0 as a return value for gtk2showuriload(). 2. Should the fprintf() call be #ifdef'ed for INTERNALBUILD'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/viewbug.do?bugid=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.

More information about the awt-dev mailing list