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
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions filenames.gni
Expand Up @@ -382,6 +382,8 @@ filenames = {
"shell/browser/file_select_helper.cc",
"shell/browser/file_select_helper.h",
"shell/browser/file_select_helper_mac.mm",
"shell/browser/font/electron_font_access_delegate.cc",
"shell/browser/font/electron_font_access_delegate.h",
"shell/browser/font_defaults.cc",
"shell/browser/font_defaults.h",
"shell/browser/javascript_environment.cc",
Expand Down
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -108,3 +108,4 @@ chore_do_not_use_chrome_windows_in_cryptotoken_webrequestsender.patch
fix_chrome_root_store_codegen_for_cross-compile_builds.patch
process_singleton.patch
fix_expose_decrementcapturercount_in_web_contents_impl.patch
fix_font_access_chooser_dcheck.patch
33 changes: 33 additions & 0 deletions patches/chromium/fix_font_access_chooser_dcheck.patch
@@ -0,0 +1,33 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Mon, 13 Sep 2021 16:06:52 +0200
Subject: Fix: font access chooser DCHECK

This patch is necessary to prevent a crash when navigator.fonts.query()
in the intermediary period prior to Electron actually implementing the
feature. 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.

diff --git a/content/browser/font_access/font_access_manager_impl.cc b/content/browser/font_access/font_access_manager_impl.cc
index 518854d5370cdb5dc5131ad47442ac2bc748eae5..8b2b6ab23d1d12a0bb202dd1227ba5a28236a934 100644
--- a/content/browser/font_access/font_access_manager_impl.cc
+++ b/content/browser/font_access/font_access_manager_impl.cc
@@ -262,8 +262,8 @@ void FontAccessManagerImpl::DidChooseLocalFonts(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

// The chooser has fulfilled its purpose. It's safe to dispose of it.
- size_t erased = choosers_.erase(frame_id);
- DCHECK_EQ(erased, 1u);
+ // size_t erased = choosers_.erase(frame_id);
+ // DCHECK_EQ(erased, 1u);
codebytere marked this conversation as resolved.
Show resolved Hide resolved

std::move(callback).Run(std::move(status), std::move(fonts));
}
6 changes: 6 additions & 0 deletions shell/browser/electron_browser_client.cc
Expand Up @@ -1670,6 +1670,12 @@ content::BluetoothDelegate* ElectronBrowserClient::GetBluetoothDelegate() {
return bluetooth_delegate_.get();
}

content::FontAccessDelegate* ElectronBrowserClient::GetFontAccessDelegate() {
if (!font_access_delegate_)
font_access_delegate_ = std::make_unique<ElectronFontAccessDelegate>();
return font_access_delegate_.get();
}

void BindBadgeServiceForServiceWorker(
const content::ServiceWorkerVersionBaseInfo& info,
mojo::PendingReceiver<blink::mojom::BadgeService> receiver) {
Expand Down
4 changes: 4 additions & 0 deletions shell/browser/electron_browser_client.h
Expand Up @@ -20,6 +20,7 @@
#include "net/ssl/client_cert_identity.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "shell/browser/bluetooth/electron_bluetooth_delegate.h"
#include "shell/browser/font/electron_font_access_delegate.h"
#include "shell/browser/serial/electron_serial_delegate.h"
#include "third_party/blink/public/mojom/badging/badging.mojom-forward.h"

Expand Down Expand Up @@ -91,6 +92,8 @@ class ElectronBrowserClient : public content::ContentBrowserClient,

content::SerialDelegate* GetSerialDelegate() override;

content::FontAccessDelegate* GetFontAccessDelegate() override;

content::BluetoothDelegate* GetBluetoothDelegate() override;

device::GeolocationManager* GetGeolocationManager() override;
Expand Down Expand Up @@ -305,6 +308,7 @@ class ElectronBrowserClient : public content::ContentBrowserClient,

std::unique_ptr<ElectronSerialDelegate> serial_delegate_;
std::unique_ptr<ElectronBluetoothDelegate> bluetooth_delegate_;
std::unique_ptr<ElectronFontAccessDelegate> font_access_delegate_;

#if defined(OS_MAC)
ElectronBrowserMainParts* browser_main_parts_ = nullptr;
Expand Down
38 changes: 38 additions & 0 deletions shell/browser/font/electron_font_access_delegate.cc
@@ -0,0 +1,38 @@
// Copyright (c) 2021 Microsoft, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#include <utility>

#include "shell/browser/font/electron_font_access_delegate.h"

#include "content/public/browser/font_access_chooser.h"
#include "content/public/browser/render_frame_host.h"

// #include <memory>
// #include <string>

class ElectronFontAccessChooser : public content::FontAccessChooser {
public:
explicit ElectronFontAccessChooser(base::OnceClosure close_callback) {
std::move(close_callback).Run();
}
~ElectronFontAccessChooser() override = default;
};

ElectronFontAccessDelegate::ElectronFontAccessDelegate() = default;
ElectronFontAccessDelegate::~ElectronFontAccessDelegate() = default;

std::unique_ptr<content::FontAccessChooser>
ElectronFontAccessDelegate::RunChooser(
content::RenderFrameHost* frame,
const std::vector<std::string>& selection,
content::FontAccessChooser::Callback callback) {
// TODO(codebytere) : implement with proper permissions model.
return std::make_unique<ElectronFontAccessChooser>(base::BindOnce(
[](content::FontAccessChooser::Callback callback) {
std::move(callback).Run(
blink::mojom::FontEnumerationStatus::kUnimplemented, {});
},
std::move(callback)));
}
28 changes: 28 additions & 0 deletions shell/browser/font/electron_font_access_delegate.h
@@ -0,0 +1,28 @@
// Copyright (c) 2021 Microsoft, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#ifndef SHELL_BROWSER_FONT_ELECTRON_FONT_ACCESS_DELEGATE_H_
#define SHELL_BROWSER_FONT_ELECTRON_FONT_ACCESS_DELEGATE_H_

#include <memory>
#include <string>
#include <vector>

#include "content/public/browser/font_access_chooser.h"
#include "content/public/browser/font_access_delegate.h"

class ElectronFontAccessDelegate : public content::FontAccessDelegate {
public:
ElectronFontAccessDelegate();
~ElectronFontAccessDelegate() override;

std::unique_ptr<content::FontAccessChooser> RunChooser(
content::RenderFrameHost* frame,
const std::vector<std::string>& selection,
content::FontAccessChooser::Callback callback) override;

DISALLOW_COPY_AND_ASSIGN(ElectronFontAccessDelegate);
};

#endif // SHELL_BROWSER_FONT_ELECTRON_FONT_ACCESS_DELEGATE_H_