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

[Anton V. Tarasov]

On 17.11.2014 17:21, Oleg Sukhodolsky wrote:
> 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 ;)

I'm afraid I don't, as I don't think this is a real problem :)

Regards,
Anton.

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