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: prevent navigator.fonts.query() from crashing #30930

Merged
merged 2 commits into from Sep 15, 2021
Merged

fix: prevent navigator.fonts.query() from crashing #30930

merged 2 commits into from Sep 15, 2021

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Sep 13, 2021

Description of Change

Closes #30884.

Follow-up at #30932.

Fixes a crash in navigator.fonts.query(). Chromium relies on a user choice via a UI they pop up to run the callback, and in contrast we don't really have a way to run the closure post-construction of the FontAccessChooser and therefore must run it in the constructor, which makes it such that FontAccessManagerImpl::DidChooseLocalFonts runs before the FontAccessChooser is assigned in the choosers_ map and DCHECKS.

As we don't yet have a standardized permissions model for this and similar APIs, this just mediates the crash and adds appropriate simple scaffolding for a proper implementation.

Tested with https://gist.github.com/bbae1d3b31214682fb6a05ec69b5d97a.

Checklist

Release Notes

Notes: Fixed a crash in navigator.fonts.query().

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 14, 2021
@jkleinsc jkleinsc merged commit c8e4cc2 into main Sep 15, 2021
@jkleinsc jkleinsc deleted the font-crash branch September 15, 2021 13:56
@release-clerk
Copy link

release-clerk bot commented Sep 15, 2021

Release Notes Persisted

Fixed a crash in navigator.fonts.query().

@trop
Copy link
Contributor

trop bot commented Sep 15, 2021

I have automatically backported this PR to "14-x-y", please check out #30984

@trop
Copy link
Contributor

trop bot commented Sep 15, 2021

I have automatically backported this PR to "15-x-y", please check out #30985

Comment on lines +18 to +22
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(
[](base::OnceClosure closure) { std::move(closure).Run(); },
std::move(close_callback)));
Copy link
Member

Choose a reason for hiding this comment

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

It seems like you could probably pass std::move(close_callback) here rather than re-wrapping it, yes? i.e.

Suggested change
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(
[](base::OnceClosure closure) { std::move(closure).Run(); },
std::move(close_callback)));
base::PostTask(FROM_HERE, {content::BrowserThread::UI}, std::move(close_callback));

@gpetrov
Copy link

gpetrov commented Feb 23, 2022

Please backport to Electron 13 as well @codebytere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Renderer process crashes after calling navigator.fonts.query()
5 participants