<AWT Dev> [PATCH]: Fix modal dialog (was Problem with modal Dialog)

[Artem Ananiev]

Roman Kennke wrote:
> Hi Artem,
> 
>> Looks fine.
> 
> Great. Do we have a bug report that I can reference in my commit
> message? And do I need another review before committing?

I filed a bug some time ago:

6809233: Modal dialog blocks calling thread after it is hidden and disposed

I hope Oleg will look at your patch soon, and this will be enough to 
commit the change. For all the subsequent bugs, new OpenJDK tools:

http://bugs.openjdk.java.net

and

https://cr.openjdk.java.net

should be used.

Thanks,

Artem

> /Roman
> 
>> Thanks,
>>
>> Artem
>>
>> Roman Kennke wrote:
>>> So this is the patch I propose to fix this modal dialog problem.
>>>
>>> /Roman
>>>
>>> Am Donnerstag, den 19.03.2009, 12:27 +0300 schrieb Artem Ananiev:
>>>> Roman Kennke wrote:
>>>>> Hi there,
>>>>>
>>>>>>>>>>>>>> BTW, simply sending this over the EQ is no solution either, because
>>>>>>>>>>>>>> then
>>>>>>>>>>>>>> later it will fail to invokeAndWait(). I will think a little more
>>>>>>>>>>>>>> about
>>>>>>>>>>>>>> this, or maybe anybody has a quick idea?
>>>>>>>>>>>>> Just an idea (have not evaluated it carefully):
>>>>>>>>>>>>> perhaps we should set keepBlockingEDT to false not in
>>>>>>>>>>>>> hideAndDisposeHandler(),
>>>>>>>>>>>>> but in WakingRunnable.run() instead.
>>>>>>>>>>>> I'm looking at this problem at the moment. The problem is the instance
>>>>>>>>>>>> of WakingRunnable is not run on EDT at all - I don't know why.
>>>>>>>>>>> I know why. It is because the DisposeAction is run _immediately_, before
>>>>>>>>>>> the WakingRunnable had a chance. This DisposeAction calls removeNotify(),
>>>>>>>>>>> which leads to all events on the EQ that are related to the Dialog beeing
>>>>>>>>>>> discarded.
>>>>>>>>>> Could you, please, point to the place where all the events are discarded?
>>>>>>>>>> I don't see any.
>>>>>>>>> In Component.removeNotify(), we call this:
>>>>>>>>>
>>>>>>>>> Toolkit.getEventQueue().removeSourceEvents(this, false);
>>>>>>>> OK, I see. What a wonderful code...
>>>>>>>>
>>>>>>>>> which removes all events related to the component. removeNotify() is
>>>>>>>>> called from inside the DisposeAction.
>>>>>>>> For this particular case it's enough to add some additional check to
>>>>>>>> hideAndDisposeHandler:
>>>>>>>>
>>>>>>>>         if (showAppContext != curAppContext) {
>>>>>>>>             // Wake up event dispatch thread on which the dialog was
>>>>>>>>             // initially shown
>>>>>>>>             SunToolkit.postEvent(showAppContext, wakingEvent);
>>>>>>>>             showAppContext = null;
>>>>>>>> +        } else if (EventQueue.isDispatchThread()) {
>>>>>>>> +            waking.run();
>>>>>>>>         } else {
>>>>>>>>             Toolkit.getEventQueue().postEvent(wakingEvent);
>>>>>>>>         }
>>>>>>> this change will fix this particular test, but the same problem may
>>>>>>> exists even when
>>>>>>> we call hide() not on EDT, it is just a little bit harder to write
>>>>>>> test for this ;)
>>>>>>>
>>>>>>> What about moving resetting keepBlockingEDT to WakingEvent.run(), or,
>>>>>>> perhaps, we can simply change
>>>>>>> target of WakingEvent from a dialog to toolkit.
>>>>>>> What do you think?
>>>>>> Setting the target of the WakingEvent to the toolkit sounds like an 
>>>>>> elegant and efficient solution to me. Simply executing the event 
>>>>>> directly when on the EDT (as Artem proposed) sounds like crying for more 
>>>>>> side-effects.
>>>>> So what should we do about it?
>>>> I'm fine with changing event's target to Toolkit. Could you send a patch 
>>>> for review then?
>>>>
>>>> Thanks,
>>>>
>>>> Artem
>>>>
>>>>> /Roman
>>>>>