Hi,
I'm getting back to this issue. Please see my comments inline:
On 05/09/2013 02:23 PM, Jose Luis Martin wrote:
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().
Please note that in JDK we use jtreg as a test harness, not JUnit. See
jdk/test/java/awt/ for examples of such tests.
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?
As I explained below, the doDispose() calls methods that may be
overridden in user applications (e.g. Window.hide()). If you stop
calling this method under certain conditions, you may break those apps
for no good reason.
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);
}
The spec for WINDOW_CLOSED is very vague and reads as:
The window closed event. This event is delivered after the window has been
closed as the result of a call to dispose.
The current implementation is such that the event gets sent
unconditionally upon every call to dispose(). This doesn't make sense if
the window has already been disposed previously, or has never allocated
its native resources (i.e. the peer) actually.
Is this the problem we're trying to resolve?
If so, then the event must only be sent if the removeNotify() call in
doDispose() actually destroys the peer. In other words, if before the
call the window has been displayable, and after the call it's no longer
displayable, then the event must be sent. It shouldn't be sent under
other circumstances.
Do you see a problem with this solution?
What are the advantages of your proposal?
Why do you think we should send the event in case hasBeenNeverShown ==
true? What would the event mean? Also, please note that
hasBeenNeverShown is not equal to "native resources have never been
created", because in fact an app may call window.addNotify() directly
(which is odd and incorrect, but oh, well..) and this will trigger the
creation of a native peer. In this case the WINDOW_CLOSED event must be
sent by the dispose() method. Do you agree?
--
best regards,
Anthony
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
