[webkit-changes] [226809] trunk/Source/WebCore

Thu, 11 Jan 2018 14:43:58 -0800

Title: [226809] trunk/Source/WebCore
Revision
226809
Author
[email protected]
Date
2018-01-11 14:43:29 -0800 (Thu, 11 Jan 2018)

Log Message

Don't call RenderElement::setStyle when nothing changes
https://bugs.webkit.org/show_bug.cgi?id=181530

Reviewed by Zalan Bujtas.

* style/StyleChange.h:

Remove 'Force' value. This essentially meant 'compute style for all descendants and call setStyle unconditionally'.
Using this value lost information about whether anything actually changed in a particular style as it was automatically
inherited by all descendants. The 'compute all descendants' part of the behavior is what is actually needed.

Instead add separate DescendantsToResolve enum for communicating what else to compute.

* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::Parent::Parent):
(WebCore::Style::computeDescendantsToResolve):

    Figure out which descendants will need resolving based on how the current elements style changed.

(WebCore::Style::TreeResolver::resolveElement):
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
(WebCore::Style::TreeResolver::pushParent):
(WebCore::Style::shouldResolveElement):

    Use DescendantsToResolve as input.

(WebCore::Style::TreeResolver::resolveComposedTree):
* style/StyleTreeResolver.h:
* style/StyleUpdate.h:
(WebCore::Style::ElementUpdates::ElementUpdates):

    Add DescendantsToResolve.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (226808 => 226809)


--- trunk/Source/WebCore/ChangeLog	2018-01-11 22:32:43 UTC (rev 226808)
+++ trunk/Source/WebCore/ChangeLog	2018-01-11 22:43:29 UTC (rev 226809)
@@ -1,3 +1,38 @@
+2018-01-11  Antti Koivisto  <[email protected]>
+
+        Don't call RenderElement::setStyle when nothing changes
+        https://bugs.webkit.org/show_bug.cgi?id=181530
+
+        Reviewed by Zalan Bujtas.
+
+        * style/StyleChange.h:
+
+        Remove 'Force' value. This essentially meant 'compute style for all descendants and call setStyle unconditionally'.
+        Using this value lost information about whether anything actually changed in a particular style as it was automatically
+        inherited by all descendants. The 'compute all descendants' part of the behavior is what is actually needed.
+
+        Instead add separate DescendantsToResolve enum for communicating what else to compute.
+
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::Parent::Parent):
+        (WebCore::Style::computeDescendantsToResolve):
+
+            Figure out which descendants will need resolving based on how the current elements style changed.
+
+        (WebCore::Style::TreeResolver::resolveElement):
+        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+        (WebCore::Style::TreeResolver::pushParent):
+        (WebCore::Style::shouldResolveElement):
+
+            Use DescendantsToResolve as input.
+
+        (WebCore::Style::TreeResolver::resolveComposedTree):
+        * style/StyleTreeResolver.h:
+        * style/StyleUpdate.h:
+        (WebCore::Style::ElementUpdates::ElementUpdates):
+
+            Add DescendantsToResolve.
+
 2018-01-11  Wenson Hsieh  <[email protected]>
 
         Send PromisedBlobInfo to the client through DragItem instead of DragClient::prepareToDragPromisedBlob

Modified: trunk/Source/WebCore/style/StyleChange.h (226808 => 226809)


--- trunk/Source/WebCore/style/StyleChange.h	2018-01-11 22:32:43 UTC (rev 226808)
+++ trunk/Source/WebCore/style/StyleChange.h	2018-01-11 22:43:29 UTC (rev 226809)
@@ -31,7 +31,7 @@
 
 namespace Style {
 
-enum Change { NoChange, NoInherit, Inherit, Force, Detach };
+enum Change { NoChange, NoInherit, Inherit, Detach };
 
 Change determineChange(const RenderStyle&, const RenderStyle&);

Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (226808 => 226809)


--- trunk/Source/WebCore/style/StyleTreeResolver.cpp	2018-01-11 22:32:43 UTC (rev 226808)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp	2018-01-11 22:43:29 UTC (rev 226809)
@@ -88,10 +88,11 @@
 {
 }
 
-TreeResolver::Parent::Parent(Element& element, const RenderStyle& style, Change change)
+TreeResolver::Parent::Parent(Element& element, const RenderStyle& style, Change change, DescendantsToResolve descendantsToResolve)
     : element(&element)
     , style(style)
     , change(change)
+    , descendantsToResolve(descendantsToResolve)
 {
 }
 
@@ -176,6 +177,26 @@
     return nullptr;
 }
 
+static DescendantsToResolve computeDescendantsToResolve(Change change, Validity validity, DescendantsToResolve parentDescendantsToResolve)
+{
+    if (parentDescendantsToResolve == DescendantsToResolve::All)
+        return DescendantsToResolve::All;
+    if (validity >= Validity::SubtreeInvalid)
+        return DescendantsToResolve::All;
+    switch (change) {
+    case NoChange:
+        return DescendantsToResolve::None;
+    case NoInherit:
+        return DescendantsToResolve::ChildrenWithExplicitInherit;
+    case Inherit:
+        return DescendantsToResolve::Children;
+    case Detach:
+        return DescendantsToResolve::All;
+    };
+    ASSERT_NOT_REACHED();
+    return DescendantsToResolve::None;
+};
+
 ElementUpdates TreeResolver::resolveElement(Element& element)
 {
     if (m_didSeePendingStylesheet && !element.renderer() && !m_document.isIgnoringPendingStylesheets()) {
@@ -196,6 +217,7 @@
     }
 
     auto update = createAnimatedElementUpdate(WTFMove(newStyle), element, parent().change);
+    auto descendantsToResolve = computeDescendantsToResolve(update.change, element.styleValidity(), parent().descendantsToResolve);
 
     if (&element == m_document.documentElement()) {
         m_documentElementStyle = RenderStyle::clonePtr(*update.style);
@@ -205,7 +227,7 @@
             // "rem" units are relative to the document element's font size so we need to recompute everything.
             // In practice this is rare.
             scope().styleResolver.invalidateMatchedPropertiesCache();
-            update.change = std::max(update.change, Force);
+            descendantsToResolve = DescendantsToResolve::All;
         }
     }
 
@@ -216,14 +238,16 @@
 
     // FIXME: These elements should not change renderer based on appearance property.
     if (element.hasTagName(HTMLNames::meterTag) || is<HTMLProgressElement>(element)) {
-        if (existingStyle && update.style->appearance() != existingStyle->appearance())
+        if (existingStyle && update.style->appearance() != existingStyle->appearance()) {
             update.change = Detach;
+            descendantsToResolve = DescendantsToResolve::All;
+        }
     }
 
     auto beforeUpdate = resolvePseudoStyle(element, update, BEFORE);
     auto afterUpdate = resolvePseudoStyle(element, update, AFTER);
 
-    return { WTFMove(update), WTFMove(beforeUpdate), WTFMove(afterUpdate) };
+    return { WTFMove(update), descendantsToResolve, WTFMove(beforeUpdate), WTFMove(afterUpdate) };
 }
 
 ElementUpdate TreeResolver::resolvePseudoStyle(Element& element, const ElementUpdate& elementUpdate, PseudoId pseudoId)
@@ -288,10 +312,8 @@
     auto change = oldStyle ? determineChange(*oldStyle, *newStyle) : Detach;
 
     auto validity = element.styleValidity();
-    if (validity >= Validity::SubtreeInvalid)
-        change = std::max(change, validity == Validity::SubtreeAndRenderersInvalid ? Detach : Force);
-    if (parentChange >= Force)
-        change = std::max(change, parentChange);
+    if (validity >= Validity::SubtreeAndRenderersInvalid || parentChange == Detach)
+        change = Detach;
 
     bool shouldRecompositeLayer = element.styleResolutionShouldRecompositeLayer() || animationUpdate.stateChanged;
 
@@ -298,11 +320,11 @@
     return { WTFMove(newStyle), change, shouldRecompositeLayer };
 }
 
-void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change change)
+void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change change, DescendantsToResolve descendantsToResolve)
 {
     scope().selectorFilter.pushParent(&element);
 
-    Parent parent(element, style, change);
+    Parent parent(element, style, change, descendantsToResolve);
 
     if (auto* shadowRoot = element.shadowRoot()) {
         pushScope(*shadowRoot);
@@ -347,22 +369,26 @@
     return pseudoElement->needsStyleRecalc();
 }
 
-static bool shouldResolveElement(const Element& element, Style::Change parentChange)
+static bool shouldResolveElement(const Element& element, DescendantsToResolve parentDescendantsToResolve)
 {
-    if (parentChange >= Inherit)
+    if (element.styleValidity() != Validity::Valid)
         return true;
-    if (parentChange == NoInherit) {
-        auto* existingStyle = renderOrDisplayContentsStyle(element);
-        if (existingStyle && existingStyle->hasExplicitlyInheritedProperties())
-            return true;
-    }
-    if (element.needsStyleRecalc())
-        return true;
     if (shouldResolvePseudoElement(element.beforePseudoElement()))
         return true;
     if (shouldResolvePseudoElement(element.afterPseudoElement()))
         return true;
 
+    switch (parentDescendantsToResolve) {
+    case DescendantsToResolve::None:
+        return false;
+    case DescendantsToResolve::Children:
+    case DescendantsToResolve::All:
+        return true;
+    case DescendantsToResolve::ChildrenWithExplicitInherit:
+        auto* existingStyle = renderOrDisplayContentsStyle(element);
+        return existingStyle && existingStyle->hasExplicitlyInheritedProperties();
+    };
+    ASSERT_NOT_REACHED();
     return false;
 }
 
@@ -452,8 +478,9 @@
 
         auto* style = renderOrDisplayContentsStyle(element);
         auto change = NoChange;
+        auto descendantsToResolve = DescendantsToResolve::None;
 
-        bool shouldResolve = shouldResolveElement(element, parent.change) || affectedByPreviousSibling;
+        bool shouldResolve = shouldResolveElement(element, parent.descendantsToResolve) || affectedByPreviousSibling;
         if (shouldResolve) {
             if (!element.hasDisplayContents())
                 element.resetComputedStyle();
@@ -469,9 +496,10 @@
 
             style = elementUpdates.update.style.get();
             change = elementUpdates.update.change;
+            descendantsToResolve = elementUpdates.descendantsToResolve;
 
-            if (affectedByPreviousSibling && change != Detach)
-                change = Force;
+            if (affectedByPreviousSibling)
+                descendantsToResolve = DescendantsToResolve::All;
 
             if (elementUpdates.update.style)
                 m_update->addElement(element, parent.element, WTFMove(elementUpdates));
@@ -484,7 +512,7 @@
             element.clearChildNeedsStyleRecalc();
         }
 
-        bool shouldIterateChildren = style && (element.childNeedsStyleRecalc() || change != NoChange);
+        bool shouldIterateChildren = style && (element.childNeedsStyleRecalc() || descendantsToResolve != DescendantsToResolve::None);
 
         if (!m_didSeePendingStylesheet)
             m_didSeePendingStylesheet = hasLoadingStylesheet(m_document.styleScope(), element, !shouldIterateChildren);
@@ -494,7 +522,7 @@
             continue;
         }
 
-        pushParent(element, *style, change);
+        pushParent(element, *style, change, descendantsToResolve);
 
         it.traverseNext();
     }

Modified: trunk/Source/WebCore/style/StyleTreeResolver.h (226808 => 226809)


--- trunk/Source/WebCore/style/StyleTreeResolver.h	2018-01-11 22:32:43 UTC (rev 226808)
+++ trunk/Source/WebCore/style/StyleTreeResolver.h	2018-01-11 22:43:29 UTC (rev 226809)
@@ -55,8 +55,10 @@
     std::unique_ptr<RenderStyle> styleForElement(Element&, const RenderStyle& inheritedStyle);
 
     void resolveComposedTree();
+
     ElementUpdates resolveElement(Element&);
-    ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, Element&, Change parentChange);
+
+    ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, Element&, Change);
     ElementUpdate resolvePseudoStyle(Element&, const ElementUpdate&, PseudoId);
 
     struct Scope : RefCounted<Scope> {
@@ -75,11 +77,12 @@
         Element* element;
         const RenderStyle& style;
         Change change { NoChange };
+        DescendantsToResolve descendantsToResolve { DescendantsToResolve::None };
         bool didPushScope { false };
         bool elementNeedingStyleRecalcAffectsNextSiblingElementStyle { false };
 
         Parent(Document&);
-        Parent(Element&, const RenderStyle&, Change);
+        Parent(Element&, const RenderStyle&, Change, DescendantsToResolve);
     };
 
     Scope& scope() { return m_scopeStack.last(); }
@@ -89,7 +92,7 @@
     void pushEnclosingScope();
     void popScope();
 
-    void pushParent(Element&, const RenderStyle&, Change);
+    void pushParent(Element&, const RenderStyle&, Change, DescendantsToResolve);
     void popParent();
     void popParentsToDepth(unsigned depth);

Modified: trunk/Source/WebCore/style/StyleUpdate.h (226808 => 226809)


--- trunk/Source/WebCore/style/StyleUpdate.h	2018-01-11 22:32:43 UTC (rev 226808)
+++ trunk/Source/WebCore/style/StyleUpdate.h	2018-01-11 22:43:29 UTC (rev 226809)
@@ -57,11 +57,14 @@
     bool recompositeLayer { false };
 };
 
+enum class DescendantsToResolve { None, ChildrenWithExplicitInherit, Children, All };
+
 struct ElementUpdates {
 #if !COMPILER_SUPPORTS(NSDMI_FOR_AGGREGATES)
     ElementUpdates() = default;
-    ElementUpdates(ElementUpdate update, std::optional<ElementUpdate> beforePseudoElementUpdate, std::optional<ElementUpdate> afterPseudoElementUpdate)
+    ElementUpdates(ElementUpdate update, DescendantsToResolve descendantsToResolve, std::optional<ElementUpdate> beforePseudoElementUpdate, std::optional<ElementUpdate> afterPseudoElementUpdate)
         : update { WTFMove(update) }
+        , descendantsToResolve(descendantsToResolve)
         , beforePseudoElementUpdate { WTFMove(beforePseudoElementUpdate) }
         , afterPseudoElementUpdate { WTFMove(afterPseudoElementUpdate) }
     {
@@ -68,6 +71,7 @@
     }
 #endif
     ElementUpdate update;
+    DescendantsToResolve descendantsToResolve { DescendantsToResolve::None };
     std::optional<ElementUpdate> beforePseudoElementUpdate;
     std::optional<ElementUpdate> afterPseudoElementUpdate;
 };

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to