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

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 assertions. 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
« no previous file with comments | « Source/core/css/resolver/StyleResolver.h ('k') | Source/core/css/resolver/StyleResolverState.h » ('j') | Source/core/css/resolver/StyleResolverState.h » ('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 edcebea5a1767097fc33503f849559530ad1c158..d750bfadd30f089272cf28d4e18ada5dda866adf 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,38 @@ static void addFontFaceRule(Document* document, CSSFontSelector* cssFontSelector
cssFontSelector->fontFaceCache()->add(cssFontSelector, fontFaceRule, fontFace);
}
+class StyleBasisContext : public ElementResolveContext {
rune 2014/11/13 14:28:42 I don't like the name StyleBasisContext. This is s
andersr 2014/11/13 14:57:20 Yeah, that's pretty good.
+ STACK_ALLOCATED();
+public:
+ StyleBasisContext(Element* element, RenderStyle* parentStyle)
+ : ElementResolveContext(*element, parentStyle)
+ , m_pseudoElement(nullptr)
+ , m_activeAnimations(element->activeAnimations())
+ , m_baseRenderStyle(m_activeAnimations ? m_activeAnimations->baseRenderStyle() : nullptr)
+ , m_allowsInheritance(this->parentStyle())
+ {
+ }
+
+ StyleBasisContext(Element* element, RenderStyle* parentStyle, const PseudoStyleRequest& pseudoRequest)
+ : ElementResolveContext(*element, parentStyle)
+ , m_pseudoElement(element->pseudoElement(pseudoRequest.pseudoId))
+ , m_activeAnimations(m_pseudoElement ? m_pseudoElement->activeAnimations() : nullptr)
+ , m_baseRenderStyle(m_activeAnimations ? m_activeAnimations->baseRenderStyle() : nullptr)
+ , m_allowsInheritance(pseudoRequest.allowsInheritance(this->parentStyle()))
+ {
+ }
+
+ PseudoElement* pseudoElement() const { return m_pseudoElement; }
+ ActiveAnimations* activeAnimations() const { return m_activeAnimations; }
+ const RenderStyle* baseRenderStyle() const { return m_baseRenderStyle; }
+ bool allowsInheritance() const { return m_allowsInheritance; }
+private:
+ PseudoElement* m_pseudoElement;
+ ActiveAnimations* m_activeAnimations;
+ const RenderStyle* m_baseRenderStyle;
+ bool m_allowsInheritance;
+};
+
StyleResolver::StyleResolver(Document& document)
: m_document(document)
, m_viewportStyleResolver(ViewportStyleResolver::create(&document))
@@ -548,6 +581,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,30 +606,19 @@ 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);
+
+ if (sharingBehavior == AllowStyleSharing && styleContext.parentStyle()) {
+ SharedStyleFinder styleFinder(styleContext, 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(), styleContext, styleForContext(styleContext), parentStyleForContext(styleContext));
- if (baseRenderStyle) {
- state.setStyle(RenderStyle::clone(baseRenderStyle));
- if (!state.parentStyle())
- state.setParentStyle(defaultStyleForElement());
- } 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()));
- }
- }
+ if (!styleContext.baseRenderStyle() && styleContext.allowsInheritance())
+ state.style()->inheritFrom(styleContext.parentStyle(), isAtShadowBoundary(element) ? RenderStyle::AtShadowBoundary : RenderStyle::NotAtShadowBoundary);
// contenteditable attribute (implemented by -webkit-user-modify) should
// be propagated from shadow host to distributed node.
@@ -617,7 +640,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
state.style()->setInsideLink(linkState);
}
- if (!baseRenderStyle) {
+ if (!styleContext.baseRenderStyle()) {
bool needsCollection = false;
CSSDefaultStyleSheets::instance().ensureDefaultStyleSheetsForElement(element, needsCollection);
@@ -638,8 +661,8 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
adjustRenderStyle(state, element);
- if (activeAnimations)
- activeAnimations->updateBaseRenderStyle(state.style());
+ if (styleContext.activeAnimations())
+ styleContext.activeAnimations()->updateBaseRenderStyle(state.style());
}
// FIXME: The CSSWG wants to specify that the effects of animations are applied before
@@ -668,15 +691,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.
@@ -708,8 +727,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);
}
@@ -719,6 +737,24 @@ PassRefPtrWillBeRawPtr<AnimatableValue> StyleResolver::createAnimatableValueSnap
return CSSAnimatableValueFactory::create(property, *state.style());
}
+PassRefPtr<RenderStyle> StyleResolver::styleForContext(const StyleBasisContext& styleContext)
+{
+ if (styleContext.baseRenderStyle())
+ return RenderStyle::clone(styleContext.baseRenderStyle());
+ if (styleContext.allowsInheritance())
+ return RenderStyle::create();
+ return initialRenderStyle();
+}
+
+PassRefPtr<RenderStyle> StyleResolver::parentStyleForContext(const StyleBasisContext& styleContext)
+{
+ if (styleContext.allowsInheritance())
+ return styleContext.parentStyle();
+ if (styleContext.baseRenderStyle())
+ return initialRenderStyle();
+ return nullptr;
rune 2014/11/13 14:28:42 Would it be better to do the clone() here instead
andersr 2014/11/13 14:57:20 I wanted to avoid this verbosity at each call site
+}
+
PassRefPtrWillBeRawPtr<PseudoElement> StyleResolver::createPseudoElementIfNeeded(Element& parent, PseudoId pseudoId)
{
RenderObject* parentRenderer = parent.renderer();
@@ -741,8 +777,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, styleForContext(styleContext), parentStyleForContext(styleContext));
+ if (!pseudoStyleForElementInternal(pseudoId, state, styleContext))
return nullptr;
RefPtr<RenderStyle> style = state.takeStyle();
ASSERT(style);
@@ -759,7 +796,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());
@@ -768,29 +805,18 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
StyleResolverParentScope::ensureParentStackIsPushed();
- Element* pseudoElement = element.pseudoElement(pseudoStyleRequest.pseudoId);
-
- 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 (!styleContext.baseRenderStyle() && styleContext.allowsInheritance())
+ state.style()->inheritFrom(styleContext.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 (!styleContext.baseRenderStyle()) {
+
// Check UA, user and author rules.
- ElementRuleCollector collector(state.elementContext(), m_selectorFilter, state.style());
+ ElementRuleCollector collector(styleContext, m_selectorFilter, state.style());
collector.setPseudoStyleRequest(pseudoStyleRequest);
matchUARules(collector);
@@ -811,14 +837,14 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
// in the adjustRenderStyle code.
adjustRenderStyle(state, 0);
- if (activeAnimations)
- activeAnimations->updateBaseRenderStyle(state.style());
+ if (styleContext.activeAnimations())
+ styleContext.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();
@@ -835,8 +861,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, styleForContext(styleContext), parentStyleForContext(styleContext));
+ if (!pseudoStyleForElementInternal(pseudoStyleRequest, state, styleContext)) {
if (pseudoStyleRequest.type == PseudoStyleRequest::ForRenderer)
return nullptr;
return state.takeStyle();
@@ -853,9 +880,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);
@@ -904,13 +930,13 @@ void StyleResolver::collectViewportRules()
viewportStyleResolver()->resolve();
}
-PassRefPtr<RenderStyle> StyleResolver::defaultStyleForElement()
+PassRefPtr<RenderStyle> StyleResolver::initialRenderStyle()
{
- StyleResolverState state(document(), 0);
- state.setStyle(RenderStyle::create());
- state.fontBuilder().setInitial(state.style()->effectiveZoom());
- state.style()->font().update(document().styleEngine()->fontSelector());
- return state.takeStyle();
+ RefPtr<RenderStyle> style = RenderStyle::create();
+ FontBuilder fontBuilder(document(), style.get());
+ fontBuilder.setInitial(style->effectiveZoom());
+ style->font().update(document().styleEngine()->fontSelector());
+ return style.release();
}
PassRefPtr<RenderStyle> StyleResolver::styleForText(Text* textNode)
@@ -919,7 +945,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForText(Text* textNode)
Node* parentNode = NodeRenderingTraversal::parent(textNode);
if (!parentNode || !parentNode->renderStyle())
- return defaultStyleForElement();
+ return initialRenderStyle();
return parentNode->renderStyle();
}
@@ -931,8 +957,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();
@@ -941,8 +967,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();
@@ -1531,8 +1557,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) {
« no previous file with comments | « Source/core/css/resolver/StyleResolver.h ('k') | Source/core/css/resolver/StyleResolverState.h » ('j') | Source/core/css/resolver/StyleResolverState.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698