The latest webrev, version 06, looks good to me now.
Approved (although I am not a "R"eviewer for AWT).
-- Kevin
Alexander Zvegintsev wrote:
HI Semyon,
Please see my answer to Dmitry
Hi Dmitry,
From my understanding JavaFX stage can't be easily integrated in JDK to support
orderWindow() approach,
addChildWindow() is a native(and the simplest) way to maintain one window above
other one, should be called only once.
IIUC the main concert of JDK-8080729 that child windows jumping to parent's
display upon focus receiving, this is not an issue with current fix,
because addChildWindow() will be called only upon dialog creation in case of
JavaFX-Swing interop.
Jump may happen if user want to create a child swing dialog on display other
than JavaFX stage's one,
but such rare scenario can be easily workarounded on a user side by calling
setLocation() right after setVisible() call.
So I would prefer to use addChildWindow() to make this fix as simple as
possible.
Thanks,
Alexander.
On 08/11/2017 02:45, Semyon Sadetsky wrote:
Hi Alexander,
In CPlatformWindow.java change you used addChildWindow(), but we get
rig of addChildWindow() in 8080729 and start to manage windows order
on java side. Can you test that this change doesn't cause any
ordering issues with modal and non-modal child and sibling windows on
mac?
--Semyon
On 11/07/2017 10:11 AM, Alexander Zvegintsev wrote:
Hi Sergey,
I am not able to crash it on several platforms, except one case:
if we are terminating JavaFX application while EDT processing some
long task. But it is unrelated to the fix and reproducible on
current builds.
I've filed a separate bug JDK-8190329
<https://bugs.openjdk.java.net/browse/JDK-8190329>.
Will the current fix cover the native dialogs like
FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by JDK-8088395
<https://bugs.openjdk.java.net/browse/JDK-8088395>
Test added:
http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/05/
Thanks,
Alexander.
On 13/10/2017 01:14, Sergey Bylokhov wrote:
Looks fine.
I am not sure but it looks like the fix has an assumption that the
CPlatformWindow.setVisible() code will be executed on EDT/Appkit
but it is not the case. This code can be executed on any
thread(intentionally for crash), and it will be good to check that
it works as expected by some stress test which will try to force
the possible crash: 4 threads:
- show/dispose Swing Node
- show/dispose Dialog1/2/3 using different timeouts
Will the current fix cover the native dialogs like
FileDialog/PrintDialog on linux and windows?
On 10/10/2017 13:54, Kevin Rushforth wrote:
Note that there is now a 04 version.
It looks good to me, although someone more familiar with AWT
should also check the AWT changes.
We will need a test program for this (as a follow-on issue if not
now).
-- Kevin
Alexander Zvegintsev wrote:
Please review the updated version
http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/02/
Now we are postponing actual window closing, it happens only
after we ensure that native window pointer is valid on Swing side.
Thanks,
Alexander.
On 23/09/2017 08:01, Sergey Bylokhov wrote:
Hi, Alexander.
How can we be sure that the parent frame will not be disposed
while we use a pointer?
long ownerWindowPtr = peer.getOverridenWindowHandle();
<<<<< Dispose the peer
if (ownerWindowPtr != 0) {
//Place window above JavaFX stage
CWrapper.NSWindow.addChildWindow(
ownerWindowPtr, ptr, CWrapper.NSWindow.NSWindowAbove);
<<<<< Boom
}
On 9/21/17 22:56, Alexander Zvegintsev wrote:
Hi Phil,
Please review the updated fix with reflection incorporated
http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/01/
New issue created JDK-8187803
<https://bugs.openjdk.java.net/browse/JDK-8187803> as JDK
counterpart of this issue.
Thanks,
Alexander.
On 21/09/2017 22:25, Phil Race wrote:
Some procedural comments :
Since 90% of this is in AWT code, I'd have thought awt-dev
should be included here.
I've added that list.
And apart from needing separate bug ids, I don't see why the
bug below is confidential.
I agree with what Kevin pointed out off-line that as in the
dialog case, the FX side
of the code can use reflection and simply be a harmless
non-functional no-op
if the SwingAccessor does not provide the new method.
BTW
264 inline HWND GetOverridenHWnd() { return m_overridenHwnd; }
should be "dd" not "d".
-phil.
On 09/21/2017 03:38 AM, Alexander Zvegintsev wrote:
Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/10/8185634/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8185634