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

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: When baseRenderStyle!=0: 1) don't inherit, and 2) use defaultStyleForElement as fallback parent. 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.h ('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 25373afd5454162b97af8bb6a2f7ffc39d2d53e9..3bbb9e6e0b9e014f1e2c16942b8310e83c877e87 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"
@@ -572,26 +573,20 @@ 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);
+ ElementResolveContext elementContext(document(), element, defaultParent);
+
+ 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(elementContext);
+
+ if (!elementContext.baseRenderStyle() && elementContext.parentStyle())
+ 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 +607,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
state.style()->setInsideLink(linkState);
}
- if (!baseRenderStyle) {
+ if (!elementContext.baseRenderStyle()) {
bool needsCollection = false;
CSSDefaultStyleSheets::instance().ensureDefaultStyleSheetsForElement(element, needsCollection);
@@ -633,8 +628,8 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
adjustRenderStyle(state, element);
- if (activeAnimations)
- activeAnimations->updateBaseRenderStyle(state.style());
+ if (elementContext.activeAnimations())
+ elementContext.activeAnimations()->updateBaseRenderStyle(state.style());
}
// FIXME: The CSSWG wants to specify that the effects of animations are applied before
@@ -663,16 +658,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));
- state.setLineHeightValue(0);
// 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.
@@ -685,10 +675,6 @@ PassRefPtr<RenderStyle> StyleResolver::styleForKeyframe(Element& element, const
// If our font got dirtied, go ahead and update it now.
updateFont(state);
- // Line-height is set when we are sure we decided on the font-size
- if (state.lineHeightValue())
- StyleBuilder::applyProperty(CSSPropertyLineHeight, state, state.lineHeightValue());
-
// Now do rest of the properties.
applyMatchedProperties<LowPriorityProperties>(state, result, false, 0, result.matchedProperties.size() - 1, inheritedOnly);
@@ -708,8 +694,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);
}
@@ -741,8 +726,9 @@ PassRefPtrWillBeRawPtr<PseudoElement> StyleResolver::createPseudoElementIfNeeded
return PseudoElement::create(&parent, pseudoId);
}
- StyleResolverState state(document(), &parent, parentStyle);
- if (!pseudoStyleForElementInternal(parent, pseudoId, parentStyle, state))
+ ElementResolveContext elementContext(document(), &parent, parentStyle);
+ StyleResolverState state(elementContext);
+ if (!pseudoStyleForElementInternal(pseudoId, parentStyle, state))
return nullptr;
RefPtr<RenderStyle> style = state.takeStyle();
ASSERT(style);
@@ -759,7 +745,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, RenderStyle* parentStyle, StyleResolverState& state)
{
ASSERT(document().frame());
ASSERT(document().settings());
@@ -767,29 +753,21 @@ 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()));
- }
+ const ElementResolveContext& elementContext = state.elementContext();
+ Element* pseudoElement = elementContext.element()->pseudoElement(pseudoStyleRequest.pseudoId);
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 (!elementContext.baseRenderStyle()) {
+
+ if (pseudoStyleRequest.allowsInheritance(elementContext.parentStyle()))
+ state.style()->inheritFrom(elementContext.parentStyle());
+
// 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);
@@ -810,8 +788,8 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
// in the adjustRenderStyle code.
adjustRenderStyle(state, 0);
- if (activeAnimations)
- activeAnimations->updateBaseRenderStyle(state.style());
+ if (elementContext.activeAnimations())
+ elementContext.activeAnimations()->updateBaseRenderStyle(state.style());
}
// FIXME: The CSSWG wants to specify that the effects of animations are applied before
@@ -834,8 +812,9 @@ PassRefPtr<RenderStyle> StyleResolver::pseudoStyleForElement(Element* element, c
if (!element)
return nullptr;
- StyleResolverState state(document(), element, parentStyle);
- if (!pseudoStyleForElementInternal(*element, pseudoStyleRequest, parentStyle, state)) {
+ ElementResolveContext elementContext(document(), element, parentStyle);
+ StyleResolverState state(elementContext);
+ if (!pseudoStyleForElementInternal(pseudoStyleRequest, parentStyle, state)) {
if (pseudoStyleRequest.type == PseudoStyleRequest::ForRenderer)
return nullptr;
return state.takeStyle();
@@ -852,9 +831,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);
@@ -866,7 +844,6 @@ PassRefPtr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
if (ScopedStyleResolver* scopedResolver = document().scopedStyleResolver())
scopedResolver->matchPageRules(collector);
- state.setLineHeightValue(0);
bool inheritedOnly = false;
MatchResult& result = collector.matchedResult();
@@ -875,10 +852,6 @@ PassRefPtr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
// If our font got dirtied, go ahead and update it now.
updateFont(state);
- // Line-height is set when we are sure we decided on the font-size.
- if (state.lineHeightValue())
- StyleBuilder::applyProperty(CSSPropertyLineHeight, state, state.lineHeightValue());
-
applyMatchedProperties<LowPriorityProperties>(state, result, false, 0, result.matchedProperties.size() - 1, inheritedOnly);
addContentAttrValuesToFeatures(state.contentAttrValues(), m_features);
@@ -908,24 +881,13 @@ void StyleResolver::collectViewportRules()
viewportStyleResolver()->resolve();
}
-PassRefPtr<RenderStyle> StyleResolver::defaultStyleForElement()
-{
- StyleResolverState state(document(), 0);
- state.setStyle(RenderStyle::create());
- state.style()->setLineHeight(RenderStyle::initialLineHeight());
- state.setLineHeightValue(0);
- state.fontBuilder().setInitial(state.style()->effectiveZoom());
- state.style()->font().update(document().styleEngine()->fontSelector());
- return state.takeStyle();
-}
-
PassRefPtr<RenderStyle> StyleResolver::styleForText(Text* textNode)
{
ASSERT(textNode);
Node* parentNode = NodeRenderingTraversal::parent(textNode);
if (!parentNode || !parentNode->renderStyle())
- return defaultStyleForElement();
+ return StyleResolverState::defaultStyleForElement(document());
return parentNode->renderStyle();
}
@@ -937,8 +899,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(document(), element, nullptr);
+ ElementRuleCollector collector(elementContext, m_selectorFilter, nullptr);
collector.setMode(SelectorChecker::CollectingStyleRules);
collectPseudoRulesForElement(element, collector, NOPSEUDO, rulesToInclude);
return collector.matchedStyleRuleList();
@@ -947,8 +909,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(document(), element, nullptr);
+ ElementRuleCollector collector(elementContext, m_selectorFilter, nullptr);
collector.setMode(SelectorChecker::CollectingCSSRules);
collectPseudoRulesForElement(element, collector, pseudoId, rulesToInclude);
return collector.matchedCSSRuleList();
@@ -1253,9 +1215,9 @@ template<> CSSPropertyID StyleResolver::firstCSSPropertyId<StyleResolver::HighPr
// This method returns the last CSSPropertyId of high priority properties.
template<> CSSPropertyID StyleResolver::lastCSSPropertyId<StyleResolver::HighPriorityProperties>()
{
- COMPILE_ASSERT(CSSPropertyLineHeight == CSSPropertyColor + 17, CSS_line_height_is_end_of_high_prioity_property_range);
- COMPILE_ASSERT(CSSPropertyZoom == CSSPropertyLineHeight - 1, CSS_zoom_is_before_line_height);
- return CSSPropertyLineHeight;
+ COMPILE_ASSERT(CSSPropertyZoom == CSSPropertyColor + 16, CSS_zoom_is_end_of_high_priority_property_range);
+ COMPILE_ASSERT(CSSPropertyTextRendering == CSSPropertyZoom - 1, CSS_text_rendering_is_before_zoom);
+ return CSSPropertyZoom;
}
// This method returns the first CSSPropertyId of remaining properties,
@@ -1266,7 +1228,7 @@ template<> CSSPropertyID StyleResolver::lastCSSPropertyId<StyleResolver::HighPri
// lastCSSPropertyId<LowPriorityProperties>.
template<> CSSPropertyID StyleResolver::firstCSSPropertyId<StyleResolver::LowPriorityProperties>()
{
- COMPILE_ASSERT(CSSPropertyAlignContent == CSSPropertyLineHeight + 1, CSS_background_is_first_low_priority_property);
+ COMPILE_ASSERT(CSSPropertyAlignContent == CSSPropertyZoom + 1, CSS_align_content_is_first_low_priority_property);
return CSSPropertyAlignContent;
}
@@ -1349,10 +1311,8 @@ void StyleResolver::applyProperties(StyleResolverState& state, const StyleProper
continue;
if (!isPropertyForPass<pass>(property))
continue;
- if (pass == HighPriorityProperties && property == CSSPropertyLineHeight)
- state.setLineHeightValue(current.value());
- else
- StyleBuilder::applyProperty(current.id(), state, current.value());
+
+ StyleBuilder::applyProperty(current.id(), state, current.value());
}
}
@@ -1435,7 +1395,6 @@ void StyleResolver::applyMatchedProperties(StyleResolverState& state, const Matc
// high-priority properties first, i.e., those properties that other properties depend on.
// The order is (1) high-priority not important, (2) high-priority important, (3) normal not important
// and (4) normal important.
- state.setLineHeightValue(0);
applyMatchedProperties<HighPriorityProperties>(state, matchResult, false, 0, matchResult.matchedProperties.size() - 1, applyInheritedOnly);
applyMatchedProperties<HighPriorityProperties>(state, matchResult, true, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, applyInheritedOnly);
applyMatchedProperties<HighPriorityProperties>(state, matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
@@ -1459,10 +1418,6 @@ void StyleResolver::applyMatchedProperties(StyleResolverState& state, const Matc
// If our font got dirtied, go ahead and update it now.
updateFont(state);
- // Line-height is set when we are sure we decided on the font-size.
- if (state.lineHeightValue())
- StyleBuilder::applyProperty(CSSPropertyLineHeight, state, state.lineHeightValue());
-
// Many properties depend on the font. If it changes we just apply all properties.
if (cachedMatchedProperties && cachedMatchedProperties->renderStyle->fontDescription() != state.style()->fontDescription())
applyInheritedOnly = false;
@@ -1543,8 +1498,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.h ('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