Changeset 223898 in webkit

Timestamp:

Oct 24, 2017 10:50:19 AM (7 years ago)

Author:

Antti Koivisto

Message:

Create inline wrappers for before/after pseudo elements that have display:contents
​https://bugs.webkit.org/show_bug.cgi?id=178722

Reviewed by Ryosuke Niwa.

Source/WebCore:

We can handle before and after pseudo elements with display:contents by giving them
inline renderers with style inherited from display:contents style. This removes
need for complicated logic for this case and handles everything correctly.

This is a better approach and replaces the one taken in bug 178584.
It also fixes two display:contents WPTs.

• dom/PseudoElement.h:

There is no need to track content renderers separately anymore. They always descendants of
pseudo element's renderer (which is an inline wrapper in case of display:contents).

• rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::copyContentFrom):

• rendering/style/RenderStyle.h:
• style/RenderTreeUpdaterGeneratedContent.cpp:

(WebCore::createContentRenderers):
(WebCore::updateStyleForContentRenderers):
(WebCore::RenderTreeUpdater::GeneratedContent::updatePseudoElement):
(WebCore::removeAndDestroyContentRenderers): Deleted.

Since content renderers are now always descendants of the pseudo renderer
there is no need for a separate destruction path.

• style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::resolvePseudoStyle):

Create ElementUpdate with a style that will produce an inline wrapper.

LayoutTests:

• TestExpectations:

Enable

imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-first-letter-001.html and
imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-001.html

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/PseudoElement.h (modified) (2 diffs)
• Source/WebCore/rendering/style/RenderStyle.cpp (modified) (1 diff)
• Source/WebCore/rendering/style/RenderStyle.h (modified) (1 diff)
• Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp (modified) (3 diffs)
• Source/WebCore/style/StyleTreeResolver.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-10-24  Antti Koivisto  <antti@apple.com>
+
+        Create inline wrappers for before/after pseudo elements that have display:contents
+        https://bugs.webkit.org/show_bug.cgi?id=178722
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestExpectations:
+
+        Enable
+
+        imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-first-letter-001.html and
+        imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-001.html
+
 2017-10-24  Per Arne Vollan  <pvollan@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -1268 +1268 @@
 ### START OF display: contents failures
 
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-first-letter-001.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-002-none.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-flex-003.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-24  Antti Koivisto  <antti@apple.com>
+
+        Create inline wrappers for before/after pseudo elements that have display:contents
+        https://bugs.webkit.org/show_bug.cgi?id=178722
+
+        Reviewed by Ryosuke Niwa.
+
+        We can handle before and after pseudo elements with display:contents by giving them
+        inline renderers with style inherited from display:contents style. This removes
+        need for complicated logic for this case and handles everything correctly.
+
+        This is a better approach and replaces the one taken in bug 178584.
+        It also fixes two display:contents WPTs.
+
+        * dom/PseudoElement.h:
+
+            There is no need to track content renderers separately anymore. They always descendants of
+            pseudo element's renderer (which is an inline wrapper in case of display:contents).
+
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::copyContentFrom):
+        * rendering/style/RenderStyle.h:
+        * style/RenderTreeUpdaterGeneratedContent.cpp:
+        (WebCore::createContentRenderers):
+        (WebCore::updateStyleForContentRenderers):
+        (WebCore::RenderTreeUpdater::GeneratedContent::updatePseudoElement):
+        (WebCore::removeAndDestroyContentRenderers): Deleted.
+
+            Since content renderers are now always descendants of the pseudo renderer
+            there is no need for a separate destruction path.
+
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::resolvePseudoStyle):
+
+            Create ElementUpdate with a style that will produce an inline wrapper.
+
 2017-10-24  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/dom/PseudoElement.h

@@ -45 +45 @@
     bool canContainRangeEndPoint() const override { return false; }
 
-    auto& contentRenderers() { return m_contentRenderers; }
-
     static String pseudoElementNameForEvents(PseudoId);
 
@@ -56 +54 @@
     Element* m_hostElement;
     PseudoId m_pseudoId;
-    Vector<WeakPtr<RenderObject>> m_contentRenderers;
 };
 

trunk/Source/WebCore/rendering/style/RenderStyle.cpp

@@ -351 +351 @@
 
     ASSERT(zoom() == initialZoom());
+}
+
+void RenderStyle::copyContentFrom(const RenderStyle& other)
+{
+    if (!other.m_rareNonInheritedData->content)
+        return;
+    m_rareNonInheritedData.access().content = other.m_rareNonInheritedData->content->clone();
 }
 

trunk/Source/WebCore/rendering/style/RenderStyle.h

@@ -160 +160 @@
     void inheritFrom(const RenderStyle& inheritParent);
     void copyNonInheritedFrom(const RenderStyle&);
+    void copyContentFrom(const RenderStyle&);
 
     ContentPosition resolvedJustifyContentPosition(const StyleContentAlignmentData& normalValueBehavior) const;

trunk/Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp

@@ -70 +70 @@
 }
 
-static void createContentRenderers(PseudoElement& pseudoElement, const RenderStyle& style, RenderTreePosition& renderTreePosition)
+static void createContentRenderers(RenderElement& pseudoRenderer, const RenderStyle& style)
 {
     ASSERT(style.contentData());
 
     for (const ContentData* content = style.contentData(); content; content = content->next()) {
-        auto child = content->createContentRenderer(renderTreePosition.parent().document(), style);
-        pseudoElement.contentRenderers().append(makeWeakPtr(*child));
-        if (renderTreePosition.parent().isChildAllowed(*child, style))
-            renderTreePosition.insert(WTFMove(child));
+        auto child = content->createContentRenderer(pseudoRenderer.document(), style);
+        if (pseudoRenderer.isChildAllowed(*child, style))
+            pseudoRenderer.addChild(WTFMove(child));
     }
 }
 
-static void updateStyleForContentRenderers(PseudoElement& pseudoElement, const RenderStyle& style)
+static void updateStyleForContentRenderers(RenderElement& pseudoRenderer, const RenderStyle& style)
 {
-    for (auto& contentRenderer : pseudoElement.contentRenderers()) {
-        if (!contentRenderer)
+    for (auto& contentRenderer : descendantsOfType<RenderElement>(pseudoRenderer)) {
+        // We only manage the style for the generated content which must be images or text.
+        if (!is<RenderImage>(contentRenderer) && !is<RenderQuote>(contentRenderer))
             continue;
-        // We only manage the style for the generated content which must be images or text.
-        if (!is<RenderImage>(*contentRenderer) && !is<RenderQuote>(*contentRenderer))
-            continue;
-        auto createdStyle = RenderStyle::createStyleInheritingFromPseudoStyle(style);
-        downcast<RenderElement>(*contentRenderer).setStyle(WTFMove(createdStyle));
+        contentRenderer.setStyle(RenderStyle::createStyleInheritingFromPseudoStyle(style));
     }
-}
-
-static void removeAndDestroyContentRenderers(PseudoElement& pseudoElement)
-{
-    for (auto& contentRenderer : pseudoElement.contentRenderers()) {
-        if (!contentRenderer)
-            continue;
-        contentRenderer->removeFromParentAndDestroy();
-    }
-    pseudoElement.contentRenderers().clear();
 }
 
@@ -114 +100 @@
     if (!needsPseudoElement(update)) {
         if (pseudoElement) {
-            removeAndDestroyContentRenderers(*pseudoElement);
-
             if (pseudoId == BEFORE)
                 current.clearBeforePseudoElement();
@@ -143 +127 @@
 
     auto* pseudoElementRenderer = pseudoElement->renderer();
-    if (!pseudoElementRenderer && update->style->display() != CONTENTS)
+    if (!pseudoElementRenderer)
         return;
 
-    auto renderTreePosition = pseudoElementRenderer ? RenderTreePosition(*pseudoElementRenderer) : m_updater.renderTreePosition();
-
-    if (update->change == Style::Detach) {
-        removeAndDestroyContentRenderers(*pseudoElement);
-
-        if (pseudoElementRenderer)
-            renderTreePosition.moveToLastChild();
-        else
-            renderTreePosition.computeNextSibling(*pseudoElement);
-
-        createContentRenderers(*pseudoElement, *update->style, renderTreePosition);
-    } else
-        updateStyleForContentRenderers(*pseudoElement, *update->style);
+    if (update->change == Style::Detach)
+        createContentRenderers(*pseudoElementRenderer, *update->style);
+    else
+        updateStyleForContentRenderers(*pseudoElementRenderer, *update->style);
 
     if (m_updater.renderView().hasQuotesNeedingUpdate()) {
-        for (auto& child : descendantsOfType<RenderQuote>(renderTreePosition.parent()))
+        for (auto& child : descendantsOfType<RenderQuote>(*pseudoElementRenderer))
             updateQuotesUpTo(&child);
     }
-    if (is<RenderListItem>(renderTreePosition.parent()))
-        ListItem::updateMarker(downcast<RenderListItem>(renderTreePosition.parent()));
+    if (is<RenderListItem>(*pseudoElementRenderer))
+        ListItem::updateMarker(downcast<RenderListItem>(*pseudoElementRenderer));
 }
 

trunk/Source/WebCore/style/StyleTreeResolver.cpp

@@ -247 +247 @@
     }
 
+    if (pseudoStyle->display() == CONTENTS) {
+        // For display:contents we create an inline wrapper that inherits its style from the display:contents style.
+        auto contentsStyle = RenderStyle::createPtr();
+        contentsStyle->setStyleType(pseudoId);
+        contentsStyle->inheritFrom(*pseudoStyle);
+        contentsStyle->copyContentFrom(*pseudoStyle);
+        pseudoStyle = WTFMove(contentsStyle);
+    }
+
     return createAnimatedElementUpdate(WTFMove(pseudoStyle), *pseudoElement, elementUpdate.change);
 }