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

[Alexander Zvegintsev]

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