On Fri, 7 Apr 2023 04:47:56 GMT, Phil Race <[email protected]> wrote:

>> This test issue was due to race condition that occured when using 
>> `-Dsun.java2d.uiScale >= 2`.
>> As a result it led to incorrect mouse position being compared as opposed to 
>> the updated mouse position after the final mouse move event.
>> 
>> Following is the event log before and after fix.
>> 
>> **Before fix:**
>> 
>> 1st mouse move
>> 2nd mouse move
>> Frame-1 Mouse Event
>> Frame-2 Mouse Event
>> java.awt.Point[x=29,y=29]
>> 
>> 
>> **After fix:**
>> 
>> 1st mouse move
>> 2nd mouse move
>> Frame-1 Mouse Event
>> 3rd mouse move
>> Frame-2 Mouse Event
>> java.awt.Point[x=50,y=50]
>> 
>> 
>> Earlier Frame-2's mouseMoved() was being triggered on Robot's 2nd mouse move 
>> instead of 3rd mouse move. To fix it, the mouseMotionListener for Frame-2 is 
>> now added after Robot's 2nd mouse move is processed to avoid the race 
>> condition.
>> 
>> The updated test fix is checked on multiple uiScales including fractional 
>> scales for windows platform.
>> CI testing works as expected on all platforms.
>
> test/jdk/java/awt/Mouse/GetMousePositionTest/GetMousePositionWithPopup.java 
> line 113:
> 
>> 111:                 frame2.setBounds(120, 120, 120, 120);
>> 112:                 frame2.setVisible(true);
>> 113:             }
> 
> So are you confident the requested location will be honoured ?
> I seem to remember a test framework fix in which you had to work quite hard 
> to be sure of the real location.
> And the implied "170-120=50" might be clearer if we had variables not just 
> literal numbers ?

@prrace I agree, adding PassFailJFrame's 
[syncLocationToWindowManager()](https://github.com/openjdk/jdk/blob/a8871f5d26e5cb42c031c7b736ec30b1b147a2bc/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java#L343)
 to this test will make it more robust.
I'll change the literals to variables to make the context clear.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13380#discussion_r1160814524

Reply via email to