From ea57b5ffb7643d633e235860ded4995980962eeb Mon Sep 17 00:00:00 2001 From: "antti@apple.com" Date: Tue, 12 Nov 2019 19:32:28 +0000 Subject: [PATCH] Skip matched declarations cache only for length resolution affecting font properties https://bugs.webkit.org/show_bug.cgi?id=204098 Reviewed by Zalan Bujtas. * css/CSSPrimitiveValue.cpp: (WebCore::CSSPrimitiveValue::equalForLengthResolution): Put this next to the length resolution function, hopefully helping to keep them in sync. * css/CSSPrimitiveValue.h: * css/StyleResolver.cpp: (WebCore::StyleResolver::applyMatchedProperties): Replace test for font declaration change with a narrower test that only looks for those properties that affect length resolution. * style/MatchedDeclarationsCache.cpp: (WebCore::Style::MatchedDeclarationsCache::Entry::isUsableAfterHighPriorityProperties const): Factor into function. * style/MatchedDeclarationsCache.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@252370 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 25 +++++++++++++++++++ Source/WebCore/css/CSSPrimitiveValue.cpp | 19 ++++++++++++++ Source/WebCore/css/CSSPrimitiveValue.h | 2 ++ Source/WebCore/css/StyleResolver.cpp | 12 ++++----- .../style/MatchedDeclarationsCache.cpp | 8 ++++++ .../WebCore/style/MatchedDeclarationsCache.h | 2 ++ 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index d4dc19345308b..fab771bb6650e 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,28 @@ +2019-11-12 Antti Koivisto + + Skip matched declarations cache only for length resolution affecting font properties + https://bugs.webkit.org/show_bug.cgi?id=204098 + + Reviewed by Zalan Bujtas. + + * css/CSSPrimitiveValue.cpp: + (WebCore::CSSPrimitiveValue::equalForLengthResolution): + + Put this next to the length resolution function, hopefully helping to keep them in sync. + + * css/CSSPrimitiveValue.h: + * css/StyleResolver.cpp: + (WebCore::StyleResolver::applyMatchedProperties): + + Replace test for font declaration change with a narrower test that only looks for those properties that affect length resolution. + + * style/MatchedDeclarationsCache.cpp: + (WebCore::Style::MatchedDeclarationsCache::Entry::isUsableAfterHighPriorityProperties const): + + Factor into function. + + * style/MatchedDeclarationsCache.h: + 2019-11-12 Peng Liu Picture-in-Picture events are not fired if we switch the Picture-in-Picture mode through modern media controls diff --git a/Source/WebCore/css/CSSPrimitiveValue.cpp b/Source/WebCore/css/CSSPrimitiveValue.cpp index 38fd6da08f81e..9da5be68a3453 100644 --- a/Source/WebCore/css/CSSPrimitiveValue.cpp +++ b/Source/WebCore/css/CSSPrimitiveValue.cpp @@ -705,6 +705,25 @@ double CSSPrimitiveValue::computeNonCalcLengthDouble(const CSSToLengthConversion return result; } +bool CSSPrimitiveValue::equalForLengthResolution(const RenderStyle& styleA, const RenderStyle& styleB) +{ + // These properties affect results of computeNonCalcLengthDouble above. + if (styleA.fontDescription().computedSize() != styleB.fontDescription().computedSize()) + return false; + if (styleA.fontDescription().specifiedSize() != styleB.fontDescription().specifiedSize()) + return false; + + if (styleA.fontMetrics().xHeight() != styleB.fontMetrics().xHeight()) + return false; + if (styleA.fontMetrics().zeroWidth() != styleB.fontMetrics().zeroWidth()) + return false; + + if (styleA.zoom() != styleB.zoom()) + return false; + + return true; +} + ExceptionOr CSSPrimitiveValue::setFloatValue(unsigned short, double) { // Keeping values immutable makes optimizations easier and allows sharing of the primitive value objects. diff --git a/Source/WebCore/css/CSSPrimitiveValue.h b/Source/WebCore/css/CSSPrimitiveValue.h index 9f954312da037..ad4a0513c3fe2 100644 --- a/Source/WebCore/css/CSSPrimitiveValue.h +++ b/Source/WebCore/css/CSSPrimitiveValue.h @@ -278,6 +278,8 @@ class CSSPrimitiveValue final : public CSSValue { static double conversionToCanonicalUnitsScaleFactor(UnitType); static double computeNonCalcLengthDouble(const CSSToLengthConversionData&, UnitType, double value); + // True if computeNonCalcLengthDouble would produce identical results when resolved against both these styles. + static bool equalForLengthResolution(const RenderStyle&, const RenderStyle&); Ref createDeprecatedCSSOMPrimitiveWrapper(CSSStyleDeclaration&) const; diff --git a/Source/WebCore/css/StyleResolver.cpp b/Source/WebCore/css/StyleResolver.cpp index 7507bb6e5de7f..6443ec32bda38 100644 --- a/Source/WebCore/css/StyleResolver.cpp +++ b/Source/WebCore/css/StyleResolver.cpp @@ -584,13 +584,11 @@ void StyleResolver::applyMatchedProperties(State& state, const MatchResult& matc // High priority properties may affect resolution of other properties (they are mostly font related). builder.applyHighPriorityProperties(); - // If the effective zoom value changes, we can't use the matched properties cache. Start over. - if (cacheEntry && cacheEntry->renderStyle->effectiveZoom() != style.effectiveZoom()) - return applyMatchedProperties(state, matchResult, UseMatchedDeclarationsCache::No); - - // If the font changed, we can't use the matched properties cache. Start over. - if (cacheEntry && cacheEntry->renderStyle->fontDescription() != style.fontDescription()) - return applyMatchedProperties(state, matchResult, UseMatchedDeclarationsCache::No); + if (cacheEntry && !cacheEntry->isUsableAfterHighPriorityProperties(style)) { + // We need to resolve all properties without caching. + applyMatchedProperties(state, matchResult, UseMatchedDeclarationsCache::No); + return; + } builder.applyLowPriorityProperties(); diff --git a/Source/WebCore/style/MatchedDeclarationsCache.cpp b/Source/WebCore/style/MatchedDeclarationsCache.cpp index 639d04ce69b10..73e9e6fca3ec2 100644 --- a/Source/WebCore/style/MatchedDeclarationsCache.cpp +++ b/Source/WebCore/style/MatchedDeclarationsCache.cpp @@ -64,6 +64,14 @@ bool MatchedDeclarationsCache::isCacheable(const Element& element, const RenderS return true; } +bool MatchedDeclarationsCache::Entry::isUsableAfterHighPriorityProperties(const RenderStyle& style) const +{ + if (style.effectiveZoom() != renderStyle->effectiveZoom()) + return false; + + return CSSPrimitiveValue::equalForLengthResolution(style, *renderStyle); +} + unsigned MatchedDeclarationsCache::computeHash(const MatchResult& matchResult) { if (!matchResult.isCacheable) diff --git a/Source/WebCore/style/MatchedDeclarationsCache.h b/Source/WebCore/style/MatchedDeclarationsCache.h index 365fa01f62c86..bdd457a852b4b 100644 --- a/Source/WebCore/style/MatchedDeclarationsCache.h +++ b/Source/WebCore/style/MatchedDeclarationsCache.h @@ -46,6 +46,8 @@ class MatchedDeclarationsCache { MatchResult matchResult; std::unique_ptr renderStyle; std::unique_ptr parentRenderStyle; + + bool isUsableAfterHighPriorityProperties(const RenderStyle&) const; }; const Entry* find(unsigned hash, const MatchResult&);