Looks fine.
On 29.11.16 12:10, dmitry markov wrote:
Hi Sergey,
You are right. The methods orderAboveSiblings() and
orderAboveSibligsImpl() are executed on the appkit thread. I have
updated the fix. Now these functions invoke
getOwnedWindows_NoClientCode() via window accessor instead of direct
call of getOwnedWindows from the Window class. The updated webrev is
located at http://cr.openjdk.java.net/~dmarkov/8169589/webrev.03/
Thanks,
Dmitry
On 28/11/2016 22:47, Sergey Bylokhov wrote:
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.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.
--
Best regards, Sergey.
