RFR: 8280982: [Wayland] [XWayland] java.awt.Robot taking screenshots

Fri May 5 10:55:21 UTC 2023

On Thu, 4 May 2023 14:18:44 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:

> Modern Linux systems often come with [Wayland](https://wayland.freedesktop.org/) by default.
> This comes with some difficulties, and one of them is the inability to get screenshots from the system.
> This is because we now use the [X Window System API](https://en.wikipedia.org/wiki/X_Window_System) to capture screenshots and it cannot access data outside the [XWayland server](https://wayland.freedesktop.org/xserver.html) 
> 
> But this functionality is a very important part of automated testing.
> 
> 
> At the moment there are two obvious solutions to this problem, and both use [xdg-desktop-portal](https://github.com/flatpak/xdg-desktop-portal):
> 
> 1. [org.freedesktop.portal.Screenshot DBUS API](https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Screenshot)
> It has several drawbacks though:
> + It saves a screenshot to disk, which must be read and deleted(may add some delays depending on the type of a disk drive).
> + There is no way to disable the visual "screen flash" after screenshot
> + It asks a user confirmation to save a screenshot. This confirmation can be saved on Gnome 43+. 
> Since we would like Ubuntu 22.04 LTS which comes with Gnome 42 this option is not acceptable for us because it would require user confirmation for each screenshot.
> But we still can consider this option as a fallback.
> 
> 
> 
> 2. [org.freedesktop.portal.ScreenCast](https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.ScreenCast)
> It typically used by applications that need to capture the contents of the user's screen or a specific window for the purpose of sharing, recording, or streaming.
> This might be a bit of overkill, but it avoids several of the problems mentioned in the Screenshot API.
> 
> + implementation is more complicated comparing to Screenshot API
> + no intermediate file, screenshot data can be obtained from memory
> + Permission to make screenshots can be stored with [`restore_token`](https://flatpak.github.io/xdg-desktop-portal/#gdbus-method-org-freedesktop-portal-ScreenCast.SelectSources)
> 
> 
> So this PR adds the ability to take screenshots using the ScreenCast API. This functionality is currently disabled by default.
> 
> This change also introduces some new behavior for the robot:
> A system window now appears asking for confirmation from the user to capture the screen.
> + The user can refuse the screen capture completely. In this case a security exception will be thrown.
> + The user can allow a screen capture for all screens or some of them. For screens without permission there will be a black image.
> + If the user wishes to change their mind about the screens allowed to be captured, the user should use the new `Robot#resetScreenCapturePermission` method
> 
> Also added several system properties:
> `awt.robot.screencastEnabled` false by default, uses the ScreenCast API if true
> `awt.robot.screencastDebug` false by default, prints some debug information if true
> 
> For convenience, this change is divided into two commits:
> 1. added [pipewire headers](https://gitlab.freedesktop.org/pipewire/pipewire/) that are needed for the build
> 2. main changes 
> 
> Changes in the documentation are in a separate draft PR (#13809) also for convenience.
> 
> At the moment, the planned supported systems are Ubuntu 22.04+, the latest Oracle Linux 9.1 and RHEL 9.1 has some issues with restore_token support.

src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java line 428:

> 426: 
> 427:     @SuppressWarnings("removal")
> 428:     public static boolean isOnWayland() {

It seems that this method doesn't have to part of a public interface of `UNIXToolkit`.

src/java.desktop/unix/classes/sun/awt/screencast/ScreencastHelper.java line 56:

> 54:     }
> 55: 
> 56:     private static final int DENIED = -11; //defined in native code

The comment is confusing as the field is defined right here, not in native code.

src/java.desktop/unix/classes/sun/awt/screencast/ScreencastHelper.java line 58:

> 56:     private static final int DENIED = -11; //defined in native code
> 57: 
> 58:     private static synchronized native int getRGBPixelsImpl(

Is `synchronized` really needed on the declaration of `getRGBPixelsImpl()`?

src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 50:

> 48: jmethodID storeTokenMethodID = NULL;
> 49: 
> 50: inline void debug_screencast(

Does this one need to be externally visible? If not, it can be (also) made `static`.

src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 138:

> 136: }
> 137: 
> 138: static inline void convertRGBxToBGRx(int* in) {

Will this work correctly on little-endian ARM systems?

src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 602:

> 600:     }
> 601: 
> 602:     pipewire_libhandle = dlopen("libpipewire-0.3.so.0",

There's a useful macro for wrapping library names: `VERSIONED_JNI_LIB_NAME("pipewire-0.3", "0")`

src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 722:

> 720: 
> 721:     const gchar *token = jtoken
> 722:                          ? token = (*env)->GetStringUTFChars(env, jtoken, NULL)

The variable `token` is unnecessarily used in its own initializer here.

src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 789:

> 787:                             env, pixelArray,
> 788:                             start, len,
> 789:                             ((jint *) screenProps->captureData)

I'm not sure if endianness of `captureData` matches the expected endianness of `pixelArray`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185887472
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185891793
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185894200
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185907122
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185917587
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185942008
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185946946
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1185951908