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