Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use generic capturer to list both screens and windows when possible #39711

Merged
merged 1 commit into from Sep 25, 2023

Conversation

aiddya
Copy link
Contributor

@aiddya aiddya commented Sep 1, 2023

Description of Change

Backport of #39111

Backporting this because the double screen sharing dialogs introduced by #38833 sometimes cause focus bugs, where one dialog is perfectly superimposed on the other, but the bottom one has focus. The top dialog is not clickable, which leaves the impression that the application has frozen.

cc @VerteDinde

Checklist

Release Notes

Notes: Fixed a redundant permission popup while fetching screens and windows using desktopCapturer.getSources() on Wayland.

@aiddya aiddya requested a review from a team as a code owner September 1, 2023 05:48
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 1, 2023
@trop trop bot added 24-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes labels Sep 1, 2023
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 1, 2023
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the patch(es) need to be updated for 24-x-y.
You can update the patch(es) by running:

e sync --3

and then running

e patches all

@aiddya aiddya force-pushed the 24-x-y-pipewire-double-popup branch from a153137 to b9e7ba5 Compare September 7, 2023 05:12
@aiddya
Copy link
Contributor Author

aiddya commented Sep 7, 2023

I've updated the patches. I didn't realize that they were out of date as gclient never complained. I don't know what e refers to (is it an alias?), but re-running git-export-patches seems to have created the same changeset as in the CI artifacts.

@jkleinsc
Copy link
Contributor

@aiddya e refers to electron-build-tools: https://github.com/electron/build-tools. e patches all calls git-export-patches, so that is fine that you called it directly.

Screensharing with PipeWire via XDG Desktop Portal requires explicit
user permission via permission dialogs. Chromium has separate tabs for
screens and windows and thus its portal implementation requests
permissions separately for each. However, the screencast portal has no
such limitation and supports both screens and windows in a single
request.

WebRTC now supports this type of capture in a new method called
called `CreateGenericCapturer`. The `desktopCapturer` implementation has
been modified to use it. Additionally, Chromium has been patched to use
same generic capturer to ensure that the source IDs remain valid for
`getUserMedia`.
@aiddya
Copy link
Contributor Author

aiddya commented Sep 22, 2023

Rebased

@codebytere codebytere dismissed jkleinsc’s stale review September 25, 2023 10:45

Failing patch application has been addressed

@jkleinsc jkleinsc merged commit e51dee4 into electron:24-x-y Sep 25, 2023
13 checks passed
@release-clerk
Copy link

release-clerk bot commented Sep 25, 2023

Release Notes Persisted

Fixed a redundant permission popup while fetching screens and windows using desktopCapturer.getSources() on Wayland.

@aiddya aiddya deleted the 24-x-y-pipewire-double-popup branch September 29, 2023 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
24-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants