<AWT Dev> [8] Request for review: 7166296 closed/java/awt/Frame/DisabledParentOfToplevel/DisabledParentOfToplevel.html failed since 1.8.0b36

[Sergey Bylokhov]

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).
>
> 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/view_bug.do?bug_id=7166296
>>>>>>>> Webrev can be found at:
>>>>>>>> http://cr.openjdk.java.net/~serb/7166296/webrev.00
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>


-- 
Best regards, Sergey.