Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(683)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: Source/core/css/resolver/StyleResolver.cpp

Issue 686723002: Improve RAII of StyleResolverState. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Fix unused var + minor diff noise. Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« Source/core/css/resolver/FontBuilder.cpp ('K') | « Source/core/css/resolver/StyleResolver.h ('k') | Source/core/css/resolver/StyleResolverState.h » ('j') | Source/core/css/resolver/StyleResolverState.cpp » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/css/resolver/StyleResolver.cpp
diff --git a/Source/core/css/resolver/StyleResolver.cpp b/Source/core/css/resolver/StyleResolver.cpp
index bcca08f5eafdebef0fd226a30c33249206d39b9c..470aeaf0206d443c558ab10b0353663b4a824319 100644
--- a/Source/core/css/resolver/StyleResolver.cpp
+++ b/Source/core/css/resolver/StyleResolver.cpp
@@ -59,6 +59,7 @@
#include "core/css/StyleRuleImport.h"
#include "core/css/StyleSheetContents.h"
#include "core/css/resolver/AnimatedStyleBuilder.h"
+#include "core/css/resolver/ElementResolveContext.h"
#include "core/css/resolver/MatchResult.h"
#include "core/css/resolver/MediaQueryResult.h"
#include "core/css/resolver/SharedStyleFinder.h"
@@ -127,6 +128,52 @@ static void addFontFaceRule(Document* document, CSSFontSelector* cssFontSelector
cssFontSelector->fontFaceCache()->add(cssFontSelector, fontFaceRule, fontFace);
}
+class AnimationResolveContext {
+ STACK_ALLOCATED();
+public:
+ AnimationResolveContext(Element* element)
+ : m_activeAnimations(element ? element->activeAnimations() : nullptr)
+ , m_baseRenderStyle(m_activeAnimations ? m_activeAnimations->baseRenderStyle() : nullptr)
+ {
+ }
+ ActiveAnimations* activeAnimations() const { return m_activeAnimations; }
+ const RenderStyle* baseRenderStyle() const { return m_baseRenderStyle; }
+private:
+ ActiveAnimations* m_activeAnimations;
+ const RenderStyle* m_baseRenderStyle;
+};
+
+class StyleBasisContext {
+ STACK_ALLOCATED();
+public:
+ StyleBasisContext(Element* element, RenderStyle* parentStyle)
+ : m_pseudoElement(nullptr)
+ , m_elementContext(*element, parentStyle)
+ , m_animationContext(element)
+ , m_allowsInheritance(m_elementContext.parentStyle())
+ {
+ }
+
+ StyleBasisContext(Element* element, RenderStyle* parentStyle, const PseudoStyleRequest& pseudoRequest)
+ : m_pseudoElement(element->pseudoElement(pseudoRequest.pseudoId))
+ , m_elementContext(*element, parentStyle)
+ , m_animationContext(m_pseudoElement)
+ , m_allowsInheritance(pseudoRequest.allowsInheritance(m_elementContext.parentStyle()))
+ {
+ }
+
+ PseudoElement* pseudoElement() const { return m_pseudoElement; }
+ const ElementResolveContext& elementContext() const { return m_elementContext; }
+ const AnimationResolveContext& animationContext() const { return m_animationContext; }
+ bool allowsInheritance() const { return m_allowsInheritance; }
+
+private:
+ PseudoElement* m_pseudoElement;
+ ElementResolveContext m_elementContext;
+ AnimationResolveContext m_animationContext;
+ bool m_allowsInheritance;
+};
+
StyleResolver::StyleResolver(Document& document)
: m_document(document)
, m_viewportStyleResolver(ViewportStyleResolver::create(&document))
@@ -548,6 +595,7 @@ void StyleResolver::loadPendingResources(StyleResolverState& state)
PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderStyle* defaultParent, StyleSharingBehavior sharingBehavior,
RuleMatchingBehavior matchingBehavior)
{
+ ASSERT(element);
ASSERT(document().frame());
ASSERT(document().settings());
ASSERT(!hasPendingAuthorStyleSheets());
@@ -572,26 +620,23 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
if (element == document().documentElement())
resetDirectionAndWritingModeOnDocument(document());
- StyleResolverState state(document(), element, defaultParent);
- if (sharingBehavior == AllowStyleSharing && state.parentStyle()) {
- SharedStyleFinder styleFinder(state.elementContext(), m_features, m_siblingRuleSet.get(), m_uncommonAttributeRuleSet.get(), *this);
+ StyleBasisContext styleContext(element, defaultParent);
+ const ElementResolveContext& elementContext = styleContext.elementContext();
+
+ if (sharingBehavior == AllowStyleSharing && elementContext.parentStyle()) {
+ SharedStyleFinder styleFinder(elementContext, m_features, m_siblingRuleSet.get(), m_uncommonAttributeRuleSet.get(), *this);
if (RefPtr<RenderStyle> sharedStyle = styleFinder.findSharedStyle())
return sharedStyle.release();
}
- ActiveAnimations* activeAnimations = element->activeAnimations();
- const RenderStyle* baseRenderStyle = activeAnimations ? activeAnimations->baseRenderStyle() : nullptr;
+ StyleResolverState state(document(), elementContext, styleForContext(styleContext), parentStyleForContext(styleContext));
+
+ const AnimationResolveContext& animationContext = styleContext.animationContext();
+
+ if (!animationContext.baseRenderStyle() && styleContext.allowsInheritance())
+ state.style()->inheritFrom(elementContext.parentStyle(), isAtShadowBoundary(element) ? RenderStyle::AtShadowBoundary : RenderStyle::NotAtShadowBoundary);
- if (baseRenderStyle) {
- state.setStyle(RenderStyle::clone(baseRenderStyle));
- } else if (state.parentStyle()) {
- state.setStyle(RenderStyle::create());
- state.style()->inheritFrom(state.parentStyle(), isAtShadowBoundary(element) ? RenderStyle::AtShadowBoundary : RenderStyle::NotAtShadowBoundary);
- } else {
- state.setStyle(defaultStyleForElement());
- state.setParentStyle(RenderStyle::clone(state.style()));
- }
// contenteditable attribute (implemented by -webkit-user-modify) should
// be propagated from shadow host to distributed node.
if (state.distributedToInsertionPoint()) {
@@ -612,7 +657,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
state.style()->setInsideLink(linkState);
}
- if (!baseRenderStyle) {
+ if (!animationContext.baseRenderStyle()) {
bool needsCollection = false;
CSSDefaultStyleSheets::instance().ensureDefaultStyleSheetsForElement(element, needsCollection);
@@ -633,8 +678,8 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
adjustRenderStyle(state, element);
- if (activeAnimations)
- activeAnimations->updateBaseRenderStyle(state.style());
+ if (animationContext.activeAnimations())
+ animationContext.activeAnimations()->updateBaseRenderStyle(state.style());
}
// FIXME: The CSSWG wants to specify that the effects of animations are applied before
@@ -663,15 +708,11 @@ PassRefPtr<RenderStyle> StyleResolver::styleForKeyframe(Element& element, const
if (&element == document().documentElement())
resetDirectionAndWritingModeOnDocument(document());
- StyleResolverState state(document(), &element, parentStyle);
+ StyleResolverState state(document(), &element, RenderStyle::clone(&elementStyle), parentStyle);
MatchResult result;
result.addMatchedProperties(&keyframe->properties());
- ASSERT(!state.style());
-
- // Create the style
- state.setStyle(RenderStyle::clone(&elementStyle));
// We don't need to bother with !important. Since there is only ever one
// decl, there's nothing to override. So just add the first properties.
@@ -703,8 +744,7 @@ PassRefPtrWillBeRawPtr<AnimatableValue> StyleResolver::createAnimatableValueSnap
style = RenderStyle::clone(element.renderStyle());
else
style = RenderStyle::create();
- StyleResolverState state(element.document(), &element);
- state.setStyle(style);
+ StyleResolverState state(element.document(), &element, style);
return createAnimatableValueSnapshot(state, property, value);
}
@@ -714,6 +754,29 @@ PassRefPtrWillBeRawPtr<AnimatableValue> StyleResolver::createAnimatableValueSnap
return CSSAnimatableValueFactory::create(property, *state.style());
}
+PassRefPtr<RenderStyle> StyleResolver::styleForContext(const StyleBasisContext& styleContext)
+{
+ const AnimationResolveContext& animationContext = styleContext.animationContext();
+
+ if (animationContext.baseRenderStyle())
+ return RenderStyle::clone(animationContext.baseRenderStyle());
+ if (styleContext.allowsInheritance())
+ return RenderStyle::create();
+ return defaultStyleForElement();
+}
+
+PassRefPtr<RenderStyle> StyleResolver::parentStyleForContext(const StyleBasisContext& styleContext)
+{
+ const ElementResolveContext& elementContext = styleContext.elementContext();
+ const AnimationResolveContext& animationContext = styleContext.animationContext();
+
+ if (styleContext.allowsInheritance())
+ return elementContext.parentStyle();
+ if (animationContext.baseRenderStyle())
+ return defaultStyleForElement();
+ return nullptr;
+}
+
PassRefPtrWillBeRawPtr<PseudoElement> StyleResolver::createPseudoElementIfNeeded(Element& parent, PseudoId pseudoId)
{
RenderObject* parentRenderer = parent.renderer();
@@ -736,8 +799,9 @@ PassRefPtrWillBeRawPtr<PseudoElement> StyleResolver::createPseudoElementIfNeeded
return PseudoElement::create(&parent, pseudoId);
}
- StyleResolverState state(document(), &parent, parentStyle);
- if (!pseudoStyleForElementInternal(parent, pseudoId, parentStyle, state))
+ StyleBasisContext styleContext(&parent, parentStyle, pseudoId);
+ StyleResolverState state(document(), styleContext.elementContext(), styleForContext(styleContext), parentStyleForContext(styleContext));
+ if (!pseudoStyleForElementInternal(pseudoId, state, styleContext))
return nullptr;
RefPtr<RenderStyle> style = state.takeStyle();
ASSERT(style);
@@ -754,7 +818,7 @@ PassRefPtrWillBeRawPtr<PseudoElement> StyleResolver::createPseudoElementIfNeeded
return pseudo.release();
}
-bool StyleResolver::pseudoStyleForElementInternal(Element& element, const PseudoStyleRequest& pseudoStyleRequest, RenderStyle* parentStyle, StyleResolverState& state)
+bool StyleResolver::pseudoStyleForElementInternal(const PseudoStyleRequest& pseudoStyleRequest, StyleResolverState& state, const StyleBasisContext& styleContext)
{
ASSERT(document().frame());
ASSERT(document().settings());
@@ -762,29 +826,21 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
StyleResolverParentScope::ensureParentStackIsPushed();
- Element* pseudoElement = element.pseudoElement(pseudoStyleRequest.pseudoId);
+ const ElementResolveContext& elementContext = styleContext.elementContext();
+ const AnimationResolveContext& animationContext = styleContext.animationContext();
- ActiveAnimations* activeAnimations = pseudoElement ? pseudoElement->activeAnimations() : nullptr;
- const RenderStyle* baseRenderStyle = activeAnimations ? activeAnimations->baseRenderStyle() : nullptr;
-
- if (baseRenderStyle) {
- state.setStyle(RenderStyle::clone(baseRenderStyle));
- } else if (pseudoStyleRequest.allowsInheritance(state.parentStyle())) {
- state.setStyle(RenderStyle::create());
- state.style()->inheritFrom(state.parentStyle());
- } else {
- state.setStyle(defaultStyleForElement());
- state.setParentStyle(RenderStyle::clone(state.style()));
- }
+ if (!animationContext.baseRenderStyle() && styleContext.allowsInheritance())
+ state.style()->inheritFrom(elementContext.parentStyle());
state.style()->setStyleType(pseudoStyleRequest.pseudoId);
// Since we don't use pseudo-elements in any of our quirk/print
// user agent rules, don't waste time walking those rules.
- if (!baseRenderStyle) {
+ if (!animationContext.baseRenderStyle()) {
+
// Check UA, user and author rules.
- ElementRuleCollector collector(state.elementContext(), m_selectorFilter, state.style());
+ ElementRuleCollector collector(elementContext, m_selectorFilter, state.style());
collector.setPseudoStyleRequest(pseudoStyleRequest);
matchUARules(collector);
@@ -805,14 +861,14 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
// in the adjustRenderStyle code.
adjustRenderStyle(state, 0);
- if (activeAnimations)
- activeAnimations->updateBaseRenderStyle(state.style());
+ if (animationContext.activeAnimations())
+ animationContext.activeAnimations()->updateBaseRenderStyle(state.style());
}
// FIXME: The CSSWG wants to specify that the effects of animations are applied before
// important rules, but this currently happens here as we require adjustment to have happened
// before deciding which properties to transition.
- if (applyAnimatedProperties(state, pseudoElement))
+ if (applyAnimatedProperties(state, styleContext.pseudoElement()))
adjustRenderStyle(state, 0);
didAccess();
@@ -829,8 +885,9 @@ PassRefPtr<RenderStyle> StyleResolver::pseudoStyleForElement(Element* element, c
if (!element)
return nullptr;
- StyleResolverState state(document(), element, parentStyle);
- if (!pseudoStyleForElementInternal(*element, pseudoStyleRequest, parentStyle, state)) {
+ StyleBasisContext styleContext(element, parentStyle, pseudoStyleRequest);
+ StyleResolverState state(document(), styleContext.elementContext(), styleForContext(styleContext), parentStyleForContext(styleContext));
+ if (!pseudoStyleForElementInternal(pseudoStyleRequest, state, styleContext)) {
if (pseudoStyleRequest.type == PseudoStyleRequest::ForRenderer)
return nullptr;
return state.takeStyle();
@@ -847,9 +904,8 @@ PassRefPtr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
{
ASSERT(!hasPendingAuthorStyleSheets());
resetDirectionAndWritingModeOnDocument(document());
- StyleResolverState state(document(), document().documentElement()); // m_rootElementStyle will be set to the document style.
+ StyleResolverState state(document(), document().documentElement(), RenderStyle::create()); // m_rootElementStyle will be set to the document style.
- state.setStyle(RenderStyle::create());
const RenderStyle* rootElementStyle = state.rootElementStyle() ? state.rootElementStyle() : document().renderStyle();
ASSERT(rootElementStyle);
state.style()->inheritFrom(rootElementStyle);
@@ -900,8 +956,7 @@ void StyleResolver::collectViewportRules()
PassRefPtr<RenderStyle> StyleResolver::defaultStyleForElement()
{
- StyleResolverState state(document(), 0);
- state.setStyle(RenderStyle::create());
+ StyleResolverState state(document(), nullptr, RenderStyle::create());
rune 2014/10/30 15:23:50 I don't think you need a StyleResolverState here.
andersr 2014/10/31 13:02:15 Done.
state.fontBuilder().setInitial(state.style()->effectiveZoom());
state.style()->font().update(document().styleEngine()->fontSelector());
return state.takeStyle();
@@ -925,8 +980,8 @@ void StyleResolver::updateFont(StyleResolverState& state)
PassRefPtrWillBeRawPtr<StyleRuleList> StyleResolver::styleRulesForElement(Element* element, unsigned rulesToInclude)
{
ASSERT(element);
- StyleResolverState state(document(), element);
- ElementRuleCollector collector(state.elementContext(), m_selectorFilter, state.style());
+ ElementResolveContext elementContext(*element, nullptr);
+ ElementRuleCollector collector(elementContext, m_selectorFilter, nullptr);
collector.setMode(SelectorChecker::CollectingStyleRules);
collectPseudoRulesForElement(element, collector, NOPSEUDO, rulesToInclude);
return collector.matchedStyleRuleList();
@@ -935,8 +990,8 @@ PassRefPtrWillBeRawPtr<StyleRuleList> StyleResolver::styleRulesForElement(Elemen
PassRefPtrWillBeRawPtr<CSSRuleList> StyleResolver::pseudoCSSRulesForElement(Element* element, PseudoId pseudoId, unsigned rulesToInclude)
{
ASSERT(element);
- StyleResolverState state(document(), element);
- ElementRuleCollector collector(state.elementContext(), m_selectorFilter, state.style());
+ ElementResolveContext elementContext(*element, nullptr);
+ ElementRuleCollector collector(elementContext, m_selectorFilter, nullptr);
collector.setMode(SelectorChecker::CollectingCSSRules);
collectPseudoRulesForElement(element, collector, pseudoId, rulesToInclude);
return collector.matchedCSSRuleList();
@@ -1525,8 +1580,7 @@ void StyleResolver::printStats()
void StyleResolver::applyPropertiesToStyle(const CSSPropertyValue* properties, size_t count, RenderStyle* style)
{
- StyleResolverState state(document(), document().documentElement(), style);
- state.setStyle(style);
+ StyleResolverState state(document(), document().documentElement(), style, style);
for (size_t i = 0; i < count; ++i) {
if (properties[i].value) {
« Source/core/css/resolver/FontBuilder.cpp ('K') | « Source/core/css/resolver/StyleResolver.h ('k') | Source/core/css/resolver/StyleResolverState.h » ('j') | Source/core/css/resolver/StyleResolverState.cpp » ('J')

Powered by Google App Engine
This is Rietveld 408576698