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

[Sergey Bylokhov]

Hi, Dmitry.
If this check is valid then probably the same check should be added to 
restoreFocus()? before:
#173 restoreFocusTo = toFocus;

But as it was mentioned before, the component can become 
non-visible/non-focusable at any point, for example after this check but 
before the assignment. And instead of data validation before assignment 
we should check them before use. As far as I understand we already do 
this, all usages of restoreFocusTo are a checks == or != to some other 
component which is visible and focusable, isn't it?

On 10/3/17 08:23, Dmitry Markov wrote:
> Hi Semyon,
> 
> I have updated the fix based on your suggestion. The new version is 
> located at http://cr.openjdk.java.net/~dmarkov/8155197/webrev.02/
> Also I slightly modified the test to simplify it.
> 
> Thanks,
> Dmitry
>> On 2 Oct 2017, at 18:32, Semyon Sadetsky <semyon.sadetsky at oracle.com 
>> <mailto:semyon.sadetsky at oracle.com>> wrote:
>>
>> Hi Dmitry,
>>
>>>> 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.
>> Before setting the restoreFocusTo to toFocus in line 190 I would 
>> recheck for toFocus.isShowing() && toFocus.canBeFocusOwner() once 
>> again because the component can be made non-focusable concurrently.
>>
>> --Semyon
>>
>>
> 


-- 
Best regards, Sergey.