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

[Sergey Bylokhov]

Hi, Alexander.
The fix looks good.

On 5/21/14 6:00 PM, Petr Pchelko wrote:
> 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.
>>>>>>>>


-- 
Best regards, Sergey.