[cp-patches] FYI: RootLayout fix

[Roman Kennke]

Hi there,

I removed the caching of the preferredSize in JRootPane.RootLayout. I
have an app here that has a problem with it. The exact problem is this:
while building the GUI and layouting, the RootLayout is asked for its
preferredSize while beeing in an invalid state. This (wrong)
preferredSize is then cached. After this, the contentPane is modified so
that the preferredSize is affected. However, when the container calls
preferredLayoutSize() the next time the cached value is returned, which
is wrong. I wrote some mauve tests that confirm that the RootLayout
should not cache its preferredSize (and one that confirms that Container
should not invalidate the layout before asking the preferredSize of an
invalid container).

Maybe we should add some comments to the API docs for LayoutManager,
this seems like a potential programming mistake that can also happen to
application developers? Caching layout info is ok and actually makes
sense, but caching preferredSize can easily lead to incorrect layout,
except when the caching is done in a way that the valid state of the
container is also considered.

2006-01-30  Roman Kennke  <[EMAIL PROTECTED]>

        * javax/swing/JRootPane.java
        (RootLayout.prefSize): Removed caching for preferredSize.
        (RootLayout.invalidateLayout): Likewise.
        (RootLayout.preferredLayoutSize): Likewise.

/Roman

Index: javax/swing/JRootPane.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/JRootPane.java,v
retrieving revision 1.32
diff -u -r1.32 JRootPane.java
--- javax/swing/JRootPane.java	8 Nov 2005 14:12:45 -0000	1.32
+++ javax/swing/JRootPane.java	30 Jan 2006 21:13:13 -0000
@@ -120,11 +120,6 @@
     private Rectangle menuBarBounds;
 
     /**
-     * The cached preferred size.
-     */
-    private Dimension prefSize;
-
-    /**
      * Creates a new <code>RootLayout</code> object.
      */
     protected RootLayout()
@@ -191,7 +186,6 @@
           layeredPaneBounds = null;
           contentPaneBounds = null;
           menuBarBounds = null;
-          prefSize = null;
         }
     }
 
@@ -287,29 +281,20 @@
      */
     public Dimension preferredLayoutSize(Container c)
     {
-      // We must synchronize here, otherwise we cannot guarantee that the
-      // prefSize is still non-null when returning.
-      synchronized (this)
+      Dimension prefSize = new Dimension();
+      Insets i = getInsets();
+      prefSize = new Dimension(i.left + i.right, i.top + i.bottom);
+      Dimension contentPrefSize = contentPane.getPreferredSize();
+      prefSize.width += contentPrefSize.width;
+      prefSize.height += contentPrefSize.height;
+      if (menuBar != null)
         {
-          if (prefSize == null)
-            {
-              Insets i = getInsets();
-              prefSize = new Dimension(i.left + i.right, i.top + i.bottom);
-              Dimension contentPrefSize = contentPane.getPreferredSize();
-              prefSize.width += contentPrefSize.width;
-              prefSize.height += contentPrefSize.height;
-              if (menuBar != null)
-                {
-                  Dimension menuBarSize = menuBar.getPreferredSize();
-                  if (menuBarSize.width > contentPrefSize.width)
-                    prefSize.width += menuBarSize.width - contentPrefSize.width;
-                  prefSize.height += menuBarSize.height;
-                }
-            }
-          // Return a copy here so the cached value won't get trashed by some
-          // other component.
-          return new Dimension(prefSize);
-      }
+          Dimension menuBarSize = menuBar.getPreferredSize();
+          if (menuBarSize.width > contentPrefSize.width)
+            prefSize.width += menuBarSize.width - contentPrefSize.width;
+          prefSize.height += menuBarSize.height;
+        }
+      return prefSize;
     }
 
     /**

signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil