On Wed, 31 May 2023 15:03:04 GMT, Alexander Zvegintsev <azveg...@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... > > Alexander Zvegintsev has updated the pull request incrementally with four > additional commits since the last revision: > > - improve retVal processing > - address token storage comments > - removing non-ascii > - EXCEPTION_CHECK_DESCRIBE_CLEAR -> EXCEPTION_CHECK_DESCRIBE I'm getting pretty consistent testing results now - meaning the ones that have known issues are the ones that fail, and I've looked at the code sufficiently. The restore token seems to be behaving properly now. I did have one case I've not understood where a test system got very confused about the display resolution of the monitor and needed a full power cycle. I don't know if a specific test caused that or it was something else, but it isn't repeatable (at least not yet) so shouldn't block integration. I am quite sure that SOME of the test failures are unstable tests that we've been lucky with all these years and xwayland is now showing up the instability. Things un-related to robot and more about timing for the most part. Although there's one test that seriously over-stresses robot with > 10,000 calls in a loop to getPixelColor() I'll see if I can push test update fixes to at least some of these to reduce noise. But I'm OK to approve now. ------------- Marked as reviewed by prr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13803#pullrequestreview-1454093265
