Commit 3407d103 authored by pdr@chromium.org's avatar pdr@chromium.org

Reduce the number of CachedUAStyles

CachedUAStyles are large (160b) and not cheap to create because they
include several Length objects which have non-trivial destructors,
making CachedUAStyles expensive to destruct. This patch reduces the
number of CachedUAStyles by only creating them when needed.

There are two changes in this patch:
1) CachedUAStyle& is no longer a member on StyleAdjuster but is
instead passed in through StyleAdjuster::adjustRenderStyle.

2) The cached UA style is now constructed as-needed in
StyleResolverState::cacheUserAgentBorderAndBackground.
StyleResolverState always owned the CachedUAStyle but this patch
makes it explicit by changing the type to OwnPtr<CachedUAStyle>. A
null check was needed in RenderTheme::adjustStyle but otherwise
CachedUAStyle was not accessed without being set.

Before patch:
  facebook.com - 19,850 CachedUAStyles constructed
  wikipedia cat - 22,686 CachedUAStyles constructed

With patch:
  facebook.com - 538 CachedUAStyles constructed
  wikipedia cat - 16 CachedUAStyles constructed

Review URL: https://codereview.chromium.org/338673002

git-svn-id: svn://svn.chromium.org/blink/trunk@176163 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent b5f0d23c
...@@ -194,7 +194,7 @@ static bool hasWillChangeThatCreatesStackingContext(const RenderStyle* style) ...@@ -194,7 +194,7 @@ static bool hasWillChangeThatCreatesStackingContext(const RenderStyle* style)
return false; return false;
} }
void StyleAdjuster::adjustRenderStyle(RenderStyle* style, RenderStyle* parentStyle, Element *e) void StyleAdjuster::adjustRenderStyle(RenderStyle* style, RenderStyle* parentStyle, Element *e, const CachedUAStyle* cachedUAStyle)
{ {
ASSERT(parentStyle); ASSERT(parentStyle);
...@@ -259,7 +259,7 @@ void StyleAdjuster::adjustRenderStyle(RenderStyle* style, RenderStyle* parentSty ...@@ -259,7 +259,7 @@ void StyleAdjuster::adjustRenderStyle(RenderStyle* style, RenderStyle* parentSty
// Let the theme also have a crack at adjusting the style. // Let the theme also have a crack at adjusting the style.
if (style->hasAppearance()) if (style->hasAppearance())
RenderTheme::theme().adjustStyle(style, e, m_cachedUAStyle); RenderTheme::theme().adjustStyle(style, e, cachedUAStyle);
// If we have first-letter pseudo style, transitions, or animations, do not share this style. // If we have first-letter pseudo style, transitions, or animations, do not share this style.
if (style->hasPseudoStyle(FIRST_LETTER) || style->transitions() || style->animations()) if (style->hasPseudoStyle(FIRST_LETTER) || style->transitions() || style->animations())
......
...@@ -36,19 +36,17 @@ class RenderStyle; ...@@ -36,19 +36,17 @@ class RenderStyle;
class StyleAdjuster { class StyleAdjuster {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
StyleAdjuster(const CachedUAStyle& cachedUAStyle, bool useQuirksModeStyles) StyleAdjuster(bool useQuirksModeStyles)
: m_cachedUAStyle(cachedUAStyle) : m_useQuirksModeStyles(useQuirksModeStyles)
, m_useQuirksModeStyles(useQuirksModeStyles)
{ } { }
void adjustRenderStyle(RenderStyle* styleToAdjust, RenderStyle* parentStyle, Element*); void adjustRenderStyle(RenderStyle* styleToAdjust, RenderStyle* parentStyle, Element*, const CachedUAStyle*);
private: private:
void adjustStyleForDisplay(RenderStyle* styleToAdjust, RenderStyle* parentStyle); void adjustStyleForDisplay(RenderStyle* styleToAdjust, RenderStyle* parentStyle);
void adjustStyleForTagName(RenderStyle* styleToAdjust, RenderStyle* parentStyle, Element&); void adjustStyleForTagName(RenderStyle* styleToAdjust, RenderStyle* parentStyle, Element&);
void adjustOverflow(RenderStyle* styleToAdjust); void adjustOverflow(RenderStyle* styleToAdjust);
const CachedUAStyle& m_cachedUAStyle;
bool m_useQuirksModeStyles; bool m_useQuirksModeStyles;
}; };
......
...@@ -581,8 +581,8 @@ static void addContentAttrValuesToFeatures(const Vector<AtomicString>& contentAt ...@@ -581,8 +581,8 @@ static void addContentAttrValuesToFeatures(const Vector<AtomicString>& contentAt
void StyleResolver::adjustRenderStyle(StyleResolverState& state, Element* element) void StyleResolver::adjustRenderStyle(StyleResolverState& state, Element* element)
{ {
StyleAdjuster adjuster(state.cachedUAStyle(), m_document.inQuirksMode()); StyleAdjuster adjuster(m_document.inQuirksMode());
adjuster.adjustRenderStyle(state.style(), state.parentStyle(), element); adjuster.adjustRenderStyle(state.style(), state.parentStyle(), element, state.cachedUAStyle());
} }
// Start loading resources referenced by this style. // Start loading resources referenced by this style.
......
...@@ -100,9 +100,14 @@ public: ...@@ -100,9 +100,14 @@ public:
// and constructing it is expensive so we avoid it if possible. // and constructing it is expensive so we avoid it if possible.
if (!style()->hasAppearance()) if (!style()->hasAppearance())
return; return;
m_cachedUAStyle = CachedUAStyle(style());
m_cachedUAStyle = CachedUAStyle::create(style());
}
const CachedUAStyle* cachedUAStyle() const
{
return m_cachedUAStyle.get();
} }
const CachedUAStyle& cachedUAStyle() const { return m_cachedUAStyle; }
ElementStyleResources& elementStyleResources() { return m_elementStyleResources; } ElementStyleResources& elementStyleResources() { return m_elementStyleResources; }
const CSSToStyleMap& styleMap() const { return m_styleMap; } const CSSToStyleMap& styleMap() const { return m_styleMap; }
...@@ -150,7 +155,7 @@ private: ...@@ -150,7 +155,7 @@ private:
FontBuilder m_fontBuilder; FontBuilder m_fontBuilder;
CachedUAStyle m_cachedUAStyle; OwnPtr<CachedUAStyle> m_cachedUAStyle;
ElementStyleResources m_elementStyleResources; ElementStyleResources m_elementStyleResources;
// CSSToStyleMap is a pure-logic class and only contains // CSSToStyleMap is a pure-logic class and only contains
......
...@@ -86,7 +86,7 @@ RenderTheme::RenderTheme() ...@@ -86,7 +86,7 @@ RenderTheme::RenderTheme()
{ {
} }
void RenderTheme::adjustStyle(RenderStyle* style, Element* e, const CachedUAStyle& uaStyle) void RenderTheme::adjustStyle(RenderStyle* style, Element* e, const CachedUAStyle* uaStyle)
{ {
// Force inline and table display styles to be inline-block (except for table- which is block) // Force inline and table display styles to be inline-block (except for table- which is block)
ControlPart part = style->appearance(); ControlPart part = style->appearance();
...@@ -98,7 +98,7 @@ void RenderTheme::adjustStyle(RenderStyle* style, Element* e, const CachedUAStyl ...@@ -98,7 +98,7 @@ void RenderTheme::adjustStyle(RenderStyle* style, Element* e, const CachedUAStyl
else if (style->display() == LIST_ITEM || style->display() == TABLE) else if (style->display() == LIST_ITEM || style->display() == TABLE)
style->setDisplay(BLOCK); style->setDisplay(BLOCK);
if (uaStyle.hasAppearance && isControlStyled(style, uaStyle)) { if (uaStyle && uaStyle->hasAppearance && isControlStyled(style, uaStyle)) {
if (part == MenulistPart) { if (part == MenulistPart) {
style->setAppearance(MenulistButtonPart); style->setAppearance(MenulistButtonPart);
part = MenulistButtonPart; part = MenulistButtonPart;
...@@ -568,8 +568,10 @@ static bool isBackgroundOrBorderStyled(const RenderStyle& style, const CachedUAS ...@@ -568,8 +568,10 @@ static bool isBackgroundOrBorderStyled(const RenderStyle& style, const CachedUAS
|| style.visitedDependentColor(CSSPropertyBackgroundColor) != uaStyle.backgroundColor; || style.visitedDependentColor(CSSPropertyBackgroundColor) != uaStyle.backgroundColor;
} }
bool RenderTheme::isControlStyled(const RenderStyle* style, const CachedUAStyle& uaStyle) const bool RenderTheme::isControlStyled(const RenderStyle* style, const CachedUAStyle* uaStyle) const
{ {
ASSERT(uaStyle);
switch (style->appearance()) { switch (style->appearance()) {
case PushButtonPart: case PushButtonPart:
case SquareButtonPart: case SquareButtonPart:
...@@ -580,14 +582,14 @@ bool RenderTheme::isControlStyled(const RenderStyle* style, const CachedUAStyle& ...@@ -580,14 +582,14 @@ bool RenderTheme::isControlStyled(const RenderStyle* style, const CachedUAStyle&
case ContinuousCapacityLevelIndicatorPart: case ContinuousCapacityLevelIndicatorPart:
case DiscreteCapacityLevelIndicatorPart: case DiscreteCapacityLevelIndicatorPart:
case RatingLevelIndicatorPart: case RatingLevelIndicatorPart:
return isBackgroundOrBorderStyled(*style, uaStyle); return isBackgroundOrBorderStyled(*style, *uaStyle);
case ListboxPart: case ListboxPart:
case MenulistPart: case MenulistPart:
case SearchFieldPart: case SearchFieldPart:
case TextAreaPart: case TextAreaPart:
case TextFieldPart: case TextFieldPart:
return isBackgroundOrBorderStyled(*style, uaStyle) || style->boxShadow(); return isBackgroundOrBorderStyled(*style, *uaStyle) || style->boxShadow();
case SliderHorizontalPart: case SliderHorizontalPart:
case SliderVerticalPart: case SliderVerticalPart:
......
...@@ -65,7 +65,7 @@ public: ...@@ -65,7 +65,7 @@ public:
// metrics and defaults given the contents of the style. This includes sophisticated operations like // metrics and defaults given the contents of the style. This includes sophisticated operations like
// selection of control size based off the font, the disabling of appearance when certain other properties like // selection of control size based off the font, the disabling of appearance when certain other properties like
// "border" are set, or if the appearance is not supported by the theme. // "border" are set, or if the appearance is not supported by the theme.
void adjustStyle(RenderStyle*, Element*, const CachedUAStyle&); void adjustStyle(RenderStyle*, Element*, const CachedUAStyle*);
// This method is called to paint the widget as a background of the RenderObject. A widget's foreground, e.g., the // This method is called to paint the widget as a background of the RenderObject. A widget's foreground, e.g., the
// text of a button, is always rendered by the engine itself. The boolean return value indicates // text of a button, is always rendered by the engine itself. The boolean return value indicates
...@@ -97,7 +97,7 @@ public: ...@@ -97,7 +97,7 @@ public:
virtual bool controlSupportsTints(const RenderObject*) const { return false; } virtual bool controlSupportsTints(const RenderObject*) const { return false; }
// Whether or not the control has been styled enough by the author to disable the native appearance. // Whether or not the control has been styled enough by the author to disable the native appearance.
virtual bool isControlStyled(const RenderStyle*, const CachedUAStyle&) const; virtual bool isControlStyled(const RenderStyle*, const CachedUAStyle*) const;
// A general method asking if any control tinting is supported at all. // A general method asking if any control tinting is supported at all.
virtual bool supportsControlTints() const { return false; } virtual bool supportsControlTints() const { return false; }
......
...@@ -44,7 +44,7 @@ public: ...@@ -44,7 +44,7 @@ public:
virtual void adjustRepaintRect(const RenderObject*, IntRect&) OVERRIDE; virtual void adjustRepaintRect(const RenderObject*, IntRect&) OVERRIDE;
virtual bool isControlStyled(const RenderStyle*, const CachedUAStyle&) const OVERRIDE; virtual bool isControlStyled(const RenderStyle*, const CachedUAStyle*) const OVERRIDE;
virtual Color platformActiveSelectionBackgroundColor() const OVERRIDE; virtual Color platformActiveSelectionBackgroundColor() const OVERRIDE;
virtual Color platformInactiveSelectionBackgroundColor() const OVERRIDE; virtual Color platformInactiveSelectionBackgroundColor() const OVERRIDE;
......
...@@ -510,10 +510,11 @@ Color RenderThemeChromiumMac::systemColor(CSSValueID cssValueId) const ...@@ -510,10 +510,11 @@ Color RenderThemeChromiumMac::systemColor(CSSValueID cssValueId) const
return color; return color;
} }
bool RenderThemeChromiumMac::isControlStyled(const RenderStyle* style, const CachedUAStyle& uaStyle) const bool RenderThemeChromiumMac::isControlStyled(const RenderStyle* style, const CachedUAStyle* uaStyle) const
{ {
ASSERT(uaStyle);
if (style->appearance() == TextFieldPart || style->appearance() == TextAreaPart || style->appearance() == ListboxPart) if (style->appearance() == TextFieldPart || style->appearance() == TextAreaPart || style->appearance() == ListboxPart)
return style->border() != uaStyle.border || style->boxShadow(); return style->border() != uaStyle->border || style->boxShadow();
// FIXME: This is horrible, but there is not much else that can be done. Menu lists cannot draw properly when // FIXME: This is horrible, but there is not much else that can be done. Menu lists cannot draw properly when
// scaled. They can't really draw properly when transformed either. We can't detect the transform case at style // scaled. They can't really draw properly when transformed either. We can't detect the transform case at style
......
...@@ -32,12 +32,17 @@ namespace WebCore { ...@@ -32,12 +32,17 @@ namespace WebCore {
// applyMatchedProperties for later use during adjustRenderStyle. // applyMatchedProperties for later use during adjustRenderStyle.
class CachedUAStyle { class CachedUAStyle {
public: public:
CachedUAStyle() static PassOwnPtr<CachedUAStyle> create(const RenderStyle* style)
: hasAppearance(false) {
, backgroundLayers(BackgroundFillLayer) return adoptPtr(new CachedUAStyle(style));
, backgroundColor(StyleColor::currentColor()) }
{ }
bool hasAppearance;
BorderData border;
FillLayer backgroundLayers;
StyleColor backgroundColor;
private:
explicit CachedUAStyle(const RenderStyle* style) explicit CachedUAStyle(const RenderStyle* style)
: hasAppearance(true) : hasAppearance(true)
, backgroundLayers(BackgroundFillLayer) , backgroundLayers(BackgroundFillLayer)
...@@ -48,14 +53,10 @@ public: ...@@ -48,14 +53,10 @@ public:
backgroundLayers = *style->backgroundLayers(); backgroundLayers = *style->backgroundLayers();
backgroundColor = style->backgroundColor(); backgroundColor = style->backgroundColor();
} }
bool hasAppearance;
BorderData border;
FillLayer backgroundLayers;
StyleColor backgroundColor;
}; };
} // namespace WebCore } // namespace WebCore
#endif // CachedUAStyle_h #endif // CachedUAStyle_h
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment