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


Reply via email to