<AWT Dev> [7] Review Request for 8047961: [TESTBUG] closed/java/awt/Focus/ActivateFocusTest/ActivateFocusTest.html fails intermittently

[Oleg Sukhodolsky]

On Mon, Nov 17, 2014 at 4:34 PM, Anton V. Tarasov
<anton.tarasov at oracle.com> wrote:
> On 17.11.2014 15:01, Oleg Sukhodolsky wrote:
>>
>> Hi Anton,
>>
>> the bug was a regression introduced in 1.4 (comparing with 1.3.1) this
>> is why it was fixed and the test was written.
>> Indeed the spec doesn't guarantee that the test will work but at the
>> time we were working on the ticket it was decided that we should not
>> allow such regressions.
>> If (according to the current policy) the spec is more important than
>> compatibility in this case I'd suggest to remove the test completely
>> since its new version doesn't test for the regression we had earlier
>> but just test spec in a very strange/complicated way.
>
>
> The test passes on Windows, but fails on Mac and I suppose Linux. I recall I
> made attemps to fix the failure on Linux (in 1.6 or 1.7) but that was quite
> complicated to make something reliable, due to the fact that in XToolkit
> even native focus is asynchronous, like in CToolkit. So, from my point of
> view hacking it doesn't worth the candles, taking into account all I've said
> below.
>
> I still like the idea of  replacing requestFocus with requestFocusInWindow.
> We can add comments saying that the original test was modified to avoid the
> endless loop. It didn't actually test for the endless loop, as otherwise it
> would have had a timer. It tested for focus gains count per window, that
> remains relevant even in its new form. That's my personal opinion, I won't
> insist on keeping it if you both vote for the opposite...

if you do not plan to fix the problem the test reproduces then I'd
vote for removing the test (you have enough tests in regression tests
suite ;)

Regards, Oleg.
>
> Thanks,
> Anton.
>
>
>>
>> So, it is you whole should make the final decision but I just do not
>> see a reason to keep a test which doesn't test for the particular
>> regression in regression tests suite.
>>
>> Regards, Oleg.
>>
>> On Mon, Nov 17, 2014 at 2:01 PM, Anton V. Tarasov
>> <anton.tarasov at oracle.com> wrote:
>>>
>>> Hi Oleg,
>>>
>>> Glad to hear from you :)
>>>
>>> On 14.11.2014 18:24, Oleg Sukhodolsky wrote:
>>>>
>>>> Sorry to interrupt you but since I do know the test let me say that
>>>> requestFocus() is an important part of the test,
>>>> if you are going to replace it with requestFocusInWindow() you
>>>> (effectively) remove it from list of regression tests.
>>>> Please check 4369903 bug for more information.
>>>
>>>
>>> In the bug I see the description of why the infinite loop happens and a
>>> suggestion to use requestFocusInWindow to prevent it. From my point of
>>> view,
>>> the behavior is expected taking into account the asynchronous nature of
>>> java
>>> focus.
>>>
>>> By default, when a toplevel window gains activation, KeyboardFocusManager
>>> requests focus in it by means of calling the requestFocusInWindow method.
>>> Thus, the test case creates an artificial situation with unclear goals.
>>> It
>>> doesn't seem to reflect a real use case (I can see the bug was filed by
>>> an
>>> internal engineer, not refering to any customer or user application). In
>>> case a developer wants to alter the component receiving focus at the
>>> toplevels's activation, he/she can achieve this by means of customizing
>>> the
>>> focus traversal policy, for instance.
>>>
>>> The javadoc [1] for the JComponent.requestFocus method warns developers
>>> about its usage:
>>>
>>> <<Note that the use of this method is discouraged because its behavior is
>>> platform dependent. Instead we recommend the use of
>>> requestFocusInWindow().>>
>>>
>>> What I'm trying to say is that we'd rather avoid hacking the focus
>>> subsystem
>>> in order to just solve the infinite loop problem.
>>>
>>> However, originally the reporter of the bug complains about the
>>> following:
>>>
>>> <<Setting focus during window activation often sends focus to the wrong
>>> place, or to multiple places.>>
>>>
>>> This behavior is incorrect, indeed. But we can verify it with the test
>>> case.
>>> When a component gains focus we can check that:
>>>
>>> 1. it is the current focus owner reported by KFM, and the current focused
>>> window is its container
>>> 2. the opposite component has lost focus prior to that (that is, every
>>> FOCUS_GAINED precedes a FOCUS_LOST received by the opposite component)
>>> 3. the same about the WINDOW_LOST_FOCUS/WINDOW_GAINED_FOCUS pair
>>>
>>> The test should be ready for the infinite loop and I can suggest to
>>> simply
>>> stop requesting focus after a couple of iterations.
>>>
>>> This could be a compromize. What do you think, Oleg, Mikhail?
>>>
>>> Thanks,
>>> Anton.
>>>
>>> [1]
>>>
>>> https://docs.oracle.com/javase/8/docs/api/javax/swing/JComponent.html#requestFocus--
>>>
>>>
>>>> Regards, Oleg.
>>>>
>>>> P.S. feel free to ask more questions if you need.
>>>>
>>>> On Fri, Nov 14, 2014 at 5:07 PM, Anton V. Tarasov
>>>> <anton.tarasov at oracle.com> wrote:
>>>>>
>>>>> Hi Mikhail,
>>>>>
>>>>> Looks fine for me. Thanks! This was an old one... Do you have any plans
>>>>> to
>>>>> fix it in 8/9 as well?
>>>>>
>>>>> Regards,
>>>>> Anton.
>>>>>
>>>>>
>>>>> On 14.11.2014 18:57, mikhail cherkasov wrote:
>>>>>>
>>>>>> Hello all,
>>>>>>
>>>>>> Could you please review a simple fix of closed test, the test was
>>>>>> moved
>>>>>> to
>>>>>> open repo
>>>>>> and requestFocus was replaced requestFocusInWindow.
>>>>>>
>>>>>> Because "requestFocus" cause infinite war for focus between two
>>>>>> windows,
>>>>>> however this
>>>>>> behavior is correct and doesn't violet spec.
>>>>>>
>>>>>> [TESTBUG]
>>>>>> closed/java/awt/Focus/ActivateFocusTest/ActivateFocusTest.html
>>>>>> fails intermittently
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8047961
>>>>>> Webrev: http://cr.openjdk.java.net/~mcherkas/8047961/webrev.00/
>>>>>>
>>>>>> Thanks,
>>>>>> Mikhail.
>>>>>>
>