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

anton nashatyrev anton.nashatyrev at oracle.com
Thu Jul 24 15:29:50 UTC 2014


Hello,

     Artem, Petr, thanks for the fix discussion and review!

     could you please review the backport to 8u-dev now?

     It is a little bit more conservative variant of the fix to jdk9 and 
was originally proposed for jdk9 as is.

Webrev: http://cr.openjdk.java.net/%7Eanashaty/8046495/8/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8046495

Thank you!
Anton.


On 24.07.2014 18:10, Artem Ananiev wrote:
>
> On 7/18/2014 2:28 PM, anton nashatyrev wrote:
>> Hello,
>>
>>      in offline discussion with Artem and Petr we decided to further
>> clean up the code and completely remove TimeHelper functions from
>> awt_Component.cpp in JDK9.
>>
>>      Could you please review the updated fix:
>> http://cr.openjdk.java.net/~anashaty/8046495/9/webrev.01/
>> <http://cr.openjdk.java.net/%7Eanashaty/8046495/9/webrev.01/>
>
> Looks fine to me.
>
> Thanks,
>
> Artem
>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8046495
>>
>> Thank you!
>> Anton.
>>
>> On 17.07.2014 13:14, anton nashatyrev wrote:
>>> Hello All,
>>>
>>>     could please anyone else take a look at the fix?
>>>
>>> Thanks!
>>> Anton.
>>>
>>> On 02.07.2014 19:37, Petr Pchelko wrote:
>>>> Hello, Anton.
>>>>
>>>> Thanks for clarifications and additional testing.
>>>> The fix looks good to me too.
>>>>
>>>> With best regards. Petr.
>>>>
>>>> On 02 июля 2014 г., at 19:34, Anton V. Tarasov
>>>> <anton.tarasov at oracle.com <mailto:anton.tarasov at oracle.com>> wrote:
>>>>
>>>>> On 02.07.2014 19:28, anton nashatyrev wrote:
>>>>>> 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:
>>>>>> [closed]/java/awt/event/*
>>>>>> [closed]/java/awt/Focus/*
>>>>>> [closed]java/awt/KeyboardFocusManager/*
>>>>>>
>>>>>> : 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).
>>>>>
>>>>> Great, thanks!
>>>>>
>>>>> Anton.
>>>>>
>>>>>>
>>>>>> Thanks for review!
>>>>>> Anton.
>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>



More information about the awt-dev mailing list