Hi,
I update the Test and include two new TestCases:
- Test if a Window that has been never shown receive the WINDOW_CLOSED
event.
- Test if a child window that never has been shown receive the
WINDOW_CLOSED event on parent dispose().
With the patch applied the second test fails and the doDispose() method
in the child Window is never called so I suppose that the patch is
wrong.
Looking at code I don't find any reason to call doDispose() on already
disposed window or a in a Window that never has been shown. Is there?
If not, we can simply test if window is displayable on doDispose() but
then the following code will not fire a WINDOW_CLOSED event.
new JFrame().dispose();
If it's not acceptable, I think that the fix should:
- Fire the event and run the dispose action on never shown Windows.
- Run the dispose action but not fire the event when re-disposed.
Something like:
doDispose() {
boolean fireClosedEvent = isDisplayable() || hasBeenNeverShown;
...
if (fireClosedEvent)
postWindowEvent(WindowEvent.WINDOW_CLOSED);
}
As Anthony suggested.
Best Regards,
-- Jose Luis Martin
El mié, 08-05-2013 a las 16:29 +0400, Anthony Petrov escribió:
> Hi,
>
> This indeed looks like a bug in AWT. I'm BCC'ing swing-dev@, and CC'ing
> awt-dev@ instead (please subscribe to this mailing list before posting
> to it).
>
> Here's a question: why do we consider this issue affects child windows
> only? If you call dispose() on a regular window several times in a row,
> you'll receive that many WINDOW_CLOSED events. Isn't it the real bug
> here actually?
>
> Note that simply guarding a call to dispose()/disposeImpl() with a check
> is not the best option because it changes the behavior. If user code
> overrides certain methods, they may be stopped being called after your
> fix, and this may break that code.
>
> Since the DisposeAction is executed synchronously anyway, perhaps it
> should set a flag based on which the doDispose() method would decide to
> send this event or not. What do you think? Also, have you run existing
> jtreg tests (in jdk/test/java/awt/) to verify that this fix doesn't
> introduce a regression?
>
> PS. I can't find the bug filed by you in our bug database. I'll file a
> new one once we review your fix on this mailing list.
>
> --
> best regards,
> Anthony
>
> On 05/06/2013 12:28 PM, Jose Luis Martin wrote:
> > Hello,
> >
> > I reported this issue one year ago but seems that it has been removed
> > from the public bug database and is still not fixed so I try now to
> > speak directly with you to see if this really is a bug or not.
> >
> > The problem that I see is that a child window already disposed is
> > re-disposed when the owner frame is disposed, multiplying the
> > WINDOW_CLOSED events.
> >
> > I guess that it could be fixed by testing if the child window is
> > displayable before disposing it in the Window dispose action. (May be
> > better in the dispose method itself).
> >
> > I attach patch over last jdk8 repository and a test case for the issue.
> >
> > Thanks,
> >
> > Best Regards.
> >
> >
> > -- Jose Luis Martin
> >
package info.joseluismartin.ui;
import java.awt.Toolkit;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import javax.swing.JDialog;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;
import junit.framework.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.BlockJUnit4ClassRunner;
/**
* Test that JDialogs multiplies the WINDOW_CLOSED event.
*
* @author Jose Luis Martin.
*/
@RunWith(BlockJUnit4ClassRunner.class)
public class DialogTest {
private static int N_LOOPS = 10;
private static int N_DIALOGS = 2;
/**
* Test WINDOW_CLOSED event received by a dialog
* that have a owner window.
* @throws Exception
*/
@Test
public void TesWithFrame() throws Exception {
System.out.println("Run with owner Frame");
doTest(true);
}
/**
* Test WINDOW_CLOSED event received by a dialog
* that don't have a owner window.
* @throws Exception
*/
@Test
public void testWithoutFrame() throws Exception {
System.out.println("Run without owner Frame");
doTest(false);
}
/**
* Test if a dialog that has never been shown fire
* the WINDOW_CLOSED event on parent dispose().
* @throws Exception
*/
@Test
public void testHidenChildDispose() throws Exception {
JFrame f = new JFrame();
JDialog dlg = new JDialog(f);
Listener l = new Listener();
dlg.addWindowListener(l);
f.dispose();
waitEvents();
Assert.assertEquals(1, l.getCount());
}
/**
* Test if a Window that has never been shown fire the
* WINDOW_CLOSED event on dispose()
*/
@Test
public void testHidenWindowDispose() throws Exception {
JFrame f = new JFrame();
Listener l = new Listener();
f.addWindowListener(l);
f.dispose();
waitEvents();
Assert.assertEquals(1, l.getCount());
}
private void doTest(final boolean useFrame) throws InterruptedException {
final Listener l = new Listener();
final JFrame f = new JFrame();
for (int i = 0; i < N_LOOPS; i++) {
SwingUtilities.invokeLater(new Runnable() {
public void run() {
JDialog[] dialogs = new JDialog[N_DIALOGS];
for (int i = 0; i < N_DIALOGS; i++) {
if (useFrame) {
dialogs[i]= new JDialog(f);
}
else {
dialogs[i] = new JDialog();
}
dialogs[i].addWindowListener(l);
}
// Dipose all
for (JDialog d : dialogs)
d.dispose();
f.dispose();
}
});
}
waitEvents();
System.out.println("Expected events: " + N_DIALOGS * N_LOOPS);
System.out.println("Received events: " + l.getCount());
Assert.assertEquals(N_DIALOGS * N_LOOPS, l.getCount());
}
private void waitEvents() throws InterruptedException {
// Wait until events are dispatched
while (Toolkit.getDefaultToolkit().getSystemEventQueue().peekEvent() != null)
Thread.sleep(100);
}
}
class Listener extends WindowAdapter {
private volatile int count = 0;
public void windowClosed(WindowEvent e) {
count++;
}
public int getCount() {
return count;
}
public void setCount(int count) {
this.count = count;
}
}
