RFR: 8150564: Migrate useful ExtendedRobot methods into awt.Robot [v15]

Alexander Zvegintsev azvegint at openjdk.org
Mon Feb 24 21:29:17 UTC 2025


On Thu, 6 Feb 2025 22:03:54 GMT, Alisen Chung <achung at openjdk.org> wrote:

>> Some useful methods in ExtendedRobot should be migrated into Robot itself so that ExtendedRobot can be removed in the future. The tests using ExtendedRobot for these migrated methods are changed to use only Robot (removing unnecessary building of ExtendedRobot).
>
> Alisen Chung has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 22 commits:
> 
>  - merge
>  - update specifications, replace default values in specifications with links to default var
>  - update glide in test
>  - merge
>  - fix test with removed robot.glide using points
>  - add code tag to specifications in Robot
>  - fix syncdelay in ER
>  - removing lesser used overridden methods
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 8150564
>  - update specification to public default fields, add waitforidle exceptions to specifications
>  - ... and 12 more: https://git.openjdk.org/jdk/compare/10791477...30ca6a6b

src/java.desktop/share/classes/java/awt/Robot.java line 130:

> 128: 
> 129:     /**
> 130:      * Default 20 milliseconds delay for mouse {@code click} and

Suggestion:

     * Default delay for mouse {@code click} and


There is no need to specify the exact value in the documentation,  in case of something it will be much easier to change it later on.

src/java.desktop/share/classes/java/awt/Robot.java line 136:

> 134: 
> 135:     /**
> 136:      * Default 2 pixel step length for mouse {@code glide}.

Suggestion:

     * Default pixel step length for mouse {@code glide}.


Same here

src/java.desktop/share/classes/java/awt/Robot.java line 138:

> 136:      * Default 2 pixel step length for mouse {@code glide}.
> 137:      */
> 138:     public static final int DEFAULT_STEP_LENGTH = 2;

Do we want to make the `DEFAULT_DELAY` and `DEFAULT_STEP_LENGTH` configurable?

src/java.desktop/share/classes/java/awt/Robot.java line 792:

> 790:      * and {@code mouseRelease}. Invokes {@code waitForIdle} with a default {@link #DEFAULT_DELAY delay} after
> 791:      * {@code mousePress} and {@code mouseRelease} calls. For specifics on valid inputs please see
> 792:      * {@link java.awt.Robot#mousePress(int)}.

It's discussable, and I may be wrong, but I'm not a fan of documentation that is very specific about its implementation.
I prefer the one that was before in the `ExtendedRobot`.


Clicks mouse button(s) by calling {@link #mousePress(int)} and {@link #mouseRelease(int)} methods

src/java.desktop/share/classes/java/awt/Robot.java line 858:

> 856:      * @since   25
> 857:      */
> 858:     public void glide(int x, int y) {

I don't see the `public void glide(Point dest)` and `public void glide(Point src, Point dest)` added, it may be convenient in some cases.

src/java.desktop/share/classes/java/awt/Robot.java line 924:

> 922:             x += tDx;
> 923:             y += tDy;
> 924:             mouseMove((int)x, (int)y);

`mouseMove` can throw an `IllegalThreadStateException` under certain circumstances.

test/jdk/java/awt/Component/PaintAll/PaintAll.java line 51:

> 49:   @summary Test Component.paintAll() method
> 50:   @author sergey.bylokhov at oracle.com: area=awt.component
> 51:   @library /lib/client/

It's not needed anymore, is it?
Suggestion:

test/jdk/lib/client/ExtendedRobot.java line 182:

> 180:     @Override
> 181:     public synchronized void waitForIdle() {
> 182:         waitForIdle(syncDelay);

This `waitForIdle(500)` is no longer called by tests(as it uses regular `java.awt.Robot#waitForIdle()`, so I assume you have verified that automated testing looks good. (I haven't gone through all the tests yet)

test/jdk/lib/client/ExtendedRobot.java line 368:

> 366:      * @see     java.awt.event.KeyEvent
> 367:      */
> 368:     public void type(char c) {

Those `type(char...` are not migrated also. 

At least one test uses it:

https://github.com/alisenchung/jdk/blob/8150564/test/jdk/java/awt/Window/ShapedAndTranslucentWindows/ShapedTranslucentWindowClick.java#L178

With your change, it now calls `type(int)` directly, which has a different implementation.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968352863
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968353101
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968417841
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968386427
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968425006
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968422144
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968431117
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968414517
PR Review Comment: https://git.openjdk.org/jdk/pull/22044#discussion_r1968435956


More information about the client-libs-dev mailing list