JDK-4993545 : NativeInLightFixer adds asynchronousity
  • Type: Bug
  • Component: client-libs
  • Sub-Component: java.awt
  • Affected Version: 6
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2004-02-12
  • Updated: 2008-04-14
  • Resolved: 2011-05-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7
7 b25Fixed
Related Reports
Relates :  
JDK-6831657 - NativeInLightFixer fails to removeReferences() and causes memory leak.
Relates :  
JDK-6871485 - Performance optimization of the HW/LW Mixing
Relates :  
JDK-6653511 - JComponent.safelyGetGraphics() may sometimes return null
Relates :  
JDK-6689468 - test/closed/java/awt/Component/VisibleHwInLwContTest/VisibleHwInLwContTest.html fails
Description
Name: dmR10075			Date: 02/12/2004


The code in NativeInLightFixer (Component.java) corrects the visibility
of the heavyweight children based on the state of the lightweight
container. However, it does that based on events which are asynchronous.
Therfore, when one makes lightweight container visible, his heavyweights
don't immediately become visible as it happens with heavyweight
containers and simple heavyweight components. This difference is
affecting native code - you can't request focus on an invisible
component, but at that time all Java properties show that such component
is eligible to receive focus. Therefore, after we have requested focus
on such component, the focus is actually nowhere.

This asynchronousity should be removed, probably by adding the children
traversal code to lightweight containers.
======================================================================
Comments
SUGGESTED FIX --- old/src/share/classes/java/awt/Component.java 2008-01-23 17:52:41.000000000 +0300 +++ new/src/share/classes/java/awt/Component.java 2008-01-23 17:52:41.000000000 +0300 @@ -6484,7 +6484,7 @@ // will need some help. Container parent = this.parent; if (parent != null && parent.peer instanceof LightweightPeer) { - nativeInLightFixer = new NativeInLightFixer(); + relocateComponent(); } } invalidate(); @@ -6595,10 +6595,6 @@ } } - if (nativeInLightFixer != null) { - nativeInLightFixer.uninstall(); - } - ComponentPeer p = peer; if (p != null) { boolean isLightweight = isLightweight(); @@ -8491,8 +8487,6 @@ setComponentOrientation(orientation); } - transient NativeInLightFixer nativeInLightFixer; - /** * Checks that this component meets the prerequesites to be focus owner: * - it is enabled, visible, focusable @@ -8517,189 +8511,26 @@ return true; } - /** - * This odd class is to help out a native component that has been - * embedded in a lightweight component. Moving lightweight - * components around and changing their visibility is not seen - * by the native window system. This is a feature for lightweights, - * but a problem for native components that depend upon the - * lightweights. An instance of this class listens to the lightweight - * parents of an associated native component (the outer class). - * - * @author Timothy Prinzing - */ - final class NativeInLightFixer implements ComponentListener, ContainerListener { - - NativeInLightFixer() { - lightParents = new Vector(); - install(parent); - } - - void install(Container parent) { - lightParents.clear(); - Container p = parent; - boolean isLwParentsVisible = true; - // stash a reference to the components that are being observed so that - // we can reliably remove ourself as a listener later. - for (; p.peer instanceof LightweightPeer; p = p.parent) { - - // register listeners and stash a reference - p.addComponentListener(this); - p.addContainerListener(this); - lightParents.addElement(p); - isLwParentsVisible &= p.isVisible(); - } - // register with the native host (native parent of associated native) - // to get notified if the top-level lightweight is removed. - nativeHost = p; - p.addContainerListener(this); - - // kick start the fixup. Since the event isn't looked at - // we can simulate movement notification. - componentMoved(null); - if (!isLwParentsVisible) { - synchronized (getTreeLock()) { - if (peer != null) { - peer.hide(); - } - } - } - } - - void uninstall() { - if (nativeHost != null) { - removeReferences(); - } - } - - // --- ComponentListener ------------------------------------------- - - /** - * Invoked when one of the lightweight parents has been resized. - * This doesn't change the position of the native child so it - * is ignored. - */ - public void componentResized(ComponentEvent e) { - } - - /** - * Invoked when one of the lightweight parents has been moved. - * The native peer must be told of the new position which is - * relative to the native container that is hosting the - * lightweight components. - */ - public void componentMoved(ComponentEvent e) { - synchronized (getTreeLock()) { - int nativeX = x; - int nativeY = y; - for(Component c = parent; (c != null) && - (c.peer instanceof LightweightPeer); - c = c.parent) { - - nativeX += c.x; - nativeY += c.y; - } - if (peer != null) { - peer.setBounds(nativeX, nativeY, width, height, - ComponentPeer.SET_LOCATION); - } - } - } - - /** - * Invoked when a lightweight parent component has been - * shown. The associated native component must also be - * shown if it hasn't had an overriding hide done on it. - */ - public void componentShown(ComponentEvent e) { - if (shouldShow()) { - synchronized (getTreeLock()) { - if (peer != null) { - peer.show(); - } - } + /** + * Fix the location of the HW component in a LW container hierarchy. + */ + final void relocateComponent() { + synchronized (getTreeLock()) { + if (peer == null) { + return; } - } - - /** - * Invoked when one of the lightweight parents become visible. - * Returns true if component and all its lightweight - * parents are visible. - */ - private boolean shouldShow() { - boolean isLwParentsVisible = visible; - for (int i = lightParents.size() - 1; - i >= 0 && isLwParentsVisible; - i--) + int nativeX = x; + int nativeY = y; + for (Component cont = getContainer(); + cont != null && cont.isLightweight(); + cont = cont.getContainer()) { - isLwParentsVisible &= - ((Container) lightParents.elementAt(i)).isVisible(); - } - return isLwParentsVisible; - } - - /** - * Invoked when component has been hidden. - */ - public void componentHidden(ComponentEvent e) { - if (visible) { - synchronized (getTreeLock()) { - if (peer != null) { - peer.hide(); - } - } - } - } - - // --- ContainerListener ------------------------------------ - - /** - * Invoked when a component has been added to a lightweight - * parent. This doesn't effect the native component. - */ - public void componentAdded(ContainerEvent e) { - } - - /** - * Invoked when a lightweight parent has been removed. - * This means the services of this listener are no longer - * required and it should remove all references (ie - * registered listeners). - */ - public void componentRemoved(ContainerEvent e) { - Component c = e.getChild(); - if (c == Component.this) { - removeReferences(); - } else { - int n = lightParents.size(); - for (int i = 0; i < n; i++) { - Container p = (Container) lightParents.elementAt(i); - if (p == c) { - removeReferences(); - break; - } - } + nativeX += cont.x; + nativeY += cont.y; } + peer.setBounds(nativeX, nativeY, width, height, + ComponentPeer.SET_LOCATION); } - - /** - * Removes references to this object so it can be - * garbage collected. - */ - void removeReferences() { - int n = lightParents.size(); - for (int i = 0; i < n; i++) { - Container c = (Container) lightParents.elementAt(i); - c.removeComponentListener(this); - c.removeContainerListener(this); - } - nativeHost.removeContainerListener(this); - lightParents.clear(); - nativeHost = null; - } - - Vector lightParents; - Container nativeHost; } /** --- old/src/share/classes/java/awt/Container.java 2008-01-23 17:52:43.000000000 +0300 +++ new/src/share/classes/java/awt/Container.java 2008-01-23 17:52:42.000000000 +0300 @@ -832,16 +832,8 @@ } if (!comp.isLightweight() && isLightweight()) { // If component is heavyweight and one of the containers is lightweight - // some NativeInLightFixer activity should be performed - if (!curParent.isLightweight()) { - // Moving from heavyweight container to lightweight container - should create NativeInLightFixer - // since addNotify does this - comp.nativeInLightFixer = new NativeInLightFixer(); - } else { - // Component already has NativeInLightFixer - just reinstall it - // because hierarchy changed and he needs to rebuild list of parents to listen. - comp.nativeInLightFixer.install(this); - } + // the location of the component should be fixed. + comp.relocateComponent(); } } } @@ -3950,6 +3942,82 @@ } } + private void recursiveShowHeavyweightChildren() { + if (!hasHeavyweightDescendants() || !isVisible()) { + return; + } + for (int index = 0; index < getComponentCount(); index++) { + Component comp = getComponent(index); + if (comp.isLightweight()) { + if (comp instanceof Container) { + ((Container)comp).recursiveShowHeavyweightChildren(); + } + } else { + if (comp.isVisible()) { + ComponentPeer peer = comp.getPeer(); + if (peer != null) { + peer.show(); + } + } + } + } + } + + private void recursiveHideHeavyweightChildren() { + if (!hasHeavyweightDescendants()) { + return; + } + for (int index = 0; index < getComponentCount(); index++) { + Component comp = getComponent(index); + if (comp.isLightweight()) { + if (comp instanceof Container) { + ((Container)comp).recursiveHideHeavyweightChildren(); + } + } else { + if (comp.isVisible()) { + ComponentPeer peer = comp.getPeer(); + if (peer != null) { + peer.hide(); + } + } + } + } + } + + private void recursiveRelocateHeavyweightChildren(Point origin) { + for (int index = 0; index < getComponentCount(); index++) { + Component comp = getComponent(index); + if (comp.isLightweight()) { + if (comp instanceof Container && + ((Container)comp).hasHeavyweightDescendants()) + { + final Point newOrigin = new Point(origin); + newOrigin.translate(comp.getX(), comp.getY()); + ((Container)comp).recursiveRelocateHeavyweightChildren(newOrigin); + } + } else { + ComponentPeer peer = comp.getPeer(); + if (peer != null) { + peer.setBounds(origin.x + comp.getX(), origin.y + comp.getY(), + comp.getWidth(), comp.getHeight(), + ComponentPeer.SET_LOCATION); + } + } + } + } + + /* + * Consider the heavyweight container hides or shows the HW descendants + * automatically. Therefore we care of LW containers' visibility only. + */ + private boolean isRecursivelyVisibleUpToHeavyweightContainer() { + return isLightweight() ? + isVisible() && (getContainer() == null || + getContainer().isRecursivelyVisibleUpToHeavyweightContainer()) + : true; + } + + @Override void mixOnShowing() { synchronized (getTreeLock()) { if (mixingLog.isLoggable(Level.FINE)) { @@ -3958,6 +4026,10 @@ boolean isLightweight = isLightweight(); + if (isLightweight && isRecursivelyVisibleUpToHeavyweightContainer()) { + recursiveShowHeavyweightChildren(); + } + if (!isLightweight || (isLightweight && hasHeavyweightDescendants())) { recursiveApplyCurrentShape(); } @@ -3966,6 +4038,42 @@ } } + @Override + void mixOnHiding(boolean isLightweight) { + synchronized (getTreeLock()) { + if (mixingLog.isLoggable(Level.FINE)) { + mixingLog.fine("this = " + this + + "; isLightweight=" + isLightweight); + } + if (isLightweight) { + recursiveHideHeavyweightChildren(); + } + super.mixOnHiding(isLightweight); + } + } + + @Override + void mixOnReshaping() { + synchronized (getTreeLock()) { + if (mixingLog.isLoggable(Level.FINE)) { + mixingLog.fine("this = " + this); + } + if (isLightweight() && hasHeavyweightDescendants()) { + final Point origin = new Point(getX(), getY()); + for (Container cont = getContainer(); + cont != null && cont.isLightweight(); + cont = cont.getContainer()) + { + origin.translate(cont.getX(), cont.getY()); + } + + recursiveRelocateHeavyweightChildren(origin); + } + super.mixOnReshaping(); + } + } + + @Override void mixOnZOrderChanging(int oldZorder, int newZorder) { synchronized (getTreeLock()) { if (mixingLog.isLoggable(Level.FINE)) {
23-01-2008
EVALUATION While fixing the issue, it is discovered that javax.swing.JComponent has a problem in its _paintImmediately() package-private method. At the bottom of the method the safelyGetGraphics() is invoked wich in turn uses the Component.getGraphics() method to obtain the Graphics object for the given component. Actually the javadoc of the method says it may return null under some circumstances. The _paintImmediately() method doesn't perform any verifications and uses the returned value directly, sometimes causing NPE to be thrown. To fix this, we need to guard the code that uses the returnd value.
22-01-2008
EVALUATION Since the HW/LW Mixing feature has been implemented, we already iterate the hierarchy of components to form the required shapes for HWs when something changes (the visibility of the component, its location, etc.). We can simply add some additional code in the loops and eliminate the NativeInLightFixer completely. Note that all these methods (like Component.mixOnShowing() and the rest) get executed synchronously, and therefore this approach should solve the problem mentioned in the description.
20-12-2007
EVALUATION No resources for Mustang
29-07-2005
CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: mustang
17-09-2004
EVALUATION Name: dmR10075 Date: 02/12/2004 Not for tiger ###@###.### 2004-2-12 ======================================================================
12-02-2004