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.

Sign up for GitHub

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

Jump to bottom

feat: add webContents.getMediaSourceId() method #31204

Merged
merged 5 commits into from
Oct 26, 2021
Merged

feat: add webContents.getMediaSourceId() method #31204

merged 5 commits into from
Oct 26, 2021

Conversation

mitchemmc
Copy link
Contributor

Description of Change

New feature: add webContents.getMediaSourceId() to provide a way to get a MediaStream from a WebContents using getUserMedia.

This aims to fix the issues from #22701 which was removed with #25414.

The first main issue was with claiming to be available in both the renderer and main process but only working in the renderer.

This is fixed here by moving the api from the DesktopCapturer to a method on a WebContents. This means the API can only be consumed in the main process but I think this is inline with restricting the access for the DesktopCapturer to the main process in v17.

The next issue was with the lack of access control to the caller.

This is fixed here by requiring the user of the api to pass in the web contents that the media source ID will be registered to.
This allows you to restrict access to only the web contents you trust.

Checklist

Release Notes

Notes: Added webContents.getMediaSourceId(), can be used with getUserMedia to get a stream for a WebContents.

@welcome
Copy link

welcome bot commented Sep 30, 2021

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 30, 2021
@jkleinsc jkleinsc added the semver/minor backwards-compatible functionality label Sep 30, 2021
zcbenz
zcbenz reviewed Oct 4, 2021
View reviewed changes
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

API looks good to me.

shell/browser/api/electron_api_web_contents.cc Outdated Show resolved Hide resolved
shell/browser/api/electron_api_web_contents.cc Outdated Show resolved Hide resolved
shell/browser/api/electron_api_web_contents.h Outdated Show resolved Hide resolved
zcbenz
zcbenz approved these changes Oct 5, 2021
View reviewed changes
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

Other than the minor stylish thing the code looks good to me.

shell/browser/api/electron_api_web_contents.cc Outdated Show resolved Hide resolved
shell/browser/api/electron_api_web_contents.cc Outdated Show resolved Hide resolved
docs/api/web-contents.md Outdated Show resolved Hide resolved
@vapormusic
Copy link

bump

@jkleinsc
Copy link
Contributor

API LGTM

@jkleinsc jkleinsc merged commit 23cdf65 into electron:main Oct 26, 2021
@welcome
Copy link

welcome bot commented Oct 26, 2021

Congrats on merging your first pull request! 🎉🎉🎉

@release-clerk
Copy link

release-clerk bot commented Oct 26, 2021

Release Notes Persisted

Added webContents.getMediaSourceId(), can be used with getUserMedia to get a stream for a WebContents.

t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
@mitchemmc @t57ser
feat: add webContents.getMediaSourceId() method (electron#31204)
30d643f 
* feat: add webContents.getMediaSourceId() method

* fix: account for null frame_hosts in webContents.getMediaSourceId()

* fix: move webContents.getMediaSourceId definition to be more organised

* fix: move webContents.getMediaSourceId implementation

* fix: move webContents.getMediaSourceId docs
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
@mitchemmc @t57ser
feat: add webContents.getMediaSourceId() method (electron#31204)
ac71068 
* feat: add webContents.getMediaSourceId() method

* fix: account for null frame_hosts in webContents.getMediaSourceId()

* fix: move webContents.getMediaSourceId definition to be more organised

* fix: move webContents.getMediaSourceId implementation

* fix: move webContents.getMediaSourceId docs
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
@mitchemmc @t57ser
feat: add webContents.getMediaSourceId() method (electron#31204)
3b8153c 
* feat: add webContents.getMediaSourceId() method

* fix: account for null frame_hosts in webContents.getMediaSourceId()

* fix: move webContents.getMediaSourceId definition to be more organised

* fix: move webContents.getMediaSourceId implementation

* fix: move webContents.getMediaSourceId docs
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
@mitchemmc @t57ser
feat: add webContents.getMediaSourceId() method (electron#31204)
69013ce 
* feat: add webContents.getMediaSourceId() method

* fix: account for null frame_hosts in webContents.getMediaSourceId()

* fix: move webContents.getMediaSourceId definition to be more organised

* fix: move webContents.getMediaSourceId implementation

* fix: move webContents.getMediaSourceId docs
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
@mitchemmc @t57ser
feat: add webContents.getMediaSourceId() method (electron#31204)
3c7277b 
* feat: add webContents.getMediaSourceId() method

* fix: account for null frame_hosts in webContents.getMediaSourceId()

* fix: move webContents.getMediaSourceId definition to be more organised

* fix: move webContents.getMediaSourceId implementation

* fix: move webContents.getMediaSourceId docs
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
@mitchemmc @t57ser
feat: add webContents.getMediaSourceId() method (electron#31204)
be56fcd 
* feat: add webContents.getMediaSourceId() method

* fix: account for null frame_hosts in webContents.getMediaSourceId()

* fix: move webContents.getMediaSourceId definition to be more organised

* fix: move webContents.getMediaSourceId implementation

* fix: move webContents.getMediaSourceId docs
@commit-lint commit-lint bot mentioned this pull request Jan 26, 2022
Open
@DanielMcAssey DanielMcAssey mentioned this pull request Feb 6, 2022
Open
@larssvensby larssvensby mentioned this pull request Mar 7, 2022
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

@zcbenz zcbenz zcbenz approved these changes

Assignees
No one assigned
Labels
api-review/requested 🗳 no-backport semver/minor backwards-compatible functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mitchemmc @vapormusic @jkleinsc @zcbenz