2009/3/24 Artem Ananiev <[email protected]>: > > 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:
The fix looks fine for me. (It is just a little surprise that someone still waits my reviews for AWT fixes ;) Oleg. > > 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 >>>>>> >
