<AWT Dev> Code Review Request for CR 7145980 - Dispose method of window.java takes long

Anthony Petrov anthony.petrov at oracle.com
Thu Feb 23 13:54:32 PST 2012


The fix looks fine. Thank you!

--
best regards,
Anthony

On 2/24/2012 1:48 AM, Oleg Pekhovskiy wrote:
> Hi Anthony,
> 
> good idea, I added logging of isDisposing() method there:
> http://cr.openjdk.java.net/~bagiras/7145980.3/
> 
> Thanks,
> Oleg
> 
> 23.02.2012 0:27, Anthony Petrov wrote:
>> Hi Oleg,
>>
>> The fix looks good. I'd also add the isDisposing() value to the logger 
>> output statement in isMixingNeeded() (in addition to the numbers of hw 
>> and lw descendants) to make sure the mixing log provides complete 
>> information about the reason isMixingNeeded() returns false.
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 2/22/2012 8:15 PM, Oleg Pekhovskiy wrote:
>>> Hi Anthony,
>>>
>>> thanks for the review, I've prepared the next version of fix 
>>> according to your proposals:
>>> http://cr.openjdk.java.net/~bagiras/7145980.2/ 
>>> <http://cr.openjdk.java.net/%7Ebagiras/7145980.2/>
>>>
>>> Thanks,
>>> Oleg
>>>
>>> 2/22/2012 5:26 PM, Anthony Petrov wrote:
>>>> Hi Oleg,
>>>>
>>>> The correct link to the webrev should be 
>>>> http://cr.openjdk.java.net/~bagiras/7145980/webrev/
>>>>
>>>> Anyways, I suggest to make isMixingNeeded() return false if the 
>>>> containing window (a reference to which is already obtained in that 
>>>> method) is currently disposing. This way we avoid a double call to 
>>>> getContainingWindow(), and also will prevent any other 
>>>> mixing-related code from execution when the window is being disposed.
>>>>
>>>> In Window.java I suggest to use the following pattern:
>>>>
>>>> disposing = true;
>>>> try {
>>>>    // do stuff
>>>> } finally {
>>>>    disposing = false;
>>>> }
>>>>
>>>> to ensure we don't leave the flag in 'true' state if some exceptions 
>>>> are thrown.
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 2/22/2012 4:57 PM, Oleg Pekhovskiy wrote:
>>>>>   Hi guys,
>>>>>
>>>>> Please review the fix for CR:
>>>>> http://bugs.sun.com/view_bug.do?bug_id=7145980
>>>>>
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~bagiras/7145980/webrev/ 
>>>>> <http://cr.openjdk.java.net/%7Edenis/7143070/webrev.04/>
>>>>>
>>>>> The main idea is to skip recalculation of component visible region 
>>>>> when a top-level window is disposing.
>>>>>
>>>>> Thanks,
>>>>> Oleg
>>>
> 



More information about the awt-dev mailing list