<AWT Dev> [9] Review request for 8046495: KeyEvent can not be accepted in quick mouse clicking

anton nashatyrev anton.nashatyrev at oracle.com
Wed Jul 2 15:28:58 UTC 2014

Hello, Anton

On 02.07.2014 18:13, Anton V. Tarasov wrote:
> On 02.07.2014 11:44, Petr Pchelko wrote:
>> Hello, Anton.
>> I'm not sure I have a detailed understanding of what's happening.
>> Before your fix the timestamp of the event represented the time when 
>> the event was created, and now it's the time when it's sent to java.
>> This might be important if some events cause other events to be 
>> issued on the java side.
>> So suppose the following:
>> Toolkit thread: InputEvent#1 created      - timestamp 0
>> Toolkit thread: InputEvent#2 created      - timestamp 1
>> Toolkit thread: InputEvent#1 sent           - timestamp 2
>> EDT:             InputEvent#1 dispatched - timestamp 3
>> EDT:               FocusEvent  created        - timestamp 4
>> Toolkit thread: InputEvent#2 sent           - timestamp 5
>> Before you fix we had the following event order: InputEvent#1(ts=0), 
>> InputEvent#2(ts=1), FocusEvent(ts=4).
>> But after your fix we will have a different order: 
>> InputEvent#1(ts=2), FocusEvent(ts=4), InputEvent#2(ts=5).
>> So now we would have troubles, because the Input Event will go to the 
>> new focused component instead of the old one.
>> Do you think that my arguments are correct? I understand that the 
>> likelihood of such a situation is very low, but for me it looks 
>> possible? Am I missing something?
> A timestamp for a FocusEvent is taken from 
> the_most_recent_KeyEvent_time which is set on EDT when the event is 
> dispatched. So the fix shouldn't affect it.
> Also, in awt_Window.cpp, when a TimedWindowEvent is created it is 
> passed a timestamp got with TimeHelper::getMessageTimeUTC(). It 
> appears, that the fix makes key events times even more consistent with 
> it. So, from the kbd focus perspective, it shouldn't make any harm.
> Anton, could you just please run the following tests, at least:
> test/java/awt/Focus/6981400/*
> test/java/awt/KeyboardFocusManager/TypeAhead/*

I've tested with the following set:

: no unexpected failures here.

I've also verified that my regression test which comes with the fix 
works fine on Linux and Mac (despite the fix is Win specific).

Thanks for review!

> Thanks,
> Anton.
>> Another thing I do not understand is why we were used to use the 
>> complicated formula instead of initializing the msg.time field with 
>> the JVM current time and using it when sending the event?
>> Wouldn't that resolve both your issue and the issue the original fix 
>> was made for?
>> I have a couple of comments about the code, but let's postpone that 
>> until we decide on the approach.
>> Thank you.
>> With best regards. Petr.
>> On 01 июля 2014 г., at 21:20, anton nashatyrev 
>> <anton.nashatyrev at oracle.com <mailto:anton.nashatyrev at oracle.com>> wrote:
>>> Hello,
>>>     could you please review the following fix:
>>> fix: http://cr.openjdk.java.net/~anashaty/8046495/9/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Eanashaty/8046495/9/webrev.00/>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8046495
>>> *Problem:*
>>> On Windows the later InputEvent may have earlier timestamp 
>>> (getWhen()), which results in incorrect processing of enqueued input 
>>> events and may also potentially be the reason of other artifacts
>>> *Evaluation*:
>>> On Windows we have some algorithm for calculating input event 
>>> timestamp: jdk/src/windows/native/sun/windows/awt_Component.cpp:2100
>>> Shortly the timestamp is calculated by the following formula:
>>>     when = JVM_CurrentTimeMillis() - (GetTickCount() - GetMessageTime())
>>> Where:
>>>   JVM_CurrentTimeMillis() - the same as System.currentTimeMillis()
>>>   GetTickCount() - Win32 function, current millis from boot time
>>>   GetMessageTime() - Win32 function, millis from boot time of the 
>>> latest native Message
>>> In theory the formula looks good: we are trying our best to 
>>> calculate the actual time of the OS message by taking the current 
>>> JVM time (JVM_CurrentTimeMillis) and adjusting it with the offset 
>>> (GetTickCount - GetMessageTime) which should indicate how many 
>>> milliseconds ago from the current moment (GetTickCount) the message 
>>> has been actually issued (GetMessageTime).
>>> In practice due to usage of different system timers by the 
>>> JVM_CurrentTimeMillis and GetTickCount their values are not updated 
>>> synchronously and we may get an earlier timestamp for the later event.
>>> *Fix*:
>>> Just to use JVM_CurrentTimeMillis only as events timestamp. On Mac 
>>> this is done in exactly the same way: 
>>> CPlatformResponder.handleMouse/KeyEvent()
>>> The message time offset calculation has been introduced with the fix 
>>> for JDK-4434193 <https://bugs.openjdk.java.net/browse/JDK-4434193> 
>>> and it seems that the issue has addressed very similar problem (At 
>>> times getWhen()in ActionEvent returns higher value than the 
>>> CurrentSystemTime) which has not been completely resolved in fact.
>>> I also didn't find any benefits in using the existing approach:
>>> - all the usages of the TimerHelper are in fact reduced to the 
>>> mentioned formula: when = JVM_CurrentTimeMillis() - (GetTickCount() 
>>> - GetMessageTime())
>>> - GetMessageTime() always increases (except of the int overflow 
>>> moments), thus we couldn't get earlier OS messages after later ones
>>> - TimerHelper::windowsToUTC(DWORD windowsTime) doesn't guarantee 
>>> returning the same timestamp across different calls for the same 
>>> message time
>>> Thanks!
>>> Anton.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20140702/9fb1e1a4/attachment-0001.html>

More information about the awt-dev mailing list