<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Dmitry,<br>
<br>
Looks fine to me.<br>
<br>
<br>
Regards,<br>
Alexey<br>
<br>
<div class="moz-cite-prefix">On 28.11.2016 14:53, dmitry markov
wrote:<br>
</div>
<blockquote cite="mid:583C1ABC.1030705@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Hi Alexey,<br>
<br>
I have updated the fix based on your suggestions. Please find new
webrev here: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edmarkov/8169589/webrev.02/">http://cr.openjdk.java.net/~dmarkov/8169589/webrev.02/</a><br>
<br>
Thanks,<br>
Dmitry<br>
<div class="moz-cite-prefix">On 28/11/2016 12:51, Alexey Ivanov
wrote:<br>
</div>
<blockquote
cite="mid:415c56f6-d0d4-fa0b-d931-7fd2af7b59cc@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hi Dmitry,<br>
<br>
If you expand imports<br>
<br>
31 import java.awt.*;<br>
<br>
you'll be able to use java.util.List via import:<br>
<br>
1123 java.util.List<Window> pwChildWindows =
new ArrayList<Window>(Arrays.asList(w.getOwnedWindows()));<br>
<br>
<br>
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().<br>
<br>
If you remove this local variable, there will be no
inconsistency:<br>
<br>
1098 ArrayList<Window> childWindows = new
ArrayList<Window>();<br>
1123 java.util.List<Window> pwChildWindows =
new ArrayList<Window>(Arrays.asList(w.getOwnedWindows()));<br>
<br>
I mean the former is declared as ArrayList whereas the latter is
List.<br>
<br>
<br>
Also you can use zero-sized array allocation in<br>
<br>
1129 orderAboveSiblingsImpl(childWindows.toArray(new
Window[childWindows.size()]));<br>
<br>
because it “seems faster, safer, and contractually cleaner”, see<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion">https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion</a><br>
<br>
<br>
Regards,<br>
Alexey<br>
<br>
<div class="moz-cite-prefix">On 26.11.2016 16:01, dmitry markov
wrote:<br>
</div>
<blockquote cite="mid:5839879D.105@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
Hi Sergey,
<div><br>
</div>
<div>I have added some remarks to the code. The updated webrev
is located at <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Edmarkov/8169589/webrev.01/">http://cr.openjdk.java.net/~dmarkov/8169589/webrev.01/</a></div>
<div><br>
</div>
<div>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.</div>
<div><br>
Summary of changes:<br>
- orderAboveSiblings() is responsible for retrieval of root
owner and initial creation of the list of the windows which
have to be ordered.<br>
- 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.<br>
- 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. <br>
</div>
<div><br>
Thanks,<br>
Dmitry<br>
</div>
<div>
<div class="AppleOriginalContents" style="direction: ltr;">
<blockquote type="cite">
<div>On 25 Nov 2016, at 16:16, Sergey Bylokhov <a
moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:Sergey.Bylokhov@oracle.com"><Sergey.Bylokhov@oracle.com></a>
wrote:</div>
<br class="Apple-interchange-newline">
<div>
<div>Hi, Dmitry.<br class="">
Can you please adds some comments to the code and
describe what is going on.<br class="">
<br class="">
On 25.11.16 16:08, dmitry markov wrote:<br class="">
<blockquote type="cite" class="">Hello,<br class="">
<br class="">
Could you review a fix for jdk9, please?<br
class="">
<br class="">
bug: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8169589">https://bugs.openjdk.java.net/browse/JDK-8169589</a><br
class="">
webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edmarkov/8169589/webrev.00/">http://cr.openjdk.java.net/~dmarkov/8169589/webrev.00/</a><br
class="">
<br class="">
Problem description:<br class="">
Current implementation of
CPlatformWindow.orderAboveSiblings() just<br
class="">
recursively pops up the windows from ‘active’
parent-child window chain.<br class="">
At the same time other child windows (which are
not in active chain)<br class="">
stayed ‘untouched’ and may be placed behind their
nearest parent/owner.<br class="">
<br class="">
Fix:<br class="">
CPlatformWindow.orderAboveSiblings() should be
modified. It has to take<br class="">
into account that a window may own more than one
child window.<br class="">
<br class="">
Note: JCK tests passed on the build with the fix.<br
class="">
<br class="">
Thanks,<br class="">
Dmitry<br class="">
</blockquote>
<br class="">
<br class="">
-- <br class="">
Best regards, Sergey.<br class="">
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>