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