Hi Semyon, > On 28 Sep 2017, at 18:34, Semyon Sadetsky <semyon.sadet...@oracle.com> 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! >> >> 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. > You are right. I agree with your evaluation.
Thanks, Dmitry > --Semyon >> >> Thanks, >> Dmitry >>> On 28 Sep 2017, at 04:02, Semyon Sadetsky < >>> <mailto:semyon.sadet...@oracle.com>semyon.sadet...@oracle.com >>> <mailto:semyon.sadet...@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 >>>> <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.sadet...@oracle.com> >>>>>>> <mailto:semyon.sadet...@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@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 >>>>> >>>> >>> >> >