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

Delete RenderTheme::cachedSystemFontDescription() because it does nothing #1318

Merged
merged 0 commits into from Jun 7, 2022
Merged

Delete RenderTheme::cachedSystemFontDescription() because it does nothing #1318

merged 0 commits into from Jun 7, 2022

Conversation

litherum
Copy link
Contributor

@litherum litherum commented Jun 6, 2022 •
edited by webkit-early-warning-system

6ead527

Delete RenderTheme::cachedSystemFontDescription() because it does nothing
https://bugs.webkit.org/show_bug.cgi?id=241329

Reviewed by Cameron McCormack.

RenderTheme::cachedSystemFontDescription() is supposed to maintain storage for cached font descriptors.
However, every time this storage is referenced, it is immediately copied from, leaving the storage
in its default uninitialized state. There's no point in keeping default initialized objects around
for no reason.

This is the second piece in a sequence of patches to fix the accessibility bold setting in web content.

* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeSystemFont):
* Source/WebCore/rendering/RenderEmbeddedObject.cpp:
(WebCore::RenderEmbeddedObject::getReplacementTextGeometry const):
* Source/WebCore/rendering/RenderTheme.cpp:
(WebCore::RenderTheme::cachedSystemFontDescription const): Deleted.
(WebCore::RenderTheme::systemFont const): Deleted.
* Source/WebCore/rendering/RenderTheme.h:
(WebCore::RenderTheme::canPaint const):
* Source/WebCore/rendering/RenderThemeAdwaita.h:
* Source/WebCore/rendering/RenderThemeCocoa.h:
* Source/WebCore/rendering/RenderThemeCocoa.mm:
(WebCore::RenderThemeCocoa::systemFont const):
(WebCore::RenderThemeCocoa::cachedSystemFontDescription const): Deleted.
(WebCore::RenderThemeCocoa::updateCachedSystemFontDescription const): Deleted.
* Source/WebCore/rendering/RenderThemeGtk.cpp:
(WebCore::RenderThemeGtk::systemFont const):
(WebCore::RenderThemeGtk::updateCachedSystemFontDescription const): Deleted.
* Source/WebCore/rendering/RenderThemeGtk.h:
* Source/WebCore/rendering/RenderThemePlayStation.cpp:
(WebCore::RenderThemePlayStation::systemFont const):
(WebCore::RenderThemePlayStation::updateCachedSystemFontDescription const): Deleted.
* Source/WebCore/rendering/RenderThemePlayStation.h:
* Source/WebCore/rendering/RenderThemeWin.cpp:
(WebCore::RenderThemeWin::systemFont const):
(WebCore::fillFontDescription): Deleted.
(WebCore::RenderThemeWin::updateCachedSystemFontDescription const): Deleted.
* Source/WebCore/rendering/RenderThemeWin.h:

Canonical link: https://commits.webkit.org/251350@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295326 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@litherum litherum self-assigned this Jun 6, 2022
@litherum litherum added Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 6, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 6, 2022
@nt1m
Copy link
Member

nt1m commented Jun 6, 2022

win/gtk/wincairo/wpe bots aren't quite happy.

@litherum litherum force-pushed the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch from fe45507 to 425ab2f Compare June 6, 2022 16:50
@litherum litherum removed merging-blocked Applied to prevent a change from being merged Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 6, 2022
@litherum litherum force-pushed the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch from 425ab2f to dd670ae Compare June 6, 2022 20:51
@litherum litherum added Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 6, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 6, 2022
@litherum litherum removed merging-blocked Applied to prevent a change from being merged Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 6, 2022
@litherum litherum force-pushed the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch from dd670ae to cfee6bd Compare June 6, 2022 22:59
@litherum litherum added Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 6, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2022
@litherum litherum removed merging-blocked Applied to prevent a change from being merged Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 7, 2022
@litherum litherum force-pushed the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch from cfee6bd to 027779c Compare June 7, 2022 00:43
@litherum litherum added Text For bugs in text layout and rendering, including international text support. WebKit Nightly Build labels Jun 7, 2022
@litherum
Copy link
Contributor Author

litherum commented Jun 7, 2022

Bots seem happy now.

heycam
heycam approved these changes Jun 7, 2022
View reviewed changes
Copy link
Contributor

@heycam heycam left a comment

Choose a reason for hiding this comment

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

s/There's not point/There's no point/ in the commit message.

Source/WebCore/page/linux/ResourceUsageOverlayLinux.cpp Outdated
@@ -75,8 +75,7 @@ class ResourceUsageOverlayPainter final : public GraphicsLayerClient {
ResourceUsageOverlayPainter(ResourceUsageOverlay& overlay)
: m_overlay(overlay)
{
FontCascadeDescription fontDescription;
RenderTheme::singleton().systemFont(CSSValueMessageBox, fontDescription);
FontCascadeDescription fontDescription = RenderTheme::singleton().systemFont(CSSValueMessageBox);
Copy link
Contributor

Choose a reason for hiding this comment

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

auto

@litherum litherum force-pushed the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch from 027779c to 26b3fd3 Compare June 7, 2022 03:32
@litherum litherum added the merge-queue Applied to send a pull request to merge-queue label Jun 7, 2022
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch from 26b3fd3 to 6ead527 Compare June 7, 2022 03:45
@webkit-early-warning-system webkit-early-warning-system merged commit 6ead527 into WebKit:main Jun 7, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r295326 (251350@main): https://commits.webkit.org/251350@main

Reviewed commits have been landed. Closing PR #1318 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label Jun 7, 2022
@litherum litherum deleted the eng/Delete-RenderThemecachedSystemFontDescription-because-it-does-nothing branch June 7, 2022 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heycam heycam heycam approved these changes

@nt1m nt1m Awaiting requested review from nt1m

@whsieh whsieh Awaiting requested review from whsieh

@pxlcoder pxlcoder Awaiting requested review from pxlcoder

@hortont424 hortont424 Awaiting requested review from hortont424

@alanbaradlay alanbaradlay Awaiting requested review from alanbaradlay

Assignees

@litherum litherum

Labels
Text For bugs in text layout and rendering, including international text support.
Projects
None yet
Milestone
No milestone
4 participants
@litherum @nt1m @webkit-early-warning-system @heycam