<AWT Dev> Bug 6913179, FileDialog uses native GTK under Linux: first patch

Peter Zhelezniakov Peter.Zhelezniakov at Sun.COM
Tue Jan 26 05:01:57 PST 2010


Hi Costantino,

I've looked at your proposed changes at
    http://cr.openjdk.java.net/~anthony/7-43-GTKFileDialog-6913179.1
(BTW i wonder what 7-43 is?)

Some comments follow:

- We're going to have two functions that check version of GTK. Does it
make sense to refactor UNIXToolkit to use the new one? And get rid of
the old one completely?

- Why would you bother with returning a String from
native_gtk_check_version()? It is effectively ignored later, so you
could as well return a boolean value from the native function.

- Most gtk2_interface functions are named gtk2_XXX. However silly this
is, i believe gtk_file_chooser_load() is better renamed, just for
consistency =)

- Note that g_thread_get_initialized() is available since glib-2.20
only. Is there a way to enable the new filechooser on older systems?

- Load/unload calls in GtkFileDialogPeer.c seems unbalanced to me. Consider:

setVisible(true)
run()
init_GtkFileDialogPeer(env) {
     if (jvm == NULL) {
         gtk2_load();
     }
}

setVisible(false)
quit()
gtk2_unload();

setVisible(true)
run()
init_GtkFileDialogPeer(env) {
     if (jvm == NULL) // this is false, gtk2_load() isn't called
}

Do you really want to load GTK each time a file dialog is shown? This is 
pretty expensive. Unloading will also break GTK LAF.

Thanks!
-- 
Peter



More information about the awt-dev mailing list