From 8b97399df330381152f0563737782bb323bbf144 Mon Sep 17 00:00:00 2001 From: Elliott Sprehn Date: Wed, 7 Jan 2015 10:47:43 -0800 Subject: [PATCH] Simplify ScopedStyleResolver keyframe handling. Just linearly search for keyframes in the set of stylesheets. Components rarely have many sheets, and sheets rarely have many keyframes so this should be quite fast. It's also much simpler than having to collect all the keyframes from all the rulesets. R=ojan@chromium.org, rafaelw@chromium.org Review URL: https://codereview.chromium.org/839473005 --- engine/core/animation/css/CSSAnimations.cpp | 4 +-- engine/core/animation/css/CSSAnimations.h | 2 +- .../core/css/resolver/ScopedStyleResolver.cpp | 31 +++++-------------- .../core/css/resolver/ScopedStyleResolver.h | 6 +--- engine/core/css/resolver/StyleResolver.cpp | 5 --- engine/core/css/resolver/StyleResolver.h | 4 --- 6 files changed, 12 insertions(+), 40 deletions(-) diff --git a/engine/core/animation/css/CSSAnimations.cpp b/engine/core/animation/css/CSSAnimations.cpp index 997f5bb2d3e..972f1140d8f 100644 --- a/engine/core/animation/css/CSSAnimations.cpp +++ b/engine/core/animation/css/CSSAnimations.cpp @@ -86,7 +86,7 @@ static void resolveKeyframes(StyleResolver* resolver, Element* element, const El { // When the element is null, use its parent for scoping purposes. const Element* elementForScoping = element ? element : &parentElement; - const StyleRuleKeyframes* keyframesRule = CSSAnimations::matchScopedKeyframesRule(resolver, elementForScoping, name.impl()); + const StyleRuleKeyframes* keyframesRule = CSSAnimations::matchScopedKeyframesRule(resolver, elementForScoping, name); if (!keyframesRule) return; @@ -193,7 +193,7 @@ static void resolveKeyframes(StyleResolver* resolver, Element* element, const El } // namespace -const StyleRuleKeyframes* CSSAnimations::matchScopedKeyframesRule(StyleResolver* resolver, const Element* element, const StringImpl* animationName) +const StyleRuleKeyframes* CSSAnimations::matchScopedKeyframesRule(StyleResolver* resolver, const Element* element, String animationName) { // FIXME: This is all implementation detail of style resolver, CSSAnimations shouldn't be reaching into any of it. Vector, 8> stack; diff --git a/engine/core/animation/css/CSSAnimations.h b/engine/core/animation/css/CSSAnimations.h index 914bd9c604d..b54f4e834ef 100644 --- a/engine/core/animation/css/CSSAnimations.h +++ b/engine/core/animation/css/CSSAnimations.h @@ -154,7 +154,7 @@ public: // FIXME: This method is only used here and in the legacy animations // implementation. It should be made private or file-scope when the legacy // engine is removed. - static const StyleRuleKeyframes* matchScopedKeyframesRule(StyleResolver*, const Element*, const StringImpl*); + static const StyleRuleKeyframes* matchScopedKeyframesRule(StyleResolver*, const Element*, String animationName); static const StylePropertyShorthand& animatableProperties(); static bool isAllowedAnimation(CSSPropertyID); diff --git a/engine/core/css/resolver/ScopedStyleResolver.cpp b/engine/core/css/resolver/ScopedStyleResolver.cpp index 8c51e2cd7d1..472ccfdc798 100644 --- a/engine/core/css/resolver/ScopedStyleResolver.cpp +++ b/engine/core/css/resolver/ScopedStyleResolver.cpp @@ -61,34 +61,19 @@ void ScopedStyleResolver::addRulesFromSheet(CSSStyleSheet* cssSheet, StyleResolv void ScopedStyleResolver::resetAuthorStyle() { m_authorStyleSheets.clear(); - m_keyframesRuleMap.clear(); m_features.clear(); } -const StyleRuleKeyframes* ScopedStyleResolver::keyframeStylesForAnimation(const StringImpl* animationName) +const StyleRuleKeyframes* ScopedStyleResolver::keyframeStylesForAnimation(String animationName) { - if (m_keyframesRuleMap.isEmpty()) - return 0; - - KeyframesRuleMap::iterator it = m_keyframesRuleMap.find(animationName); - if (it == m_keyframesRuleMap.end()) - return 0; - - return it->value.get(); -} - -void ScopedStyleResolver::addKeyframeStyle(PassRefPtr rule) -{ - AtomicString s(rule->name()); - if (rule->isVendorPrefixed()) { - KeyframesRuleMap::iterator it = m_keyframesRuleMap.find(rule->name().impl()); - if (it == m_keyframesRuleMap.end()) - m_keyframesRuleMap.set(s.impl(), rule); - else if (it->value->isVendorPrefixed()) - m_keyframesRuleMap.set(s.impl(), rule); - } else { - m_keyframesRuleMap.set(s.impl(), rule); + for (auto& sheet : m_authorStyleSheets) { + // TODO(esprehn): Maybe just store the keyframes in a map? + for (auto& rule : sheet->contents()->ruleSet().keyframesRules()) { + if (rule->name() == animationName) + return rule.get(); + } } + return nullptr; } void ScopedStyleResolver::collectMatchingAuthorRules(ElementRuleCollector& collector, bool includeEmptyRules, bool applyAuthorStyles, CascadeScope cascadeScope, CascadeOrder cascadeOrder) diff --git a/engine/core/css/resolver/ScopedStyleResolver.h b/engine/core/css/resolver/ScopedStyleResolver.h index 59f0558f6bb..7ae23d4342e 100644 --- a/engine/core/css/resolver/ScopedStyleResolver.h +++ b/engine/core/css/resolver/ScopedStyleResolver.h @@ -54,8 +54,7 @@ public: const TreeScope& treeScope() const { return *m_scope; } public: - const StyleRuleKeyframes* keyframeStylesForAnimation(const StringImpl* animationName); - void addKeyframeStyle(PassRefPtr); + const StyleRuleKeyframes* keyframeStylesForAnimation(String animationName); void collectMatchingAuthorRules(ElementRuleCollector&, bool includeEmptyRules, bool applyAuthorStyles, CascadeScope, CascadeOrder = ignoreCascadeOrder); void addRulesFromSheet(CSSStyleSheet*, StyleResolver*); @@ -70,9 +69,6 @@ private: RawPtr m_scope; Vector > m_authorStyleSheets; - typedef HashMap > KeyframesRuleMap; - KeyframesRuleMap m_keyframesRuleMap; - RuleFeatureSet m_features; }; diff --git a/engine/core/css/resolver/StyleResolver.cpp b/engine/core/css/resolver/StyleResolver.cpp index 521b28c7367..762a3d6c92e 100644 --- a/engine/core/css/resolver/StyleResolver.cpp +++ b/engine/core/css/resolver/StyleResolver.cpp @@ -195,11 +195,6 @@ void StyleResolver::finishAppendAuthorStyleSheets() void StyleResolver::processScopedRules(const RuleSet& authorRules, CSSStyleSheet* parentStyleSheet, unsigned parentIndex, ContainerNode& scope) { - const Vector > keyframesRules = authorRules.keyframesRules(); - ScopedStyleResolver* resolver = &scope.treeScope().scopedStyleResolver(); - for (unsigned i = 0; i < keyframesRules.size(); ++i) - resolver->addKeyframeStyle(keyframesRules[i]); - // FIXME(BUG 72461): We don't add @font-face rules of scoped style sheets for the moment. if (scope.isDocumentNode()) { const Vector > fontFaceRules = authorRules.fontFaceRules(); diff --git a/engine/core/css/resolver/StyleResolver.h b/engine/core/css/resolver/StyleResolver.h index 4f25549d2c5..90371f51204 100644 --- a/engine/core/css/resolver/StyleResolver.h +++ b/engine/core/css/resolver/StyleResolver.h @@ -196,10 +196,6 @@ private: template void applyAllProperty(StyleResolverState&, CSSValue*); - // FIXME: This likely belongs on RuleSet. - typedef HashMap > KeyframesRuleMap; - KeyframesRuleMap m_keyframesRuleMap; - static RenderStyle* s_styleNotYetAvailable; MatchedPropertiesCache m_matchedPropertiesCache;