On Fri, 5 May 2023 14:35:46 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 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. > > Alexander Zvegintsev has updated the pull request incrementally with three > additional commits since the last revision: > > - update, based on review comments > - remove wayland detection > - BUILD_LIBPIPEWIRE_HEADER_DIRS -> LIBPIPEWIRE_HEADER_DIRS I'm thinking about the re-set API "If the user wishes to change their mind about the screens allowed to be captured, the user should use the new Robot#resetScreenCapturePermission method" Well the "user" is hosed unless the app calls this every time ... or provides UI for it .. I want to work through the scenarios and how much of it is specific to the behaviours of the API you are using and so forth. Since you use the Preferences API for saving the token, if you keyed it off the Robot class rather than the internal API class then anyone using the Preferences API could delete the token, couldn't they ? And we presumably can store alongside the token, info about the screens attached at the time we obtained the token ? Can you point (here in the implementation review) to the exact native functions that popup up the dialog ? I'm still trying to find my way through that code. Is it opaque to the caller (us) what the user actually approved ? I can't imagine why we would need to reset "we have all permissions" to "we have no permissions" so I'm supposing the issue is when they didn't grant permissions .. but if they "deny" permissions that's surely different than "this token is no longer valid" ? The first time we need a permission for the screencast API there is a dialog. The user makes some choices and we then choose to save a token so that the user isn't prompted again. We must at least know though that they approved SOMETHING but we don't know what ? Or do we ? If they DENY, would we save that ? Interesting question because if we don't save something we'll keep spamming them until they approve :-) Next time, what happens if for some reaon the token is no longer valid ? And what might make it invalid ? If the user denied screen 2, or added screen 2 later, would that not result in an invalid token and we'd know to ask again ? No reset needed ? Also I can't figure out if "validating" the token can cause the dialog as a side effect or if there's some API we explicitly call to request a new token. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13803#issuecomment-1540864851
