Re: <AWT Dev> [10] Review request for 185634: Java Fx-Swing dialogs appearing behind main stage

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 185634: Java Fx-Swing dialogs appearing behind main stage

Philip Race
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


Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Alexander Zvegintsev

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 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



Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Philip Race
I need to look more closely but at a quick glance I don't see any problems so a tentative +1 ..
but a final +1 will go in the bug.
Have you tested the various configurations .. running this on top of current JDK (not FX) 10
both with and without the FX changes ?

-phil.


On 09/21/2017 10:56 PM, 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 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




Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Sergey Bylokhov
In reply to this post by Alexander Zvegintsev
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
>>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Dmitry Markov
In reply to this post by Alexander Zvegintsev
Hi Alexander,

In CPlatformWindow class you call CWrapper.NSWindow.addChildWindow() to place a window above JavaFX stage. The usage of addChilWindow() or/and removeChildWindow() may cause ‘jumping window issue’ in multi-monitor environment, see JDK-8080729 for more details.

To avoid possible problems in multi monitor set-up I suggest that you should replace addChildWindow() with orderWindow() or use some another approach to locate the window above the stage.

Thanks,
Dmitry

> On 22 Sep 2017, at 06:56, Alexander Zvegintsev <[hidden email]> 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
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Alexander Zvegintsev
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 23/09/2017 21:21, Dmitry Markov wrote:

> Hi Alexander,
>
> In CPlatformWindow class you call CWrapper.NSWindow.addChildWindow() to place a window above JavaFX stage. The usage of addChilWindow() or/and removeChildWindow() may cause ‘jumping window issue’ in multi-monitor environment, see JDK-8080729 for more details.
>
> To avoid possible problems in multi monitor set-up I suggest that you should replace addChildWindow() with orderWindow() or use some another approach to locate the window above the stage.
>
> Thanks,
> Dmitry
>> On 22 Sep 2017, at 06:56, Alexander Zvegintsev <[hidden email]> 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
>>>>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Alexander Zvegintsev
In reply to this post by Sergey Bylokhov
Hi Sergey,

Thanks for catching this, this is an issue which should be addressed.

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
>>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

semyon.sadetsky
In reply to this post by Alexander Zvegintsev

Hi Alexander,

On the Windows platform you've only modified the dialog native peer while on other platforms all window types are modified to use FX window as a parent. Shouldn't the frame native peer  be modified too on the Windows platform?

--Semyon


On 09/21/2017 10:56 PM, 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 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




Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Dmitry Markov
In reply to this post by Alexander Zvegintsev
Hi Alexander,

Sounds reasonable. I am OK with usage of addChildWindow() in this case.

Thanks,
Dmitry

> On 25 Sep 2017, at 13:05, Alexander Zvegintsev <[hidden email]> wrote:
>
> 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 23/09/2017 21:21, Dmitry Markov wrote:
>> Hi Alexander,
>>
>> In CPlatformWindow class you call CWrapper.NSWindow.addChildWindow() to place a window above JavaFX stage. The usage of addChilWindow() or/and removeChildWindow() may cause ‘jumping window issue’ in multi-monitor environment, see JDK-8080729 for more details.
>>
>> To avoid possible problems in multi monitor set-up I suggest that you should replace addChildWindow() with orderWindow() or use some another approach to locate the window above the stage.
>>
>> Thanks,
>> Dmitry
>>> On 22 Sep 2017, at 06:56, Alexander Zvegintsev <[hidden email]> 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
>>>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Alexander Zvegintsev
In reply to this post by Sergey Bylokhov
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
>>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Kevin Rushforth
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
>>>>>
>>>>
>>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Sergey Bylokhov
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
>>>>>>
>>>>>
>>>>
>>>
>>>
>>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Alexander Zvegintsev

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.

Will the current fix cover the native dialogs like FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by 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









Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Sergey Bylokhov
Thank yo for clarification, looks fine.

On 07/11/2017 10:11, 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

semyon.sadetsky
In reply to this post by Alexander Zvegintsev

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.

Will the current fix cover the native dialogs like FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by 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










Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Kevin Rushforth
In reply to this post by Alexander Zvegintsev
FX patch

The new tests run fine for me on all three platforms. I noticed a couple issues while reviewing it:

1. All robot-based tests should be in the test.robot.** package hierarchy, such that they are only enabled with "-PUSE_ROBOT=true" -- I recommend to put them in a new test.robot.javafx.embed.swing package.

2. Can you replace the wild-card imports in the tests with explicit imports?


JDK patch

You need to rebase the patch against the latest tip of jdk/client. Once you do that, you will notice that CPlatformWindow.java no longer compiles on Mac -- you will need to add an explicit import of sun.lwawt.LWLightweightFramePeer.

-- Kevin


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.

Will the current fix cover the native dialogs like FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by 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









Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Alexander Zvegintsev
In reply to this post by semyon.sadetsky

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.

Will the current fix cover the native dialogs like FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by 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











Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

Kevin Rushforth
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.

Will the current fix cover the native dialogs like FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by 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











Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review request for 8185634, 8185634: Java Fx-Swing dialogs appearing behind main stage

semyon.sadetsky
In reply to this post by Alexander Zvegintsev

Looks good.

--Semyon


On 11/07/2017 06:59 PM, 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.

Will the current fix cover the native dialogs like FileDialog/PrintDialog on linux and windows?
It will not, Windows already fixed by 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