<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html;
      charset=windows-1252">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Hi Krishna,<br>
    </p>
    This seems good to me except one thing. You are checking
    getRowCount() == 0 but there is a chance of test extending
    VariableHeightLayoutCache and overriding getRowCount to return -ve
    also as it is an int. In that case, I guess this function will not
    return -1 which spec mandates [If there are no rows, -1 is returned]
    so I guess we should check for <=0. <br>
    Also, there is no need of calling getRowCount() twice as it will not
    change between 929, 936.<br>
    <br>
    Regards<br>
    Prasanta<br>
    <div class="moz-cite-prefix">On 12/7/2017 4:41 PM, Krishna Addepalli
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:8ff212fc-7c49-4963-99af-477ac4a0f4e1@default">
      <meta http-equiv="Content-Type" content="text/html;
        charset=windows-1252">
      <meta name="Generator" content="Microsoft Word 15 (filtered
        medium)">
      <style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:#954F72;
        text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
        {mso-style-priority:34;
        margin-top:0in;
        margin-right:0in;
        margin-bottom:0in;
        margin-left:.5in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
p.msonormal0, li.msonormal0, div.msonormal0
        {mso-style-name:msonormal;
        mso-margin-top-alt:auto;
        margin-right:0in;
        mso-margin-bottom-alt:auto;
        margin-left:0in;
        font-size:12.0pt;
        font-family:"Times New Roman",serif;}
span.EmailStyle19
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
span.EmailStyle20
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
/* List Definitions */
@list l0
        {mso-list-id:931817106;
        mso-list-type:hybrid;
        mso-list-template-ids:1614576216 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l0:level1
        {mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-.25in;}
@list l0:level2
        {mso-level-number-format:alpha-lower;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-.25in;}
@list l0:level3
        {mso-level-number-format:roman-lower;
        mso-level-tab-stop:none;
        mso-level-number-position:right;
        text-indent:-9.0pt;}
@list l0:level4
        {mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-.25in;}
@list l0:level5
        {mso-level-number-format:alpha-lower;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-.25in;}
@list l0:level6
        {mso-level-number-format:roman-lower;
        mso-level-tab-stop:none;
        mso-level-number-position:right;
        text-indent:-9.0pt;}
@list l0:level7
        {mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-.25in;}
@list l0:level8
        {mso-level-number-format:alpha-lower;
        mso-level-tab-stop:none;
        mso-level-number-position:left;
        text-indent:-.25in;}
@list l0:level9
        {mso-level-number-format:roman-lower;
        mso-level-tab-stop:none;
        mso-level-number-position:right;
        text-indent:-9.0pt;}
ol
        {margin-bottom:0in;}
ul
        {margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
      <div class="WordSection1">
        <p class="MsoNormal"><span style="color:#1F497D">Hi Sergey,<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span style="color:#1F497D">Per our
            conversation, I have done the following changes:<o:p></o:p></span></p>
        <p class="MsoListParagraph"
          style="text-indent:-.25in;mso-list:l0 level1 lfo1"><!--[if !supportLists]--><span
            style="color:#1F497D"><span style="mso-list:Ignore">1.<span
                style="font:7.0pt "Times New Roman"">       </span></span></span><!--[endif]--><span
            style="color:#1F497D">Found that the .class size increases
            by 1kb when streams are used, so reverted the changes
            related to it.<o:p></o:p></span></p>
        <p class="MsoListParagraph"
          style="text-indent:-.25in;mso-list:l0 level1 lfo1"><!--[if !supportLists]--><span
            style="color:#1F497D"><span style="mso-list:Ignore">2.<span
                style="font:7.0pt "Times New Roman"">       </span></span></span><!--[endif]--><span
            style="color:#1F497D">Moved the “++nextIndex” into the
            conditional, so that there is no logical change.<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:#1F497D">Here is the
            updated webrev: </span><a
            href="http://cr.openjdk.java.net/%7Ekaddepalli/8190281/webrev01/"
            moz-do-not-send="true">http://cr.openjdk.java.net/~kaddepalli/8190281/webrev01/</a><o:p></o:p></p>
        <p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span style="color:#1F497D">Thanks,<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:#1F497D">Krishna<o:p></o:p></span></p>
        <p class="MsoNormal"><a name="_MailEndCompose"
            moz-do-not-send="true"><span style="color:#1F497D"><o:p> </o:p></span></a></p>
        <span style="mso-bookmark:_MailEndCompose"></span>
        <div>
          <div style="border:none;border-top:solid #E1E1E1
            1.0pt;padding:3.0pt 0in 0in 0in">
            <p class="MsoNormal"><b>From:</b> Krishna Addepalli <br>
              <b>Sent:</b> Wednesday, December 6, 2017 2:43 PM<br>
              <b>To:</b> <a class="moz-txt-link-abbreviated" href="mailto:swing-dev@openjdk.java.net">swing-dev@openjdk.java.net</a><br>
              <b>Subject:</b> [10][JDK-8190281] Code cleanup in
src\java.desktop\share\classes\javax\swing\tree\VariableHeightLayoutCache.java<o:p></o:p></p>
          </div>
        </div>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p class="MsoNormal">Hi All,<o:p></o:p></p>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p class="MsoNormal">Please review the fix for bug:<o:p></o:p></p>
        <p class="MsoNormal">Bug: JDK-8190281 <a
            href="https://bugs.openjdk.java.net/browse/JDK-8190281"
            moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8190281</a><o:p></o:p></p>
        <p class="MsoNormal">JDK 10 Webrev: <a
            href="http://cr.openjdk.java.net/%7Ekaddepalli/8190281/webrev00/"
            moz-do-not-send="true">http://cr.openjdk.java.net/~kaddepalli/8190281/webrev00/</a><o:p></o:p></p>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p class="MsoNormal">This bug was created while root causing
          JDK-8187936, and the following refactoring points have been
          addressed:<o:p></o:p></p>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p class="MsoNormal">1. Line 927: Uninitialized variables,
          checking for trivial reject case multiple times. <br>
          2. Line 999: Traditional code written to find maximum size of
          components, which can be done without any local variables and
          explicit looping by replacing with streams. <br>
          3. Line 1365: Code repetition for differenct conditions, which
          can be ored together to reduce the repetition. <br>
          4. Line 1482: A large code block gets repeated only because of
          different values need to be passed in one line. This can be
          moved to a variable initialization, and the repeating code
          blocks can be reduced to one. <br>
          5. Line 1505: Variable initialization can be simplified by
          combining different conditions. <br>
          6. Line 1540: An explicit loop to apply a function over a
          collection, can be achieved in one line by a forEach
          construct.  – This is producing some visual artifacts, so
          ignored.<br>
          7. Line 1747: Combine all the trivial reject cases into one
          condition, and also, a potential bug which increments the
          "nextIndex" value beyond the length of the containing
          elements. The increment should happen only if the trivial
          reject case fails.<o:p></o:p></p>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p class="MsoNormal">Thanks,<o:p></o:p></p>
        <p class="MsoNormal">Krishna<o:p></o:p></p>
      </div>
    </blockquote>
    <br>
  </body>
</html>