8150564: Migrate useful ExtendedRobot methods into awt.Robot #22044

alisenchung · 2024-11-12T17:39:55Z

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).

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)
Change requires CSR request JDK-8349593 to be approved

Issues

JDK-8150564: Migrate useful ExtendedRobot methods into awt.Robot (Enhancement - P3)
JDK-8349593: Migrate useful ExtendedRobot methods into awt.Robot (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22044/head:pull/22044
$ git checkout pull/22044

Update a local copy of the PR:
$ git checkout pull/22044
$ git pull https://git.openjdk.org/jdk.git pull/22044/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22044

View PR using the GUI difftool:
$ git pr show -t 22044

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22044.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-11-12T17:40:56Z

👋 Welcome back achung! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-11-12T17:41:24Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2024-11-12T17:41:57Z

@alisenchung this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout 8150564
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

openjdk · 2024-11-12T17:42:26Z

@alisenchung The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-11-12T17:45:24Z

Webrevs

prrace · 2024-11-13T19:48:16Z

Please resolve the merge conflict.

src/java.desktop/share/classes/java/awt/Robot.java

prrace · 2024-11-13T19:51:19Z

/csr

prrace · 2024-11-13T19:51:37Z

/reviewers reviewers 2

openjdk · 2024-11-13T19:52:12Z

@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@alisenchung please create a CSR request for issue JDK-8150564 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

openjdk · 2024-11-13T19:52:52Z

@prrace Usage: /reviewers <n> [<role>] where <n> is the number of required reviewers. If role is set, the reviewers need to have that project role. If omitted, role defaults to authors.

prrace · 2024-11-13T20:05:25Z

/reviewers 2 reviewers

openjdk · 2024-11-13T20:06:46Z

@prrace
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

openjdk · 2025-01-28T04:57:52Z

@alisenchung This pull request is now open

azvegint · 2025-02-24T20:10:58Z

src/java.desktop/share/classes/java/awt/Robot.java

@@ -126,6 +126,17 @@ public class Robot {

    private DirectColorModel screenCapCM = null;

+    /**
+     * Default 20 milliseconds delay for mouse {@code click} and


Suggested change

* Default 20 milliseconds delay for mouse {@code click} and

* 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.

Be aware that values of static fields are automatically documented.
eg see
(https://docs.oracle.com/en/java/javase/21/docs/api/constant-values.html#java.awt.Font.BOLD)

And an app really could use knowing what the default is.
And these defaults have been battle-tested in ExtendedRobot

azvegint · 2025-02-24T20:11:12Z

src/java.desktop/share/classes/java/awt/Robot.java

+    public static final int DEFAULT_DELAY = 20;
+
+    /**
+     * Default 2 pixel step length for mouse {@code glide}.


Suggested change

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

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

Same here

azvegint · 2025-02-24T20:38:33Z

src/java.desktop/share/classes/java/awt/Robot.java

+     * A convenience method that simulates clicking a mouse button by calling {@code mousePress}
+     * and {@code mouseRelease}. Invokes {@code waitForIdle} with a default {@link #DEFAULT_DELAY delay} after
+     * {@code mousePress} and {@code mouseRelease} calls. For specifics on valid inputs please see
+     * {@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

I understand the concern but in this case it is more or less explanation of what the method does. Perhaps it can be adjusted or made in to an apiNote or whatever is best.

If this method doesn't call waitForIdle there's no reason to document here that you'll get IllegalThreadStateException if called on the EDT, and waitForIdle is the sole source of ITSE.
So since we are obliged to document the IllegalThreadStateException I don't think it does any harm to say it calls waitForIdle.

azvegint · 2025-02-24T21:03:02Z

test/jdk/lib/client/ExtendedRobot.java

-     */
-    @Override
-    public synchronized void waitForIdle() {
-        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)

azvegint · 2025-02-24T21:05:57Z

src/java.desktop/share/classes/java/awt/Robot.java

+    /**
+     * Default 2 pixel step length for mouse {@code glide}.
+     */
+    public static final int DEFAULT_STEP_LENGTH = 2;


Do we want to make the DEFAULT_DELAY and DEFAULT_STEP_LENGTH configurable?

"glide has over-rides step length"

Sorry, that text was not clear. What I mean is that glide(..) accepts delay and step parameters, and so at least in that case you can over-ride with your preferred values.

Other than that, it isn't configurable in the ExtendedRobot, so the need isn't clear.

Even so, we could do this but whether now or later, but in either case,
we ought to think now as to what it would look like so it can be compatibly added.

In ExtendedRobot the equivalent fields are private but here they are public.
Tests might start to use them directly. I'm not sure what for, but they might.
But if it is configurable tests probably should instead call a getter() for the currently set delay/step.

Then all the methods that now mention DEFAULT_ would probably need to mention something like
getMouseDelay() and getStepLength() instead.

So it may be better to do it now ? Thoughts ?

azvegint · 2025-02-24T21:09:56Z

src/java.desktop/share/classes/java/awt/Robot.java

+             stepNum++) {
+            x += tDx;
+            y += tDy;
+            mouseMove((int)x, (int)y);


mouseMove can throw an IllegalThreadStateException under certain circumstances.

yes should be added to all glide API methods

azvegint · 2025-02-24T21:12:15Z

src/java.desktop/share/classes/java/awt/Robot.java

+     * @see     #glide(int, int, int, int, int, int)
+     * @since   25
+     */
+    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.

I've taken a look at all the tests running glide and very few of them actually used glide(Point dest) and glide(Point src, Point dest) so I decided to remove them from them from the migration

azvegint · 2025-02-24T21:18:08Z

test/jdk/java/awt/Component/PaintAll/PaintAll.java

@@ -48,7 +49,6 @@
  @summary Test Component.paintAll() method
  @author [email protected]: area=awt.component
  @library /lib/client/


It's not needed anymore, is it?

Suggested change

@library /lib/client/

azvegint · 2025-02-24T21:22:48Z

test/jdk/lib/client/ExtendedRobot.java

-     * @see     #type(int)
-     * @see     java.awt.event.KeyEvent
-     */
-    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.

i've now also migrated type(int)

bridgekeeper · 2025-03-25T03:34:29Z

@alisenchung This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2025-04-22T03:36:12Z

@alisenchung This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

alisenchung · 2025-05-07T15:51:25Z

/open

openjdk · 2025-05-07T15:52:23Z

@alisenchung This pull request is now open

…tation in glide

bridgekeeper · 2025-06-10T05:10:41Z

@alisenchung This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

honkar-jdk

I see couple of tests that use ExtendedRobot which can be changed to use Robot. Can you double check these tests.

java/awt/Frame/DisposeParentGC/DisposeParentGC.java
java/awt/List/SetBackgroundTest/SetBackgroundTest.java
java/awt/Modal/ToBack/ToBackFDFTest.java

honkar-jdk · 2025-06-11T17:45:30Z

test/jdk/lib/client/ExtendedRobot.java

-    public void glide(Point src, Point dest) {
-        glide(src.x, src.y, dest.x, dest.y, DEFAULT_STEP_LENGTH, DEFAULT_SPEED);
-    }


@alisenchung

Shouldn't glide(Point src, Point dest) and glide(Point dest) be retained in ExtendedRobot
In previous comment it was mentioned that these methods won't be migrated so do they need to be retained in ExtendedRobot ?

I see MultipleMouseButtonsTest.java using glide(Point src, Point dest). There are two options here:

Either update the test to use Robot's glide()
change the line robot.glide(origin, center) to robot.glide(origin.x, origin.y, center.x, center.y); and completely remove ExtendedRobot versions of glide.

Or retain these two convenience methods in ExtendedRobot.

There are few other tests that use ExtendedRobot version of glide() so probably option 2 (retaining the two convenience methods in ExtendedRobot) might be a better option.

I think ideally ER will be removed in the future so depending on the number of tests using glide(point), I will either change the tests or readd glide(point) to Robot.

honkar-jdk

There are couple of tests failing due to compilation error. I have added the list to JBS, please take a look.

honkar-jdk · 2025-06-11T18:33:49Z

test/jdk/java/awt/TrayIcon/TrayIconPopup/TrayIconPopupTest.java

@@ -61,7 +61,7 @@
 public class TrayIconPopupTest {

    TrayIcon icon;
-    ExtendedRobot robot;
+    Robot robot;


This test still builds ExtendedRobot which is no longer needed. Applicable for other tests as well.

@library /lib/client @build ExtendedRobot

honkar-jdk

Since there are many test changes it is good to run CI on latest update.

honkar-jdk · 2025-06-17T16:47:05Z

test/jdk/java/awt/TrayIcon/TrayIconPopup/TrayIconPopupTest.java

@@ -53,7 +54,6 @@
 *          java.desktop/java.awt.Helper
 *          jdk.test.lib.Platform
 *          jtreg.SkippedException
- *          ExtendedRobot


Since ExtendedRobot is removed /lib/client is no longer required in @library tag. Applicable for other tests as well.

There are couple of tests with /lib/client in @library tag which is no longer required. This cleanup can be handled in subsequent PR.

honkar-jdk · 2025-06-17T17:00:28Z

test/jdk/java/awt/Modal/ModalBlockingTests/BlockingDDModelessTest.java

 * @library ../helpers /lib/client/
 * @library /test/lib


Same here /lib/client is no longer required.

honkar-jdk

Latest changes LGTM.

@alisenchung
Does CI run look good with the latest changes?

src/java.desktop/share/classes/java/awt/Robot.java

prrace · 2025-07-08T18:42:57Z

src/java.desktop/share/classes/java/awt/Robot.java

+     * A convenience method that simulates clicking a mouse button by calling {@code mousePress}
+     * and {@code mouseRelease}. Invokes {@code waitForIdle} with a default {@link #DEFAULT_DELAY delay} after
+     * {@code mousePress} and {@code mouseRelease} calls. For specifics on valid inputs please see
+     * {@link java.awt.Robot#mousePress(int)}.


If this method doesn't call waitForIdle there's no reason to document here that you'll get IllegalThreadStateException if called on the EDT, and waitForIdle is the sole source of ITSE.
So since we are obliged to document the IllegalThreadStateException I don't think it does any harm to say it calls waitForIdle.

src/java.desktop/share/classes/java/awt/Robot.java

alisenchung added 6 commits October 23, 2024 13:34

init commit

514a813

added tests

b21c56b

fix tests

b60c093

fix spacing

633318c

Merge branch 'master' of https://github.com/openjdk/jdk into 8150564

b39bd2e

extendedRobot

1c54001

openjdk bot added merge-conflict Pull request has merge conflict with target branch rfr Pull request is ready for review labels Nov 12, 2024

openjdk bot added the client [email protected] label Nov 12, 2024

prrace reviewed Nov 13, 2024

View reviewed changes

src/java.desktop/share/classes/java/awt/Robot.java Outdated Show resolved Hide resolved

src/java.desktop/share/classes/java/awt/Robot.java Outdated Show resolved Hide resolved

src/java.desktop/share/classes/java/awt/Robot.java Outdated Show resolved Hide resolved

openjdk bot added the csr Pull request needs approved CSR before integration label Nov 13, 2024

honkar-jdk mentioned this pull request Nov 14, 2024

8339524: Clean up a few ExtendedRobot tests #20846

Closed

3 tasks

alisenchung added 2 commits November 15, 2024 07:00

move mouseMove back to ER

955f551

merge

4cb44bc

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Nov 15, 2024

alisenchung added 3 commits November 15, 2024 07:08

Merge branch 'master' of https://github.com/openjdk/jdk into 8150564

c0733de

update specifications

a11b8ac

fix var names in ER

adb0f58

openjdk bot reopened this Jan 28, 2025

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Feb 6, 2025

merge

30ca6a6

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Feb 6, 2025

azvegint reviewed Feb 24, 2025

View reviewed changes

bridgekeeper bot closed this Apr 22, 2025

openjdk bot reopened this May 7, 2025

remove specified numbers in constant fields, add exception to documen…

c0cd6e7

…tation in glide

alisenchung added 3 commits June 10, 2025 14:09

Merge branch 'master' of https://github.com/openjdk/jdk into 8150564

cbcf3fb

migrate type(char) method

7eab183

synchronized for type(char)

a46b067

honkar-jdk reviewed Jun 11, 2025

View reviewed changes

honkar-jdk suggested changes Jun 11, 2025

View reviewed changes

honkar-jdk reviewed Jun 11, 2025

View reviewed changes

alisenchung added 2 commits June 12, 2025 11:24

update tests

0abc123

update tests, remove extended robot from some tests

9aa8201

honkar-jdk suggested changes Jun 17, 2025

View reviewed changes

honkar-jdk reviewed Jun 17, 2025

View reviewed changes

alisenchung added 2 commits July 2, 2025 14:14

remove /lib/client/ from tests with ER removed

8347d2f

remove ER from remaining modal test

9f19885

honkar-jdk reviewed Jul 3, 2025

View reviewed changes

Merge branch 'master' of https://github.com/openjdk/jdk into 8150564

3a9372c

prrace reviewed Jul 8, 2025

View reviewed changes

update robot specs

c02d418

	* Default 20 milliseconds delay for mouse {@code click} and
	* Default delay for mouse {@code click} and

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

8150564: Migrate useful ExtendedRobot methods into awt.Robot #22044

Are you sure you want to change the base?

8150564: Migrate useful ExtendedRobot methods into awt.Robot #22044

Conversation

alisenchung commented Nov 12, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewing

Uh oh!

bridgekeeper bot commented Nov 12, 2024

Uh oh!

openjdk bot commented Nov 12, 2024

Uh oh!

openjdk bot commented Nov 12, 2024

Uh oh!

openjdk bot commented Nov 12, 2024

Uh oh!

mlbridge bot commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

prrace commented Nov 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prrace commented Nov 13, 2024

Uh oh!

prrace commented Nov 13, 2024

Uh oh!

openjdk bot commented Nov 13, 2024

Uh oh!

openjdk bot commented Nov 13, 2024

Uh oh!

prrace commented Nov 13, 2024

Uh oh!

openjdk bot commented Nov 13, 2024

Uh oh!

openjdk bot commented Jan 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prrace May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bridgekeeper bot commented Mar 25, 2025

Uh oh!

bridgekeeper bot commented Apr 22, 2025

Uh oh!

alisenchung commented May 7, 2025

alisenchung commented Nov 12, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 12, 2024 •

edited

Loading

prrace May 8, 2025 •

edited

Loading

honkar-jdk Jun 11, 2025 •

edited

Loading

honkar-jdk Jun 11, 2025 •

edited

Loading

honkar-jdk left a comment •

edited

Loading