StyleResolver state should store user agent appearance style as RenderStyle
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Nov 2019 21:10:11 +0000 (21:10 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Nov 2019 21:10:11 +0000 (21:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204008

Reviewed by Zalan Bujtas.

Generate and pass around user agent style as RenderStyle for apperance computation instead
of awkwardly passing around separate property values.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::styleForElement):
(WebCore::StyleResolver::styleForKeyframe):
(WebCore::StyleResolver::pseudoStyleForElement):
(WebCore::StyleResolver::adjustRenderStyle):
(WebCore::StyleResolver::applyMatchedProperties):
(WebCore::StyleResolver::State::cacheBorderAndBackground): Deleted.
* css/StyleResolver.h:
(WebCore::StyleResolver::element const):
(WebCore::StyleResolver::State::userAgentAppearanceStyle const):
(WebCore::StyleResolver::State::setUserAgentAppearanceStyle):
(WebCore::StyleResolver::element): Deleted.
(WebCore::StyleResolver::State::hasUAAppearance const): Deleted.
(WebCore::StyleResolver::State::borderData const): Deleted.
(WebCore::StyleResolver::State::backgroundData const): Deleted.
(WebCore::StyleResolver::State::backgroundColor const): Deleted.
* rendering/RenderTheme.cpp:
(WebCore::RenderTheme::adjustStyle):
(WebCore::RenderTheme::isControlStyled const):
* rendering/RenderTheme.h:
* rendering/RenderThemeIOS.h:
* rendering/RenderThemeIOS.mm:
(WebCore::RenderThemeIOS::isControlStyled const):
* rendering/RenderThemeMac.h:
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::isControlStyled const):
* style/StyleBuilder.cpp:
(WebCore::Style::Builder::Builder):

Allow providing style separately from StyleResolver state.

* style/StyleBuilder.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252255 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h
Source/WebCore/rendering/RenderTheme.cpp
Source/WebCore/rendering/RenderTheme.h
Source/WebCore/rendering/RenderThemeIOS.h
Source/WebCore/rendering/RenderThemeIOS.mm
Source/WebCore/rendering/RenderThemeMac.h
Source/WebCore/rendering/RenderThemeMac.mm
Source/WebCore/style/StyleBuilder.cpp
Source/WebCore/style/StyleBuilder.h

index 96d5483..7697acc 100644 (file)
@@ -1,3 +1,46 @@
+2019-11-08  Antti Koivisto  <antti@apple.com>
+
+        StyleResolver state should store user agent appearance style as RenderStyle
+        https://bugs.webkit.org/show_bug.cgi?id=204008
+
+        Reviewed by Zalan Bujtas.
+
+        Generate and pass around user agent style as RenderStyle for apperance computation instead
+        of awkwardly passing around separate property values.
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::styleForElement):
+        (WebCore::StyleResolver::styleForKeyframe):
+        (WebCore::StyleResolver::pseudoStyleForElement):
+        (WebCore::StyleResolver::adjustRenderStyle):
+        (WebCore::StyleResolver::applyMatchedProperties):
+        (WebCore::StyleResolver::State::cacheBorderAndBackground): Deleted.
+        * css/StyleResolver.h:
+        (WebCore::StyleResolver::element const):
+        (WebCore::StyleResolver::State::userAgentAppearanceStyle const):
+        (WebCore::StyleResolver::State::setUserAgentAppearanceStyle):
+        (WebCore::StyleResolver::element): Deleted.
+        (WebCore::StyleResolver::State::hasUAAppearance const): Deleted.
+        (WebCore::StyleResolver::State::borderData const): Deleted.
+        (WebCore::StyleResolver::State::backgroundData const): Deleted.
+        (WebCore::StyleResolver::State::backgroundColor const): Deleted.
+        * rendering/RenderTheme.cpp:
+        (WebCore::RenderTheme::adjustStyle):
+        (WebCore::RenderTheme::isControlStyled const):
+        * rendering/RenderTheme.h:
+        * rendering/RenderThemeIOS.h:
+        * rendering/RenderThemeIOS.mm:
+        (WebCore::RenderThemeIOS::isControlStyled const):
+        * rendering/RenderThemeMac.h:
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::isControlStyled const):
+        * style/StyleBuilder.cpp:
+        (WebCore::Style::Builder::Builder):
+
+        Allow providing style separately from StyleResolver state.
+
+        * style/StyleBuilder.h:
+
 2019-11-08  Daniel Bates  <dabates@apple.com>
 
         Add WebKit Legacy SPI to retrieve editable elements in rect
index 6de23a5..6dca902 100644 (file)
@@ -97,21 +97,12 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-inline void StyleResolver::State::cacheBorderAndBackground()
-{
-    m_hasUAAppearance = m_style->hasAppearance();
-    if (m_hasUAAppearance) {
-        m_borderData = m_style->border();
-        m_backgroundData = m_style->backgroundLayers();
-        m_backgroundColor = m_style->backgroundColor();
-    }
-}
-
 inline void StyleResolver::State::clear()
 {
     m_element = nullptr;
     m_parentStyle = nullptr;
     m_ownedParentStyle = nullptr;
+    m_userAgentAppearanceStyle = nullptr;
 }
 
 StyleResolver::StyleResolver(Document& document)
@@ -285,7 +276,7 @@ ElementStyle StyleResolver::styleForElement(const Element& element, const Render
     applyMatchedProperties(collector.matchResult(), element);
 
     // Clean up our style object's display and text decorations (among other fixups).
-    adjustRenderStyle(*state.style(), *state.parentStyle(), parentBoxStyle, &element);
+    adjustRenderStyle(*state.style(), *state.parentStyle(), parentBoxStyle, &element, state.userAgentAppearanceStyle());
 
     if (state.style()->hasViewportUnits())
         document().setHasStyleWithViewportUnits();
@@ -313,7 +304,7 @@ std::unique_ptr<RenderStyle> StyleResolver::styleForKeyframe(const RenderStyle*
     Style::Builder builder(*this, result, { Style::CascadeLevel::Author });
     builder.applyAllProperties();
 
-    adjustRenderStyle(*state.style(), *state.parentStyle(), nullptr, nullptr);
+    adjustRenderStyle(*state.style(), *state.parentStyle(), nullptr, nullptr, state.userAgentAppearanceStyle());
 
     // Add all the animating properties to the keyframe.
     unsigned propertyCount = keyframe->properties().propertyCount();
@@ -469,7 +460,7 @@ std::unique_ptr<RenderStyle> StyleResolver::pseudoStyleForElement(const Element&
     applyMatchedProperties(collector.matchResult(), element);
 
     // Clean up our style object's display and text decorations (among other fixups).
-    adjustRenderStyle(*state.style(), *m_state.parentStyle(), parentBoxStyle, nullptr);
+    adjustRenderStyle(*state.style(), *m_state.parentStyle(), parentBoxStyle, nullptr, state.userAgentAppearanceStyle());
 
     if (state.style()->hasViewportUnits())
         document().setHasStyleWithViewportUnits();
@@ -779,7 +770,7 @@ bool StyleResolver::adjustRenderStyleForTextAutosizing(RenderStyle& style, const
 }
 #endif
 
-void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const Element* element)
+void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const Element* element, const RenderStyle* userAgentAppearanceStyle)
 {
     // If the composed tree parent has display:contents, the parent box style will be different from the parent style.
     // We don't have it when resolving computed style for display:none subtree. Use parent style for adjustments in that case.
@@ -999,7 +990,7 @@ void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& par
 
     // Let the theme also have a crack at adjusting the style.
     if (style.hasAppearance())
-        RenderTheme::singleton().adjustStyle(*this, style, element, m_state.hasUAAppearance(), m_state.borderData(), m_state.backgroundData(), m_state.backgroundColor());
+        RenderTheme::singleton().adjustStyle(*this, style, element, userAgentAppearanceStyle);
 
     // If we have first-letter pseudo style, do not share this style.
     if (style.hasPseudoStyle(PseudoId::FirstLetter))
@@ -1138,14 +1129,14 @@ void StyleResolver::applyMatchedProperties(const MatchResult& matchResult, const
     }
 
     if (elementTypeHasAppearanceFromUAStyle(element)) {
-        // FIXME: This is such a hack.
         // Find out if there's a -webkit-appearance property in effect from the UA sheet.
         // If so, we cache the border and background styles so that RenderTheme::adjustStyle()
         // can look at them later to figure out if this is a styled form control or not.
-        Style::Builder builder(*this, matchResult, { Style::CascadeLevel::UserAgent }, includedProperties);
+        auto userAgentStyle = RenderStyle::clonePtr(style);
+        Style::Builder builder(*userAgentStyle, *this, matchResult, { Style::CascadeLevel::UserAgent });
         builder.applyAllProperties();
 
-        state.cacheBorderAndBackground();
+        state.setUserAgentAppearanceStyle(WTFMove(userAgentStyle));
     }
 
     Style::Builder builder(*this, matchResult, Style::allCascadeLevels(), includedProperties);
index 9c8205a..849d569 100644 (file)
@@ -117,7 +117,7 @@ public:
     RenderStyle* style() const { return m_state.style(); }
     const RenderStyle* parentStyle() const { return m_state.parentStyle(); }
     const RenderStyle* rootElementStyle() const { return m_state.rootElementStyle(); }
-    const Element* element() { return m_state.element(); }
+    const Element* element() const { return m_state.element(); }
     Document& document() { return m_document; }
     const Document& document() const { return m_document; }
     const Settings& settings() const { return m_document.settings(); }
@@ -183,7 +183,7 @@ public:
     void clearCachedDeclarationsAffectedByViewportUnits();
 
 private:
-    void adjustRenderStyle(RenderStyle&, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const Element*);
+    void adjustRenderStyle(RenderStyle&, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const Element*, const RenderStyle* userAgentStyle);
     void adjustRenderStyleForSiteSpecificQuirks(RenderStyle&, const Element&);
 
     enum class UseMatchedDeclarationsCache { Yes, No };
@@ -215,11 +215,8 @@ public:
         const RenderStyle* parentStyle() const { return m_parentStyle; }
         const RenderStyle* rootElementStyle() const { return m_rootElementStyle; }
 
-        void cacheBorderAndBackground();
-        bool hasUAAppearance() const { return m_hasUAAppearance; }
-        BorderData borderData() const { return m_borderData; }
-        FillLayer backgroundData() const { return m_backgroundData; }
-        const Color& backgroundColor() const { return m_backgroundColor; }
+        const RenderStyle* userAgentAppearanceStyle() const { return m_userAgentAppearanceStyle.get(); }
+        void setUserAgentAppearanceStyle(std::unique_ptr<RenderStyle> style) { m_userAgentAppearanceStyle = WTFMove(style); }
 
         const SelectorFilter* selectorFilter() const { return m_selectorFilter; }
         
@@ -232,11 +229,7 @@ public:
 
         const SelectorFilter* m_selectorFilter { nullptr };
 
-        BorderData m_borderData;
-        FillLayer m_backgroundData { FillLayerType::Background };
-        Color m_backgroundColor;
-
-        bool m_hasUAAppearance { false };
+        std::unique_ptr<RenderStyle> m_userAgentAppearanceStyle;
     };
 
     State& state() { return m_state; }
index 437f4c5..9b2dfec 100644 (file)
@@ -78,7 +78,7 @@ RenderTheme::RenderTheme()
 {
 }
 
-void RenderTheme::adjustStyle(StyleResolver& styleResolver, RenderStyle& style, const Element* element, bool UAHasAppearance, const BorderData& border, const FillLayer& background, const Color& backgroundColor)
+void RenderTheme::adjustStyle(StyleResolver& styleResolver, RenderStyle& style, const Element* element, const RenderStyle* userAgentAppearanceStyle)
 {
     // Force inline and table display styles to be inline-block (except for table- which is block)
     ControlPart part = style.appearance();
@@ -90,7 +90,7 @@ void RenderTheme::adjustStyle(StyleResolver& styleResolver, RenderStyle& style,
     else if (style.display() == DisplayType::Compact || style.display() == DisplayType::ListItem || style.display() == DisplayType::Table)
         style.setDisplay(DisplayType::Block);
 
-    if (UAHasAppearance && isControlStyled(style, border, background, backgroundColor)) {
+    if (userAgentAppearanceStyle && isControlStyled(style, *userAgentAppearanceStyle)) {
         switch (part) {
         case MenulistPart:
             style.setAppearance(MenulistButtonPart);
@@ -730,7 +730,7 @@ bool RenderTheme::isControlContainer(ControlPart appearance) const
     return appearance != CheckboxPart && appearance != RadioPart;
 }
 
-bool RenderTheme::isControlStyled(const RenderStyle& style, const BorderData& border, const FillLayer& background, const Color& backgroundColor) const
+bool RenderTheme::isControlStyled(const RenderStyle& style, const RenderStyle& userAgentStyle) const
 {
     switch (style.appearance()) {
     case PushButtonPart:
@@ -752,9 +752,9 @@ bool RenderTheme::isControlStyled(const RenderStyle& style, const BorderData& bo
     case TextFieldPart:
     case TextAreaPart:
         // Test the style to see if the UA border and background match.
-        return style.border() != border
-            || style.backgroundLayers() != background
-            || !style.backgroundColorEqualsToColorIgnoringVisited(backgroundColor);
+        return style.border() != userAgentStyle.border()
+            || style.backgroundLayers() != userAgentStyle.backgroundLayers()
+            || !style.backgroundColorEqualsToColorIgnoringVisited(userAgentStyle.backgroundColor());
     default:
         return false;
     }
index 3af56ae..19acf0b 100644 (file)
@@ -65,7 +65,7 @@ public:
     // 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
     // "border" are set, or if the appearance is not supported by the theme.
-    void adjustStyle(StyleResolver&, RenderStyle&, const Element*,  bool UAHasAppearance, const BorderData&, const FillLayer&, const Color& backgroundColor);
+    void adjustStyle(StyleResolver&, RenderStyle&, const Element*, const RenderStyle* userAgentAppearanceStyle);
 
     // 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
@@ -116,7 +116,7 @@ public:
     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.
-    virtual bool isControlStyled(const RenderStyle&, const BorderData&, const FillLayer&, const Color& backgroundColor) const;
+    virtual bool isControlStyled(const RenderStyle&, const RenderStyle& userAgentStyle) const;
 
     // A general method asking if any control tinting is supported at all.
     virtual bool supportsControlTints() const { return false; }
index 7385910..7e39ec6 100644 (file)
@@ -66,7 +66,7 @@ protected:
     void updateCachedSystemFontDescription(CSSValueID, FontCascadeDescription&) const override;
     int baselinePosition(const RenderBox&) const override;
 
-    bool isControlStyled(const RenderStyle&, const BorderData&, const FillLayer& background, const Color& backgroundColor) const override;
+    bool isControlStyled(const RenderStyle&, const RenderStyle& userAgentStyle) const override;
 
     // Methods for each appearance value.
     void adjustCheckboxStyle(StyleResolver&, RenderStyle&, const Element*) const override;
index e31a837..10af040 100644 (file)
@@ -447,16 +447,16 @@ int RenderThemeIOS::baselinePosition(const RenderBox& box) const
     return RenderTheme::baselinePosition(box);
 }
 
-bool RenderThemeIOS::isControlStyled(const RenderStyle& style, const BorderData& border, const FillLayer& background, const Color& backgroundColor) const
+bool RenderThemeIOS::isControlStyled(const RenderStyle& style, const RenderStyle& userAgentStyle) const
 {
     // Buttons and MenulistButtons are styled if they contain a background image.
     if (style.appearance() == PushButtonPart || style.appearance() == MenulistButtonPart)
         return !style.visitedDependentColor(CSSPropertyBackgroundColor).isVisible() || style.backgroundLayers().hasImage();
 
     if (style.appearance() == TextFieldPart || style.appearance() == TextAreaPart)
-        return style.backgroundLayers() != background;
+        return style.backgroundLayers() != userAgentStyle.backgroundLayers();
 
-    return RenderTheme::isControlStyled(style, border, background, backgroundColor);
+    return RenderTheme::isControlStyled(style, userAgentStyle);
 }
 
 void RenderThemeIOS::adjustRadioStyle(StyleResolver&, RenderStyle& style, const Element*) const
index a1361af..1b9f460 100644 (file)
@@ -49,7 +49,7 @@ public:
 
     void adjustRepaintRect(const RenderObject&, FloatRect&) final;
 
-    bool isControlStyled(const RenderStyle&, const BorderData&, const FillLayer&, const Color& backgroundColor) const final;
+    bool isControlStyled(const RenderStyle&, const RenderStyle& userAgentStyle) const final;
 
     bool supportsSelectionForegroundColors(OptionSet<StyleColor::Options>) const final;
 
index a42ce6e..aa5991c 100644 (file)
@@ -882,11 +882,10 @@ bool RenderThemeMac::usesTestModeFocusRingColor() const
     return WebCore::usesTestModeFocusRingColor();
 }
 
-bool RenderThemeMac::isControlStyled(const RenderStyle& style, const BorderData& border,
-                                     const FillLayer& background, const Color& backgroundColor) const
+bool RenderThemeMac::isControlStyled(const RenderStyle& style, const RenderStyle& userAgentStyle) const
 {
     if (style.appearance() == TextFieldPart || style.appearance() == TextAreaPart || style.appearance() == ListboxPart)
-        return style.border() != border;
+        return style.border() != userAgentStyle.border();
 
     // 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
@@ -895,7 +894,7 @@ bool RenderThemeMac::isControlStyled(const RenderStyle& style, const BorderData&
     if (style.appearance() == MenulistPart && style.effectiveZoom() != 1.0f)
         return true;
 
-    return RenderTheme::isControlStyled(style, border, background, backgroundColor);
+    return RenderTheme::isControlStyled(style, userAgentStyle);
 }
 
 static FloatRect inflateRect(const FloatRect& rect, const IntSize& size, const int* margins, float zoomLevel)
index e58c203..132896c 100644 (file)
@@ -49,12 +49,17 @@ static PropertyCascade::Direction directionFromStyle(const RenderStyle& style)
     return { style.direction(), style.writingMode() };
 }
 
+Builder::Builder(RenderStyle& style, const StyleResolver& resolver, const MatchResult& matchResult, OptionSet<CascadeLevel> cascadeLevels, PropertyCascade::IncludedProperties includedProperties)
+    : m_cascade(matchResult, cascadeLevels, includedProperties, directionFromStyle(style))
+    , m_state(*this, style, *resolver.parentStyle(), resolver.rootElementStyle(), resolver.document(), resolver.element())
+{
+    ASSERT(resolver.parentStyle());
+}
+
 Builder::Builder(StyleResolver& resolver, const MatchResult& matchResult, OptionSet<CascadeLevel> cascadeLevels, PropertyCascade::IncludedProperties includedProperties)
-    : m_cascade(matchResult, cascadeLevels, includedProperties, directionFromStyle(*resolver.style()))
-    , m_state(*this, *resolver.style(), *resolver.parentStyle(), resolver.rootElementStyle(), resolver.document(), resolver.element())
+    : Builder(*resolver.style(), resolver, matchResult, cascadeLevels, includedProperties)
 {
     ASSERT(resolver.style());
-    ASSERT(resolver.parentStyle());
 }
 
 Builder::~Builder() = default;
index 9fc080b..463d857 100644 (file)
@@ -35,6 +35,7 @@ namespace Style {
 class Builder {
     WTF_MAKE_FAST_ALLOCATED;
 public:
+    Builder(RenderStyle&, const StyleResolver&, const MatchResult&, OptionSet<CascadeLevel>, PropertyCascade::IncludedProperties = PropertyCascade::IncludedProperties::All);
     Builder(StyleResolver&, const MatchResult&, OptionSet<CascadeLevel>, PropertyCascade::IncludedProperties = PropertyCascade::IncludedProperties::All);
     ~Builder();