Changeset 218318 in webkit

Timestamp:

Jun 14, 2017 11:58:02 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

Don't always recalc the style of display: contents elements.
​https://bugs.webkit.org/show_bug.cgi?id=172753

Patch by Emilio Cobos Álvarez <​ecobos@igalia.com> on 2017-06-15
Reviewed by Antti Koivisto.

No new tests (no functionality change). This only removes an
inefficiency.

• dom/Element.cpp:

(WebCore::Element::existingComputedStyle):

• dom/Element.h:
• style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::updateRenderTree):
(WebCore::RenderTreeUpdater::updateElementRenderer):

• style/StyleTreeResolver.cpp:

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

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)
• dom/Element.h (modified) (2 diffs)
• style/RenderTreeUpdater.cpp (modified) (3 diffs)
• style/StyleTreeResolver.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-06-15 Emilio Cobos Álvarez  <ecobos@igalia.com>
+
+        Don't always recalc the style of display: contents elements.
+        https://bugs.webkit.org/show_bug.cgi?id=172753
+
+        Reviewed by Antti Koivisto.
+
+        No new tests (no functionality change). This only removes an
+        inefficiency.
+
+        * dom/Element.cpp:
+        (WebCore::Element::existingComputedStyle):
+        * dom/Element.h:
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::updateRenderTree):
+        (WebCore::RenderTreeUpdater::updateElementRenderer):
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::resolveElement):
+        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+        (WebCore::Style::shouldResolveElement):
+        (WebCore::Style::TreeResolver::resolveComposedTree):
+
 2017-06-14  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -2664 +2664 @@
 }
 
-const RenderStyle* Element::existingComputedStyle()
+const RenderStyle* Element::existingComputedStyle() const
 {
     if (auto* renderTreeStyle = renderStyle())

trunk/Source/WebCore/dom/Element.h

@@ -506 +506 @@
     LayoutRect absoluteEventHandlerBounds(bool& includesFixedPositionElements) override;
 
+    const RenderStyle* existingComputedStyle() const;
+
     void setBeforePseudoElement(Ref<PseudoElement>&&);
     void setAfterPseudoElement(Ref<PseudoElement>&&);
@@ -644 +646 @@
     void removeShadowRoot();
 
-    const RenderStyle* existingComputedStyle();
     const RenderStyle& resolveComputedStyle();
 

trunk/Source/WebCore/style/RenderTreeUpdater.cpp

@@ -158 +158 @@
         if (auto* renderer = node.renderer())
             renderTreePosition().invalidateNextSibling(*renderer);
+        else if (is<Element>(node) && downcast<Element>(node).hasDisplayContents())
+            renderTreePosition().invalidateNextSibling();
 
         if (is<Text>(node)) {
@@ -171 +173 @@
 
         auto* elementUpdate = m_styleUpdate->elementUpdate(element);
-        if (!elementUpdate) {
+
+        // We hop through display: contents elements in findRenderingRoot, so
+        // there may be other updates down the tree.
+        if (!elementUpdate && !element.hasDisplayContents()) {
             it.traverseNextSkippingChildren();
             continue;
         }
 
-        updateElementRenderer(element, *elementUpdate);
+        if (elementUpdate)
+            updateElementRenderer(element, *elementUpdate);
 
         bool mayHaveRenderedDescendants = element.renderer() || (element.hasDisplayContents() && shouldCreateRenderer(element, renderTreePosition().parent()));
@@ -275 +281 @@
         else
             element.storeDisplayContentsStyle(RenderStyle::clonePtr(*update.style));
-        // Render tree position needs to be recomputed as rendering siblings may be found from the display:contents subtree.
-        renderTreePosition().invalidateNextSibling();
     }
 

trunk/Source/WebCore/style/StyleTreeResolver.cpp

@@ -180 +180 @@
         return { };
 
-    auto* existingStyle = element.renderStyle();
+    auto* existingStyle = element.existingComputedStyle();
 
     if (m_didSeePendingStylesheet && (!existingStyle || existingStyle->isNotFinal())) {
@@ -249 +249 @@
 
     if (!renderer) {
-        auto keepsDisplayContents = newStyle->display() == CONTENTS && element.hasDisplayContents();
-        // Some inherited property might have changed.
-        return makeUpdate(WTFMove(newStyle), keepsDisplayContents ? Inherit : Detach);
+        auto change = Detach;
+        if (auto* oldStyle = element.existingComputedStyle())
+            change = determineChange(*oldStyle, *newStyle);
+        return makeUpdate(WTFMove(newStyle), change);
     }
 
@@ -332 +333 @@
         return true;
     if (parentChange == NoInherit) {
-        auto* existingStyle = element.renderStyle();
+        auto* existingStyle = element.existingComputedStyle();
         if (existingStyle && existingStyle->hasExplicitlyInheritedProperties())
             return true;
     }
     if (element.needsStyleRecalc())
-        return true;
-    if (element.hasDisplayContents())
         return true;
     if (shouldResolvePseudoElement(element.beforePseudoElement()))
@@ -418 +417 @@
             parent.elementNeedingStyleRecalcAffectsNextSiblingElementStyle = element.affectsNextSiblingElementStyle();
 
-        auto* style = element.renderStyle();
+        auto* style = element.existingComputedStyle();
         auto change = NoChange;