Delete RenderTheme::cachedSystemFontDescription() because it does not…

…hing

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

Source/WebCore/css/parser/CSSPropertyParser.cpp

@@ -1,5 +1,5 @@
 // Copyright 2015 The Chromium Authors. All rights reserved.
-// Copyright (C) 2016-2021 Apple Inc. All rights reserved.
+// Copyright (C) 2016-2022 Apple Inc. All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
@@ -5196,8 +5196,7 @@ bool CSSPropertyParser::consumeSystemFont(bool important)
     if (!m_range.atEnd())
         return false;
 
-    FontCascadeDescription fontDescription;
-    RenderTheme::singleton().systemFont(systemFontID, fontDescription);
+    auto fontDescription = RenderTheme::singleton().systemFont(systemFontID);
     if (!fontDescription.isAbsoluteSize())
         return false;
 

Source/WebCore/page/linux/ResourceUsageOverlayLinux.cpp

@@ -75,8 +75,7 @@ class ResourceUsageOverlayPainter final : public GraphicsLayerClient {
     ResourceUsageOverlayPainter(ResourceUsageOverlay& overlay)
         : m_overlay(overlay)
     {
-        FontCascadeDescription fontDescription;
-        RenderTheme::singleton().systemFont(CSSValueMessageBox, fontDescription);
+        auto fontDescription = RenderTheme::singleton().systemFont(CSSValueMessageBox);
         fontDescription.setComputedSize(gFontSize);
         m_textFont = FontCascade(WTFMove(fontDescription), 0, 0);
         m_textFont.update(nullptr);

Source/WebCore/rendering/RenderEmbeddedObject.cpp

@@ -2,7 +2,7 @@
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2000 Simon Hausmann <hausmann@kde.org>
  *           (C) 2000 Stefan Schimanski (1Stein@gmx.de)
- * Copyright (C) 2004, 2005, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -304,8 +304,7 @@ void RenderEmbeddedObject::getReplacementTextGeometry(const LayoutPoint& accumul
     contentRect = contentBoxRect();
     contentRect.moveBy(roundedIntPoint(accumulatedOffset));
 
-    FontCascadeDescription fontDescription;
-    RenderTheme::singleton().systemFont(CSSValueWebkitSmallControl, fontDescription);
+    auto fontDescription = RenderTheme::singleton().systemFont(CSSValueWebkitSmallControl);
     fontDescription.setWeight(boldWeightValue());
     fontDescription.setRenderingMode(settings().fontRenderingMode());
     fontDescription.setComputedSize(12);

Source/WebCore/rendering/RenderTheme.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2022 Apple Inc. All rights reserved.
  * Copyright (C) 2014 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ -1324,45 +1324,6 @@ auto RenderTheme::colorCache(OptionSet<StyleColorOptions> options) const -> Colo
     }).iterator->value;
 }
 
-FontCascadeDescription& RenderTheme::cachedSystemFontDescription(CSSValueID systemFontID) const
-{
-    static NeverDestroyed<std::array<FontCascadeDescription, 10>> fontDescriptions;
-    switch (systemFontID) {
-    case CSSValueCaption:
-        return fontDescriptions.get()[0];
-    case CSSValueIcon:
-        return fontDescriptions.get()[1];
-    case CSSValueMenu:
-        return fontDescriptions.get()[2];
-    case CSSValueMessageBox:
-        return fontDescriptions.get()[3];
-    case CSSValueSmallCaption:
-        return fontDescriptions.get()[4];
-    case CSSValueStatusBar:
-        return fontDescriptions.get()[5];
-    case CSSValueWebkitMiniControl:
-        return fontDescriptions.get()[6];
-    case CSSValueWebkitSmallControl:
-        return fontDescriptions.get()[7];
-    case CSSValueWebkitControl:
-        return fontDescriptions.get()[8];
-    case CSSValueNone:
-        return fontDescriptions.get()[9];
-    default:
-        ASSERT_NOT_REACHED();
-        return fontDescriptions.get()[9];
-    }
-}
-
-void RenderTheme::systemFont(CSSValueID systemFontID, FontCascadeDescription& fontDescription) const
-{
-    fontDescription = cachedSystemFontDescription(systemFontID);
-    if (fontDescription.isAbsoluteSize())
-        return;
-
-    updateCachedSystemFontDescription(systemFontID, fontDescription);
-}
-
 Color RenderTheme::systemColor(CSSValueID cssValueId, OptionSet<StyleColorOptions> options) const
 {
     switch (cssValueId) {

Source/WebCore/rendering/RenderTheme.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -187,7 +187,7 @@ class RenderTheme {
     virtual Seconds caretBlinkInterval() const { return 500_ms; }
 
     // System fonts and colors for CSS.
-    void systemFont(CSSValueID, FontCascadeDescription&) const;
+    virtual FontCascadeDescription systemFont(CSSValueID) const = 0;
     virtual Color systemColor(CSSValueID, OptionSet<StyleColorOptions>) const;
 
     virtual int minimumMenuListSize(const RenderStyle&) const { return 0; }
@@ -256,8 +256,6 @@ class RenderTheme {
 
 protected:
     virtual bool canPaint(const PaintInfo&, const Settings&) const { return true; }
-    virtual FontCascadeDescription& cachedSystemFontDescription(CSSValueID systemFontID) const;
-    virtual void updateCachedSystemFontDescription(CSSValueID systemFontID, FontCascadeDescription&) const = 0;
 
     // The platform selection color.
     virtual Color platformActiveSelectionBackgroundColor(OptionSet<StyleColorOptions>) const;

Source/WebCore/rendering/RenderThemeAdwaita.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2020 Igalia S.L.
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -55,7 +56,7 @@ class RenderThemeAdwaita : public RenderTheme {
     bool supportsListBoxSelectionForegroundColors(OptionSet<StyleColorOptions>) const final { return true; }
     bool shouldHaveCapsLockIndicator(const HTMLInputElement&) const final;
 
-    void updateCachedSystemFontDescription(CSSValueID, FontCascadeDescription&) const override { };
+    FontCascadeDescription systemFont(CSSValueID) const override { return FontCascadeDescription(); };
 
     Color platformActiveSelectionBackgroundColor(OptionSet<StyleColorOptions>) const final;
     Color platformInactiveSelectionBackgroundColor(OptionSet<StyleColorOptions>) const final;

Source/WebCore/rendering/RenderThemeCocoa.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -48,8 +48,7 @@ class RenderThemeCocoa : public RenderTheme {
     bool paintApplePayButton(const RenderObject&, const PaintInfo&, const IntRect&) override;
 #endif
 
-    FontCascadeDescription& cachedSystemFontDescription(CSSValueID systemFontID) const override;
-    void updateCachedSystemFontDescription(CSSValueID systemFontID, FontCascadeDescription&) const override;
+    FontCascadeDescription systemFont(CSSValueID systemFontID) const override;
 
 #if ENABLE(VIDEO) && ENABLE(MODERN_MEDIA_CONTROLS)
     String mediaControlsStyleSheet() override;

Source/WebCore/rendering/RenderThemeCocoa.mm

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -176,54 +176,6 @@ @implementation WebCoreRenderThemeBundle
 
 #endif // ENABLE(VIDEO) && ENABLE(MODERN_MEDIA_CONTROLS)
 
-FontCascadeDescription& RenderThemeCocoa::cachedSystemFontDescription(CSSValueID valueID) const
-{
-    static NeverDestroyed<std::array<FontCascadeDescription, 17>> fontDescriptions;
-
-    ASSERT(std::all_of(std::begin(fontDescriptions.get()), std::end(fontDescriptions.get()), [](auto& description) {
-        return !description.isAbsoluteSize();
-    }));
-
-    switch (valueID) {
-    case CSSValueAppleSystemHeadline:
-        return fontDescriptions.get()[0];
-    case CSSValueAppleSystemBody:
-        return fontDescriptions.get()[1];
-    case CSSValueAppleSystemTitle0:
-        return fontDescriptions.get()[2];
-    case CSSValueAppleSystemTitle1:
-        return fontDescriptions.get()[3];
-    case CSSValueAppleSystemTitle2:
-        return fontDescriptions.get()[4];
-    case CSSValueAppleSystemTitle3:
-        return fontDescriptions.get()[5];
-    case CSSValueAppleSystemTitle4:
-        return fontDescriptions.get()[6];
-    case CSSValueAppleSystemSubheadline:
-        return fontDescriptions.get()[7];
-    case CSSValueAppleSystemFootnote:
-        return fontDescriptions.get()[8];
-    case CSSValueAppleSystemCaption1:
-        return fontDescriptions.get()[9];
-    case CSSValueAppleSystemCaption2:
-        return fontDescriptions.get()[10];
-    case CSSValueAppleSystemShortHeadline:
-        return fontDescriptions.get()[11];
-    case CSSValueAppleSystemShortBody:
-        return fontDescriptions.get()[12];
-    case CSSValueAppleSystemShortSubheadline:
-        return fontDescriptions.get()[13];
-    case CSSValueAppleSystemShortFootnote:
-        return fontDescriptions.get()[14];
-    case CSSValueAppleSystemShortCaption1:
-        return fontDescriptions.get()[15];
-    case CSSValueAppleSystemTallBody:
-        return fontDescriptions.get()[16];
-    default:
-        return RenderTheme::cachedSystemFontDescription(valueID);
-    }
-}
-
 static inline FontSelectionValue cssWeightOfSystemFont(CTFontRef font)
 {
     auto resultRef = adoptCF(static_cast<CFNumberRef>(CTFontCopyAttribute(font, kCTFontCSSWeightAttribute)));
@@ -243,7 +195,7 @@ static inline FontSelectionValue cssWeightOfSystemFont(CTFontRef font)
     return FontSelectionValue(900);
 }
 
-void RenderThemeCocoa::updateCachedSystemFontDescription(CSSValueID valueID, FontCascadeDescription& fontDescription) const
+FontCascadeDescription RenderThemeCocoa::systemFont(CSSValueID valueID) const
 {
     auto cocoaFontClass = [] {
 #if PLATFORM(IOS_FAMILY)
@@ -380,11 +332,13 @@ static inline FontSelectionValue cssWeightOfSystemFont(CTFontRef font)
 
     ASSERT(fontDescriptor);
     auto font = adoptCF(CTFontCreateWithFontDescriptor(fontDescriptor.get(), 0, nullptr));
+    FontCascadeDescription fontDescription;
     fontDescription.setIsAbsoluteSize(true);
     fontDescription.setOneFamily(style);
     fontDescription.setSpecifiedSize(CTFontGetSize(font.get()));
     fontDescription.setWeight(cssWeightOfSystemFont(font.get()));
     fontDescription.setItalic(normalItalicValue());
+    return fontDescription;
 }
 
 }

Source/WebCore/rendering/RenderThemeGtk.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2020 Igalia S.L.
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -36,22 +37,23 @@ RenderTheme& RenderTheme::singleton()
     return theme;
 }
 
-void RenderThemeGtk::updateCachedSystemFontDescription(CSSValueID, FontCascadeDescription& fontDescription) const
+FontCascadeDescription RenderThemeGtk::systemFont(CSSValueID) const
 {
     GtkSettings* settings = gtk_settings_get_default();
     if (!settings)
-        return;
+        return FontCascadeDescription();
 
     // This will be a font selection string like "Sans 10" so we cannot use it as the family name.
     GUniqueOutPtr<gchar> fontName;
     g_object_get(settings, "gtk-font-name", &fontName.outPtr(), nullptr);
     if (!fontName || !fontName.get()[0])
-        return;
+        return FontCascadeDescription();
 
     PangoFontDescription* pangoDescription = pango_font_description_from_string(fontName.get());
     if (!pangoDescription)
-        return;
+        return FontCascadeDescription();
 
+    FontCascadeDescription fontDescription;
     fontDescription.setOneFamily(AtomString::fromLatin1(pango_font_description_get_family(pangoDescription)));
 
     int size = pango_font_description_get_size(pangoDescription) / PANGO_SCALE;
@@ -64,6 +66,7 @@ void RenderThemeGtk::updateCachedSystemFontDescription(CSSValueID, FontCascadeDe
     fontDescription.setWeight(normalWeightValue());
     fontDescription.setItalic(FontSelectionValue());
     pango_font_description_free(pangoDescription);
+    return fontDescription;
 }
 
 Seconds RenderThemeGtk::caretBlinkInterval() const

Source/WebCore/rendering/RenderThemeGtk.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2020 Igalia S.L.
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -28,7 +29,7 @@ class RenderThemeGtk final : public RenderThemeAdwaita {
 private:
     virtual ~RenderThemeGtk() = default;
 
-    void updateCachedSystemFontDescription(CSSValueID, FontCascadeDescription&) const override;
+    FontCascadeDescription systemFont(CSSValueID) const override;
     Seconds caretBlinkInterval() const override;
 };
 

Source/WebCore/rendering/RenderThemePlayStation.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2018 Sony Interactive Entertainment Inc.
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -36,9 +37,10 @@ RenderTheme& RenderTheme::singleton()
     return theme;
 }
 
-void RenderThemePlayStation::updateCachedSystemFontDescription(CSSValueID, FontCascadeDescription&) const
+FontCascadeDescription RenderThemePlayStation::systemFont(CSSValueID) const
 {
     notImplemented();
+    return FontCascadeDescription();
 }
 
 } // namespace WebCore

Source/WebCore/rendering/RenderThemePlayStation.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2018 Sony Interactive Entertainment Inc.
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -34,7 +35,7 @@ class RenderThemePlayStation final : public RenderTheme {
     friend NeverDestroyed<RenderThemePlayStation>;
 
 private:
-    void updateCachedSystemFontDescription(CSSValueID systemFontID, FontCascadeDescription&) const final;
+    FontCascadeDescription systemFont(CSSValueID systemFontID) const final;
 };
 
 } // namespace WebCore

Source/WebCore/rendering/RenderThemeWin.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2022 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Kenneth Rohde Christiansen
  *
  * This library is free software; you can redistribute it and/or
@@ -308,16 +308,7 @@ Color RenderThemeWin::platformInactiveSelectionForegroundColor(OptionSet<StyleCo
     return platformActiveSelectionForegroundColor(options);
 }
 
-static void fillFontDescription(FontCascadeDescription& fontDescription, LOGFONT& logFont, float fontSize)
-{    
-    fontDescription.setIsAbsoluteSize(true);
-    fontDescription.setOneFamily(logFont.lfFaceName);
-    fontDescription.setSpecifiedSize(fontSize);
-    fontDescription.setWeight(logFont.lfWeight >= 700 ? boldWeightValue() : normalWeightValue()); // FIXME: Use real weight.
-    fontDescription.setIsItalic(logFont.lfItalic);
-}
-
-void RenderThemeWin::updateCachedSystemFontDescription(CSSValueID valueID, FontCascadeDescription& fontDescription) const
+FontCascadeDescription RenderThemeWin::systemFont(CSSValueID valueID) const
 {
     static bool initialized;
     static NONCLIENTMETRICS ncm;
@@ -357,12 +348,18 @@ void RenderThemeWin::updateCachedSystemFontDescription(CSSValueID valueID, FontC
     default: { // Everything else uses the stock GUI font.
         HGDIOBJ hGDI = ::GetStockObject(DEFAULT_GUI_FONT);
         if (!hGDI)
-            return;
+            return FontCascadeDescription();
         if (::GetObject(hGDI, sizeof(logFont), &logFont) <= 0)
-            return;
+            return FontCascadeDescription();
     }
     }
-    fillFontDescription(fontDescription, logFont, shouldUseDefaultControlFontPixelSize ? defaultControlFontPixelSize : abs(logFont.lfHeight));
+    FontCascadeDescription fontDescription;
+    fontDescription.setIsAbsoluteSize(true);
+    fontDescription.setOneFamily(logFont.lfFaceName);
+    fontDescription.setSpecifiedSize(shouldUseDefaultControlFontPixelSize ? defaultControlFontPixelSize : abs(logFont.lfHeight));
+    fontDescription.setWeight(logFont.lfWeight >= 700 ? boldWeightValue() : normalWeightValue()); // FIXME: Use real weight.
+    fontDescription.setIsItalic(logFont.lfItalic);
+    return fontDescription;
 }
 
 bool RenderThemeWin::supportsFocus(ControlPart appearance) const

Source/WebCore/rendering/RenderThemeWin.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2022 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Kenneth Rohde Christiansen
  *
  * This library is free software; you can redistribute it and/or
@@ -140,7 +140,7 @@ class RenderThemeWin final : public RenderTheme {
     virtual ~RenderThemeWin();
 
     // System fonts.
-    void updateCachedSystemFontDescription(CSSValueID, FontCascadeDescription&) const override;
+    FontCascadeDescription systemFont(CSSValueID) const override;
 
     void addIntrinsicMargins(RenderStyle&) const;
     void close();