Create inline wrappers for before/after pseudo elements that have dis…

…play: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


Canonical link: https://commits.webkit.org/194894@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223898 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,17 @@
+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>
 
         Mark fast/css-generated-content/details-summary-before-after.html as failing on Windows.

LayoutTests/TestExpectations

@@ -1267,8 +1267,6 @@ imported/w3c/web-platform-tests/fetch/nosniff [ DumpJSConsoleLogInStdErr ]
 ########################################
 ### 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 ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-inline.html [ ImageOnlyFailure ]

Source/WebCore/ChangeLog

@@ -1,3 +1,39 @@
+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>
 
         [iOS] MediaPlayer::isAvailable() takes ~150 ms during web process initialization

Source/WebCore/dom/PseudoElement.h

@@ -44,8 +44,6 @@ class PseudoElement final : public Element {
     bool canStartSelection() const override { return false; }
     bool canContainRangeEndPoint() const override { return false; }
 
-    auto& contentRenderers() { return m_contentRenderers; }
-
     static String pseudoElementNameForEvents(PseudoId);
 
 private:
@@ -55,7 +53,6 @@ class PseudoElement final : public Element {
 
     Element* m_hostElement;
     PseudoId m_pseudoId;
-    Vector<WeakPtr<RenderObject>> m_contentRenderers;
 };
 
 const QualifiedName& pseudoElementTagName();

Source/WebCore/rendering/style/RenderStyle.cpp

@@ -352,6 +352,13 @@ void RenderStyle::copyNonInheritedFrom(const RenderStyle& other)
     ASSERT(zoom() == initialZoom());
 }
 
+void RenderStyle::copyContentFrom(const RenderStyle& other)
+{
+    if (!other.m_rareNonInheritedData->content)
+        return;
+    m_rareNonInheritedData.access().content = other.m_rareNonInheritedData->content->clone();
+}
+
 bool RenderStyle::operator==(const RenderStyle& other) const
 {
     // compare everything except the pseudoStyle pointer

Source/WebCore/rendering/style/RenderStyle.h

@@ -159,6 +159,7 @@ class RenderStyle {
 
     void inheritFrom(const RenderStyle& inheritParent);
     void copyNonInheritedFrom(const RenderStyle&);
+    void copyContentFrom(const RenderStyle&);
 
     ContentPosition resolvedJustifyContentPosition(const StyleContentAlignmentData& normalValueBehavior) const;
     ContentDistributionType resolvedJustifyContentDistribution(const StyleContentAlignmentData& normalValueBehavior) const;

Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp

@@ -69,39 +69,25 @@ void RenderTreeUpdater::GeneratedContent::updateQuotesUpTo(RenderQuote* lastQuot
     ASSERT(!lastQuote);
 }
 
-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)
-            continue;
+    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;
-        auto createdStyle = RenderStyle::createStyleInheritingFromPseudoStyle(style);
-        downcast<RenderElement>(*contentRenderer).setStyle(WTFMove(createdStyle));
-    }
-}
-
-static void removeAndDestroyContentRenderers(PseudoElement& pseudoElement)
-{
-    for (auto& contentRenderer : pseudoElement.contentRenderers()) {
-        if (!contentRenderer)
+        if (!is<RenderImage>(contentRenderer) && !is<RenderQuote>(contentRenderer))
             continue;
-        contentRenderer->removeFromParentAndDestroy();
+        contentRenderer.setStyle(RenderStyle::createStyleInheritingFromPseudoStyle(style));
     }
-    pseudoElement.contentRenderers().clear();
 }
 
 void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current, const std::optional<Style::ElementUpdate>& update, PseudoId pseudoId)
@@ -113,8 +99,6 @@ void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current,
 
     if (!needsPseudoElement(update)) {
         if (pseudoElement) {
-            removeAndDestroyContentRenderers(*pseudoElement);
-
             if (pseudoId == BEFORE)
                 current.clearBeforePseudoElement();
             else
@@ -142,29 +126,20 @@ void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current,
     m_updater.updateElementRenderer(*pseudoElement, *update);
 
     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));
 }
 
 bool RenderTreeUpdater::GeneratedContent::needsPseudoElement(const std::optional<Style::ElementUpdate>& update)

Source/WebCore/style/StyleTreeResolver.cpp

@@ -246,6 +246,15 @@ ElementUpdate TreeResolver::resolvePseudoStyle(Element& element, const ElementUp
             element.setAfterPseudoElement(WTFMove(newPseudoElement));
     }
 
+    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);
 }