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

[Anton V. Tarasov]

Hi Anthony,

On 11.06.2013 13:35, Anthony Petrov wrote:
> Hi Anton,
>
> Thanks for the explanation. I'm still not sure I fully understand the whole Type-Ahead (and focus, 
> for that matter) thing, but the new fix looks much cleaner and safer to me. As long as all focus 
> tests pass with this fix, I'm fine with it.

Thanks. I've checked only single platform for now, but I'll test it on the rest ones as well.
>
>
>
> 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.

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