The attached patch fixes some issues with Container/Component wrt to the pref/min/max size:
- Added proper locking of the tree.
- Corrected handling of cached values.
- Need to return copy of the real thing, so that apps can play tricks with the actual values.

This causes no regressions in Mauve and Intel's testsuite, and actually fix Tania's new test for invalidate.

2006-07-13  Roman Kennke  <[EMAIL PROTECTED]>

        * java/awt/Component.java
        (DEFAULT_MAX_SIZE): New static constant.
        (preferredSize): Return copy of the actual value computed
        by new helper method.
        (preferredSizeImpl): New helper method. Adds locking and
        correct handling of cached value.
        (minimumSize): Return copy of the actual value computed
        by new helper method.
        (minimumSizeImpl): New helper method. Adds locking and
        correct handling of cached value.
        (getMaximumSize):  Return copy of the actual value computed
        by new helper method.
        (maximumSizeImpl): New helper method. Adds locking and
        correct handling of cached value.
        (invalidate): Correct handling of cached layout information.
        Added locking.
        * java/awt/Container.java
        (preferredSize): Minimized locking. Corrected handling of cached
        values. Return copy of real value.
        (minimumSize): Minimized locking. Corrected handling of cached
        values. Return copy of real value.
        (getMaximumSize): Minimized locking. Corrected handling of cached
        values. Return copy of real value.

/Roman
Index: java/awt/Component.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Component.java,v
retrieving revision 1.132
diff -u -1 -2 -r1.132 Component.java
--- java/awt/Component.java	7 Jul 2006 14:08:44 -0000	1.132
+++ java/awt/Component.java	13 Jul 2006 19:22:26 -0000
@@ -205,24 +205,30 @@
    * @see #getAlignmentX()
    */
   public static final float LEFT_ALIGNMENT = 0;
 
   /**
    * Make the treelock a String so that it can easily be identified
    * in debug dumps. We clone the String in order to avoid a conflict in
    * the unlikely event that some other package uses exactly the same string
    * as a lock object.
    */
   static final Object treeLock = new String("AWT_TREE_LOCK");
 
+  /**
+   * The default maximum size.
+   */
+  private static final Dimension DEFAULT_MAX_SIZE 
+                             = new Dimension(Short.MAX_VALUE, Short.MAX_VALUE);
+
   // Serialized fields from the serialization spec.
 
   /**
    * The x position of the component in the parent's coordinate system.
    *
    * @see #getLocation()
    * @serial the x position
    */
   int x;
 
   /**
    * The y position of the component in the parent's coordinate system.
@@ -1651,32 +1657,54 @@
   {
     return prefSizeSet;
   }
   
   /**
    * Returns the component's preferred size.
    *
    * @return the component's preferred size
    * @deprecated use [EMAIL PROTECTED] #getPreferredSize()} instead
    */
   public Dimension preferredSize()
   {
-    if (!prefSizeSet)
+    // Create a new Dimension object, so that the application doesn't mess
+    // with the actual values.
+    return new Dimension(preferredSizeImpl());
+  }
+
+  /**
+   * The actual calculation is pulled out of preferredSize() so that
+   * we can call it from Container.preferredSize() and avoid creating a
+   * new intermediate Dimension object.
+   * 
+   * @return the preferredSize of the component
+   */
+  Dimension preferredSizeImpl()
+  {
+    Dimension size = prefSize;
+    // Try to use a cached value.
+    if (size == null || !(valid || prefSizeSet))
       {
-        if (peer == null)
-          prefSize = minimumSize();
-        else
-          prefSize = peer.getPreferredSize();
+        // We need to lock here, because the calculation depends on the
+        // component structure not changing.
+        synchronized (getTreeLock())
+          {
+            ComponentPeer p = peer;
+            if (p != null)
+              size = peer.preferredSize();
+            else
+              size = minimumSizeImpl();
+          }
       }
-    return prefSize;
+    return size;
   }
 
   /**
    * Returns the component's minimum size.
    * 
    * @return the component's minimum size
    * @see #getPreferredSize()
    * @see #setMinimumSize(Dimension)
    * @see LayoutManager
    */
   public Dimension getMinimumSize()
   {
@@ -1715,45 +1743,85 @@
   {
     return minSizeSet;
   }
   
   /**
    * Returns the component's minimum size.
    *
    * @return the component's minimum size
    * @deprecated use [EMAIL PROTECTED] #getMinimumSize()} instead
    */
   public Dimension minimumSize()
   {
-    if (minSize == null)
-      minSize = (peer != null ? peer.getMinimumSize()
-                 : new Dimension(width, height));
-    return minSize;
+    // Create a new Dimension object, so that the application doesn't mess
+    // with the actual values.
+    return new Dimension(minimumSizeImpl());
+  }
+
+  /**
+   * The actual calculation is pulled out of minimumSize() so that
+   * we can call it from Container.preferredSize() and
+   * Component.preferredSizeImpl and avoid creating a
+   * new intermediate Dimension object.
+   * 
+   * @return the minimum size of the component
+   */
+  Dimension minimumSizeImpl()
+  {
+    Dimension size = minSize;
+    if (size == null || !(valid || minSizeSet))
+      {
+        // We need to lock here, because the calculation depends on the
+        // component structure not changing.
+        synchronized (getTreeLock())
+          {
+            ComponentPeer p = peer;
+            if (p != null)
+              size = peer.minimumSize();
+            else
+              size = size();
+          }
+      }
+    return size;
   }
 
   /**
    * Returns the component's maximum size.
    *
    * @return the component's maximum size
    * @see #getMinimumSize()
    * @see #setMaximumSize(Dimension)
    * @see #getPreferredSize()
    * @see LayoutManager
    */
   public Dimension getMaximumSize()
   {
+    return new Dimension(maximumSizeImpl());
+  }
+
+  /**
+   * This is pulled out from getMaximumSize(), so that we can access it
+   * from Container.getMaximumSize() without creating an additional
+   * intermediate Dimension object.
+   *
+   * @return the maximum size of the component
+   */
+  Dimension maximumSizeImpl()
+  {
+    Dimension size;
     if (maxSizeSet)
-      return maxSize;
+      size = maxSize;
     else
-      return new Dimension(Short.MAX_VALUE, Short.MAX_VALUE);
+      size = DEFAULT_MAX_SIZE;
+    return size;
   }
 
   /**
    * Sets the maximum size that will be returned by [EMAIL PROTECTED] #getMaximumSize()}
    * always, and sends a [EMAIL PROTECTED] PropertyChangeEvent} (with the property name
    * 'maximumSize') to all registered listeners.
    * 
    * @param size  the maximum size (<code>null</code> permitted).
    * 
    * @since 1.5
    * 
    * @see #getMaximumSize()
@@ -1839,29 +1907,43 @@
   public void validate()
   {
     valid = true;
   }
 
   /**
    * Invalidates this component and all of its parent components. This will
    * cause them to have their layout redone. This is called frequently, so
    * make it fast.
    */
   public void invalidate()
   {
-    valid = false;
-    prefSize = null;
-    minSize = null;
-    if (parent != null && parent.isValid())
-      parent.invalidate();
+    // Need to lock here, to avoid races and other ugly stuff when doing
+    // layout or structure changes in other threads.
+    synchronized (getTreeLock())
+      {
+        // Invalidate.
+        valid = false;
+
+        // Throw away cached layout information.
+        if (! minSizeSet)
+          minSize = null;
+        if (! prefSizeSet)
+          prefSize = null;
+        if (! maxSizeSet)
+          maxSize = null;
+
+        // Also invalidate the parent, if it hasn't already been invalidated.
+        if (parent != null && parent.isValid())
+          parent.invalidate();
+      }
   }
 
   /**
    * Returns a graphics object for this component. Returns <code>null</code>
    * if this component is not currently displayed on the screen.
    *
    * @return a graphics object for this component
    * @see #paint(Graphics)
    */
   public Graphics getGraphics()
   {
     if (peer != null)
Index: java/awt/Container.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Container.java,v
retrieving revision 1.97
diff -u -1 -2 -r1.97 Container.java
--- java/awt/Container.java	6 Jul 2006 20:42:46 -0000	1.97
+++ java/awt/Container.java	13 Jul 2006 19:22:26 -0000
@@ -663,92 +663,111 @@
     return preferredSize ();
   }
 
   /**
    * Returns the preferred size of this container.
    *
    * @return The preferred size of this container.
    *
    * @deprecated use [EMAIL PROTECTED] #getPreferredSize()} instead
    */
   public Dimension preferredSize()
   {
-    synchronized(treeLock)
-      {  
-        if(valid && prefSize != null)
-          return new Dimension(prefSize);
-        LayoutManager layout = getLayout();
-        if (layout != null)
+    Dimension size = prefSize;
+    // Try to return cached value if possible.
+    if (size == null || !(prefSizeSet || valid))
+      {
+        // Need to lock here.
+        synchronized (getTreeLock())
           {
-            Dimension layoutSize = layout.preferredLayoutSize(this);
-            if(valid)
-              prefSize = layoutSize;
-            return new Dimension(layoutSize);
+            LayoutManager l = layoutMgr;
+            if (l != null)
+              prefSize = l.preferredLayoutSize(this);
+            else
+              prefSize = super.preferredSizeImpl();
+            size = prefSize;
           }
-        else
-          return super.preferredSize ();
       }
+    if (size != null)
+      return new Dimension(size);
+    else
+      return size;
   }
 
   /**
    * Returns the minimum size of this container.
    *
    * @return The minimum size of this container.
    */
   public Dimension getMinimumSize()
   {
     return minimumSize ();
   }
 
   /**
    * Returns the minimum size of this container.
    *
    * @return The minimum size of this container.
    *
    * @deprecated use [EMAIL PROTECTED] #getMinimumSize()} instead
    */
   public Dimension minimumSize()
   {
-    if(valid && minSize != null)
-      return new Dimension(minSize);
-
-    LayoutManager layout = getLayout();
-    if (layout != null)
+    Dimension size = minSize;
+    // Try to return cached value if possible.
+    if (size == null || !(minSizeSet || valid))
       {
-        minSize = layout.minimumLayoutSize (this);
-        return minSize;
-      }    
+        // Need to lock here.
+        synchronized (getTreeLock())
+          {
+            LayoutManager l = layoutMgr;
+            if (l != null)
+              minSize = l.minimumLayoutSize(this);
+            else
+              minSize = super.minimumSizeImpl();
+            size = minSize;
+          }
+      }
+    if (size != null)
+      return new Dimension(size);
     else
-      return super.minimumSize ();
+      return size;
   }
 
   /**
    * Returns the maximum size of this container.
    *
    * @return The maximum size of this container.
    */
   public Dimension getMaximumSize()
   {
-    if (valid && maxSize != null)
-      return new Dimension(maxSize);
-
-    LayoutManager layout = getLayout();
-    if (layout != null && layout instanceof LayoutManager2)
+    Dimension size = maxSize;
+    // Try to return cached value if possible.
+    if (size == null || !(maxSizeSet || valid))
       {
-        LayoutManager2 lm2 = (LayoutManager2) layout;
-        maxSize = lm2.maximumLayoutSize(this);
-        return maxSize;
+        // Need to lock here.
+        synchronized (getTreeLock())
+          {
+            LayoutManager l = layoutMgr;
+            if (l instanceof LayoutManager2)
+              maxSize = ((LayoutManager2) l).maximumLayoutSize(this);
+            else
+              maxSize = super.maximumSizeImpl();
+            size = maxSize;
+          }
       }
+    if (size != null)
+      return new Dimension(size);
     else
-      return super.getMaximumSize();
+      return size;
   }
 
   /**
    * Returns the preferred alignment along the X axis.  This is a value
    * between 0 and 1 where 0 represents alignment flush left and
    * 1 means alignment flush right, and 0.5 means centered.
    *
    * @return The preferred alignment along the X axis.
    */
   public float getAlignmentX()
   {
     LayoutManager layout = getLayout();

Reply via email to