From 25f43abc6e10e195a41db5db0b383bef04a208ac Mon Sep 17 00:00:00 2001 From: Antti Koivisto Date: Tue, 22 Aug 2023 09:02:52 -0700 Subject: [PATCH] Add ScopedName type https://bugs.webkit.org/show_bug.cgi?id=260493 rdar://114224001 Reviewed by Alan Baradlay. Refactor animation name type to a generic struct. There are some other things in CSS that need scoped names too. * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/animation/CSSAnimation.cpp: (WebCore::CSSAnimation::CSSAnimation): * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::getKeyframes): (WebCore::KeyframeEffect::computeCSSAnimationBlendingKeyframes): * Source/WebCore/css/CSSToStyleMap.cpp: (WebCore::CSSToStyleMap::mapAnimationName): * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::valueForScopedName): (WebCore::ComputedStyleExtractor::addValueForAnimationPropertyToList): (WebCore::valueForAnimationName): Deleted. * Source/WebCore/platform/animation/Animation.cpp: (WebCore::Animation::Animation): (WebCore::Animation::animationsMatch const): (WebCore::Animation::initialName const): (WebCore::operator<<): * Source/WebCore/platform/animation/Animation.h: (WebCore::Animation::isValidAnimation const): (WebCore::Animation::name const): (WebCore::Animation::setName): (WebCore::Animation::nameStyleScopeOrdinal const): Deleted. * Source/WebCore/style/ScopedName.h: Added. * Source/WebCore/style/Styleable.cpp: (WebCore::keyframesRuleExistsForAnimation): (WebCore::Styleable::animationListContainsNewlyValidAnimation const): (WebCore::Styleable::updateCSSAnimations const): Canonical link: https://commits.webkit.org/267135@main --- Source/WebCore/Headers.cmake | 1 + .../WebCore/WebCore.xcodeproj/project.pbxproj | 4 ++ Source/WebCore/animation/CSSAnimation.cpp | 2 +- Source/WebCore/animation/KeyframeEffect.cpp | 6 +-- Source/WebCore/css/CSSToStyleMap.cpp | 2 +- Source/WebCore/css/ComputedStyleExtractor.cpp | 10 ++--- .../WebCore/platform/animation/Animation.cpp | 10 ++--- Source/WebCore/platform/animation/Animation.h | 21 +++------ Source/WebCore/style/ScopedName.h | 43 +++++++++++++++++++ Source/WebCore/style/Styleable.cpp | 6 +-- 10 files changed, 71 insertions(+), 34 deletions(-) create mode 100644 Source/WebCore/style/ScopedName.h diff --git a/Source/WebCore/Headers.cmake b/Source/WebCore/Headers.cmake index 5a891be45f0c..44f3fb7726a5 100644 --- a/Source/WebCore/Headers.cmake +++ b/Source/WebCore/Headers.cmake @@ -2399,6 +2399,7 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS storage/StorageType.h storage/StorageUtilities.h + style/ScopedName.h style/StyleAppearance.h style/StyleChange.h style/StyleScope.h diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj index b4ea080abb0a..ffbc7d707a31 100644 --- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj +++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj @@ -5845,6 +5845,7 @@ E3D3C00B2881D90B00ECD5F6 /* GPUAvailabilityMetal.mm in Sources */ = {isa = PBXBuildFile; fileRef = E3D3C00A2881D90B00ECD5F6 /* GPUAvailabilityMetal.mm */; }; E3E4E2A81E3B17100023BB8A /* ScriptElementCachedScriptFetcher.h in Headers */ = {isa = PBXBuildFile; fileRef = E3E4E2A61E3B16FC0023BB8A /* ScriptElementCachedScriptFetcher.h */; settings = {ATTRIBUTES = (Private, ); }; }; E3FA38641D71812D00AA5950 /* PendingScriptClient.h in Headers */ = {isa = PBXBuildFile; fileRef = E3FA38611D716E7600AA5950 /* PendingScriptClient.h */; }; + E401B3632A942E3F0018B557 /* ScopedName.h in Headers */ = {isa = PBXBuildFile; fileRef = E401B3622A942E3E0018B557 /* ScopedName.h */; settings = {ATTRIBUTES = (Private, ); }; }; E401C27517CE53EC00C41A35 /* ElementIteratorAssertions.h in Headers */ = {isa = PBXBuildFile; fileRef = E401C27417CE53EC00C41A35 /* ElementIteratorAssertions.h */; settings = {ATTRIBUTES = (Private, ); }; }; E401E0A41C3C0B8300F34D10 /* StyleChange.h in Headers */ = {isa = PBXBuildFile; fileRef = E401E0A31C3C0B8300F34D10 /* StyleChange.h */; settings = {ATTRIBUTES = (Private, ); }; }; E403B7A2251B11930019E800 /* LayoutIntegrationCoverage.h in Headers */ = {isa = PBXBuildFile; fileRef = E403B7A1251B11930019E800 /* LayoutIntegrationCoverage.h */; }; @@ -19101,6 +19102,7 @@ E3E4E2A51E3B16FC0023BB8A /* ScriptElementCachedScriptFetcher.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ScriptElementCachedScriptFetcher.cpp; sourceTree = ""; }; E3E4E2A61E3B16FC0023BB8A /* ScriptElementCachedScriptFetcher.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ScriptElementCachedScriptFetcher.h; sourceTree = ""; }; E3FA38611D716E7600AA5950 /* PendingScriptClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PendingScriptClient.h; sourceTree = ""; }; + E401B3622A942E3E0018B557 /* ScopedName.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ScopedName.h; sourceTree = ""; }; E401C27417CE53EC00C41A35 /* ElementIteratorAssertions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ElementIteratorAssertions.h; sourceTree = ""; }; E401E0A31C3C0B8300F34D10 /* StyleChange.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StyleChange.h; sourceTree = ""; }; E401E0A51C3C0CF700F34D10 /* StyleChange.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = StyleChange.cpp; sourceTree = ""; }; @@ -34511,6 +34513,7 @@ A79BADA0161E7F3F00C2E652 /* RuleSet.h */, E47CEBEB2709DFDC00B8D8F5 /* RuleSetBuilder.cpp */, E47CEBE82709DFD000B8D8F5 /* RuleSetBuilder.h */, + E401B3622A942E3E0018B557 /* ScopedName.h */, E4643288273D2E7C005B756B /* SelectorMatchingState.h */, 7173DEBD2614DF040097DF32 /* Styleable.cpp */, 713922BC2518AB70005DB3C2 /* Styleable.h */, @@ -40652,6 +40655,7 @@ 5DFE8F570D16477C0076E937 /* ScheduledAction.h in Headers */, 9BD0BF9312A42BF50072FD43 /* ScopedEventQueue.h in Headers */, 7B7311FB25C092B7003B2796 /* ScopedHighPerformanceGPURequest.h in Headers */, + E401B3632A942E3F0018B557 /* ScopedName.h in Headers */, BCEC01BE0C274DAC009F4EC9 /* Screen.h in Headers */, 070BED98273F415D00583926 /* ScreenCaptureKitCaptureSource.h in Headers */, 073955BB27AB277F009A08D2 /* ScreenCaptureKitSharingSessionManager.h in Headers */, diff --git a/Source/WebCore/animation/CSSAnimation.cpp b/Source/WebCore/animation/CSSAnimation.cpp index 4179cc66a316..5e57069e856c 100644 --- a/Source/WebCore/animation/CSSAnimation.cpp +++ b/Source/WebCore/animation/CSSAnimation.cpp @@ -49,7 +49,7 @@ Ref CSSAnimation::create(const Styleable& owningElement, const Ani CSSAnimation::CSSAnimation(const Styleable& element, const Animation& backingAnimation) : DeclarativeAnimation(element, backingAnimation) - , m_animationName(backingAnimation.name().string) + , m_animationName(backingAnimation.name().name) { } diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp index 8c04c3b6e6ce..64e078dbacca 100644 --- a/Source/WebCore/animation/KeyframeEffect.cpp +++ b/Source/WebCore/animation/KeyframeEffect.cpp @@ -680,7 +680,7 @@ auto KeyframeEffect::getKeyframes(Document& document) -> Vector(*animation()).backingAnimation(); - auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.nameStyleScopeOrdinal()); + auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.name().scopeOrdinal); if (!styleScope) return { }; @@ -1072,8 +1072,8 @@ void KeyframeEffect::computeCSSAnimationBlendingKeyframes(const RenderStyle& una auto& backingAnimation = downcast(*animation()).backingAnimation(); - KeyframeList keyframeList(AtomString { backingAnimation.name().string }); - if (auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.nameStyleScopeOrdinal())) + KeyframeList keyframeList(AtomString { backingAnimation.name().name }); + if (auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.name().scopeOrdinal)) styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList); // Ensure resource loads for all the frames. diff --git a/Source/WebCore/css/CSSToStyleMap.cpp b/Source/WebCore/css/CSSToStyleMap.cpp index 268ba525137b..d668d382a3eb 100644 --- a/Source/WebCore/css/CSSToStyleMap.cpp +++ b/Source/WebCore/css/CSSToStyleMap.cpp @@ -396,7 +396,7 @@ void CSSToStyleMap::mapAnimationName(Animation& layer, const CSSValue& value) if (primitiveValue.valueID() == CSSValueNone) layer.setIsNoneAnimation(true); else - layer.setName({ primitiveValue.stringValue(), primitiveValue.isCustomIdent() }, m_builderState.styleScopeOrdinal()); + layer.setName({ AtomString { primitiveValue.stringValue() }, primitiveValue.isCustomIdent(), m_builderState.styleScopeOrdinal() }); } void CSSToStyleMap::mapAnimationPlayState(Animation& layer, const CSSValue& value) diff --git a/Source/WebCore/css/ComputedStyleExtractor.cpp b/Source/WebCore/css/ComputedStyleExtractor.cpp index a238c8bc8f09..1933255e93b2 100644 --- a/Source/WebCore/css/ComputedStyleExtractor.cpp +++ b/Source/WebCore/css/ComputedStyleExtractor.cpp @@ -1532,11 +1532,11 @@ static Ref valueForAnimationPlayState(AnimationPlayState play RELEASE_ASSERT_NOT_REACHED(); } -static Ref valueForAnimationName(const Animation::Name& name) +static Ref valueForScopedName(const Style::ScopedName& scopedName) { - if (name.isIdentifier) - return CSSPrimitiveValue::createCustomIdent(name.string); - return CSSPrimitiveValue::create(name.string); + if (scopedName.isIdentifier) + return CSSPrimitiveValue::createCustomIdent(scopedName.name); + return CSSPrimitiveValue::create(scopedName.name); } static Ref valueForAnimationTimingFunction(const TimingFunction& timingFunction) @@ -1605,7 +1605,7 @@ void ComputedStyleExtractor::addValueForAnimationPropertyToList(CSSValueListBuil if (!animation || !animation->isPlayStateFilled()) list.append(valueForAnimationPlayState(animation ? animation->playState() : Animation::initialPlayState())); } else if (property == CSSPropertyAnimationName) - list.append(valueForAnimationName(animation ? animation->name() : Animation::initialName())); + list.append(valueForScopedName(animation ? animation->name() : Animation::initialName())); else if (property == CSSPropertyAnimationComposition) { if (!animation || !animation->isCompositeOperationFilled()) list.append(valueForAnimationComposition(animation ? animation->compositeOperation() : Animation::initialCompositeOperation())); diff --git a/Source/WebCore/platform/animation/Animation.cpp b/Source/WebCore/platform/animation/Animation.cpp index 5ccc88cd8b98..ee240e6ed132 100644 --- a/Source/WebCore/platform/animation/Animation.cpp +++ b/Source/WebCore/platform/animation/Animation.cpp @@ -70,7 +70,6 @@ Animation::Animation(const Animation& o) , m_delay(o.m_delay) , m_duration(o.m_duration) , m_timingFunction(o.m_timingFunction) - , m_nameStyleScopeOrdinal(o.m_nameStyleScopeOrdinal) , m_direction(o.m_direction) , m_fillMode(o.m_fillMode) , m_playState(o.m_playState) @@ -102,7 +101,7 @@ Animation::~Animation() = default; bool Animation::animationsMatch(const Animation& other, bool matchProperties) const { - bool result = m_name.string == other.m_name.string + bool result = m_name == other.m_name && m_playState == other.m_playState && m_compositeOperation == other.m_compositeOperation && m_playStateSet == other.m_playStateSet @@ -110,7 +109,6 @@ bool Animation::animationsMatch(const Animation& other, bool matchProperties) co && m_delay == other.m_delay && m_duration == other.m_duration && *(m_timingFunction.get()) == *(other.m_timingFunction.get()) - && m_nameStyleScopeOrdinal == other.m_nameStyleScopeOrdinal && m_direction == other.m_direction && m_fillMode == other.m_fillMode && m_delaySet == other.m_delaySet @@ -129,9 +127,9 @@ bool Animation::animationsMatch(const Animation& other, bool matchProperties) co return !matchProperties || (m_property.mode == other.m_property.mode && m_property.animatableProperty == other.m_property.animatableProperty && m_propertySet == other.m_propertySet); } -auto Animation::initialName() -> const Name& +auto Animation::initialName() -> const Style::ScopedName& { - static NeverDestroyed initialValue { Name { noneAtom(), true } }; + static NeverDestroyed initialValue { Style::ScopedName { noneAtom() } }; return initialValue; } @@ -160,7 +158,7 @@ TextStream& operator<<(TextStream& ts, Animation::Direction direction) TextStream& operator<<(TextStream& ts, const Animation& animation) { ts.dumpProperty("property", animation.property()); - ts.dumpProperty("name", animation.name().string); + ts.dumpProperty("name", animation.name().name); ts.dumpProperty("iteration count", animation.iterationCount()); ts.dumpProperty("delay", animation.iterationCount()); ts.dumpProperty("duration", animation.duration()); diff --git a/Source/WebCore/platform/animation/Animation.h b/Source/WebCore/platform/animation/Animation.h index 54bf9f435006..f66867ae95ff 100644 --- a/Source/WebCore/platform/animation/Animation.h +++ b/Source/WebCore/platform/animation/Animation.h @@ -27,7 +27,7 @@ #include "CSSPropertyNames.h" #include "CompositeOperation.h" #include "RenderStyleConstants.h" -#include "StyleScopeOrdinal.h" +#include "ScopedName.h" #include "TimingFunction.h" #include "WebAnimationTypes.h" @@ -56,7 +56,7 @@ class Animation : public RefCounted { // We can make placeholder Animation objects to keep the comma-separated lists // of properties in sync. isValidAnimation means this is not a placeholder. - bool isValidAnimation() const { return !m_isNone && !m_name.string.isEmpty(); } + bool isValidAnimation() const { return !m_isNone && !m_name.name.isEmpty(); } bool isEmpty() const { @@ -125,15 +125,9 @@ class Animation : public RefCounted { double duration() const { return m_duration; } double playbackRate() const { return m_playbackRate; } - struct Name { - String string; - bool isIdentifier { false }; - }; - static constexpr double IterationCountInfinite = -1; double iterationCount() const { return m_iterationCount; } - const Name& name() const { return m_name; } - Style::ScopeOrdinal nameStyleScopeOrdinal() const { return m_nameStyleScopeOrdinal; } + const Style::ScopedName& name() const { return m_name; } AnimationPlayState playState() const { return static_cast(m_playState); } TransitionProperty property() const { return m_property; } TimingFunction* timingFunction() const { return m_timingFunction.get(); } @@ -145,10 +139,9 @@ class Animation : public RefCounted { void setPlaybackRate(double d) { m_playbackRate = d; } void setFillMode(AnimationFillMode f) { m_fillMode = static_cast(f); m_fillModeSet = true; } void setIterationCount(double c) { m_iterationCount = c; m_iterationCountSet = true; } - void setName(const Name& name, Style::ScopeOrdinal scope = Style::ScopeOrdinal::Element) + void setName(const Style::ScopedName& name) { m_name = name; - m_nameStyleScopeOrdinal = scope; m_nameSet = true; } void setPlayState(AnimationPlayState d) { m_playState = static_cast(d); m_playStateSet = true; } @@ -197,7 +190,7 @@ class Animation : public RefCounted { // Packs with m_refCount from the base class. TransitionProperty m_property { TransitionMode::All, CSSPropertyInvalid }; - Name m_name; + Style::ScopedName m_name; double m_iterationCount; double m_delay; double m_duration; @@ -205,8 +198,6 @@ class Animation : public RefCounted { RefPtr m_timingFunction; RefPtr m_defaultTimingFunctionForKeyframes; - Style::ScopeOrdinal m_nameStyleScopeOrdinal { Style::ScopeOrdinal::Element }; - unsigned m_direction : 2; // Direction unsigned m_fillMode : 2; // AnimationFillMode unsigned m_playState : 2; // AnimationPlayState @@ -241,7 +232,7 @@ class Animation : public RefCounted { static double initialDuration() { return 0; } static AnimationFillMode initialFillMode() { return AnimationFillMode::None; } static double initialIterationCount() { return 1.0; } - static const Name& initialName(); + static const Style::ScopedName& initialName(); static AnimationPlayState initialPlayState() { return AnimationPlayState::Playing; } static CompositeOperation initialCompositeOperation() { return CompositeOperation::Replace; } static TransitionProperty initialProperty() { return { TransitionMode::All, CSSPropertyInvalid }; } diff --git a/Source/WebCore/style/ScopedName.h b/Source/WebCore/style/ScopedName.h new file mode 100644 index 000000000000..801b97221a17 --- /dev/null +++ b/Source/WebCore/style/ScopedName.h @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2023 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 met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#pragma once + +#include "StyleScopeOrdinal.h" +#include + +namespace WebCore { +namespace Style { + +struct ScopedName { + AtomString name; + bool isIdentifier { true }; + ScopeOrdinal scopeOrdinal { ScopeOrdinal::Element }; + + bool operator==(const ScopedName&) const = default; +}; + +} +} + diff --git a/Source/WebCore/style/Styleable.cpp b/Source/WebCore/style/Styleable.cpp index f87072f9407d..262ac265d0c2 100644 --- a/Source/WebCore/style/Styleable.cpp +++ b/Source/WebCore/style/Styleable.cpp @@ -271,7 +271,7 @@ void Styleable::cancelDeclarativeAnimations() const static bool keyframesRuleExistsForAnimation(Element& element, const Animation& animation, const String& animationName) { - auto* styleScope = Style::Scope::forOrdinal(element, animation.nameStyleScopeOrdinal()); + auto* styleScope = Style::Scope::forOrdinal(element, animation.name().scopeOrdinal); return styleScope && styleScope->resolver().isAnimationNameValid(animationName); } @@ -282,7 +282,7 @@ bool Styleable::animationListContainsNewlyValidAnimation(const AnimationList& an return false; for (auto& animation : animations) { - auto& name = animation->name().string; + auto& name = animation->name().name; if (name != noneAtom() && !name.isEmpty() && keyframeEffectStack.containsInvalidCSSAnimationName(name) && keyframesRuleExistsForAnimation(element, animation.get(), name)) return true; } @@ -327,7 +327,7 @@ void Styleable::updateCSSAnimations(const RenderStyle* currentStyle, const Rende if (!currentAnimation->isValidAnimation()) continue; - auto& animationName = currentAnimation->name().string; + auto& animationName = currentAnimation->name().name; if (animationName == noneAtom() || animationName.isEmpty()) continue;