Skip to content

Commit

Permalink
fix: prevent navigator.fonts.query() from crashing
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Sep 13, 2021
1 parent eb955af commit 13a052b
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 0 deletions.
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);

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_

0 comments on commit 13a052b

Please sign in to comment.