[8] Request for review: 7166296 closed/java/awt/Frame/DisabledParentOfToplevel/DisabledParentOfToplevel.html failed since 1.8.0b36 (original) (raw)

Anthony Petrov anthony.petrov at oracle.com
Tue Apr 23 06:23:29 PDT 2013

Hi Sergey,

On 04/23/2013 04:46 PM, Sergey Bylokhov wrote:

On 23.04.2013 16:24, Anthony Petrov wrote:

This code path exists in order to ensure that a heavyweight component (e.g. a Button) has a valid native container to be inserted to. Obviously, the container must always reside in the same top-level window where the component itself resides. This code should never traverse containers in other top-level windows (even though it does now, which is the real bug here). Not sure that It used for container validation, to me it is used for the creation of the window with appropriate parent(in AwtWindow::Create).

Indeed, it does. But in e.g. AwtButton::Create() it uses the same thing as a container. This code is ugly, and the Toolkit/Component.getNativeContainer() is named incorrectly.

I see that this erroneous pattern comes from the Windows implementation, but it doesn't make any sense on any of other platforms because they do make a clear distinction between owners and containers and use completely different APIs/concepts to set them. So I'd try and get rid of the pattern. I think we should investigate all use cases (~15 in JDK) and see whether we can rework them to something meaningful.

I believe that in most cases a get*Container() method must really return a container belonging to the same top-level window only. And in cases where we do want to get an owner of a window, we must use the Window.getParent() explicitly. I think the latter is only really needed when we create a native window, actually.

This should resolve this bug and also make our code clearer.

-- best regards, Anthony

I still think that the real bug here is an absent override method Window.getNativeContainer() that should return "this". Do you agree? It should be null, in this case, but will it be better than owner? -- best regards, Anthony On 04/23/2013 01:37 AM, Sergey Bylokhov wrote: Hi, Anthony. WComponentPeer.java:762 -> WComponentPeer.java:764 -> WWindowPeer.java:create(WComponentPeer parent).

On 23.04.2013 0:01, Anthony Petrov wrote: I don't believe getNativeContainer() should ever traverse a hierarchy outside of the toplevel window for component of which it's been called (or for the window itself if it's called on a Window instance). If some code expects otherwise, then there is a bug in that code which needs fixing.

Could you please point me to exact locations where the code does that? -- best regards, Anthony On 04/22/2013 06:24 PM, Sergey Bylokhov wrote: On 22.04.2013 17:55, Anthony Petrov wrote: Ah, I see the Component.getNativeContainer() checks whether the peer is lightweight or not. I believe it needs to be overridden in the Window class to return "this", though. Otherwise there's still a bug in this method. But window's implementation mix owner/container during initialization of WWindowPeer. There is only one field "parent", which assigned from the SunToolkit.getNativeContainer(). I assume that, when this method was added the window's parent/container and the window's owner were one concept. I don't quite understand this point. Please elaborate.

What I see is that we call SunToolkit.getNativeContainer() which isn't overridden in UNIXToolkit nor XToolkit, and thus results in a call to Toolkit.getNativeContainer(). The latter calls Component.getNativeContainer(). Which, w/o an override suggested above, produces an incorrect result and is the root cause of our bug, isn't it? The question is, what the result is correct. Currently, some code expects the owner will be refunded. -- best regards, Anthony

-- best regards, Anthony On 04/22/13 16:06, Sergey Bylokhov wrote: Hello, Please review the fix for jdk 8. SetEnable method check status of all parent containers and windows(via getParent() in SunToolkit.getNativeContainer()). But only containers in the same window should be checked. The new method was added to return the peer of the nearest heavyweight container. Bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7166296 Webrev can be found at: http://cr.openjdk.java.net/~serb/7166296/webrev.00

More information about the awt-dev mailing list