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

[Semyon Sadetsky]

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