<AWT Dev> [8] Request for review: java/awt/Focus/TypeAhead/TestFocusFreeze.java hangs with jdk8 since b56

[Anton V. Tarasov]

Thanks for the review, Anthony!

Anton.

On 11.06.2013 18:59, Anthony Petrov wrote:
> On 06/11/2013 06:55 PM, Anton V. Tarasov wrote:
>>> On a side note (and I'm not suggesting to rewrite the machinery,
>>> really, I'm just curious), wouldn't it be simpler to maintain separate
>>> key-event queues per window, and then per component, rather than have
>>> two global enqueuedKeyEvents and typeAheadMarkers queues in the KFM?
>>> Why would we want to have key events belonging to different top-level
>>> windows in a single queue? What are the benefits of this architecture?
>>
>> When focus is requested to a component a marker for that component is
>> set. However, the request may fail asynchronously (by a number of
>> reasons), and in that case the marker will be deleted. Currently,
>> deletion of the marker doesn't affect the type-ahead queue, but with the
>> approach you're asking about, the key events previously targeted to the
>> component and residing in a separate component-tight queue should be
>> moved either out of the queue (and be then dispatched) or to the queue
>> tight to the next marker/component (there may be multiple markers
>> installed).
>>
>> So, all key events are simply kept in a single queue because their
>> targets are not actually constants and may vary.
>
> Oh, I see. It makes sense. Thanks for clarifying that.
>
> -- 
> best regards,
> Anthony
>
>>
>> Thanks,
>> Anton.
>>
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 06/05/2013 03:42 PM, Anton V. Tarasov wrote:
>>>> Hi Anthony,
>>>>
>>>> Thank you for the question. (Sorry for the delay, my laptop has broken).
>>>>
>>>> At least, I've found my fix incorrect. Namely, I was wrong in the
>>>> following statement: "originally it wasn't assumed that key events are
>>>> left in the queue more than a single repost cycle." It happens. When a
>>>> series of TAB keys are pressed each of the key events initiates focus
>>>> transfer. Each focus transfer (request) initiates type-ahead marker
>>>> installation (a marker consists of the time and the component requesting
>>>> focus). All TAB events with greater origin time are put into the
>>>> type-ahead queue (separate from the event queue). When a component gains
>>>> focus, pending key events are pulled from the type-ahead queue and are
>>>> dispatched in order. The appropriate marker gets removed. The difficulty
>>>> with a TAB event is that on its dispatching a focus transfer is
>>>> immediately initiated which causes another marker installation which
>>>> causes all the subsequent TAB events be put back to the type-ahead queue
>>>> until the focus request is satisfied (asynchronously). There's also some
>>>> special stamping of focus requests and markers, so that key events are
>>>> dispatched to right components. In case with TABs, there should
>>>> eventually be correct number of focus transfers matching the number of
>>>> key presses. That was a tough scenario...
>>>>
>>>> So, a toplevel focus switch should be delayed (from DKFM high level
>>>> perspective) until all those key events are dispatched to their targets.
>>>> Taking into account an synchronous focus delivery (which every TAB
>>>> initiates) this may take a number of cycles of reposting. As to your
>>>> question, placing a WINDOW_LOST_FOCUS/WINDOW_GAINED_FOCUS event in the
>>>> event queue according to its time stamp won't help. Because its place is
>>>> just at the end of the queue (after all the FOCUS_LOST/FOCUS_GAINED
>>>> events initiated by the TABs dispatching). Theoretically, we could
>>>> consider placing FOCUS_LOST/FOCUS_GAINED events in the queue so that
>>>> they appear before any possible timed window focus events which came
>>>> later. However, this approach seems to me no less risky than the
>>>> original. I can't see benefits worth switching to it, as principally
>>>> that would do the same disposition of focus events.
>>>>
>>>> Let me suggest the following:
>>>>
>>>> http://cr.openjdk.java.net/~ant/JDK-8015454/webrev.1
>>>>
>>>> The idea is to add a condition that the key events pending dispatching
>>>> are targeted to a component which belongs to the current focused window.
>>>> In that case, delaying focus window switch won't prevent from focusing
>>>> the target component and dispatching the key events.
>>>>
>>>> Also, I've slightly refactored the code. There's no need to walk through
>>>> the whole type-ahead queue, it's enough to check the time of the first
>>>> key event only (if any).
>>>>
>>>> Thanks,
>>>> Anton.
>>>>
>>>>
>>>> On 31.05.2013 17:41, Anthony Petrov wrote:
>>>>> Hi Anton,
>>>>>
>>>>> My knowledge of the focus machinery is limited, but according to your
>>>>> description of the problem, the main issue here is that we re-post
>>>>> window events to the end of the queue as opposed to re-posting them to
>>>>> a specific position in the queue.
>>>>>
>>>>> In other words my question is: can we move the timed window events to
>>>>> a specific position in the event queue according to their timestamp,
>>>>> so that both the key and window events get processed in the same order
>>>>> as they have actually been generated by the native system?
>>>>>
>>>>> -- 
>>>>> best regards,
>>>>> Anthony
>>>>>
>>>>> On 05/30/2013 05:48 PM, Anton V. Tarasov wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please, review the fix:
>>>>>>
>>>>>> jira: https://jbs.oracle.com/bugs/browse/JDK-8015454
>>>>>> webrev: http://cr.openjdk.java.net/~ant/JDK-8015454/webrev.0
>>>>>>
>>>>>> It's a regression of 6981400 "Tabbing between textfield do not work
>>>>>> properly when ALT+TAB".
>>>>>>
>>>>>> That fix resolved the following case: in a frame with several
>>>>>> components, one pressed TAB multiple times.
>>>>>> This should have transferred focus to the N'th component. But, to make
>>>>>> things complicated, one switched
>>>>>> back and forth active windows with ALT-TAB in the middle. Namely, when
>>>>>> the first TAB gets dispatched
>>>>>> (this is possible when if the app performs lengthy tasks on EDT). As a
>>>>>> result, the appropriate focus window
>>>>>> events were dispatched before the key events and the key events were
>>>>>> dispatched improperly.
>>>>>>
>>>>>> In order to solve it, there were created a timed window event. When a
>>>>>> timed focus window event is dispatched
>>>>>> by DKFM, it inspects the type-ahead queue and if it contains a key
>>>>>> event
>>>>>> with a time stamp less than the window
>>>>>> event's time, the window event gets reposted to the end of the queue.
>>>>>> For more details, please refer to the CR.
>>>>>>
>>>>>> One aspect wasn't taken into account. A key event, waiting in the
>>>>>> type-ahead queue, may depend on the
>>>>>> delivery of the focus window event which gets reposted. So, quite the
>>>>>> contrary, it should be dispatched after
>>>>>> the focus window event is delivered to its target. This may happen
>>>>>> when
>>>>>> a key event is generated after a type-ahead
>>>>>> marker is added (as a result of, say, a window switch), but before the
>>>>>> targeted component (which may reside in another
>>>>>> window) gets focused. In such situation, the window focus events
>>>>>> may get
>>>>>> reposted infinitely waiting for the key
>>>>>> events, because the latter will wait for the delivery of the focus
>>>>>> window event. So, a kind of a deadlock...
>>>>>>
>>>>>> How to solve it? Somehow we could analyze that the waiting key events
>>>>>> depend on the delivery of the focus
>>>>>> window event. However, originally it wasn't assumed that key events
>>>>>> are
>>>>>> left in the queue more than a single repost
>>>>>> cycle. If they do, it just means the dependency. So, I suggest simply
>>>>>> bound the number of reposts by one.
>>>>>>
>>>>>> Additionally, I've fixed the test. The key event time stamp should not
>>>>>> be equal to the type-ahead marker time stamp.
>>>>>> Sometimes they appear to match on Windows. I've put a 1ms delay.
>>>>>>
>>>>>> The fix also fixes the following CRs: JDK-8015584
>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8015584>, JDK-8015450
>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8015450>, JDK-8015446
>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8015446>.
>>>>>>
>>>>>> Thanks,
>>>>>> Anton.
>>>>
>>