<AWT Dev> [10] Review request for 8155197: Focus transition issue

[Alexey Ivanov]

Hi Semyon and Dmitry,

On 28/09/2017 18:34, Semyon Sadetsky wrote:
> Hi Dmitry and Alexey,
>
> On 9/28/2017 8:14 AM, Dmitry Markov wrote:
>> I have updated the regression test for the fix. The new version is 
>> located at http://cr.openjdk.java.net/~dmarkov/8155197/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Edmarkov/8155197/webrev.01/>
>> Now the test is failed on the build without fix and passed on the 
>> build with the fix. Tested on Mac and Windows.
>>
>> Many thanks to Alexey for his help with testing and valuable advice 
>> regarding test modifications.
> Now I can reproduce the issue. Thanks Alexey!

Both of you are very welcome! ;)

>>
>> Semyon,
>> I performed the test, you requested, (i.e. replaced the code in 
>> LWComponentPeer and ran the regression test on jdk10 build w/o the 
>> fix). Both versions of the regression test were failed. It looks like 
>> that result is quite expected, isn’t it?
> Could be. I have my Mac env not ready yet and couldn't run your test 
> there. So I followed the root cause you've suggested but it doesn't 
> seem right.
>
> Actually the parent frame doesn't own the input focus when the issue 
> happens. The focus is on the dialog already and when FOCUS_GAINED 
> event comes for the dialog the KFM discovers that the dialog should 
> not have the focus and rejects it in line 588 of the 
> DefaultKeyboardFocusManager:
>
>                             restoreFocus(fe, newFocusedWindow);
>
> This happens when the dialog became non-focusable (non-visible) after 
> the focus request is sent, but before the corresponding FOCUS_GAINED 
> event is handled on the EDT. In this case the focus is directly 
> restored to the previously focused window which doesn't have the focus 
> at this moment and input focus cannot be requested to one of its 
> components synchronously.
> Please confirm whether you agree on the root cause of the bug.

Let me describe what I observed when I worked on this bug.

The test case was like this: Click the button, it transfers focus to the 
first text field (out of three) and then shows a quick progress dialog. 
The focus moves to the third text field.

I modified the test case so that button-click only moves focus to the 
text field. Once the text field receives FOCUS_GAINED event, it shows 
the progress dialog. I would have expected that it worked correctly. But 
the result was the same: the focus moved to the third text field.

Dmitry took over and completed the evaluation along with providing the fix.


Regards,
Alexey

>
> --Semyon
>>
>> Thanks,
>> Dmitry
>>> On 28 Sep 2017, at 04:02, Semyon Sadetsky 
>>> <semyon.sadetsky at oracle.com> wrote:
>>>
>>> Hi Alexey & Dmitry,
>>>
>>> The bug may be a Mac specific issue.
>>> Can you try to replace in the LWComponentPeer::requestFocus
>>>
>>> 960                boolean res = parentPeer.requestWindowFocus(cause);
>>> with
>>>                 KeyboardFocusManagerPeer kfmPeer = 
>>> LWKeyboardFocusManagerPeer.getInstance();
>>>                 Component focusOwner = 
>>> kfmPeer.getCurrentFocusedWindow();
>>>                 boolean res = parentPeer.getTarget() == focusOwner 
>>> || parentPeer.requestWindowFocus(cause);
>>>
>>> and run the test without the current fix on jdk10?
>>>
>>> --Semyon
>>>
>>> On 9/27/2017 1:45 PM, Alexey Ivanov wrote:
>>>> Hi Semyon,
>>>>
>>>> You're right: the test does not fail for me too on Windows 10 using 
>>>> jdk10. At the same time, I can reproduce the problem using 
>>>> focus8Test.jar attached to the bug [1]. However, it's harder to 
>>>> reproduce the issue using jdk10 or jdk9. It takes more attempts 
>>>> than with jdk8.
>>>>
>>>>
>>>> I could reproduce the problem on Windows 7 and Windows 10.
>>>>
>>>> However, if I switch the theme to Windows Basic or Windows Classic 
>>>> in Windows 7, the issue cannot be reproduced any more. It looks as 
>>>> if it's related to window animation when a new dialog appears and 
>>>> disappears. If the dialog appears on the screen for a longer while, 
>>>> the test does not fail either.
>>>>
>>>>
>>>> Regards,
>>>> Alexey
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8155197
>>>>
>>>> On 26/09/2017 23:46, Semyon Sadetsky wrote:
>>>>> Hi Dmitry,
>>>>>
>>>>> On 9/26/2017 1:56 AM, Dmitry Markov wrote:
>>>>>> Hi Semyon,
>>>>>>
>>>>>> Please find my answer inline.
>>>>>>
>>>>>> Thanks,
>>>>>> Dmitry
>>>>>>> On 25 Sep 2017, at 22:26, Semyon Sadetsky<semyon.sadetsky at oracle.com>  wrote:
>>>>>>>
>>>>>>> Hi Dmitry,
>>>>>>>
>>>>>>>
>>>>>>> On 09/25/2017 01:09 PM, Dmitry Markov wrote:
>>>>>>>> Hi Semyon,
>>>>>>>>
>>>>>>>> This issue and the problem addressed by 8139218 and 8159432 are slightly different. This one is still reproducible on latest 9 and 10 builds using the test case attached to the bug or regression test provided with the fix.
>>>>>>> I couldn't get the test failed on 9 and 10. But I only tried it on Ubuntu. Is the issue a platform dependent?
>>>>>> I was able to reproduce the issue on Windows and Mac OSX. I didn’t test Linux, but I guess it is present there too.
>>>>>> Note: if you use VirtualBoxVM, it is necessary to enable 3D acceleration to reproduce the issue.
>>>>> Not sure that 3D may change focus behavior. But I did not uses 
>>>>> physical Ubuntu to test.
>>>>> I also couldn't get the test failed on Windows platform after 10 
>>>>> attempts:
>>>>>
>>>>> ssadetsk at SSADETSK-LAP1 /data/projects/client10/jdk
>>>>> $ for ((n=0;n<10;n++)); do `/data/jtreg/bin/jtreg 
>>>>> -jdk:../build/windows-x86_64-normal-server-fastdebug/jdk -nr 
>>>>> test/java/awt/Focus/FocusTransitionTest/FocusTransitionTest.java`; 
>>>>> done
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>> Test: extra argument ‘passed:’
>>>>>
>>>>>>>> The problem takes place when we restore focus to a component and its parent window owns the focus. In this case we invoke doRestoreFocus(), (i.e. restore focus synchronously). If the parent window loses the focus during this invocation, focus request will fail and focus target will be shifted to next in focus traversal cycle. This case is not covered by the changes introduced by 8139218 and 8159432.
>>>>>>> I see. What may prevent to set the input focus synchronously? Since the native window already has the focus this should be very deterministic.
>>>>>> The native window loses focus if another window or dialog is displayed. Proposed fix is intended for the following case:
>>>>>> The native window owns focus and we set the focus to one of its child components synchronously, (i.e. invoke doRestoreFocus() -> requestFocus()). At the same time another window/dialog pops up, requestFocus() fails because the native parent window is not the focus owner any more and we shift focus target to next component.
>>>>> The requestFocus() calls the requestFocusHelper() which has several outcomes that returns "false". Which one of them should be involved?
>>>>>
>>>>> --Semyon
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20171002/894cc810/attachment-0001.html>