[cp-patches] Patch: Checkbox group fix

Lillian Angel langel at redhat.com
Fri Jun 30 19:31:21 UTC 2006


Hi,

I fixed the issues Tom stated below.

On Fri, 2006-06-30 at 13:03 -0400, Thomas Fitzsimmons wrote:
> Hi,
> 
> Lillian Angel wrote:
> > This patch fixes the long standing problem of not being able to
> > dynamically switch between a checkbox and a radio button. I.e. moving a
> > checkbox to a checkbox group changes the checkbox to a radio button.
> > 
> > Tom helped a lot with this patch. He removed the CheckboxGroupPeer and
> > we fixed it so everything is handled in GtkCheckboxPeer.
> > 
> > There is a mauve test for this. The harmony test
> > (test.java.awt.CheckboxTest) now passes.

[...]
> > +  if (groupPointer != 0)
> > +  {
> > +    native_group = JLONG_TO_PTR (GSList, groupPointer);
> > +    if(! GTK_IS_RADIO_BUTTON (native_group->data))
> > +      native_group = NULL;
> 
> When does it happen that the data field is not a GtkRadioButton?  I suspect only 
> after removing a Checkbox from a CheckboxGroup?  This is likely a memory leak -- 
> if the native_group's data field is no longer valid then we should probably free 
> the GSList.

Gtk actually frees the list when needed. I changed these checks to
asserts and set the native_group to null in *_removeFromGroup (when
needed).

[...]

> > +  gdk_threads_leave ();
> > +  
> > +  return PTR_TO_JLONG (native_group);
> 
> I realized that there is a race condition here.  Instead of returning the 
> pointer value, we should use JNI to set the groupPointer field with the GDK lock 
> held.

I changed all functions in GtkCheckboxPeer to be syncronized, and added
a new function to put the new group in the groupMap. This function is
called from the native peer.

[...]
> > +  NSA_DEL_PTR (env, obj);
> > +  NSA_SET_PTR (env, obj, container);
> 
> The NSA table points to the containing event box, so I don't think these two 
> lines are necessary.

Removed
> 
2006-06-30  Lillian Angel  <langel at redhat.com>

        * gnu/java/awt/peer/gtk/GtkCheckboxPeer.java:
        Changed all return values of native functions to void.
        (create): Changed function to be syncronized. Removed
        call to put value in groupMap, this is now done from
        the native code.
        (setState): Changed function to be syncronized.
        (setLabel): Changed function to be syncronized.
        (setCheckboxGroup): Changed function to be syncronized. Removed
        call to put value in groupMap, this is now done from
        the native code.
        (postItemEvent): Changed function to be syncronized.
        (addToGroupMap): New function. Called by native code to add
        new value to the group.
        (dispose): Changed function to be syncronized.
        * include/gnu_java_awt_peer_gtk_GtkCheckboxPeer.h: Updated
        all functions.
        * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c
        (cp_gtk_checkbox_init_jni): Added code to link to
        java function.
        (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_createRadioButton):
        Changed return value to void. Added call
        to java function to set pointer in groupMap.
        (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_addtoGroup): 
	Likewise. Also, changed check to an assert. Also, removed call 
	to set/del pointer.
        (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_removeFromGroup):
        Likewise. Also, added check to determine if native_group should 
	be set to NULL.
        (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_switchToGroup): 
	Likewise.

Committed,
Lillian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 12917 bytes
Desc: not available
Url : http://developer.classpath.org/pipermail/classpath-patches/attachments/20060630/401413a7/patch.bin


More information about the Classpath-patches mailing list