<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hello, Anton.<div><br></div><div>Thanks for clarifications and additional testing.</div><div>The fix looks good to me too.</div><div><br></div><div>With best regards. Petr.</div><div><br><div><div>On 02 июля 2014 г., at 19:34, Anton V. Tarasov <<a href="mailto:anton.tarasov@oracle.com">anton.tarasov@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
  
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  
  <div bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 02.07.2014 19:28, anton nashatyrev
      wrote:<br>
    </div>
    <blockquote cite="mid:53B4253A.3040406@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      Hello, Anton<br>
      <br>
      <div class="moz-cite-prefix">On 02.07.2014 18:13, Anton V. Tarasov
        wrote:<br>
      </div>
      <blockquote cite="mid:53B413A7.4000809@oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
        <div class="moz-cite-prefix">On 02.07.2014 11:44, Petr Pchelko
          wrote:<br>
        </div>
        <blockquote cite="mid:B4A1F8A6-25D8-4CA2-A138-95EC14A6A881@oracle.com" type="cite">
          <meta http-equiv="Content-Type" content="text/html;
            charset=UTF-8">
          Hello, Anton.
          <div><br>
          </div>
          <div>I'm not sure I have a detailed understanding of what's
            happening. </div>
          <div><br>
          </div>
          <div>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.</div>
          <div>This might be important if some events cause other events
            to be issued on the java side. </div>
          <div><br>
          </div>
          <div>So suppose the following:</div>
          <div>Toolkit thread: InputEvent#1 created      - timestamp 0</div>
          <div>Toolkit thread: InputEvent#2 created      - timestamp 1</div>
          <div>Toolkit thread: InputEvent#1 sent           - timestamp 2</div>
          <div>EDT:<span class="Apple-tab-span" style="white-space:pre">
            </span>               InputEvent#1 dispatched - timestamp 3</div>
          <div>EDT:               FocusEvent  created        - timestamp
            4</div>
          <div>Toolkit thread: InputEvent#2 sent           - timestamp 5</div>
          <div><br>
          </div>
          <div>Before you fix we had the following event order:
            InputEvent#1(ts=0), InputEvent#2(ts=1), FocusEvent(ts=4).</div>
          <div>But after your fix we will have a different order:
            InputEvent#1(ts=2), FocusEvent(ts=4), InputEvent#2(ts=5).</div>
          <div>So now we would have troubles, because the Input Event
            will go to the new focused component instead of the old
            one. </div>
          <div>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?</div>
        </blockquote>
        <br>
        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.<br>
        <br>
        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.<br>
        <br>
        Anton, could you just please run the following tests, at least:<br>
        <br>
        test/java/awt/Focus/6981400/*<br>
        test/java/awt/KeyboardFocusManager/TypeAhead/*<br>
      </blockquote>
      <br>
      I've tested with the following set: <br>
      [closed]/java/awt/event/* <br>
      [closed]/java/awt/Focus/* <br>
      [closed]java/awt/KeyboardFocusManager/*<br>
      <br>
      : no unexpected failures here.<br>
      <br>
      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).
      <br>
    </blockquote>
    <br>
    Great, thanks!<br>
    <br>
    Anton.<br>
    <br>
    <blockquote cite="mid:53B4253A.3040406@oracle.com" type="cite"> <br>
      Thanks for review!<br>
      Anton.<br>
      <br>
      <blockquote cite="mid:53B413A7.4000809@oracle.com" type="cite"> <br>
        Thanks,<br>
        Anton.<br>
        <br>
        <blockquote cite="mid:B4A1F8A6-25D8-4CA2-A138-95EC14A6A881@oracle.com" type="cite">
          <div><br>
          </div>
          <div>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?</div>
          <div>Wouldn't that resolve both your issue and the issue the
            original fix was made for?</div>
          <div><br>
          </div>
          <div>I have a couple of comments about the code, but let's
            postpone that until we decide on the approach.</div>
          <div><br>
          </div>
          <div>Thank you.</div>
          <div>With best regards. Petr.</div>
          <div><br>
          </div>
          <div>
            <div>
              <div>On 01 июля 2014 г., at 21:20, anton nashatyrev <<a moz-do-not-send="true" href="mailto:anton.nashatyrev@oracle.com">anton.nashatyrev@oracle.com</a>>


                wrote:</div>
              <br class="Apple-interchange-newline">
              <blockquote type="cite">
                <meta http-equiv="content-type" content="text/html;
                  charset=UTF-8">
                <div text="#000000" bgcolor="#FFFFFF"> Hello, <br>
                      could you please review the following fix:<br>
                  <br>
                  fix: <a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eanashaty/8046495/9/webrev.00/">http://cr.openjdk.java.net/~anashaty/8046495/9/webrev.00/</a><br>
                  bug: <a moz-do-not-send="true" href="https://bugs.openjdk.java.net/browse/JDK-8046495">https://bugs.openjdk.java.net/browse/JDK-8046495</a><br>
                  <br>
                      <b>Problem:</b> <br>
                  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 <br>
                  <br>
                      <b>Evaluation</b>: <br>
                  On Windows we have some algorithm for calculating
                  input event timestamp:
                  jdk/src/windows/native/sun/windows/awt_Component.cpp:2100<br>
                  Shortly the timestamp is calculated by the following
                  formula: <br>
                      when = JVM_CurrentTimeMillis() - (GetTickCount() -
                  GetMessageTime())<br>
                  <br>
                  Where: <br>
                    JVM_CurrentTimeMillis() - the same as
                  System.currentTimeMillis()<br>
                    GetTickCount() - Win32 function, current millis from
                  boot time<br>
                    GetMessageTime() - Win32 function, millis from boot
                  time of the latest native Message<br>
                  <br>
                  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).<br>
                  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. <br>
                  <br>
                      <b>Fix</b>: <br>
                  Just to use JVM_CurrentTimeMillis only as events
                  timestamp. On Mac this is done in exactly the same
                  way: CPlatformResponder.handleMouse/KeyEvent()<br>
                  <br>
                  The message time offset calculation has been
                  introduced with the fix for <a moz-do-not-send="true" href="https://bugs.openjdk.java.net/browse/JDK-4434193" title="ActionEvents (and other events) need
                    timestamps"><strike>JDK-4434193</strike></a> 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.<br>
                  I also didn't find any benefits in using the existing
                  approach: <br>
                  - all the usages of the TimerHelper are in fact
                  reduced to the mentioned formula: when =
                  JVM_CurrentTimeMillis() - (GetTickCount() -
                  GetMessageTime())<br>
                  - GetMessageTime() always increases (except of the int
                  overflow moments), thus we couldn't get earlier OS
                  messages after later ones<br>
                  - TimerHelper::windowsToUTC(DWORD windowsTime) doesn't
                  guarantee returning the same timestamp across
                  different calls for the same message time <br>
                  <br>
                  Thanks! <br>
                  Anton. <br>
                </div>
              </blockquote>
            </div>
            <br>
          </div>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </div>

</blockquote></div><br></div></body></html>