<AWT Dev> [9] Review request for 8169589: [macosx] Activating a JDialog puts to back another dialog

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Mon Nov 28 19:47:27 UTC 2016


Hi, Dmitry.
As far as I understand the orderAboveSiblingsImpl() is execute on the 
appkit thread? if yes, then you should not call target.getOwnedWindows() 
on it, because it can be overridden by the user. You should call 
"getOwnedWindows_NoClientCode" via accessor, or get a list of child 
windows via the native api.

On 28.11.16 15:12, Alexey Ivanov wrote:
> Hi Dmitry,
>
> Looks fine to me.
>
>
> Regards,
> Alexey
>
> On 28.11.2016 14:53, dmitry markov wrote:
>> Hi Alexey,
>>
>> I have updated the fix based on your suggestions. Please find new
>> webrev here: http://cr.openjdk.java.net/~dmarkov/8169589/webrev.02/
>>
>> Thanks,
>> Dmitry
>> On 28/11/2016 12:51, Alexey Ivanov wrote:
>>> Hi Dmitry,
>>>
>>> If you expand imports
>>>
>>>  31 import java.awt.*;
>>>
>>> you'll be able to use java.util.List via import:
>>>
>>> 1123             java.util.List<Window> pwChildWindows = new
>>> ArrayList<Window>(Arrays.asList(w.getOwnedWindows()));
>>>
>>>
>>> Actually you don't need this local variable as well as new ArrayList
>>> object: you can pass the result of Arrays.asList directly to
>>> childWindows.addAll().
>>>
>>> If you remove this local variable, there will be no inconsistency:
>>>
>>> 1098         ArrayList<Window> childWindows = new ArrayList<Window>();
>>> 1123             java.util.List<Window> pwChildWindows = new
>>> ArrayList<Window>(Arrays.asList(w.getOwnedWindows()));
>>>
>>> I mean the former is declared as ArrayList whereas the latter is List.
>>>
>>>
>>> Also you can use zero-sized array allocation in
>>>
>>> 1129             orderAboveSiblingsImpl(childWindows.toArray(new
>>> Window[childWindows.size()]));
>>>
>>> because it “seems faster, safer, and contractually cleaner”, see
>>> https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion
>>>
>>>
>>> Regards,
>>> Alexey
>>>
>>> On 26.11.2016 16:01, dmitry markov wrote:
>>>> Hi Sergey,
>>>>
>>>> I have added some remarks to the code. The updated webrev is located
>>>> at http://cr.openjdk.java.net/~dmarkov/8169589/webrev.01/
>>>> <http://cr.openjdk.java.net/%7Edmarkov/8169589/webrev.01/>
>>>>
>>>> Proposed functionality performs ordering operation from the very
>>>> bottom, (i.e. root owner) so that the windows are ordered above
>>>> their nearest parent; ancestors of the window, which is going to
>>>> become ‘main window’, are placed above their siblings.
>>>>
>>>> Summary of changes:
>>>> - orderAboveSiblings() is responsible for retrieval of root owner
>>>> and initial creation of the list of the windows which have to be
>>>> ordered.
>>>> - orderAboveSiblingsImpl(Window[] windows) performs ordering of the
>>>> windows specified by input array. If the window is one of ancestors
>>>> of 'main window' or is going to become main by itself, the window
>>>> will be ordered above its siblings; otherwise the window is just
>>>> ordered above its nearest parent. This method is recursively called
>>>> until all windows in window hierarchy are ordered.
>>>> - Two helper methods: getRootOwner() is responsible for retrieval of
>>>> root owner for the window and isOneOfOwnersOrSelf(CPlatformWindow
>>>> window) - tests whether the current window is one of ancestors of
>>>> the specified window.
>>>>
>>>> Thanks,
>>>> Dmitry
>>>>> On 25 Nov 2016, at 16:16, Sergey Bylokhov
>>>>> <Sergey.Bylokhov at oracle.com> wrote:
>>>>>
>>>>> Hi, Dmitry.
>>>>> Can you please adds some comments to the code and describe what is
>>>>> going on.
>>>>>
>>>>> On 25.11.16 16:08, dmitry markov wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Could you review a fix for jdk9, please?
>>>>>>
>>>>>>    bug: https://bugs.openjdk.java.net/browse/JDK-8169589
>>>>>>    webrev: http://cr.openjdk.java.net/~dmarkov/8169589/webrev.00/
>>>>>>
>>>>>> Problem description:
>>>>>> Current implementation of CPlatformWindow.orderAboveSiblings() just
>>>>>> recursively pops up the windows from ‘active’ parent-child window
>>>>>> chain.
>>>>>> At the same time other child windows (which are not in active chain)
>>>>>> stayed ‘untouched’ and may be placed behind their nearest
>>>>>> parent/owner.
>>>>>>
>>>>>> Fix:
>>>>>> CPlatformWindow.orderAboveSiblings() should be modified. It has to
>>>>>> take
>>>>>> into account that a window may own more than one child window.
>>>>>>
>>>>>> Note: JCK tests passed on the build with the fix.
>>>>>>
>>>>>> Thanks,
>>>>>> Dmitry
>>>>>
>>>>>
>>>>> --
>>>>> Best regards, Sergey.
>>>>
>>>
>>
>


-- 
Best regards, Sergey.


More information about the awt-dev mailing list