<AWT Dev> Review request: 6913179 (The java.awt.FileDialog should use native GTK file chooser on linux distros)

[Damjan Jovanovic]

Hi Costantino

You still use C++ (//) comments, but other OpenJDK JNI code uses them
too so I guess it's fine.

But I cannot find any OpenJDK code that declares variables in the
middle of a block - a C99 feature - while you do. So it may compile
fine on Linux+GCC but what about other compilers and platforms? When
last I heard, Microsoft's compilers (used for the Windows build)
definitely didn't support C99, and what is the official policy for C99
code in OpenJDK anyway?

I see this in some makefiles:
corba/make/common/Defs-solaris.gmk:#       -xc99=%none     Do NOT
allow for c99 extensions to be used.
jdk/make/common/Defs-solaris.gmk:# Do not allow C99 language features
like declarations in code etc.
so I guess C99 is not allowed after all?

Your error checking still seems lacking, for example:

+	jclass stringCls = (*env)->FindClass(env, "java/lang/String");
+	GSList *iterator = NULL;
+
+	jobjectArray array;
+	array = (*env)->NewObjectArray(env, fp_gtk_g_slist_length(list),
stringCls, NULL);
+	int i = 0;
+	for (iterator = list; iterator; iterator = iterator->next) {
+		char* entry = (char*)iterator->data;
+		entry = strrchr(entry, '/') + 1;
+		str = (*env)->NewStringUTF(env, entry);
+		(*env)->SetObjectArrayElement(env, array, i, str);
+		i++;
+	}

What if the FindClass or NewObjectArray fail for some reason? Do we
need to worry about that - other JNI code I've seen is not
particularly strict?

Otherwise good work, thank you
Damjan

On Mon, Mar 1, 2010 at 1:52 PM, Costantino Cerbo <c.cerbo at gmail.com> wrote:
> Hello Anthony, Hello Peter,
>
> what about my patch for the issue 6913179?
> Did you start the review?
>
> I report again the most relevant changes:
>  1. Dynamic link to libgthread-2.0.so.0 instead of libgthread-2.0.so
>  2. GTK multithread support (gdk_threads_enter() and
> gdk_threads_leave() as outlined by Damjan)
>  3. New Thread to don't block the EDT
>  4. Support for multiple file selection (bugs 6705345)
>
> Best regards,
> Costantino
>
> 2010/2/19 Anthony Petrov <Anthony.Petrov at sun.com>:
>> Hi Costantino,
>>
>> Here's the latest version of your fix:
>>
>> http://cr.openjdk.java.net/~anthony/7-43-GTKFileDialog-6913179.3/
>>
>> Thanks!
>>
>>
>>> Okay, but meanwhile we can open an issue for this feature in the Sun
>>> bug database, what do you think about?
>>
>> Just filed the following:
>>
>> 6927978 : Directory Selection standard dialog support
>>
>> (should become visible on the bugs.sun.com in a day or two).
>>
>> --
>> best regards,
>> Anthony
>>
>