Introduce StyleFontData to store the FontCascade object #22531

smfr · 2024-01-09T05:14:17Z

`b965532`

Introduce StyleFontData to store the FontCascade object
https://bugs.webkit.org/show_bug.cgi?id=267252
rdar://120688639

FontCascade::operator== shows up on profiles under RenderStyle::diff() in the Design subtest
of MotionMark, which is animating the `color` property on many elements. `color` and
`fontCascade` are both members of `StyleInheritedData`, so we end up copying and comparing
`StyleInheritedData` a lot.

But FontCascade is large (232 bytes) and expensive to compare, so we can optimize by moving
it into its own ref-counted class that `StyleInheritedData` can own via a `DataRef`. This
approach was chosen, rather than making `FontCascade` directly ref-counted, because it's
used on the stack in various places.

Now RenderStyle can test for `fontData` pointer equality in various places before doing more
expensive checks.

This reveals some cases where we fail to issue correct style invalidations;
fast/css/ios/update-user-interface-level.html shows that we need to explicitly update
the base view background color when "elevated user interface" mode changes, so do this
via `Document::updateElementsAffectedBySystemColors()`.

* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::updateElementsAffectedBySystemColors):
(WebCore::Document::updateElementsAffectedByMediaQueries):
* Source/WebCore/dom/Document.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::appearanceDidChange):
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::hashForTextAutosizing const):
(WebCore::RenderStyle::equalForTextAutosizing const):
(WebCore::RenderStyle::changeRequiresLayout const):
(WebCore::RenderStyle::conservativelyCollectChangedAnimatableProperties const):
(WebCore::RenderStyle::fontCascade const):
(WebCore::RenderStyle::metricsOfPrimaryFont const):
(WebCore::RenderStyle::fontDescription const):
(WebCore::RenderStyle::setFontDescription):
(WebCore::RenderStyle::setLetterSpacing):
(WebCore::RenderStyle::setWordSpacing):
* Source/WebCore/rendering/style/RenderStyleInlines.h:
(WebCore::RenderStyle::letterSpacing const):
(WebCore::RenderStyle::wordSpacing const):
* Source/WebCore/rendering/style/StyleFontData.cpp: Added.
(WebCore::StyleFontData::StyleFontData):
(WebCore::StyleFontData::copy const):
(WebCore::StyleFontData::operator== const):
* Source/WebCore/rendering/style/StyleFontData.h: Added.
(WebCore::StyleFontData::create):
* Source/WebCore/rendering/style/StyleInheritedData.cpp:
(WebCore::StyleInheritedData::StyleInheritedData):
(WebCore::StyleInheritedData::nonFastPathInheritedEqual const):
* Source/WebCore/rendering/style/StyleInheritedData.h:

b965532

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	❌ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	✅ 🛠 wpe-skia
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-01-09T05:14:34Z

EWS run on previous version of this PR (hash db653db)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 gtk~~
⏳ 🛠 🧪 jsc	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
⏳ 🛠 🧪 jsc-arm64	⏳ 🛠 tv	⏳ 🧪 mac-AS-debug-wk2	~~🧪 api-gtk~~
	⏳ 🛠 tv-sim
	⏳ 🛠 watch
	✅ 🛠 watch-sim

heycam

r=me with comments addressed.

Source/WebCore/rendering/style/RenderStyle.cpp

webkit-early-warning-system · 2024-01-09T05:24:52Z

Starting EWS tests for 2744fcf. Live statuses available at the PR page, #22531

webkit-early-warning-system · 2024-01-09T05:24:52Z

EWS run on previous version of this PR (hash 2744fcf)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	❌ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 gtk
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

mdubet · 2024-01-09T05:55:22Z

Source/WebCore/rendering/style/StyleFontData.cpp

+    return adoptRef(*new StyleFontData(*this));
+}
+
+bool StyleFontData::operator==(const StyleFontData& o) const


= default ?

webkit-early-warning-system · 2024-01-09T16:44:22Z

EWS run on previous version of this PR (hash a1376ed)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	✅ 🛠 gtk
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-01-09T16:44:22Z

Starting EWS tests for a1376ed. Live statuses available at the PR page, #22531

…changes https://bugs.webkit.org/show_bug.cgi?id=267423 rdar://120854167 Reviewed by Antti Koivisto. WebKit#22531 caused the TestWebKitAPI.WKWebViewThemeColor.KVO API test to break, when changing the to the `print` media type failed to cause a change of theme color when the theme meta tag had `media=screen`. That PR reduces the frequency of style diffs caused by FontCascade differences. In the API test, this results in a change to the `print` media type no longer triggering a layout on the (empty) document. Since the layout no longer occurs, we never hit `updateStyleForLayout()`, so don't end up calling `updateElementsAffectedByMediaQueries()`. Fix by calling `document.updateElementsAffectedByMediaQueries()` from `Page::updateStyleAfterChangeInEnvironment()`, and have `WebPage::setOverriddenMediaType()` call `updateStyleAfterChangeInEnvironment()` which seems like the right thing to do. * Source/WebCore/html/HTMLMetaElement.cpp: Whitespace. * Source/WebCore/page/LocalFrameViewLayoutContext.cpp: Whitespace. * Source/WebCore/page/Page.cpp: (WebCore::Page::updateStyleAfterChangeInEnvironment): * Source/WebCore/page/Page.h: * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::setOverriddenMediaType): Canonical link: https://commits.webkit.org/272947@main

webkit-early-warning-system · 2024-01-12T05:09:39Z

Starting EWS tests for 8f8b49d. Live statuses available at the PR page, #22531

webkit-early-warning-system · 2024-01-12T05:09:39Z

EWS run on previous version of this PR (hash 8f8b49d)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	⏳ 🛠 wpe-skia
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

…y changes https://bugs.webkit.org/show_bug.cgi?id=267564 rdar://121024996 Reviewed by Antoine Quint. On iOS with WebKit#22531, fast/attachment/attachment-dynamic-type.html fails, showing that fonts failed to update after we change `contentSizeCategory()` via Internals. When the content size category changes via the user setting it in the UI, we get notified via a `UIContentSizeCategoryDidChangeNotification` notification, and in response to this we call `FontCache::invalidateAllFontCaches()` (see `fontCacheRegisteredFontsChangedNotificationCallback()`). So we need to do the same when the content size category is changed through Internals. This caused webanimations/css-animation-effect-target-change-and-get-keyframes-crash.html to crash with a null m_target, so add a null check in `computeCSSAnimationBlendingKeyframes()`. * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::computeCSSAnimationBlendingKeyframes): * Source/WebCore/platform/graphics/cocoa/FontCacheCocoa.mm: (WebCore::setContentSizeCategory): * Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp: (WebCore::FontCache::platformInit): Canonical link: https://commits.webkit.org/273070@main

webkit-early-warning-system · 2024-04-04T00:52:25Z

EWS run on previous version of this PR (hash bde6612)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	✅ 🛠 wpe-skia
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=267252 rdar://120688639 FontCascade::operator== shows up on profiles under RenderStyle::diff() in the Design subtest of MotionMark, which is animating the `color` property on many elements. `color` and `fontCascade` are both members of `StyleInheritedData`, so we end up copying and comparing `StyleInheritedData` a lot. But FontCascade is large (232 bytes) and expensive to compare, so we can optimize by moving it into its own ref-counted class that `StyleInheritedData` can own via a `DataRef`. This approach was chosen, rather than making `FontCascade` directly ref-counted, because it's used on the stack in various places. Now RenderStyle can test for `fontData` pointer equality in various places before doing more expensive checks. This reveals some cases where we fail to issue correct style invalidations; fast/css/ios/update-user-interface-level.html shows that we need to explicitly update the base view background color when "elevated user interface" mode changes, so do this via `Document::updateElementsAffectedBySystemColors()`. * Source/WebCore/Headers.cmake: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/dom/Document.cpp: (WebCore::Document::updateElementsAffectedBySystemColors): (WebCore::Document::updateElementsAffectedByMediaQueries): * Source/WebCore/dom/Document.h: * Source/WebCore/page/Page.cpp: (WebCore::Page::appearanceDidChange): * Source/WebCore/rendering/style/RenderStyle.cpp: (WebCore::RenderStyle::hashForTextAutosizing const): (WebCore::RenderStyle::equalForTextAutosizing const): (WebCore::RenderStyle::changeRequiresLayout const): (WebCore::RenderStyle::conservativelyCollectChangedAnimatableProperties const): (WebCore::RenderStyle::fontCascade const): (WebCore::RenderStyle::metricsOfPrimaryFont const): (WebCore::RenderStyle::fontDescription const): (WebCore::RenderStyle::setFontDescription): (WebCore::RenderStyle::setLetterSpacing): (WebCore::RenderStyle::setWordSpacing): * Source/WebCore/rendering/style/RenderStyleInlines.h: (WebCore::RenderStyle::letterSpacing const): (WebCore::RenderStyle::wordSpacing const): * Source/WebCore/rendering/style/StyleFontData.cpp: Added. (WebCore::StyleFontData::StyleFontData): (WebCore::StyleFontData::copy const): (WebCore::StyleFontData::operator== const): * Source/WebCore/rendering/style/StyleFontData.h: Added. (WebCore::StyleFontData::create): * Source/WebCore/rendering/style/StyleInheritedData.cpp: (WebCore::StyleInheritedData::StyleInheritedData): (WebCore::StyleInheritedData::nonFastPathInheritedEqual const): * Source/WebCore/rendering/style/StyleInheritedData.h:

webkit-early-warning-system · 2024-04-05T22:26:53Z

EWS run on current version of this PR (hash b965532)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	❌ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	✅ 🛠 wpe-skia
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

smfr self-assigned this Jan 9, 2024

smfr added the CSS Cascading Style Sheets implementation label Jan 9, 2024

heycam approved these changes Jan 9, 2024

View reviewed changes

Source/WebCore/rendering/style/RenderStyle.cpp Outdated Show resolved Hide resolved

Source/WebCore/rendering/style/RenderStyle.cpp Outdated Show resolved Hide resolved

smfr force-pushed the eng/Introduce-StyleFontData-to-store-the-FontCascade-object branch from db653db to 2744fcf Compare January 9, 2024 05:24

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 9, 2024

mdubet reviewed Jan 9, 2024

View reviewed changes

smfr removed the merging-blocked Applied to prevent a change from being merged label Jan 9, 2024

smfr force-pushed the eng/Introduce-StyleFontData-to-store-the-FontCascade-object branch from 2744fcf to a1376ed Compare January 9, 2024 16:44

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 9, 2024

smfr mentioned this pull request Jan 11, 2024

Media-query sensitive meta tags may not get updated after some style changes #22669

Merged

smfr removed the merging-blocked Applied to prevent a change from being merged label Jan 12, 2024

smfr force-pushed the eng/Introduce-StyleFontData-to-store-the-FontCascade-object branch from a1376ed to 8f8b49d Compare January 12, 2024 05:09

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 12, 2024

smfr mentioned this pull request Jan 16, 2024

Font caches need to be invalidated when the font content size category changes #22806

Merged

smfr removed the merging-blocked Applied to prevent a change from being merged label Apr 4, 2024

smfr force-pushed the eng/Introduce-StyleFontData-to-store-the-FontCascade-object branch from 8f8b49d to bde6612 Compare April 4, 2024 00:52

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 4, 2024

smfr removed the merging-blocked Applied to prevent a change from being merged label Apr 5, 2024

smfr force-pushed the eng/Introduce-StyleFontData-to-store-the-FontCascade-object branch from bde6612 to b965532 Compare April 5, 2024 22:26

smfr requested review from cdumez and rniwa as code owners April 5, 2024 22:26

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce StyleFontData to store the FontCascade object #22531

Introduce StyleFontData to store the FontCascade object #22531

smfr commented Jan 9, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Jan 9, 2024 •

edited

Loading

heycam left a comment

webkit-early-warning-system commented Jan 9, 2024

webkit-early-warning-system commented Jan 9, 2024 •

edited

Loading

mdubet Jan 9, 2024

webkit-early-warning-system commented Jan 9, 2024 •

edited

Loading

webkit-early-warning-system commented Jan 9, 2024

webkit-early-warning-system commented Jan 12, 2024

webkit-early-warning-system commented Jan 12, 2024 •

edited

Loading

webkit-early-warning-system commented Apr 4, 2024 •

edited

Loading

webkit-early-warning-system commented Apr 5, 2024 •

edited

Loading

Introduce StyleFontData to store the FontCascade object #22531

Are you sure you want to change the base?

Introduce StyleFontData to store the FontCascade object #22531

Conversation

smfr commented Jan 9, 2024 • edited by webkit-early-warning-system Loading

b965532

webkit-early-warning-system commented Jan 9, 2024 • edited Loading

heycam left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Jan 9, 2024

webkit-early-warning-system commented Jan 9, 2024 • edited Loading

mdubet Jan 9, 2024

Choose a reason for hiding this comment

webkit-early-warning-system commented Jan 9, 2024 • edited Loading

webkit-early-warning-system commented Jan 9, 2024

webkit-early-warning-system commented Jan 12, 2024

webkit-early-warning-system commented Jan 12, 2024 • edited Loading

webkit-early-warning-system commented Apr 4, 2024 • edited Loading

webkit-early-warning-system commented Apr 5, 2024 • edited Loading

smfr commented Jan 9, 2024 •

edited by webkit-early-warning-system

Loading

`b965532`

webkit-early-warning-system commented Jan 9, 2024 •

edited

Loading

webkit-early-warning-system commented Jan 9, 2024 •

edited

Loading

webkit-early-warning-system commented Jan 9, 2024 •

edited

Loading

webkit-early-warning-system commented Jan 12, 2024 •

edited

Loading

webkit-early-warning-system commented Apr 4, 2024 •

edited

Loading

webkit-early-warning-system commented Apr 5, 2024 •

edited

Loading