<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi Prasanta,<br>
    <br>
    Please find my answer inline.<br>
    <br>
    Thanks,<br>
    Dmitry<br>
    <div class="moz-cite-prefix">On 24/01/2017 08:29, Prasanta Sadhukhan
      wrote:<br>
    </div>
    <blockquote
      cite="mid:68ccebf0-3619-7796-a791-05ebec257fd2@oracle.com"
      type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      <p>Hi Dmitri,<br>
      </p>
      <br>
      <div class="moz-cite-prefix">On 1/23/2017 11:52 PM, Dmitry Markov
        wrote:<br>
      </div>
      <blockquote
        cite="mid:E0926A23-85D4-4667-8FAB-9E5CA15B2638@oracle.com"
        type="cite">
        <meta http-equiv="Content-Type" content="text/html;
          charset=UTF-8">
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">Hi Prasanta,</span></div>
        <div style="margin: 0px; line-height: normal; min-height: 14px;"
          class=""><span style="font-kerning: none" class=""></span><br
            class="">
        </div>
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">As far as I know
            imageDataProvider_UnholdJavaPixels() is only invoked for
            images with ‘custom’ type. We read the data directly from
            Java memory for such images. <br>
          </span></div>
      </blockquote>
      I understand that but my question was coming from the fact that,
      if we are going to print more than 1 "custom" image, then
      isdo->pixels will point to the same "direct buffer address"
      (which we obtained for the first custom image) since
      isdo->pixels will not be NULL (so line 969 will not be
      executed), whereas ideally, isdo->pixels should obtain the data
      again. Am I misinterpreting something?<br>
    </blockquote>
    Actually we initialize isdo structure based on data from Java layer
    anytime when an image is printed, see LockImage() function. So
    isdo->pixels will be correctly initialized inside
    holdJavaPixels() for every "custom" image. I am sorry, but the
    situation you described above is not possible here. <br>
    <blockquote
      cite="mid:68ccebf0-3619-7796-a791-05ebec257fd2@oracle.com"
      type="cite">
      <blockquote
        cite="mid:E0926A23-85D4-4667-8FAB-9E5CA15B2638@oracle.com"
        type="cite">
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">At the same time for
            other image types we use Get/ReleasePrimitiveArrayCritical()
            and copy the data from Java to native memory.</span></div>
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">I guess that’s the main
            reason why we don’t set isdo->pixels to null inside
            imageDataProvider_UnholdJavaPixels().</span></div>
        <div style="margin: 0px; line-height: normal; min-height: 14px;"
          class=""><span style="font-kerning: none" class=""></span><br
            class="">
        </div>
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">Also I updated the test
            based on your suggestions. Please find the new version here:
            <a moz-do-not-send="true"
              href="http://cr.openjdk.java.net/%7Edmarkov/8163889/webrev.03/"
              class="">http://cr.openjdk.java.net/~dmarkov/8163889/webrev.03/</a></span></div>
      </blockquote>
      test looks good.<br>
      <br>
      Regards<br>
      Prasanta<br>
      <blockquote
        cite="mid:E0926A23-85D4-4667-8FAB-9E5CA15B2638@oracle.com"
        type="cite">
        <div style="margin: 0px; line-height: normal; min-height: 14px;"
          class=""><span style="font-kerning: none" class=""></span><br
            class="">
        </div>
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">Thanks,</span></div>
        <div style="margin: 0px; line-height: normal;" class=""><span
            style="font-kerning: none" class="">Dmitry </span></div>
        <div style="margin: 0px; line-height: normal; min-height: 14px;"
          class=""><span style="font-kerning: none" class=""></span><br
            class="">
        </div>
        <div>
          <blockquote type="cite" class="">
            <div class="">On 23 Jan 2017, at 09:17, Prasanta Sadhukhan
              <<a moz-do-not-send="true"
                href="mailto:prasanta.sadhukhan@oracle.com" class="">prasanta.sadhukhan@oracle.com</a>>

              wrote:</div>
            <br class="Apple-interchange-newline">
            <div class="">
              <meta content="text/html; charset=UTF-8"
                http-equiv="Content-Type" class="">
              <div bgcolor="#FFFFFF" text="#000000" class="">
                <p class="">Hi Dmitry,<br class="">
                </p>
                <p class="">Any particular reason why in
                  imageDataProvider_UnholdJavaPixels(), we are not doing
                  <br class="">
                </p>
                <p class="">isdo->pixels = null; which we do in
                  unholdJavaPixels() </p>
                since in holdJavaPixels() , we are doing <br class="">
                <pre class="">967     else if (isdo->pixels == NULL)
 968     {
 969         isdo->pixels = (Pixel8bit*)((*env)->GetDirectBufferAddress(env, isdo->array));
 970     }

</pre>
                <div class="moz-cite-prefix">Also, in the test, I guess
                  there's no point catching Exception and rethrowing
                  RuntimeException since we are not doing any processing
                  in catch block. We can just add throws Exception in
                  main() without this catch block and have try-finally.<br
                    class="">
                  Also, I guess we do not follow adding "author" in the
                  new test anymore. <br class="">
                  <br class="">
                  Regards<br class="">
                  Prasanta<br class="">
                  On 1/22/2017 12:35 AM, Dmitry Markov wrote:<br
                    class="">
                </div>
                <blockquote
                  cite="mid:C43DC296-7D01-40F9-93A3-9BF69FED6654@oracle.com"
                  type="cite" class="">
                  <meta http-equiv="Content-Type" content="text/html;
                    charset=UTF-8" class="">
                  Hi Phil,
                  <div class=""><br class="">
                  </div>
                  <div class="">I agree ‘othervm’ is not necessary here.
                    That is ‘copy-paste’ error. </div>
                  <div class="">Also I updated the part related to the
                    file deletion based on your suggestion.</div>
                  <div class="">Please find new webrev here: <a
                      moz-do-not-send="true"
                      href="http://cr.openjdk.java.net/%7Edmarkov/8163889/webrev.02/"
                      class="">http://cr.openjdk.java.net/~dmarkov/8163889/webrev.02/</a></div>
                  <div class=""><br class="">
                  </div>
                  <div class="">Thanks,</div>
                  <div class="">Dmitry <br class="">
                    <div class="">
                      <blockquote type="cite" class="">
                        <div class="">On 21 Jan 2017, at 00:24, Philip
                          Race <<a moz-do-not-send="true"
                            href="mailto:philip.race@oracle.com"
                            class="">philip.race@oracle.com</a>>
                          wrote:</div>
                        <br class="Apple-interchange-newline">
                        <div class="">
                          <meta content="text/html; charset=UTF-8"
                            http-equiv="Content-Type" class="">
                          <div bgcolor="#FFFFFF" text="#000000" class="">
                            Hi Dmitry,<br class="">
                            <meta http-equiv="content-type"
                              content="text/html; charset=UTF-8"
                              class="">
                            > 29 * @run main/othervm PrintCrashTest<br
                              class="">
                            <pre class="">why othervm ?

I don't think that is strictly necessary just because you are using deleteOnExit.
And FWIW I think the test could "more promptly" delete the file anyway after print returns.

-phil.
</pre>
                            <br class="">
                            <br class="">
                            On 1/20/17, 9:36 AM, Dmitry Markov wrote:
                            <blockquote
                              cite="mid:C1BB8E44-79B5-40EB-BFAC-CAC1EA4F35B0@oracle.com"
                              type="cite" class="">
                              <pre class="" wrap="">Hi Phil, Prasanta,

I have updated the fix as you suggested, (i.e. added the regression test). The new webrev is located at <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Edmarkov/8163889/webrev.01/">http://cr.openjdk.java.net/~dmarkov/8163889/webrev.01/</a>
Could you review the new version, please?

Thanks,
Dmitry
</pre>
                              <blockquote type="cite" class="">
                                <pre class="" wrap="">On 20 Jan 2017, at 19:50, Phil Race <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:philip.race@oracle.com"><philip.race@oracle.com></a> wrote:

I haven't looked at the fix (yet) but I definitely agree that a manual regression test
for this  is better than none. What else should we do ? Just not test printing ?

In my view which I've expressed to SQE for a really long time, if you aren't testing with
printers installed you aren't testing the whole platform. Whilst it may be convenient
that tests (silently) don't complain when there are no printers, it is a slippery slope ..

-phil.

On 01/20/2017 04:04 AM, Prasanta Sadhukhan wrote:
</pre>
                                <blockquote type="cite" class="">
                                  <pre class="" wrap="">It is possible to create manual regression test for this problem. Also the test will require some additional set up steps such as printer installation and so on. It seems to me that is overhead for person who runs it. However if you insist on test creation, I will add it.
</pre>
                                </blockquote>
                              </blockquote>
                            </blockquote>
                          </div>
                        </div>
                      </blockquote>
                    </div>
                    <br class="">
                  </div>
                </blockquote>
                <br class="">
              </div>
            </div>
          </blockquote>
        </div>
        <br class="">
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>