From 13a052be5cd05ebd0d64bfc63b13b858238fa0bb Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 8 Sep 2021 21:52:22 +0200 Subject: [PATCH] fix: prevent navigator.fonts.query() from crashing --- filenames.gni | 2 + patches/chromium/.patches | 1 + .../fix_font_access_chooser_dcheck.patch | 33 ++++++++++++++++ shell/browser/electron_browser_client.cc | 6 +++ shell/browser/electron_browser_client.h | 4 ++ .../font/electron_font_access_delegate.cc | 38 +++++++++++++++++++ .../font/electron_font_access_delegate.h | 28 ++++++++++++++ 7 files changed, 112 insertions(+) create mode 100644 patches/chromium/fix_font_access_chooser_dcheck.patch create mode 100644 shell/browser/font/electron_font_access_delegate.cc create mode 100644 shell/browser/font/electron_font_access_delegate.h diff --git a/filenames.gni b/filenames.gni index 7ac65a0903446..d38ea4303681a 100644 --- a/filenames.gni +++ b/filenames.gni @@ -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", diff --git a/patches/chromium/.patches b/patches/chromium/.patches index c37f869376c3c..538c7b12964f7 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -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 diff --git a/patches/chromium/fix_font_access_chooser_dcheck.patch b/patches/chromium/fix_font_access_chooser_dcheck.patch new file mode 100644 index 0000000000000..1706a0cc0108f --- /dev/null +++ b/patches/chromium/fix_font_access_chooser_dcheck.patch @@ -0,0 +1,33 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shelley Vohr +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); + + std::move(callback).Run(std::move(status), std::move(fonts)); + } diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index 069c64c36104e..4a0af0243ddfe 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -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(); + return font_access_delegate_.get(); +} + void BindBadgeServiceForServiceWorker( const content::ServiceWorkerVersionBaseInfo& info, mojo::PendingReceiver receiver) { diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index 6b694d8796bca..7e7131ebd67fc 100644 --- a/shell/browser/electron_browser_client.h +++ b/shell/browser/electron_browser_client.h @@ -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" @@ -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; @@ -305,6 +308,7 @@ class ElectronBrowserClient : public content::ContentBrowserClient, std::unique_ptr serial_delegate_; std::unique_ptr bluetooth_delegate_; + std::unique_ptr font_access_delegate_; #if defined(OS_MAC) ElectronBrowserMainParts* browser_main_parts_ = nullptr; diff --git a/shell/browser/font/electron_font_access_delegate.cc b/shell/browser/font/electron_font_access_delegate.cc new file mode 100644 index 0000000000000..5c71583b610fa --- /dev/null +++ b/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 + +#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 +// #include + +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 +ElectronFontAccessDelegate::RunChooser( + content::RenderFrameHost* frame, + const std::vector& selection, + content::FontAccessChooser::Callback callback) { + // TODO(codebytere) : implement with proper permissions model. + return std::make_unique(base::BindOnce( + [](content::FontAccessChooser::Callback callback) { + std::move(callback).Run( + blink::mojom::FontEnumerationStatus::kUnimplemented, {}); + }, + std::move(callback))); +} diff --git a/shell/browser/font/electron_font_access_delegate.h b/shell/browser/font/electron_font_access_delegate.h new file mode 100644 index 0000000000000..1f15da275cbd5 --- /dev/null +++ b/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 +#include +#include + +#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 RunChooser( + content::RenderFrameHost* frame, + const std::vector& selection, + content::FontAccessChooser::Callback callback) override; + + DISALLOW_COPY_AND_ASSIGN(ElectronFontAccessDelegate); +}; + +#endif // SHELL_BROWSER_FONT_ELECTRON_FONT_ACCESS_DELEGATE_H_