Hi,

On Tue, 2006-02-21 at 10:50 -0500, Thomas Fitzsimmons wrote:
> > If you think propagating value changes to awt components through the
> > event queue and dispatchEventImp() is a good idea then I will change
> > GtkCheckboxPeer accordingly and audit the other awt gtk-peers.
> 
> Yes, I've thought about different approaches to this and this seems like
> the simplest one that doesn't contain race conditions.

OK, here is the corresponding change for Checkbox. I also simplified
some code and made sure no state change method is called unnecessary.

2006-02-22  Mark Wielaard  <[EMAIL PROTECTED]>

    * java/awt/Checkbox.java (setState): Check that state actually changed
    before calling peer.
    (dispatchEventImpl): Set new state if ItemEvent.
    * gnu/java/awt/peer/gtk/GtkCheckboxPeer.java (changing): Removed.
    (create): Set currentState.
    (setState): Make synchronized, check and set currentState before
    calling gtkToggleButtonSetActive.
    (postItemEvent): Make synchronized, check and set currentState before
    posting ItemEvent.
    * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c
    (postItemEventID): Method now takes boolean.
    (item_toggled_cb): Likewise.

Committed.

Now reviewing the other gtk-peers that post events/change component states.

Cheers,

Mark
Index: java/awt/Checkbox.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Checkbox.java,v
retrieving revision 1.19
diff -u -r1.19 Checkbox.java
--- java/awt/Checkbox.java	19 Sep 2005 08:57:03 -0000	1.19
+++ java/awt/Checkbox.java	22 Feb 2006 14:45:53 -0000
@@ -1,5 +1,6 @@
 /* Checkbox.java -- An AWT checkbox widget
-   Copyright (C) 1999, 2000, 2001, 2002, 2005  Free Software Foundation, Inc.
+   Copyright (C) 1999, 2000, 2001, 2002, 2005, 2006
+   Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -459,11 +460,14 @@
 public synchronized void
 setState(boolean state)
 {
-  this.state = state;
-  if (peer != null)
+  if (this.state != state)
     {
-      CheckboxPeer cp = (CheckboxPeer) peer;
-      cp.setState (state);
+      this.state = state;
+      if (peer != null)
+	{
+	  CheckboxPeer cp = (CheckboxPeer) peer;
+	  cp.setState (state);
+	}
     }
 }
 
@@ -599,10 +603,15 @@
 dispatchEventImpl(AWTEvent e)
 {
   if (e.id <= ItemEvent.ITEM_LAST
-      && e.id >= ItemEvent.ITEM_FIRST
-      && (item_listeners != null 
-	  || (eventMask & AWTEvent.ITEM_EVENT_MASK) != 0))
-    processEvent(e);
+      && e.id >= ItemEvent.ITEM_FIRST)
+    {
+      ItemEvent ie = (ItemEvent) e;
+      int itemState = ie.getStateChange();
+      setState(itemState == ItemEvent.SELECTED ? true : false);
+      if (item_listeners != null 
+	  || (eventMask & AWTEvent.ITEM_EVENT_MASK) != 0)
+	processEvent(e);
+    }
   else
     super.dispatchEventImpl(e);
 }
Index: gnu/java/awt/peer/gtk/GtkCheckboxPeer.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/peer/gtk/GtkCheckboxPeer.java,v
retrieving revision 1.23
diff -u -r1.23 GtkCheckboxPeer.java
--- gnu/java/awt/peer/gtk/GtkCheckboxPeer.java	13 Feb 2006 18:57:03 -0000	1.23
+++ gnu/java/awt/peer/gtk/GtkCheckboxPeer.java	22 Feb 2006 14:45:53 -0000
@@ -42,6 +42,8 @@
 import java.awt.CheckboxGroup;
 import java.awt.peer.CheckboxPeer;
 
+import java.awt.event.ItemEvent;
+
 public class GtkCheckboxPeer extends GtkComponentPeer
   implements CheckboxPeer
 {
@@ -49,7 +51,6 @@
   public GtkCheckboxGroupPeer old_group;
   // The current state of the GTK checkbox.
   private boolean currentState;  
-  private boolean changing = false;
 
   public native void create (GtkCheckboxGroupPeer group);
   public native void nativeSetCheckboxGroup (GtkCheckboxGroupPeer group);
@@ -75,23 +76,24 @@
     CheckboxGroup g = checkbox.getCheckboxGroup ();
     old_group = GtkCheckboxGroupPeer.getCheckboxGroupPeer (g);
     create (old_group);
-    gtkToggleButtonSetActive (checkbox.getState ());
+    currentState = checkbox.getState();
+    gtkToggleButtonSetActive(currentState);
     gtkButtonSetLabel (checkbox.getLabel ());
   }
 
-  public void setState (boolean state)
+  /**
+   * Sets native GtkCheckButton is state is different from current
+   * state.  Will set currentState to state to prevent posting an
+   * event since events should only be posted for user initiated
+   * clicks on the GtkCheckButton.
+   */
+  synchronized public void setState (boolean state)
   {
-    // prevent item_toggled_cb -> postItemEvent ->
-    // awtComponent.setState -> this.setState ->
-    // gtkToggleButtonSetActive self-deadlock on the GDK lock.
-    if (changing && Thread.currentThread() == GtkToolkit.mainThread)
+    if (currentState != state)
       {
-        changing = false;
-        return;
+	currentState = state;
+	gtkToggleButtonSetActive(state);
       }
-
-    if (currentState != state)
-      gtkToggleButtonSetActive (state);
   }
 
   public void setLabel (String label)
@@ -115,22 +117,15 @@
   // Override the superclass postItemEvent so that the peer doesn't
   // need information that we have.
   // called back by native side: item_toggled_cb
-  public void postItemEvent (Object item, int stateChange)
+  synchronized public void postItemEvent(Object item, boolean state)
   {
-    Checkbox currentCheckBox = ((Checkbox)awtComponent);
-    // A firing of the event is only desired if the state has changed due to a 
-    // button press. The currentCheckBox's state must be different from the 
-    // one that the stateChange is changing to. 
-    // stateChange = 1 if it goes from false -> true
-    // stateChange = 2 if it goes from true -> false
-    if (( !currentCheckBox.getState() && stateChange == 1)
-        || (currentCheckBox.getState() && stateChange == 2))
-    {
-      super.postItemEvent (awtComponent, stateChange);
-      currentState = !currentCheckBox.getState();
-      changing = true;
-      currentCheckBox.setState(currentState);
-    }
+    // Only fire event is state actually changed.
+    if (currentState != state)
+      {
+	currentState = state;
+	super.postItemEvent(awtComponent,
+			    state ? ItemEvent.SELECTED : ItemEvent.DESELECTED);
+      }
   }
 
   public void dispose ()
Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c
===================================================================
RCS file: /cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c,v
retrieving revision 1.21
diff -u -r1.21 gnu_java_awt_peer_gtk_GtkCheckboxPeer.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c	22 Sep 2005 20:25:39 -0000	1.21
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c	22 Feb 2006 14:45:53 -0000
@@ -1,5 +1,6 @@
 /* gtkcheckboxpeer.c -- Native implementation of GtkCheckboxPeer
-   Copyright (C) 1998, 1999, 2002, 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 1998, 1999, 2002, 2003, 2004, 2006
+   Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -52,7 +53,7 @@
 
   postItemEventID = (*cp_gtk_gdk_env())->GetMethodID (cp_gtk_gdk_env(), gtkcheckboxpeer,
                                                "postItemEvent", 
-                                               "(Ljava/lang/Object;I)V");
+                                               "(Ljava/lang/Object;Z)V");
 }
 
 static void item_toggled_cb (GtkToggleButton *item, jobject peer);
@@ -230,7 +231,5 @@
   (*cp_gtk_gdk_env())->CallVoidMethod (cp_gtk_gdk_env(), peer,
                                 postItemEventID,
                                 peer,
-                                item->active ?
-                                (jint) AWT_ITEM_SELECTED :
-                                (jint) AWT_ITEM_DESELECTED);
+                                item->active);
 }

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to