<AWT Dev> [9] Review request for 8040291 [macosx] Http-Images are not fully loaded when using ImageIcon

[Petr Pchelko]

Hello, Alexander.

The fix looks good.

With best regards. Petr.

On 21 мая 2014 г., at 17:47, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:

> 
> Could you review the updated fix:
>  http://cr.openjdk.java.net/~alexsch/8040291/webrev.03/
> 
> - The test checks that the resolution-variant observer is called at least once.
> 
> Thanks,
> Alexandr.
> 
> On 5/21/2014 2:50 PM, Alexander Scherbatiy wrote:
>> 
>> Could you review the updated fix:
>>   http://cr.openjdk.java.net/~alexsch/8040291/webrev.02/
>> 
>> - The isRVObserevr typo is fixed
>> 
>> On 5/20/2014 7:32 PM, Petr Pchelko wrote:
>>>> Could you review the updated fix:
>>>>  http://cr.openjdk.java.net/~alexsch/8040291/webrev.01
>>>> 
>>>> - The getRVSize() method from the SunToolkit is fixed
>>> Thank you for the update, but I still have questions about the test:
>>> 1. There’s a typo in isRVObserevr.
>>> 2. Do we need the isRVObserevr method? How could it be that this
>>> method return false in this test and it’s a correct behavior and not a bug?
>>> I see it’s only possible from SunToolkit.prepareImage if bits are ERROR | ABORT.
>>> Couldn’t we get rid of this method? Walking up the stack doesn’t look like the most
>>> reliable solution..
>> 
>>  The ImageObserver is used for two purposes in the Toolkit prepareImage() method:
>>    - to query which part of the image should be loaded
>>    - to notify an object about which image information is available
>> 
>>  To prepare a multi-resolution image it needs to prepare its resolution variant as well.
>>  The only way to know which part of the resolution variant should be loaded is requesting
>>  the base image observer. It leads that there will be a notification for the object that contains
>>  the resolution variant instead of the base image. To improve it the MultiResolutionToolkitImage
>>  wraps the base image observer for the resolution variant. The object gets notification
>>  which contains only the base image and updated sizes after that.
>> 
>>  The another case which is fixed in this issue is that  the resolution variant can be loaded first
>>  and it notifies the object that the image is loaded. The fix reduces the resolution variant info
>>  flags so the object should wait for image loading notification from the base image.
>> 
>>  The test needs to check that the wrapped observer from the resolution variant does not send
>>  more info than the original image has already had. The isRVObserver() just checks
>>  that the observer is called from the resolution variant and not is from the base image.
>> 
>>  Thanks,
>>  Alexandr.
>> 
>>> 
>>> With best regards. Petr.
>>> 
>>> On May 20, 2014, at 7:07 PM, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:
>>> 
>>>> Could you review the updated fix:
>>>>  http://cr.openjdk.java.net/~alexsch/8040291/webrev.01
>>>> 
>>>> - The getRVSize() method from the SunToolkit is fixed
>>>> 
>>>> On 5/20/2014 6:46 PM, Petr Pchelko wrote:
>>>>> Hello, Alexander.
>>>>> 
>>>>> SunToolkit:876 size == -1 ? -1 : size
>>>>> 
>>>>> What’s going on here? Isn’t this equal to just size?
>>>>> I believe is’t wrong and the size should be multiplied by 2 somewhere?
>>>>> If the method is wrong how does the test pass?
>>>>    The test passes because it uses SunToolkit.prepareImage() method with the -1 size.
>>>>    It also uses the image observer that requires to load all image bits.
>>>> 
>>>>   Thanks,
>>>>   Alexandr.
>>>> 
>>>>>  With best regards. Petr.
>>>>> 
>>>>> On May 20, 2014, at 6:32 PM, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:
>>>>> 
>>>>>>  Hello,
>>>>>> 
>>>>>>  Could you look at the fix?
>>>>>> 
>>>>>>  Thanks,
>>>>>>  Alexandr.
>>>>>> 
>>>>>> On 4/30/2014 6:34 PM, Alexander Scherbatiy wrote:
>>>>>>> Hello,
>>>>>>> 
>>>>>>> Could you review the fix:
>>>>>>>  bug: https://bugs.openjdk.java.net/browse/JDK-8040291
>>>>>>>  webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00
>>>>>>> 
>>>>>>> 
>>>>>>>  The SunToolkit.prepareResolutionVariant() method wraps the base image observer and
>>>>>>>  passes it to the resolution variant. It leads that the resolution variant notifies
>>>>>>>  the base image about info changing.
>>>>>>> 
>>>>>>>  When the base image is loaded by the MediaTracker and the resolution variant is loaded
>>>>>>>  first it calls the base image observer and wrongly finishes the base image loading.
>>>>>>> 
>>>>>>> 
>>>>>>>  The fix passes the reduced resolution variant info flags to the base image
>>>>>>>  so the base image loading is finished only after notifiing by the original base image observer.
>>>>>>> 
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Alexandr.
>>>>>>> 
>> 
>