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

[Alexey Ivanov]

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.
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20161128/edaee18e/attachment.html>