RFR: 6176679: Application freezes when copying an animated gif image to the system clipboard [v10]

[Alan Snyder]

Your changes look reasonable.

I have a suggestion. Unless IDEA is lying to me, all uses of reconstruct() pass ALLBITS as the parameter.

The net effect is to wait for any of the three terminating conditions, ALLBITS, FRAMEBITS, and ERROR.

My suggestion would be to eliminate the parameter, add ALLBITS to the test in the loop, and rename the method waitForTermination() or something like that.

(The existing method can hang if ALLBITS is not true in the parameter, which makes the method unnecessarily fragile.)

> I am not sure what you mean by native image. Could you please clarify ?


I think I was mistaken. I tried again and was able to run your test.



> On Apr 30, 2023, at 2:36 PM, Rajat Mahajan <rmahajan at openjdk.org> wrote:
> 
> On Thu, 20 Apr 2023 19:26:52 GMT, Rajat Mahajan <rmahajan at openjdk.org <mailto:rmahajan at openjdk.org>> wrote:
> 
>>> **Problem:**
>>> 
>>> On pressing the Copy button we keep waiting in AWT-EventQueue thread in reconstruct() function as we detect that we have missing information for the animated image since we are copying single frame at a time.
>>> Due to this infinite wait the application freezes.
>>> 
>>> **Proposed Fix:**
>>> 
>>> There are conditions in the reconstruct() function that avoid entering the wait() code if we have some error reading the image , etc. So, I added the condition to avoid entering the wait() code if we are copying single frame at a time. This sounded logical to me since if we have incomplete information(single frame) about the animated image we shouldn’t keep waiting, as it leads to infinite wait. 
>>> After this change I see the GIF image being correctly copied and animated.
>>> 
>>> 
>>> **Testing:**
>>> 
>>> Added a test for this (bug6176679.java) and tested locally on my Windows machine and on mach5.
>> 
>> Rajat Mahajan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>  Update CopyAnimatedGIFTest.java
> 
>> _Mailing list message from [Alan Snyder](mailto:javalists at cbfiddle.com) on [client-libs-dev](mailto:client-libs-dev at mail.openjdk.org):_
>> 
>> I was hoping to be able to run your test program, but I cannot because on macOS there is no support for transferring a native image.
>> 
>> It also appears that I cannot simulate the transfer code because that code depends upon the reconstruct method, which is not public.
>> 
>> Just looking at the code, I think it is not correct.
>> 
>> It looks to me that the ImageRepresentation.drawToBufImage method is called only once, and it will do nothing unless ALLBITS or FRAMEBITS is true. I do not see any loop that would call it multiple times. (If I am correct about this, then only the first frame is drawn into the buffered image, which makes much more sense than drawing every frame into the buffered image. One might ask, however, what a user is trying to accomplish by copying an animated image to the system clipboard?)
>> 
>> So, it works if reconstruct() waits until ALLBITS or FRAMEBITS is true. But your change does not test for FRAMEBITS in the loop, so it will only work if FRAMEBITS is already true when reconstruct() is called.
>> 
>> How can that happen?
>> 
>> I think it works because you are displaying the image on the screen, and the animation code is reading frames.
>> 
>> I suggest trying the test without displaying the image.
> 
> Thanks for your suggestions. I have modified the code so it works for both cases where image is and is not displayed on screen.
> I have modified the test so it tests for both these cases.
> 
>> Also, your test would better if it retrieved the native image from the system clipboard to verify that it was successfully transferred. You would need to clear the system clipboard before running the test to be sure you are not retrieving something from a previous test run.
> 
> I am not sure what you mean by native image. Could you please clarify ?
> 
> -------------
> 
> PR Comment: https://git.openjdk.org/jdk/pull/13414#issuecomment-1529141085

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/client-libs-dev/attachments/20230430/3e4de822/attachment.htm>